From ff25742a1d5b47c2ada1b3fcce58b898fd03d4de Mon Sep 17 00:00:00 2001 From: Andreas Farre Date: Wed, 8 Feb 2023 12:57:09 +0000 Subject: [PATCH] Bug 1795312 - Don't Base64 encode principals when serializing. r=peterv Base64 encoding is done to top-level principal and the sub-principals of expanded principals. Getting rid of the Base64 encoding entirely also lets us use jsoncpp consider expanded principals to be entirely serialized as JSON. Differential Revision: https://phabricator.services.mozilla.com/D166783 --- .../browser_principalSerialization_json.js | 16 +--- caps/BasePrincipal.cpp | 89 +++++++++++++++---- caps/BasePrincipal.h | 3 + caps/ExpandedPrincipal.cpp | 72 +++++++++++---- caps/ExpandedPrincipal.h | 7 ++ .../gtest/TestPrincipalSerialization.cpp | 61 +++++++++++-- dom/base/nsContentUtils.cpp | 14 ++- toolkit/modules/E10SUtils.sys.mjs | 50 +++++++---- 8 files changed, 231 insertions(+), 81 deletions(-) diff --git a/browser/base/content/test/caps/browser_principalSerialization_json.js b/browser/base/content/test/caps/browser_principalSerialization_json.js index d0a8bed35d31..4c7fce892598 100644 --- a/browser/base/content/test/caps/browser_principalSerialization_json.js +++ b/browser/base/content/test/caps/browser_principalSerialization_json.js @@ -54,7 +54,7 @@ add_task(async function test_nullPrincipal() { let p = Services.scriptSecurityManager.createNullPrincipal(test.input.OA); let sp = E10SUtils.serializePrincipal(p); // Not sure why cppjson is adding a \n here - let spr = atob(sp).replace(nullReplaceRegex, NULL_REPLACE); + let spr = sp.replace(nullReplaceRegex, NULL_REPLACE); is( test.expected, spr, @@ -125,16 +125,7 @@ add_task(async function test_contentPrincipal() { test.input.OA ); let sp = E10SUtils.serializePrincipal(p); - is( - test.expected, - atob(sp), - "Expected serialized object for " + test.input.uri - ); - is( - btoa(test.expected), - sp, - "Expected serialized string for " + test.input.uri - ); + is(test.expected, sp, "Expected serialized object for " + test.input.uri); let dp = E10SUtils.deserializePrincipal(sp); is(dp.URI.spec, test.input.uri, "Ensure spec is the same"); @@ -159,8 +150,7 @@ add_task(async function test_systemPrincipal() { let p = Services.scriptSecurityManager.getSystemPrincipal(); let sp = E10SUtils.serializePrincipal(p); - is(expected, atob(sp), "Expected serialized object for system principal"); - is(btoa(expected), sp, "Expected serialized string for system principal"); + is(expected, sp, "Expected serialized object for system principal"); let dp = E10SUtils.deserializePrincipal(sp); is( dp, diff --git a/caps/BasePrincipal.cpp b/caps/BasePrincipal.cpp index 97f066255eeb..a9e3a4be818d 100644 --- a/caps/BasePrincipal.cpp +++ b/caps/BasePrincipal.cpp @@ -278,12 +278,53 @@ already_AddRefed BasePrincipal::FromJSON( return nullptr; } + return FromJSON(root); +} + +// Checks if an ExpandedPrincipal is using the legacy format, where +// sub-principals are Base64 encoded. +// +// Given a legacy expanded principal: +// +// * +// {"2": {"0": "eyIxIjp7IjAiOiJodHRwczovL2EuY29tLyJ9fQ=="}} +// | | | +// | ---------- Value +// | | +// PrincipalKind | +// | +// SerializableKeys +// +// The value is a CSV list of Base64 encoded prinipcals. The new format for this +// principal is: +// +// Subsumed principals +// | +// ------------------------------------ +// * | | +// {"2": {"0": [{"1": {"0": https://mozilla.com"}}]}} +// | | | +// -------------- Value +// | +// PrincipalKind +// +// It is possible to tell these apart by checking the type of the property noted +// in both diagrams with an asterisk. In the legacy format the type will be a +// string and in the new format it will be an array. +static bool IsLegacyFormat(const Json::Value& aValue) { + const auto& specs = std::to_string(ExpandedPrincipal::eSpecs); + return aValue.isMember(specs) && aValue[specs].isString(); +} + +/* static */ +already_AddRefed BasePrincipal::FromJSON( + const Json::Value& aJSON) { int principalKind = -1; - const Json::Value* value = GetPrincipalObject(root, principalKind); + const Json::Value* value = GetPrincipalObject(aJSON, principalKind); if (!value) { #ifdef DEBUG fprintf(stderr, "Unexpected JSON principal %s\n", - root.toStyledString().c_str()); + aJSON.toStyledString().c_str()); #endif MOZ_ASSERT(false, "Unexpected JSON to deserialize as a principal"); @@ -310,9 +351,15 @@ already_AddRefed BasePrincipal::FromJSON( } if (principalKind == eExpandedPrincipal) { - nsTArray res = - GetJSONKeys(value); - return ExpandedPrincipal::FromProperties(res); + // Check if expanded principals is stored in the new or the old format. See + // comment for `IsLegacyFormat`. + if (IsLegacyFormat(*value)) { + nsTArray res = + GetJSONKeys(value); + return ExpandedPrincipal::FromProperties(res); + } + + return ExpandedPrincipal::FromProperties(*value); } MOZ_RELEASE_ASSERT(false, "Unexpected enum to deserialize as a principal"); @@ -325,26 +372,36 @@ nsresult BasePrincipal::PopulateJSONObject(Json::Value& aObject) { // Returns a JSON representation of the principal. // Calling BasePrincipal::FromJSON will deserialize the JSON into // the corresponding principal type. -nsresult BasePrincipal::ToJSON(nsACString& aResult) { - MOZ_ASSERT(aResult.IsEmpty(), "ToJSON only supports an empty result input"); - aResult.Truncate(); +nsresult BasePrincipal::ToJSON(nsACString& aJSON) { + MOZ_ASSERT(aJSON.IsEmpty(), "ToJSON only supports an empty result input"); + aJSON.Truncate(); Json::StreamWriterBuilder builder; builder["indentation"] = ""; - Json::Value innerJSONObject = Json::objectValue; - - nsresult rv = PopulateJSONObject(innerJSONObject); - NS_ENSURE_SUCCESS(rv, rv); + builder["emitUTF8"] = true; Json::Value root = Json::objectValue; - std::string key = std::to_string(Kind()); - root[key] = innerJSONObject; + nsresult rv = ToJSON(root); + NS_ENSURE_SUCCESS(rv, rv); + std::string result = Json::writeString(builder, root); - aResult.Append(result); - if (aResult.Length() == 0) { + aJSON.Append(result); + if (aJSON.Length() == 0) { MOZ_ASSERT(false, "JSON writer failed to output a principal serialization"); return NS_ERROR_UNEXPECTED; } + + return NS_OK; +} + +nsresult BasePrincipal::ToJSON(Json::Value& aObject) { + Json::Value innerJSONObject = Json::objectValue; + nsresult rv = PopulateJSONObject(innerJSONObject); + NS_ENSURE_SUCCESS(rv, rv); + + std::string key = std::to_string(Kind()); + aObject[key] = innerJSONObject; + return NS_OK; } diff --git a/caps/BasePrincipal.h b/caps/BasePrincipal.h index 3b845cb7b43d..921cba501683 100644 --- a/caps/BasePrincipal.h +++ b/caps/BasePrincipal.h @@ -188,7 +188,10 @@ class BasePrincipal : public nsJSPrincipals { NS_IMETHOD GetPrecursorPrincipal(nsIPrincipal** aPrecursor) override; nsresult ToJSON(nsACString& aJSON); + nsresult ToJSON(Json::Value& aObject); + static already_AddRefed FromJSON(const nsACString& aJSON); + static already_AddRefed FromJSON(const Json::Value& aJSON); // Method populates a passed Json::Value with serializable fields // which represent all of the fields to deserialize the principal virtual nsresult PopulateJSONObject(Json::Value& aObject); diff --git a/caps/ExpandedPrincipal.cpp b/caps/ExpandedPrincipal.cpp index f71344a3a646..d08e3841ea0a 100644 --- a/caps/ExpandedPrincipal.cpp +++ b/caps/ExpandedPrincipal.cpp @@ -291,23 +291,16 @@ nsresult ExpandedPrincipal::GetSiteIdentifier(SiteIdentifier& aSite) { } nsresult ExpandedPrincipal::PopulateJSONObject(Json::Value& aObject) { - nsAutoCString principalList; - // First item through we have a blank separator and append the next result - nsAutoCString sep; - for (auto& principal : mPrincipals) { - nsAutoCString JSON; - BasePrincipal::Cast(principal)->ToJSON(JSON); - // This is blank for the first run through so the last in the list doesn't - // add a separator - principalList.Append(sep); - sep = ','; - // Values currently only copes with strings so encode into base64 to allow a - // CSV safely. - nsresult rv; - rv = Base64EncodeAppend(JSON, principalList); + Json::Value principalList = Json::arrayValue; + + for (const auto& principal : mPrincipals) { + Json::Value object = Json::objectValue; + nsresult rv = BasePrincipal::Cast(principal)->ToJSON(object); NS_ENSURE_SUCCESS(rv, rv); + + principalList.append(object); } - aObject[std::to_string(eSpecs)] = principalList.get(); + aObject[std::to_string(eSpecs)] = principalList; nsAutoCString suffix; OriginAttributesRef().CreateSuffix(suffix); @@ -325,6 +318,7 @@ already_AddRefed ExpandedPrincipal::FromProperties( OriginAttributes attrs; // The odd structure here is to make the code to not compile // if all the switch enum cases haven't been codified + for (const auto& field : aFields) { switch (field.key) { case ExpandedPrincipal::eSpecs: @@ -365,6 +359,54 @@ already_AddRefed ExpandedPrincipal::FromProperties( return expandedPrincipal.forget(); } +/* static */ +already_AddRefed ExpandedPrincipal::FromProperties( + const Json::Value& aJSON) { + MOZ_ASSERT(aJSON.size() <= eMax + 1, "Must have at most, all the properties"); + const std::string specs = std::to_string(eSpecs); + const std::string suffix = std::to_string(eSuffix); + MOZ_ASSERT(aJSON.isMember(specs), "The eSpecs member is required"); + MOZ_ASSERT(aJSON.size() == 1 || aJSON.isMember(suffix), + "eSuffix is optional"); + + const auto* specsValue = + aJSON.find(specs.c_str(), specs.c_str() + specs.length()); + if (!specsValue) { + MOZ_ASSERT(false, "Expanded principals require specs in serialized JSON"); + return nullptr; + } + + nsTArray> allowList; + for (const auto& principalJSON : *specsValue) { + if (nsCOMPtr principal = + BasePrincipal::FromJSON(principalJSON)) { + allowList.AppendElement(principal); + } + } + + if (allowList.Length() == 0) { + return nullptr; + } + + OriginAttributes attrs; + if (aJSON.isMember(suffix)) { + const auto& value = aJSON[suffix]; + if (!value.isString()) { + return nullptr; + } + + bool ok = attrs.PopulateFromSuffix(nsDependentCString(value.asCString())); + if (!ok) { + return nullptr; + } + } + + RefPtr expandedPrincipal = + ExpandedPrincipal::Create(allowList, attrs); + + return expandedPrincipal.forget(); +} + NS_IMETHODIMP ExpandedPrincipal::IsThirdPartyURI(nsIURI* aURI, bool* aRes) { // ExpandedPrincipal for extension content scripts consist of two principals, diff --git a/caps/ExpandedPrincipal.h b/caps/ExpandedPrincipal.h index 18f8ff8e4e20..748a0c41385d 100644 --- a/caps/ExpandedPrincipal.h +++ b/caps/ExpandedPrincipal.h @@ -62,9 +62,16 @@ class ExpandedPrincipal : public nsIExpandedPrincipal, enum SerializableKeys : uint8_t { eSpecs = 0, eSuffix, eMax = eSuffix }; typedef mozilla::BasePrincipal::KeyValT KeyVal; + // This is the legacy serializer for expanded principals. See note for + // `IsLegacyFormat` in BasePrincipal.cpp. static already_AddRefed FromProperties( nsTArray& aFields); + // This is the new serializer for expanded principals. See note for + // `IsLegacyFormat` in BasePrincipal.cpp. + static already_AddRefed FromProperties( + const Json::Value& aJSON); + class Deserializer : public BasePrincipal::Deserializer { public: NS_IMETHOD Read(nsIObjectInputStream* aStream) override; diff --git a/caps/tests/gtest/TestPrincipalSerialization.cpp b/caps/tests/gtest/TestPrincipalSerialization.cpp index e3abbdeebf53..e63d57aa6cd8 100644 --- a/caps/tests/gtest/TestPrincipalSerialization.cpp +++ b/caps/tests/gtest/TestPrincipalSerialization.cpp @@ -2,6 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ #include "gtest/gtest.h" +#include "mozilla/Base64.h" #include "mozilla/BasePrincipal.h" #include "mozilla/ContentPrincipal.h" #include "mozilla/NullPrincipal.h" @@ -127,10 +128,9 @@ TEST(PrincipalSerialization, ExpandedPrincipal) nsAutoCString JSON; rv = BasePrincipal::Cast(result)->ToJSON(JSON); ASSERT_EQ(rv, NS_OK); - ASSERT_STREQ( - JSON.get(), - "{\"2\":{\"0\":\"eyIxIjp7IjAiOiJodHRwczovL21vemlsbGEuY29tLyJ9fQ==," - "eyIxIjp7IjAiOiJodHRwczovL21vemlsbGEub3JnLyJ9fQ==\"}}"); + ASSERT_STREQ(JSON.get(), + "{\"2\":{\"0\":[{\"1\":{\"0\":\"https://mozilla.com/" + "\"}},{\"1\":{\"0\":\"https://mozilla.org/\"}}]}}"); nsCOMPtr returnedPrincipal = BasePrincipal::FromJSON(JSON); auto outPrincipal = BasePrincipal::Cast(returnedPrincipal); @@ -190,11 +190,10 @@ TEST(PrincipalSerialization, ExpandedPrincipalOA) nsAutoCString JSON; rv = BasePrincipal::Cast(result)->ToJSON(JSON); ASSERT_EQ(rv, NS_OK); - ASSERT_STREQ( - JSON.get(), - "{\"2\":{\"0\":\"eyIxIjp7IjAiOiJodHRwczovL21vemlsbGEuY29tLyJ9fQ==," - "eyIxIjp7IjAiOiJodHRwczovL21vemlsbGEub3JnLyJ9fQ==\",\"1\":\"^" - "userContextId=1\"}}"); + ASSERT_STREQ(JSON.get(), + "{\"2\":{\"0\":[{\"1\":{\"0\":\"https://mozilla.com/" + "\"}},{\"1\":{\"0\":\"https://mozilla.org/" + "\"}}],\"1\":\"^userContextId=1\"}}"); nsCOMPtr returnedPrincipal = BasePrincipal::FromJSON(JSON); auto outPrincipal = BasePrincipal::Cast(returnedPrincipal); @@ -213,3 +212,47 @@ TEST(PrincipalSerialization, ExpandedPrincipalOA) ASSERT_FALSE(outPrincipal->FastSubsumesIgnoringFPD(principalDev)); } + +static void MeasurePerformance(const std::function& aOldCallback, + const std::function& aNewCallback) { + mozilla::TimeDuration old; + mozilla::TimeStamp then = mozilla::TimeStamp::Now(); + for (uint32_t i = 0; i < 1000; ++i) { + aOldCallback(); + } + old = mozilla::TimeStamp::Now() - then; + + then = mozilla::TimeStamp::Now(); + for (uint32_t i = 0; i < 1000; ++i) { + aNewCallback(); + } + + mozilla::TimeStamp now = mozilla::TimeStamp::Now(); + ASSERT_LT(now - then, old); +} + +static void FromToJSON(const nsCString& aString, const nsCString& aReference) { + nsCOMPtr principal = BasePrincipal::FromJSON(aString); + nsCString result; + ASSERT_EQ(BasePrincipal::Cast(principal)->ToJSON(result), NS_OK); + ASSERT_STREQ(result.get(), aReference.get()); +} + +TEST(PrincipalSerialization, ExpandedPrincipalJsonCpp) +{ + nsCString base64 = + R"({"2":{"0":"eyIxIjp7IjAiOiJodHRwczovL21vemlsbGEuY29tLyJ9fQ==,)"_ns + R"(eyIxIjp7IjAiOiJodHRwczovL21vemlsbGEub3JnLyJ9fQ==","1":"^userContextId=1"}})"_ns; + + nsCString json = + "{\"2\":{\"0\":[{\"1\":{\"0\":\"https://mozilla.com/\"}},"_ns + "{\"1\":{\"0\":\"https://mozilla.org/\"}}],\"1\":\"^userContextId=1\"}}"_ns; + + FromToJSON(base64, json); + + FromToJSON(json, json); + + MeasurePerformance( + [&]() { mozilla::Unused << BasePrincipal::FromJSON(base64); }, + [&]() { mozilla::Unused << BasePrincipal::FromJSON(json); }); +} diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 66838dc51768..7e57bf923da8 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -9952,15 +9952,11 @@ bool nsContentUtils::QueryTriggeringPrincipal( } nsCString binary; - nsresult rv = Base64Decode(NS_ConvertUTF16toUTF8(loadingStr), binary); - if (NS_SUCCEEDED(rv)) { - nsCOMPtr serializedPrin = BasePrincipal::FromJSON(binary); - if (serializedPrin) { - result = true; - serializedPrin.forget(aTriggeringPrincipal); - } - } else { - MOZ_ASSERT(false, "Unable to deserialize base64 principal"); + nsCOMPtr serializedPrin = + BasePrincipal::FromJSON(NS_ConvertUTF16toUTF8(loadingStr)); + if (serializedPrin) { + result = true; + serializedPrin.forget(aTriggeringPrincipal); } if (!result) { diff --git a/toolkit/modules/E10SUtils.sys.mjs b/toolkit/modules/E10SUtils.sys.mjs index 41f4f1f01b19..0e5a2fd5ef7a 100644 --- a/toolkit/modules/E10SUtils.sys.mjs +++ b/toolkit/modules/E10SUtils.sys.mjs @@ -745,15 +745,15 @@ export var E10SUtils = { * Serialize principal data. * * @param {nsIPrincipal} principal The principal to serialize. - * @return {String} The base64 encoded principal data. + * @return {String} The serialized principal data. */ serializePrincipal(principal) { let serializedPrincipal = null; try { if (principal) { - serializedPrincipal = btoa( - Services.scriptSecurityManager.principalToJSON(principal) + serializedPrincipal = Services.scriptSecurityManager.principalToJSON( + principal ); } } catch (e) { @@ -764,14 +764,13 @@ export var E10SUtils = { }, /** - * Deserialize a base64 encoded principal (serialized with - * serializePrincipal). + * Deserialize a principal (serialized with serializePrincipal). * - * @param {String} principal_b64 A base64 encoded serialized principal. + * @param {String} serializedPincipal A serialized principal. * @return {nsIPrincipal} A deserialized principal. */ - deserializePrincipal(principal_b64, fallbackPrincipalCallback = null) { - if (!principal_b64) { + deserializePrincipal(serializedPincipal, fallbackPrincipalCallback = null) { + if (!serializedPincipal) { if (!fallbackPrincipalCallback) { this.log().warn( "No principal passed to deserializePrincipal and no fallbackPrincipalCallback" @@ -784,21 +783,32 @@ export var E10SUtils = { try { let principal; - let tmpa = atob(principal_b64); - // Both the legacy and new JSON representation of principals are stored as base64 - // The new kind are the only ones that will start with "{" when decoded. - // We check here for the new JSON serialized, if it doesn't start with that continue using nsISerializable. - // JSONToPrincipal accepts a *non* base64 encoded string and returns a principal or a null. - if (tmpa.startsWith("{")) { - principal = Services.scriptSecurityManager.JSONToPrincipal(tmpa); + // The current JSON representation of principal is not stored as base64. We start by checking + // if the serialized data starts with '{' to determine if we're using the new JSON representation. + // If it doesn't we try the two legacy formats, old JSON and nsISerializable. + if (serializedPincipal.startsWith("{")) { + principal = Services.scriptSecurityManager.JSONToPrincipal( + serializedPincipal + ); } else { - principal = lazy.serializationHelper.deserializeObject(principal_b64); + // Both the legacy and legacy JSON representation of principals are stored as base64 + // The legacy JSON kind are the only ones that will start with "{" when decoded. + // We check here for the legacy JSON serialized, if it doesn't start with that continue using nsISerializable. + // JSONToPrincipal accepts a *non* base64 encoded string and returns a principal or a null. + let tmpa = atob(serializedPincipal); + if (tmpa.startsWith("{")) { + principal = Services.scriptSecurityManager.JSONToPrincipal(tmpa); + } else { + principal = lazy.serializationHelper.deserializeObject( + serializedPincipal + ); + } } principal.QueryInterface(Ci.nsIPrincipal); return principal; } catch (e) { this.log().error( - `Failed to deserialize principal_b64 '${principal_b64}' ${e}` + `Failed to deserialize serializedPincipal '${serializedPincipal}' ${e}` ); } if (!fallbackPrincipalCallback) { @@ -987,8 +997,10 @@ XPCOMUtils.defineLazyGetter( E10SUtils, "SERIALIZED_SYSTEMPRINCIPAL", function() { - return E10SUtils.serializePrincipal( - Services.scriptSecurityManager.getSystemPrincipal() + return btoa( + E10SUtils.serializePrincipal( + Services.scriptSecurityManager.getSystemPrincipal() + ) ); } );