From bcb426d989211c6e17d92a146d17bd40b9ac3b8c Mon Sep 17 00:00:00 2001 From: "wilander@apple.com" Date: Tue, 25 Sep 2018 01:13:26 +0000 Subject: [PATCH] Cap lifetime of persistent cookies created client-side through document.cookie https://bugs.webkit.org/show_bug.cgi?id=189933 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 --- LayoutTests/ChangeLog | 19 ++++++ LayoutTests/TestExpectations | 1 + ...pped-lifetime-for-cookie-set-in-js-expected.txt | 11 ++++ .../capped-lifetime-for-cookie-set-in-js.html | 73 ++++++++++++++++++++++ .../tests/cookies/resources/cookie-utilities.js | 7 +++ LayoutTests/platform/ios/TestExpectations | 1 + LayoutTests/platform/mac-wk2/TestExpectations | 1 + Source/WebCore/ChangeLog | 45 +++++++++++++ .../network/cocoa/NetworkStorageSessionCocoa.mm | 11 ++++ Source/WebCore/testing/Internals.cpp | 14 +++++ Source/WebCore/testing/Internals.h | 33 ++++++++++ Source/WebCore/testing/Internals.idl | 17 +++++ 12 files changed, 233 insertions(+) create mode 100644 LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js-expected.txt create mode 100644 LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 53ce7c5..6af2cb7c 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,22 @@ +2018-09-24 John Wilander + + Cap lifetime of persistent cookies created client-side through document.cookie + https://bugs.webkit.org/show_bug.cgi?id=189933 + + + 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 Remove filterRes parameter from filters diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations index 14760f5..d9112cb 100644 --- a/LayoutTests/TestExpectations +++ b/LayoutTests/TestExpectations @@ -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 index 0000000..484620d --- /dev/null +++ b/LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js-expected.txt @@ -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 index 0000000..ff0796a --- /dev/null +++ b/LayoutTests/http/tests/cookies/capped-lifetime-for-cookie-set-in-js.html @@ -0,0 +1,73 @@ + + + + + + + + + + diff --git a/LayoutTests/http/tests/cookies/resources/cookie-utilities.js b/LayoutTests/http/tests/cookies/resources/cookie-utilities.js index 07c0c03..91ff860 100644 --- a/LayoutTests/http/tests/cookies/resources/cookie-utilities.js +++ b/LayoutTests/http/tests/cookies/resources/cookie-utilities.js @@ -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(); +} diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations index 587d5b5..f0ba15d 100644 --- a/LayoutTests/platform/ios/TestExpectations +++ b/LayoutTests/platform/ios/TestExpectations @@ -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 ] diff --git a/LayoutTests/platform/mac-wk2/TestExpectations b/LayoutTests/platform/mac-wk2/TestExpectations index 7ff697c..9b17383 100644 --- a/LayoutTests/platform/mac-wk2/TestExpectations +++ b/LayoutTests/platform/mac-wk2/TestExpectations @@ -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 ] diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index e43da57..5c82f5e 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,48 @@ +2018-09-24 John Wilander + + Cap lifetime of persistent cookies created client-side through document.cookie + https://bugs.webkit.org/show_bug.cgi?id=189933 + + + 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 Remove filterRes parameter from SVG filters diff --git a/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm b/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm index 315e50e..ce0caa0 100644 --- a/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm +++ b/Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm @@ -266,6 +266,7 @@ static RetainPtr filterCookies(NSArray *unfilteredCookies) NSUInteger count = [unfilteredCookies count]; RetainPtr 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 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> properties = adoptNS([[cookie properties] mutableCopy]); + RetainPtr dateInAWeek = adoptNS([[NSDate alloc] initWithTimeIntervalSinceNow:secondsPerWeek]); + [properties setObject:dateInAWeek.get() forKey:NSHTTPCookieExpires]; + cookie = [NSHTTPCookie cookieWithProperties:properties.get()]; + } + } + [filteredCookies.get() addObject:cookie]; } diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp index 8685487..69b3094 100644 --- a/Source/WebCore/testing/Internals.cpp +++ b/Source/WebCore/testing/Internals.cpp @@ -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 Internals::parseHEVCCodecParameters(const String return WebCore::parseHEVCCodecParameters(codecString); } +auto Internals::getCookies() const -> Vector +{ + auto* document = contextDocument(); + if (!document) + return { }; + + Vector cookies; + getRawCookies(*document, document->cookieURL(), cookies); + return WTF::map(cookies, [](auto& cookie) { + return CookieData { cookie }; + }); +} + } // namespace WebCore diff --git a/Source/WebCore/testing/Internals.h b/Source/WebCore/testing/Internals.h index 8dd3167..b81b954 100644 --- a/Source/WebCore/testing/Internals.h +++ b/Source/WebCore/testing/Internals.h @@ -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 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 getCookies() const; + private: explicit Internals(Document&); Document* contextDocument() const; diff --git a/Source/WebCore/testing/Internals.idl b/Source/WebCore/testing/Internals.idl index 9445718..f415a32 100644 --- a/Source/WebCore/testing/Internals.idl +++ b/Source/WebCore/testing/Internals.idl @@ -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 getCookies(); }; -- 1.8.3.1