diff --git a/dom/cookiestore/CookieStoreParent.cpp b/dom/cookiestore/CookieStoreParent.cpp index b646c338ff35..26a9f9b7cafd 100644 --- a/dom/cookiestore/CookieStoreParent.cpp +++ b/dom/cookiestore/CookieStoreParent.cpp @@ -417,11 +417,10 @@ bool CookieStoreParent::SetRequestOnMainThread( rv = service->AddNative( aCookieURI, domainWithDot, NS_ConvertUTF16toUTF8(aPath), NS_ConvertUTF16toUTF8(aName), NS_ConvertUTF16toUTF8(aValue), - true, // secure - false, // mHttpOnly, - aSession, aSession ? INT64_MAX : aExpires, &attrs, aSameSite, - nsICookie::SCHEME_HTTPS, aPartitioned, &aOperationID, - [&](mozilla::net::CookieStruct& aCookieStruct) -> bool { + /* secure: */ true, + /* http-only: */ false, aSession, aSession ? INT64_MAX : aExpires, &attrs, + aSameSite, nsICookie::SCHEME_HTTPS, aPartitioned, /* from http: */ false, + &aOperationID, [&](mozilla::net::CookieStruct& aCookieStruct) -> bool { return CookieParser::CheckCookieStruct(aCookieStruct, aCookieURI, ""_ns, domain, requireMatch, false) == CookieParser::NoRejection; @@ -521,7 +520,8 @@ bool CookieStoreParent::DeleteRequestOnMainThread( notificationWatcher->CallbackWhenNotified(aOperationID, notificationCb); rv = cookieManager->RemoveNative(cookie->Host(), matchName, cookie->Path(), - &attrs, &aOperationID); + &attrs, /* from http: */ false, + &aOperationID); if (NS_WARN_IF(NS_FAILED(rv))) { return false; } diff --git a/netwerk/cookie/CookieService.cpp b/netwerk/cookie/CookieService.cpp index 9f16fa5f4f7c..90db6f7fde67 100644 --- a/netwerk/cookie/CookieService.cpp +++ b/netwerk/cookie/CookieService.cpp @@ -722,7 +722,7 @@ CookieService::Add(const nsACString& aHost, const nsACString& aPath, return AddNative(nullptr, aHost, aPath, aName, aValue, aIsSecure, aIsHttpOnly, aIsSession, aExpiry, &attrs, aSameSite, aSchemeMap, - aIsPartitioned, nullptr, + aIsPartitioned, /* from-http: */ true, nullptr, [](CookieStruct&) -> bool { return true; }); } @@ -733,7 +733,7 @@ CookieService::AddNative(nsIURI* aCookieURI, const nsACString& aHost, bool aIsHttpOnly, bool aIsSession, int64_t aExpiry, OriginAttributes* aOriginAttributes, int32_t aSameSite, nsICookie::schemeType aSchemeMap, bool aIsPartitioned, - const nsID* aOperationID, + bool aFromHttp, const nsID* aOperationID, const std::function& aCheck) { if (NS_WARN_IF(!aOriginAttributes)) { return NS_ERROR_FAILURE; @@ -770,7 +770,7 @@ CookieService::AddNative(nsIURI* aCookieURI, const nsACString& aHost, CookieStorage* storage = PickStorage(*aOriginAttributes); storage->AddCookie(nullptr, baseDomain, *aOriginAttributes, cookie, - currentTimeInUsec, aCookieURI, VoidCString(), true, + currentTimeInUsec, aCookieURI, VoidCString(), aFromHttp, !aOriginAttributes->mPartitionKey.IsEmpty(), nullptr, aOperationID); return NS_OK; @@ -779,7 +779,7 @@ CookieService::AddNative(nsIURI* aCookieURI, const nsACString& aHost, nsresult CookieService::Remove(const nsACString& aHost, const OriginAttributes& aAttrs, const nsACString& aName, const nsACString& aPath, - const nsID* aOperationID) { + bool aFromHttp, const nsID* aOperationID) { // first, normalize the hostname, and fail if it contains illegal characters. nsAutoCString host(aHost); nsresult rv = NormalizeHost(host); @@ -797,7 +797,7 @@ nsresult CookieService::Remove(const nsACString& aHost, CookieStorage* storage = PickStorage(aAttrs); storage->RemoveCookie(baseDomain, aAttrs, host, PromiseFlatCString(aName), - PromiseFlatCString(aPath), aOperationID); + PromiseFlatCString(aPath), aFromHttp, aOperationID); return NS_OK; } @@ -812,19 +812,21 @@ CookieService::Remove(const nsACString& aHost, const nsACString& aName, return NS_ERROR_INVALID_ARG; } - return RemoveNative(aHost, aName, aPath, &attrs, nullptr); + return RemoveNative(aHost, aName, aPath, &attrs, /* from http: */ true, + nullptr); } NS_IMETHODIMP_(nsresult) CookieService::RemoveNative(const nsACString& aHost, const nsACString& aName, const nsACString& aPath, - OriginAttributes* aOriginAttributes, + OriginAttributes* aOriginAttributes, bool aFromHttp, const nsID* aOperationID) { if (NS_WARN_IF(!aOriginAttributes)) { return NS_ERROR_FAILURE; } - nsresult rv = Remove(aHost, *aOriginAttributes, aName, aPath, aOperationID); + nsresult rv = + Remove(aHost, *aOriginAttributes, aName, aPath, aFromHttp, aOperationID); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } @@ -1486,7 +1488,8 @@ class RemoveAllSinceRunnable : public Runnable { auto* cookie = static_cast(mList[mIndex].get()); if (cookie->CreationTime() > mSinceWhen && NS_FAILED(mSelf->Remove(cookie->Host(), cookie->OriginAttributesRef(), - cookie->Name(), cookie->Path(), nullptr))) { + cookie->Name(), cookie->Path(), + /* from http: */ true, nullptr))) { continue; } } diff --git a/netwerk/cookie/CookieService.h b/netwerk/cookie/CookieService.h index e787809d1a15..b2b32483c161 100644 --- a/netwerk/cookie/CookieService.h +++ b/netwerk/cookie/CookieService.h @@ -98,7 +98,7 @@ class CookieService final : public nsICookieService, */ nsresult Remove(const nsACString& aHost, const OriginAttributes& aAttrs, const nsACString& aName, const nsACString& aPath, - const nsID* aOperationID); + bool aFromHttp, const nsID* aOperationID); bool SetCookiesFromIPC(const nsACString& aBaseDomain, const OriginAttributes& aAttrs, nsIURI* aHostURI, diff --git a/netwerk/cookie/CookieStorage.cpp b/netwerk/cookie/CookieStorage.cpp index 7b3f48e35866..1646042492d2 100644 --- a/netwerk/cookie/CookieStorage.cpp +++ b/netwerk/cookie/CookieStorage.cpp @@ -322,13 +322,19 @@ void CookieStorage::RemoveCookie(const nsACString& aBaseDomain, const OriginAttributes& aOriginAttributes, const nsACString& aHost, const nsACString& aName, - const nsACString& aPath, + const nsACString& aPath, bool aFromHttp, const nsID* aOperationID) { CookieListIter matchIter{}; RefPtr cookie; if (FindCookie(aBaseDomain, aOriginAttributes, aHost, aName, aPath, matchIter)) { cookie = matchIter.Cookie(); + + // If the old cookie is httponly, make sure we're not coming from script. + if (cookie && !aFromHttp && cookie->IsHttpOnly()) { + return; + } + RemoveCookieFromList(matchIter); } diff --git a/netwerk/cookie/CookieStorage.h b/netwerk/cookie/CookieStorage.h index ae7cb41338f3..09f7ea787328 100644 --- a/netwerk/cookie/CookieStorage.h +++ b/netwerk/cookie/CookieStorage.h @@ -110,7 +110,8 @@ class CookieStorage : public nsIObserver, public nsSupportsWeakReference { void RemoveCookie(const nsACString& aBaseDomain, const OriginAttributes& aOriginAttributes, const nsACString& aHost, const nsACString& aName, - const nsACString& aPath, const nsID* aOperationID); + const nsACString& aPath, bool aFromHttp, + const nsID* aOperationID); virtual void RemoveCookiesWithOriginAttributes( const OriginAttributesPattern& aPattern, const nsACString& aBaseDomain); diff --git a/netwerk/cookie/nsICookieManager.idl b/netwerk/cookie/nsICookieManager.idl index e9da30d93bef..202f6f3c5330 100644 --- a/netwerk/cookie/nsICookieManager.idl +++ b/netwerk/cookie/nsICookieManager.idl @@ -88,7 +88,8 @@ interface nsICookieManager : nsISupports in ACString aName, in AUTF8String aPath, in OriginAttributesPtr aOriginAttributes, - in nsIDPtr aOperationID); + in boolean aFromHttp, + in nsIDPtr aOperationID); /** * Add a cookie. nsICookieService is the normal way to do this. This @@ -158,6 +159,7 @@ interface nsICookieManager : nsISupports in int32_t aSameSite, in nsICookie_schemeType aSchemeMap, in boolean aIsPartitioned, + in boolean aFromHttp, in nsIDPtr aOperationID, in CheckStructFunctionRef aCheckValid); diff --git a/netwerk/test/gtest/TestCookie.cpp b/netwerk/test/gtest/TestCookie.cpp index 8c04bfa9c6ae..208d1ce5c70a 100644 --- a/netwerk/test/gtest/TestCookie.cpp +++ b/netwerk/test/gtest/TestCookie.cpp @@ -771,6 +771,7 @@ TEST(TestCookie, TestCookieMain) &attrs, // originAttributes nsICookie::SAMESITE_NONE, nsICookie::SCHEME_HTTPS, false, // is partitioned + true, // from http nullptr, // operation ID [](CookieStruct&) -> bool { return true; }))); EXPECT_TRUE(NS_SUCCEEDED( @@ -786,6 +787,7 @@ TEST(TestCookie, TestCookieMain) &attrs, // originAttributes nsICookie::SAMESITE_NONE, nsICookie::SCHEME_HTTPS, false, // is partitioned + true, // from http nullptr, // operation ID [](CookieStruct&) -> bool { return true; }))); EXPECT_TRUE(NS_SUCCEEDED( @@ -801,6 +803,7 @@ TEST(TestCookie, TestCookieMain) &attrs, // originAttributes nsICookie::SAMESITE_NONE, nsICookie::SCHEME_HTTPS, false, // is partitioned + true, // from http nullptr, // operation ID [](CookieStruct&) -> bool { return true; }))); // confirm using enumerator diff --git a/testing/web-platform/tests/cookie-store/httponly_cookies.https.window.js b/testing/web-platform/tests/cookie-store/httponly_cookies.https.window.js index 8a10e358ef6d..605e94e67440 100644 --- a/testing/web-platform/tests/cookie-store/httponly_cookies.https.window.js +++ b/testing/web-platform/tests/cookie-store/httponly_cookies.https.window.js @@ -67,3 +67,33 @@ cookie_test(async t => { 'cookie1=value1; cookie2=value2; cookie3=value3', 'httpOnly is not an option for CookieStore.set()'); }, 'HttpOnly cookies can not be set by CookieStore'); + +cookie_test(async t => { + await setCookieStringHttp('HTTPONLY-cookie=value; path=/; httponly'); + assert_equals( + await getCookieString(), + undefined, + 'HttpOnly cookie we wrote using HTTP in cookie jar' + + ' is invisible to script'); + assert_equals( + await getCookieStringHttp(), + 'HTTPONLY-cookie=value', + 'HttpOnly cookie we wrote using HTTP in HTTP cookie jar'); + + try { + await cookieStore.set('HTTPONLY-cookie', 'dummy'); + } catch(e) {} + + assert_equals( + await getCookieString(), + undefined, + 'HttpOnly cookie is not overwritten'); + + try { + await cookieStore.delete('HTTPONLY-cookie'); + } catch(e) {} + + assert_equals(await getCookieString(), undefined, 'HttpOnly cookie is not overwritten'); + + assert_equals(await getCookieStringHttp(), 'HTTPONLY-cookie=value', 'HttpOnly cookie is not deleted'); +}, 'HttpOnly cookies are not deleted/overwritten'); diff --git a/toolkit/components/cookiebanners/nsCookieInjector.cpp b/toolkit/components/cookiebanners/nsCookieInjector.cpp index 02789eb62c31..f67af1ee121f 100644 --- a/toolkit/components/cookiebanners/nsCookieInjector.cpp +++ b/toolkit/components/cookiebanners/nsCookieInjector.cpp @@ -327,7 +327,7 @@ nsresult nsCookieInjector::InjectCookiesFromRules( nullptr, c.Host(), c.Path(), c.Name(), c.Value(), c.IsSecure(), c.IsHttpOnly(), c.IsSession(), c.Expiry(), &aOriginAttributes, c.SameSite(), static_cast(c.SchemeMap()), - /* is partitioned: */ false, nullptr, + /* is partitioned: */ false, /* is from http: */ true, nullptr, [](mozilla::net::CookieStruct&) { return true; }); NS_ENSURE_SUCCESS(rv, rv);