From 382a3aa03026cd3bbee2ccd0a67b050a14638033 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Wed, 5 Jun 2019 12:19:54 +0000 Subject: [PATCH] Bug 1551798 - Store sameSite value as received from the wire in the database - migration 10, r=Ehsan,robwu Differential Revision: https://phabricator.services.mozilla.com/D32482 --HG-- extra : moz-landing-system : lando --- netwerk/cookie/CookieServiceChild.cpp | 9 ++- netwerk/cookie/nsCookie.cpp | 22 ++++-- netwerk/cookie/nsCookie.h | 7 +- netwerk/cookie/nsCookieService.cpp | 76 ++++++++++++++++--- netwerk/cookie/test/unit/test_bug1321912.js | 2 +- netwerk/ipc/NeckoChannelParams.ipdlh | 1 + .../test/unit/test_cookies_sync_failure.js | 2 +- 7 files changed, 95 insertions(+), 24 deletions(-) diff --git a/netwerk/cookie/CookieServiceChild.cpp b/netwerk/cookie/CookieServiceChild.cpp index 9906badd19b9..e4818e985f16 100644 --- a/netwerk/cookie/CookieServiceChild.cpp +++ b/netwerk/cookie/CookieServiceChild.cpp @@ -104,7 +104,8 @@ void CookieServiceChild::MoveCookies() { cookie->Name(), cookie->Value(), cookie->Host(), cookie->Path(), cookie->Expiry(), cookie->LastAccessed(), cookie->CreationTime(), cookie->IsSession(), cookie->IsSecure(), cookie->IsHttpOnly(), - cookie->OriginAttributesRef(), cookie->SameSite()); + cookie->OriginAttributesRef(), cookie->SameSite(), + cookie->RawSameSite()); newCookiesList.AppendElement(newCookie); } cookiesList->SwapElements(newCookiesList); @@ -208,7 +209,7 @@ mozilla::ipc::IPCResult CookieServiceChild::RecvAddCookie( aCookie.name(), aCookie.value(), aCookie.host(), aCookie.path(), aCookie.expiry(), aCookie.lastAccessed(), aCookie.creationTime(), aCookie.isSession(), aCookie.isSecure(), aCookie.isHttpOnly(), aAttrs, - aCookie.sameSite()); + aCookie.sameSite(), aCookie.rawSameSite()); RecordDocumentCookie(cookie, aAttrs); return IPC_OK(); } @@ -232,7 +233,7 @@ mozilla::ipc::IPCResult CookieServiceChild::RecvTrackCookiesLoad( aCookiesList[i].path(), aCookiesList[i].expiry(), aCookiesList[i].lastAccessed(), aCookiesList[i].creationTime(), aCookiesList[i].isSession(), aCookiesList[i].isSecure(), false, aAttrs, - aCookiesList[i].sameSite()); + aCookiesList[i].sameSite(), aCookiesList[i].rawSameSite()); RecordDocumentCookie(cookie, aAttrs); } @@ -381,7 +382,7 @@ void CookieServiceChild::SetCookieInternal( aCookieData.path(), aCookieData.expiry(), currentTimeInUsec, nsCookie::GenerateUniqueCreationTime(currentTimeInUsec), aCookieData.isSession(), aCookieData.isSecure(), aCookieData.isHttpOnly(), - aAttrs, aCookieData.sameSite()); + aAttrs, aCookieData.sameSite(), aCookieData.rawSameSite()); RecordDocumentCookie(cookie, aAttrs); } diff --git a/netwerk/cookie/nsCookie.cpp b/netwerk/cookie/nsCookie.cpp index 52769a9b062e..6b8d0c9463a1 100644 --- a/netwerk/cookie/nsCookie.cpp +++ b/netwerk/cookie/nsCookie.cpp @@ -41,11 +41,12 @@ already_AddRefed nsCookie::Create( const nsACString& aName, const nsACString& aValue, const nsACString& aHost, const nsACString& aPath, int64_t aExpiry, int64_t aLastAccessed, int64_t aCreationTime, bool aIsSession, bool aIsSecure, bool aIsHttpOnly, - const OriginAttributes& aOriginAttributes, int32_t aSameSite) { + const OriginAttributes& aOriginAttributes, int32_t aSameSite, + int32_t aRawSameSite) { mozilla::net::CookieStruct cookieData( nsCString(aName), nsCString(aValue), nsCString(aHost), nsCString(aPath), aExpiry, aLastAccessed, aCreationTime, aIsHttpOnly, aIsSession, aIsSecure, - aSameSite); + aSameSite, aRawSameSite); return Create(cookieData, aOriginAttributes); } @@ -60,18 +61,23 @@ already_AddRefed nsCookie::Create( UTF_8_ENCODING->DecodeWithoutBOMHandling(aCookieData.value(), cookie->mData.value()); - // If the creationTime given to us is higher than the running maximum, update - // our maximum. + // If the creationTime given to us is higher than the running maximum, + // update our maximum. if (cookie->mData.creationTime() > gLastCreationTime) { gLastCreationTime = cookie->mData.creationTime(); } - // If aSameSite is not a sensible value, assume strict + // If sameSite is not a sensible value, assume strict if (cookie->mData.sameSite() < 0 || cookie->mData.sameSite() > nsICookie::SAMESITE_STRICT) { cookie->mData.sameSite() = nsICookie::SAMESITE_STRICT; } + // If rawSameSite is not a sensible value, assume equal to sameSite. + if (!nsCookie::ValidateRawSame(cookie->mData)) { + cookie->mData.rawSameSite() = nsICookie::SAMESITE_NONE; + } + return cookie.forget(); } @@ -172,4 +178,10 @@ nsCookie::GetExpires(uint64_t* aExpires) { return NS_OK; } +// static +bool nsCookie::ValidateRawSame(const mozilla::net::CookieStruct& aCookieData) { + return aCookieData.rawSameSite() == aCookieData.sameSite() || + aCookieData.rawSameSite() == nsICookie::SAMESITE_NONE; +} + NS_IMPL_ISUPPORTS(nsCookie, nsICookie) diff --git a/netwerk/cookie/nsCookie.h b/netwerk/cookie/nsCookie.h index b9e50540321e..770a24e55b0e 100644 --- a/netwerk/cookie/nsCookie.h +++ b/netwerk/cookie/nsCookie.h @@ -40,6 +40,9 @@ class nsCookie final : public nsICookie { : mData(aCookieData), mOriginAttributes(aOriginAttributes) {} public: + // Returns false if rawSameSite has an invalid value, compared to sameSite. + static bool ValidateRawSame(const mozilla::net::CookieStruct& aCookieData); + // Generate a unique and monotonically increasing creation time. See comment // in nsCookie.cpp. static int64_t GenerateUniqueCreationTime(int64_t aCreationTime); @@ -50,7 +53,8 @@ class nsCookie final : public nsICookie { const nsACString& aHost, const nsACString& aPath, int64_t aExpiry, int64_t aLastAccessed, int64_t aCreationTime, bool aIsSession, bool aIsSecure, bool aIsHttpOnly, - const OriginAttributes& aOriginAttributes, int32_t aSameSite); + const OriginAttributes& aOriginAttributes, int32_t aSameSite, + int32_t aRawSameSite); static already_AddRefed Create( const mozilla::net::CookieStruct& aCookieData, @@ -81,6 +85,7 @@ class nsCookie final : public nsICookie { return mOriginAttributes; } inline int32_t SameSite() const { return mData.sameSite(); } + inline int32_t RawSameSite() const { return mData.rawSameSite(); } // setters inline void SetExpiry(int64_t aExpiry) { mData.expiry() = aExpiry; } diff --git a/netwerk/cookie/nsCookieService.cpp b/netwerk/cookie/nsCookieService.cpp index 7340f91d5416..745b43ac37f8 100644 --- a/netwerk/cookie/nsCookieService.cpp +++ b/netwerk/cookie/nsCookieService.cpp @@ -87,7 +87,7 @@ static StaticRefPtr gCookieService; #define HTTP_ONLY_PREFIX "#HttpOnly_" #define COOKIES_FILE "cookies.sqlite" -#define COOKIES_SCHEMA_VERSION 9 +#define COOKIES_SCHEMA_VERSION 10 // parameter indexes; see |Read| #define IDX_NAME 0 @@ -102,6 +102,7 @@ static StaticRefPtr gCookieService; #define IDX_BASE_DOMAIN 9 #define IDX_ORIGIN_ATTRIBUTES 10 #define IDX_SAME_SITE 11 +#define IDX_RAW_SAME_SITE 12 #define TOPIC_CLEAR_ORIGIN_DATA "clear-origin-attributes-data" @@ -1212,7 +1213,24 @@ OpenDBResult nsCookieService::TryInitDB(bool aRecreateDB) { NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Create a new_moz_cookies table without the appId field. - rv = CreateTableWorker("new_moz_cookies"); + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL( + NS_LITERAL_CSTRING("CREATE TABLE new_moz_cookies(" + "id INTEGER PRIMARY KEY, " + "baseDomain TEXT, " + "originAttributes TEXT NOT NULL DEFAULT '', " + "name TEXT, " + "value TEXT, " + "host TEXT, " + "path TEXT, " + "expiry INTEGER, " + "lastAccessed INTEGER, " + "creationTime INTEGER, " + "isSecure INTEGER, " + "isHttpOnly INTEGER, " + "inBrowserElement INTEGER DEFAULT 0, " + "CONSTRAINT moz_uniqueid UNIQUE (name, host, " + "path, originAttributes)" + ")")); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); // Move the data over. @@ -1271,15 +1289,32 @@ OpenDBResult nsCookieService::TryInitDB(bool aRecreateDB) { // Add the sameSite column to the table. rv = mDefaultDBState->syncConn->ExecuteSimpleSQL( NS_LITERAL_CSTRING("ALTER TABLE moz_cookies ADD sameSite INTEGER")); + NS_ENSURE_SUCCESS(rv, RESULT_RETRY); + COOKIE_LOGSTRING(LogLevel::Debug, ("Upgraded database to schema version 9")); } + MOZ_FALLTHROUGH; + + case 9: { + // Add the rawSameSite column to the table. + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + "ALTER TABLE moz_cookies ADD rawSameSite INTEGER")); + NS_ENSURE_SUCCESS(rv, RESULT_RETRY); + + // Copy the current sameSite value into rawSameSite. + rv = mDefaultDBState->syncConn->ExecuteSimpleSQL(NS_LITERAL_CSTRING( + "UPDATE moz_cookies SET rawSameSite = sameSite")); + NS_ENSURE_SUCCESS(rv, RESULT_RETRY); + + COOKIE_LOGSTRING(LogLevel::Debug, + ("Upgraded database to schema version 10")); // No more upgrades. Update the schema version. rv = mDefaultDBState->syncConn->SetSchemaVersion(COOKIES_SCHEMA_VERSION); NS_ENSURE_SUCCESS(rv, RESULT_RETRY); - + } MOZ_FALLTHROUGH; case COOKIES_SCHEMA_VERSION: @@ -1323,7 +1358,8 @@ OpenDBResult nsCookieService::TryInitDB(bool aRecreateDB) { "creationTime, " "isSecure, " "isHttpOnly, " - "sameSite " + "sameSite, " + "rawSameSite " "FROM moz_cookies"), getter_AddRefs(stmt)); if (NS_SUCCEEDED(rv)) break; @@ -1396,7 +1432,7 @@ void nsCookieService::InitDBConn() { tuple.cookie->lastAccessed(), tuple.cookie->creationTime(), tuple.cookie->isSession(), tuple.cookie->isSecure(), tuple.cookie->isHttpOnly(), tuple.originAttributes, - tuple.cookie->sameSite()); + tuple.cookie->sameSite(), tuple.cookie->rawSameSite()); AddCookieToList(tuple.key, cookie, mDefaultDBState, nullptr, false); } @@ -1470,7 +1506,8 @@ nsresult nsCookieService::InitDBConnInternal() { "creationTime, " "isSecure, " "isHttpOnly, " - "sameSite " + "sameSite, " + "rawSameSite " ") VALUES (" ":baseDomain, " ":originAttributes, " @@ -1483,7 +1520,8 @@ nsresult nsCookieService::InitDBConnInternal() { ":creationTime, " ":isSecure, " ":isHttpOnly, " - ":sameSite" + ":sameSite, " + ":rawSameSite " ")"), getter_AddRefs(mDefaultDBState->stmtInsert)); NS_ENSURE_SUCCESS(rv, rv); @@ -1527,6 +1565,7 @@ nsresult nsCookieService::CreateTableWorker(const char* aName) { "isHttpOnly INTEGER, " "inBrowserElement INTEGER DEFAULT 0, " "sameSite INTEGER DEFAULT 0, " + "rawSameSite INTEGER DEFAULT 0, " "CONSTRAINT moz_uniqueid UNIQUE (name, host, path, originAttributes)" ")"); return mDefaultDBState->syncConn->ExecuteSimpleSQL(command); @@ -2466,7 +2505,7 @@ nsCookieService::AddNative(const nsACString& aHost, const nsACString& aPath, RefPtr cookie = nsCookie::Create( aName, aValue, host, aPath, aExpiry, currentTimeInUsec, nsCookie::GenerateUniqueCreationTime(currentTimeInUsec), aIsSession, - aIsSecure, aIsHttpOnly, key.mOriginAttributes, aSameSite); + aIsSecure, aIsHttpOnly, key.mOriginAttributes, aSameSite, aSameSite); if (!cookie) { return NS_ERROR_OUT_OF_MEMORY; } @@ -2584,11 +2623,12 @@ mozilla::UniquePtr nsCookieService::GetCookieFromRow( bool isSecure = 0 != aRow->AsInt32(IDX_SECURE); bool isHttpOnly = 0 != aRow->AsInt32(IDX_HTTPONLY); int32_t sameSite = aRow->AsInt32(IDX_SAME_SITE); + int32_t rawSameSite = aRow->AsInt32(IDX_RAW_SAME_SITE); // Create a new constCookie and assign the data. return mozilla::MakeUnique( name, value, host, path, expiry, lastAccessed, creationTime, isHttpOnly, - false, isSecure, sameSite); + false, isSecure, sameSite, rawSameSite); } void nsCookieService::EnsureReadComplete(bool aInitDBConn) { @@ -2663,7 +2703,8 @@ OpenDBResult nsCookieService::Read() { "isHttpOnly, " "baseDomain, " "originAttributes, " - "sameSite " + "sameSite, " + "rawSameSite " "FROM moz_cookies " "WHERE baseDomain NOTNULL"), getter_AddRefs(stmt)); @@ -2844,7 +2885,8 @@ nsCookieService::ImportCookies(nsIFile* aCookieFile) { nsCookie::GenerateUniqueCreationTime(currentTimeInUsec), false, Substring(buffer, secureIndex, expiresIndex - secureIndex - 1) .EqualsLiteral(kTrue), - isHttpOnly, key.mOriginAttributes, nsICookie::SAMESITE_NONE); + isHttpOnly, key.mOriginAttributes, nsICookie::SAMESITE_NONE, + nsICookie::SAMESITE_NONE); if (!newCookie) { return NS_ERROR_OUT_OF_MEMORY; } @@ -3354,7 +3396,7 @@ bool nsCookieService::SetCookieInternal( cookieData.path(), cookieData.expiry(), currentTimeInUsec, nsCookie::GenerateUniqueCreationTime(currentTimeInUsec), cookieData.isSession(), cookieData.isSecure(), cookieData.isHttpOnly(), - aKey.mOriginAttributes, cookieData.sameSite()); + aKey.mOriginAttributes, cookieData.sameSite(), cookieData.rawSameSite()); if (!cookie) return newCookie; // check permissions from site permission list, or ask the user, @@ -3745,6 +3787,7 @@ bool nsCookieService::ParseAttributes(nsCString& aCookieHeader, aCookieData.isSecure() = false; aCookieData.isHttpOnly() = false; aCookieData.sameSite() = nsICookie::SAMESITE_NONE; + aCookieData.rawSameSite() = nsICookie::SAMESITE_NONE; if (StaticPrefs::network_cookie_sameSite_laxByDefault()) { aCookieData.sameSite() = nsICookie::SAMESITE_LAX; @@ -3803,10 +3846,13 @@ bool nsCookieService::ParseAttributes(nsCString& aCookieHeader, else if (tokenString.LowerCaseEqualsLiteral(kSameSite)) { if (tokenValue.LowerCaseEqualsLiteral(kSameSiteLax)) { aCookieData.sameSite() = nsICookie::SAMESITE_LAX; + aCookieData.rawSameSite() = nsICookie::SAMESITE_LAX; } else if (tokenValue.LowerCaseEqualsLiteral(kSameSiteStrict)) { aCookieData.sameSite() = nsICookie::SAMESITE_STRICT; + aCookieData.rawSameSite() = nsICookie::SAMESITE_STRICT; } else if (tokenValue.LowerCaseEqualsLiteral(kSameSiteNone)) { aCookieData.sameSite() = nsICookie::SAMESITE_NONE; + aCookieData.rawSameSite() = nsICookie::SAMESITE_NONE; } } } @@ -3825,6 +3871,8 @@ bool nsCookieService::ParseAttributes(nsCString& aCookieHeader, // re-assign aCookieHeader, in case we need to process another cookie aCookieHeader.Assign(Substring(cookieStart, cookieEnd)); + + MOZ_ASSERT(nsCookie::ValidateRawSame(aCookieData)); return newCookie; } @@ -4999,6 +5047,10 @@ void bindCookieParameters(mozIStorageBindingParamsArray* aParamsArray, aCookie->SameSite()); NS_ASSERT_SUCCESS(rv); + rv = params->BindInt32ByName(NS_LITERAL_CSTRING("rawSameSite"), + aCookie->RawSameSite()); + NS_ASSERT_SUCCESS(rv); + // Bind the params to the array. rv = aParamsArray->AddParams(params); NS_ASSERT_SUCCESS(rv); diff --git a/netwerk/cookie/test/unit/test_bug1321912.js b/netwerk/cookie/test/unit/test_bug1321912.js index da06350e7b3a..4dff0eb3a3e8 100644 --- a/netwerk/cookie/test/unit/test_bug1321912.js +++ b/netwerk/cookie/test/unit/test_bug1321912.js @@ -49,7 +49,7 @@ conn.executeSimpleSQL("INSERT INTO moz_cookies(" + // Get sessionEnumerator to wait for the initialization in cookie thread const enumerator = Services.cookies.sessionEnumerator; -Assert.equal(conn.schemaVersion, 9); +Assert.equal(conn.schemaVersion, 10); let stmt = conn.createStatement("SELECT sql FROM sqlite_master " + "WHERE type = 'table' AND " + " name = 'moz_cookies'"); diff --git a/netwerk/ipc/NeckoChannelParams.ipdlh b/netwerk/ipc/NeckoChannelParams.ipdlh index 258fd0f3c4f4..a2cb09777c8a 100644 --- a/netwerk/ipc/NeckoChannelParams.ipdlh +++ b/netwerk/ipc/NeckoChannelParams.ipdlh @@ -332,6 +332,7 @@ struct CookieStruct bool isSession; bool isSecure; int32_t sameSite; + int32_t rawSameSite; }; } // namespace ipc diff --git a/netwerk/test/unit/test_cookies_sync_failure.js b/netwerk/test/unit/test_cookies_sync_failure.js index ebfd1f0d309a..f7fab76df17f 100644 --- a/netwerk/test/unit/test_cookies_sync_failure.js +++ b/netwerk/test/unit/test_cookies_sync_failure.js @@ -18,7 +18,7 @@ // c) Schema 3: the 'creationTime' column already exists; or the // 'moz_uniqueid' index already exists. -var COOKIE_DATABASE_SCHEMA_CURRENT = 9; +var COOKIE_DATABASE_SCHEMA_CURRENT = 10; var test_generator = do_run_test();