Cap lifetime of persistent cookies created client-side through document.cookie
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Sep 2018 01:13:26 +0000 (01:13 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 25 Sep 2018 01:13:26 +0000 (01:13 +0000)
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 ].

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@236448 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html [new file with mode: 0644]
LayoutTests/http/tests/cookies/resources/cookie-utilities.js
LayoutTests/platform/ios/TestExpectations
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index 53ce7c5..6af2cb7 100644 (file)
@@ -1,3 +1,22 @@
+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>
 
         Remove filterRes parameter from filters
index 14760f5..d9112cb 100644 (file)
@@ -153,6 +153,7 @@ fast/media/mq-prefers-reduced-motion-live-update.html [ Skip ]
 http/tests/loading/basic-auth-remove-credentials.html [ Skip ]
 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.
 http/tests/ssl/applepay/ [ Skip ]
diff --git a/LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js-expected.txt b/LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js-expected.txt
new file mode 100644 (file)
index 0000000..484620d
--- /dev/null
@@ -0,0 +1,11 @@
+Check that cookies created by JavaScript with max-age or expiry longer than a week get capped to a week.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS The two short-lived cookies don't expire after more than 172830 seconds.
+PASS The two long-lived cookies don't expire after more than 604830 seconds.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html b/LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html
new file mode 100644 (file)
index 0000000..ff0796a
--- /dev/null
@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script src="/js-test-resources/js-test.js"></script>
+    <script src="resources/cookie-utilities.js"></script>
+</head>
+<body>
+<script>
+    description("Check that cookies created by JavaScript with max-age or expiry longer than a week get capped to a week.");
+    jsTestIsAsync = true;
+
+    let passedTests = 0;
+    function checkThatCookieDoesNotExpireAfter(cookieData, maxAgeInSeconds) {
+        let now = new Date();
+        let maxExpiryDateInMilliseconds = now.getTime() + (maxAgeInSeconds * 1000);
+
+        if (maxExpiryDateInMilliseconds > cookieData["expires"])
+            ++passedTests;
+        else
+            testFailed("Cookie named " + cookieData["name"] + " expires in more than " + maxAgeInSeconds + " seconds.");
+    }
+
+    const twoDaysInSeconds = 2 * 24 * 60 * 60;
+    const shortLivedCookieMaxAge = { name : "shortLivedCookieMaxAge", lifetime : "Max-Age=" + twoDaysInSeconds + ";" };
+    document.cookie = shortLivedCookieMaxAge.name + "=foobar; " + shortLivedCookieMaxAge.lifetime + " path=/";
+
+    const twoDaysAsExpiresDate = createExpiresDateFromMaxAge(twoDaysInSeconds);
+    const shortLivedCookieExpires = { name : "shortLivedCookieExpires", lifetime : "Expires=" + twoDaysAsExpiresDate + ";" };
+    document.cookie = shortLivedCookieExpires.name + "=foobar; " + shortLivedCookieExpires.lifetime + " path=/";
+
+    const oneWeekInSeconds = 7 * 24 * 60 * 60;
+    const twoWeeksInSeconds = 2 * oneWeekInSeconds;
+    const longLivedCookieMaxAge = { name : "longLivedCookieMaxAge", lifetime : "Max-Age=" + twoWeeksInSeconds + ";" };
+    document.cookie = longLivedCookieMaxAge.name + "=foobar; " + longLivedCookieMaxAge.lifetime + " path=/";
+
+    const twoWeeksAsExpiresDate = createExpiresDateFromMaxAge(twoWeeksInSeconds);
+    const longLivedCookieExpires = { name : "longLivedCookieExpires", lifetime : "Expires=" + twoWeeksAsExpiresDate + ";" };
+    document.cookie = longLivedCookieExpires.name + "=foobar; " + longLivedCookieExpires.lifetime + " path=/";
+
+    const overTwoDaysInSeconds = twoDaysInSeconds + 30;
+    const overOneWeekInSeconds = oneWeekInSeconds + 30;
+    if (internals) {
+        let cookies = internals.getCookies();
+        if (!cookies.length)
+            testFailed("No cookies found.");
+        for (let cookie of cookies) {
+            switch (cookie.name) {
+                case shortLivedCookieMaxAge.name:
+                    checkThatCookieDoesNotExpireAfter(cookie, overTwoDaysInSeconds);
+                    break;
+                case shortLivedCookieExpires.name:
+                    checkThatCookieDoesNotExpireAfter(cookie, overTwoDaysInSeconds);
+                    break;
+                case longLivedCookieMaxAge.name:
+                    checkThatCookieDoesNotExpireAfter(cookie, overOneWeekInSeconds);
+                    break;
+                case longLivedCookieExpires.name:
+                    checkThatCookieDoesNotExpireAfter(cookie, overOneWeekInSeconds);
+                    break;
+            }
+        }
+        if (passedTests === 4) {
+            testPassed("The two short-lived cookies don't expire after more than " + overTwoDaysInSeconds + " seconds.");
+            testPassed("The two long-lived cookies don't expire after more than " + overOneWeekInSeconds + " seconds.");
+        } else
+            testFailed("At least one cookie's expiry attribute was beyond the test thresholds.");
+    } else
+        testFailed("No internals object.");
+
+    finishJSTest();
+</script>
+</body>
+</html>
index 07c0c03..91ff860 100644 (file)
@@ -239,3 +239,10 @@ function setCookieUsingWebSocketFromHost(host)
     });
     return promise;
 }
+
+function createExpiresDateFromMaxAge(maxAgeInSeconds)
+{
+    let date = new Date();
+    date.setTime(date.getTime() + (maxAgeInSeconds * 1000));
+    return date.toUTCString();
+}
index 587d5b5..f0ba15d 100644 (file)
@@ -2822,6 +2822,7 @@ http/tests/resourceLoadStatistics/strip-referrer-to-origin-for-prevalent-subreso
 http/tests/storageAccess/deny-storage-access-under-opener.html [ Pass ]
 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.
 http/tests/security/strip-referrer-to-origin-for-third-party-redirects-in-private-mode.html [ Pass ]
index 7ff697c..9b17383 100644 (file)
@@ -769,6 +769,7 @@ webkit.org/b/185994 [ Debug ] fast/text/user-installed-fonts/shadow-postscript-f
 [ HighSierra+ ] http/tests/resourceLoadStatistics/strip-referrer-to-origin-for-prevalent-subresource-redirects.html [ Pass ]
 [ 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.
 http/tests/security/strip-referrer-to-origin-for-third-party-redirects-in-private-mode.html [ Pass ]
index e43da57..5c82f5e 100644 (file)
@@ -1,3 +1,48 @@
+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>
 
         Remove filterRes parameter from SVG filters
index 315e50e..ce0caa0 100644 (file)
@@ -266,6 +266,7 @@ static RetainPtr<NSArray> filterCookies(NSArray *unfilteredCookies)
     NSUInteger count = [unfilteredCookies count];
     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];
 
@@ -279,6 +280,16 @@ static RetainPtr<NSArray> filterCookies(NSArray *unfilteredCookies)
         if ([cookie isHTTPOnly])
             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];
     }
 
index 8685487..69b3094 100644 (file)
@@ -48,6 +48,7 @@
 #include "Chrome.h"
 #include "ClientOrigin.h"
 #include "ComposedTreeIterator.h"
+#include "CookieJar.h"
 #include "Cursor.h"
 #include "DOMRect.h"
 #include "DOMRectList.h"
@@ -4761,4 +4762,17 @@ std::optional<HEVCParameterSet> Internals::parseHEVCCodecParameters(const String
     return WebCore::parseHEVCCodecParameters(codecString);
 }
 
+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
index 8dd3167..b81b954 100644 (file)
@@ -28,6 +28,7 @@
 
 #include "CSSComputedStyleDeclaration.h"
 #include "ContextDestructionObserver.h"
+#include "Cookie.h"
 #include "ExceptionOr.h"
 #include "HEVCUtilities.h"
 #include "JSDOMPromiseDeferred.h"
@@ -750,6 +751,38 @@ public:
     using HEVCParameterSet = WebCore::HEVCParameterSet;
     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&);
     Document* contextDocument() const;
index 9445718..f415a32 100644 (file)
@@ -145,6 +145,21 @@ enum CompositingPolicy {
 
 [
     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 {
     DOMString address(Node node);
@@ -699,4 +714,6 @@ enum CompositingPolicy {
     boolean supportsVCPEncoder();
 
     HEVCParameterSet? parseHEVCCodecParameters(DOMString codecParameters);
+
+    sequence<CookieData> getCookies();
 };