Changeset 236448 in webkit

Timestamp:

Sep 24, 2018, 6:13:26 PM (8 years ago)

Author:

wilander@apple.com

Message:

Cap lifetime of persistent cookies created client-side through document.cookie
​https://bugs.webkit.org/show_bug.cgi?id=189933
<rdar://problem/44741888>

Reviewed by Chris Dumez.

Source/WebCore:

Test: http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html

As pointed out in ​https://github.com/mikewest/http-state-tokens:

1) Cookies are available to JavaScript by default via document.cookie, which
enables a smooth upgrade from one-time XSS to theft of persistent credentials
and also makes cookies available to Spectre-like attacks on memory.

2) Though the HttpOnly attribute was introduced well over a decade ago, only
~8.31% of Set-Cookie operations use it today (stats from Chrome). We need
developer incentives to put proper protections in place.

3) The median (uncompressed) Cookie request header is 409 bytes, while the 90th
percentile is 1,589 bytes, the 95th 2,549 bytes, the 99th 4,601 bytes, and
~0.1% of Cookie headers are over 10kB (stats from Chrome). This is bad for load
performance.

In addition to this, third-party scripts running in first-party contexts can
read user data through document.cookie and even store cross-site tracking data
in them.

Authentication cookies should be HttpOnly and thus not be affected by
restrictions to document.cookie. Cookies that persist for a long time should
be Secure, HttpOnly, and SameSite to provide good security and privacy.

By capping the lifetime of persistent cookies set through document.cookie we
embark on a journey towards better cookie management on the web.

• platform/network/cocoa/NetworkStorageSessionCocoa.mm:

(WebCore::filterCookies):

Now caps the life time of persistent cookies to one week (seven days).

• testing/Internals.cpp:

(WebCore::Internals::getCookies const):

New test function to get to cookie meta data such as expiry.

• testing/Internals.h:
• testing/Internals.idl:

LayoutTests:

• TestExpectations:

Skipped the new test by default since the behavior change is for
Cocoa platforms only.

• http/tests/cookies/capped-lifetime-for-cookie-set-in-js-expected.txt: Added.
• http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html: Added.
• http/tests/cookies/resources/cookie-utilities.js:
• platform/ios/TestExpectations:

Marked the new test as [ Pass ].

• platform/mac-wk2/TestExpectations:

Marked the new test as [ Pass ].

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js-expected.txt (added)
• LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html (added)
• LayoutTests/http/tests/cookies/resources/cookie-utilities.js (modified) (1 diff)
• LayoutTests/platform/ios/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac-wk2/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm (modified) (2 diffs)
• Source/WebCore/testing/Internals.cpp (modified) (2 diffs)
• Source/WebCore/testing/Internals.h (modified) (2 diffs)
• Source/WebCore/testing/Internals.idl (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-09-24  John Wilander  <wilander@apple.com>
+
+        Cap lifetime of persistent cookies created client-side through document.cookie
+        https://bugs.webkit.org/show_bug.cgi?id=189933
+        <rdar://problem/44741888>
+
+        Reviewed by Chris Dumez.
+
+        * TestExpectations:
+            Skipped the new test by default since the behavior change is for
+            Cocoa platforms only.
+        * http/tests/cookies/capped-lifetime-for-cookie-set-in-js-expected.txt: Added.
+        * http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html: Added.
+        * http/tests/cookies/resources/cookie-utilities.js:
+        * platform/ios/TestExpectations:
+            Marked the new test as [ Pass ].
+        * platform/mac-wk2/TestExpectations:
+            Marked the new test as [ Pass ].
+
 2018-09-24  Simon Fraser  <simon.fraser@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -154 +154 @@
 http/tests/security/strip-referrer-to-origin-for-third-party-redirects-in-private-mode.html [ Skip ]
 http/tests/security/strip-referrer-to-origin-for-third-party-requests-in-private-mode.html [ Skip ]
+http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html [ Skip ]
 
 # ApplePay is only available on iOS (greater than iOS 10) and macOS (greater than macOS 10.12) and only for WebKit2.

trunk/LayoutTests/http/tests/cookies/resources/cookie-utilities.js

@@ -240 +240 @@
     return promise;
 }
+
+function createExpiresDateFromMaxAge(maxAgeInSeconds)
+{
+    let date = new Date();
+    date.setTime(date.getTime() + (maxAgeInSeconds * 1000));
+    return date.toUTCString();
+}

trunk/LayoutTests/platform/ios/TestExpectations

@@ -2823 +2823 @@
 http/tests/storageAccess/grant-storage-access-under-opener.html [ Pass ]
 http/tests/resourceLoadStatistics/cap-cache-max-age-for-prevalent-resource.html [ Pass ]
+http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html [ Pass ]
 
 # Skipped in general expectations since they only work on iOS and Mac, WK2.

trunk/LayoutTests/platform/mac-wk2/TestExpectations

@@ -770 +770 @@
 [ HighSierra+ ] http/tests/resourceLoadStatistics/strip-referrer-to-origin-for-prevalent-subresource-requests.html [ Pass ]
 [ HighSierra+ ] http/tests/resourceLoadStatistics/cap-cache-max-age-for-prevalent-resource.html [ Pass ]
+http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html [ Pass ]
 
 # Skipped in general expectations since they only work on iOS and Mac, WK2.

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-09-24  John Wilander  <wilander@apple.com>
+
+        Cap lifetime of persistent cookies created client-side through document.cookie
+        https://bugs.webkit.org/show_bug.cgi?id=189933
+        <rdar://problem/44741888>
+
+        Reviewed by Chris Dumez.
+
+        Test: http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html
+
+        As pointed out in https://github.com/mikewest/http-state-tokens:
+
+        1) Cookies are available to JavaScript by default via document.cookie, which
+        enables a smooth upgrade from one-time XSS to theft of persistent credentials
+        and also makes cookies available to Spectre-like attacks on memory.
+
+        2) Though the HttpOnly attribute was introduced well over a decade ago, only
+        ~8.31% of Set-Cookie operations use it today (stats from Chrome). We need
+        developer incentives to put proper protections in place.
+
+        3) The median (uncompressed) Cookie request header is 409 bytes, while the 90th
+        percentile is 1,589 bytes, the 95th 2,549 bytes, the 99th 4,601 bytes, and
+        ~0.1% of Cookie headers are over 10kB (stats from Chrome). This is bad for load
+        performance.
+
+        In addition to this, third-party scripts running in first-party contexts can
+        read user data through document.cookie and even store cross-site tracking data
+        in them.
+
+        Authentication cookies should be HttpOnly and thus not be affected by
+        restrictions to document.cookie. Cookies that persist for a long time should
+        be Secure, HttpOnly, and SameSite to provide good security and privacy.
+
+        By capping the lifetime of persistent cookies set through document.cookie we
+        embark on a journey towards better cookie management on the web.
+
+        * platform/network/cocoa/NetworkStorageSessionCocoa.mm:
+        (WebCore::filterCookies):
+            Now caps the life time of persistent cookies to one week (seven days).
+        * testing/Internals.cpp:
+        (WebCore::Internals::getCookies const):
+            New test function to get to cookie meta data such as expiry.
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2018-09-24  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm

@@ -267 +267 @@
     RetainPtr<NSMutableArray> filteredCookies = adoptNS([[NSMutableArray alloc] initWithCapacity:count]);
 
+    const NSTimeInterval secondsPerWeek = 7 * 24 * 60 * 60;
     for (NSUInteger i = 0; i < count; ++i) {
         NSHTTPCookie *cookie = (NSHTTPCookie *)[unfilteredCookies objectAtIndex:i];
@@ -280 +281 @@
             continue;
 
+        // Cap lifetime of persistent, client-side cookies to a week.
+        if (![cookie isSessionOnly]) {
+            if (!cookie.expiresDate || cookie.expiresDate.timeIntervalSinceNow > secondsPerWeek) {
+                RetainPtr<NSMutableDictionary<NSHTTPCookiePropertyKey, id>> properties = adoptNS([[cookie properties] mutableCopy]);
+                RetainPtr<NSDate> dateInAWeek = adoptNS([[NSDate alloc] initWithTimeIntervalSinceNow:secondsPerWeek]);
+                [properties setObject:dateInAWeek.get() forKey:NSHTTPCookieExpires];
+                cookie = [NSHTTPCookie cookieWithProperties:properties.get()];
+            }
+        }
+
         [filteredCookies.get() addObject:cookie];
     }

trunk/Source/WebCore/testing/Internals.cpp

@@ -49 +49 @@
 #include "ClientOrigin.h"
 #include "ComposedTreeIterator.h"
+#include "CookieJar.h"
 #include "Cursor.h"
 #include "DOMRect.h"
@@ -4762 +4763 @@
 }
 
+auto Internals::getCookies() const -> Vector<CookieData>
+{
+    auto* document = contextDocument();
+    if (!document)
+        return { };
+
+    Vector<Cookie> cookies;
+    getRawCookies(*document, document->cookieURL(), cookies);
+    return WTF::map(cookies, [](auto& cookie) {
+        return CookieData { cookie };
+    });
+}
+
 } // namespace WebCore

trunk/Source/WebCore/testing/Internals.h

@@ -29 +29 @@
 #include "CSSComputedStyleDeclaration.h"
 #include "ContextDestructionObserver.h"
+#include "Cookie.h"
 #include "ExceptionOr.h"
 #include "HEVCUtilities.h"
@@ -751 +752 @@
     std::optional<HEVCParameterSet> parseHEVCCodecParameters(const String& codecString);
 
+    struct CookieData {
+        String name;
+        String value;
+        String domain;
+        // Expiration dates are expressed as milliseconds since the UNIX epoch.
+        double expires { 0 };
+        bool isHttpOnly { false };
+        bool isSecure { false };
+        bool isSession { false };
+        bool isSameSiteLax { false };
+        bool isSameSiteStrict { false };
+
+        CookieData(Cookie cookie)
+            : name(cookie.name)
+            , value(cookie.value)
+            , domain(cookie.domain)
+            , expires(cookie.expires)
+            , isHttpOnly(cookie.httpOnly)
+            , isSecure(cookie.secure)
+            , isSession(cookie.session)
+            , isSameSiteLax(cookie.sameSite == Cookie::SameSitePolicy::Lax)
+            , isSameSiteStrict(cookie.sameSite == Cookie::SameSitePolicy::Strict)
+        {
+            ASSERT(!(isSameSiteLax && isSameSiteStrict));
+        }
+
+        CookieData()
+        {
+        }
+    };
+    Vector<CookieData> getCookies() const;
+
 private:
     explicit Internals(Document&);

trunk/Source/WebCore/testing/Internals.idl

@@ -146 +146 @@
 [
     ExportMacro=WEBCORE_TESTSUPPORT_EXPORT,
+    JSGenerateToJSObject,
+] dictionary CookieData {
+    DOMString name;
+    DOMString value;
+    DOMString domain;
+    double expires;
+    boolean isHttpOnly;
+    boolean isSecure;
+    boolean isSession;
+    boolean isSameSiteLax;
+    boolean isSameSiteStrict;
+};
+
+[
+    ExportMacro=WEBCORE_TESTSUPPORT_EXPORT,
     NoInterfaceObject,
 ] interface Internals {
@@ -700 +715 @@
 
     HEVCParameterSet? parseHEVCCodecParameters(DOMString codecParameters);
-};
+
+    sequence<CookieData> getCookies();
+};