Allow an opaque url::Origin to remember where it came from.

The problem being solved here is that, although various web platform features
can cause documents to be placed in opaque origins, sometimes doing so
obscures the actual source of the documents, which itself can be a
security risk. "data:" URLs, "srcdoc" plus "sandbox" are particular tricky cases
of this, as neither the URL nor the committed origin retains information about
which network host the content is originally from.

This CL is the first step towards solving this problem by keeping that
information around in url::Origin. It is just the url::Origin changes
from nick@'s work on precursor origins started in https://crrev.com/c/1028985.

The precursor information must be used carefully. Opaque origins should
generally not inherit privileges from the origins they derive from. However, in
some cases (such as restrictions on process placement, or determining the http
lock icon, or determining content script injection) this information may be
relevant to ensure that entering an opaque origin does not grant privileges
initially denied to the original non-opaque origin.

This new tracking is transitive: meaning if a page loaded from
http://example.com navigates to a data URL, which then navigates to a blob:null
URL, which embeds an <iframe sandbox srcdoc="...">, the precursor origin for the
sandboxed iframe is retained to be "http://example.com".

Bug: 882053
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I021245c624b78f08bd835c5cae9fde7ec5e44b80
Reviewed-on: https://chromium-review.googlesource.com/1214745
Commit-Queue: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Ian Clelland <iclelland@chromium.org>
Reviewed-by: Luna Lu <loonybear@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591867}

content/child/blink_platform_impl_unittest.cc

@@ -17,15 +17,16 @@ namespace content {
 
 void CheckCastedOriginsAlreadyNormalized(
     const blink::WebSecurityOrigin& origin) {
-  url::Origin checked_origin =
-      url::Origin::UnsafelyCreateOriginWithoutNormalization(
+  if (origin.IsOpaque())
+    return;
+
+  base::Optional<url::Origin> checked_origin =
+      url::Origin::UnsafelyCreateTupleOriginWithoutNormalization(
           origin.Protocol().Utf8(), origin.Host().Utf8(),
           origin.EffectivePort());
   url::Origin non_checked_origin = url::Origin::CreateFromNormalizedTuple(
       origin.Protocol().Utf8(), origin.Host().Utf8(), origin.EffectivePort());
-  EXPECT_EQ(checked_origin.scheme(), non_checked_origin.scheme());
-  EXPECT_EQ(checked_origin.host(), non_checked_origin.host());
-  EXPECT_EQ(checked_origin.port(), non_checked_origin.port());
+  EXPECT_EQ(checked_origin, non_checked_origin);
 }
 
 TEST(BlinkPlatformTest, CastWebSecurityOrigin) {

net/http/http_server_properties_impl_unittest.cc

@@ -450,17 +450,17 @@ TEST_F(AlternateProtocolServerPropertiesTest, Set) {
   ASSERT_EQ(3u, map.size());
   AlternativeServiceMap::const_iterator map_it = map.begin();
 
-  EXPECT_TRUE(map_it->first.Equals(test_server2));
+  EXPECT_EQ(map_it->first, test_server2);
   ASSERT_EQ(1u, map_it->second.size());
   EXPECT_EQ(alternative_service3, map_it->second[0].alternative_service());
   EXPECT_EQ(expiration3, map_it->second[0].expiration());
   ++map_it;
-  EXPECT_TRUE(map_it->first.Equals(test_server1));
+  EXPECT_EQ(map_it->first, test_server1);
   ASSERT_EQ(1u, map_it->second.size());
   EXPECT_EQ(alternative_service1, map_it->second[0].alternative_service());
   EXPECT_EQ(expiration1, map_it->second[0].expiration());
   ++map_it;
-  EXPECT_TRUE(map_it->first.Equals(test_server3));
+  EXPECT_EQ(map_it->first, test_server3);
   ASSERT_EQ(1u, map_it->second.size());
   EXPECT_EQ(alternative_service4, map_it->second[0].alternative_service());
   EXPECT_EQ(expiration4, map_it->second[0].expiration());
@@ -603,7 +603,7 @@ TEST_F(AlternateProtocolServerPropertiesTest, MRUOfGetAlternativeServiceInfos) {
 
   const AlternativeServiceMap& map = impl_.alternative_service_map();
   AlternativeServiceMap::const_iterator it = map.begin();
-  EXPECT_TRUE(it->first.Equals(test_server2));
+  EXPECT_EQ(it->first, test_server2);
   ASSERT_EQ(1u, it->second.size());
   EXPECT_EQ(alternative_service2, it->second[0].alternative_service());
 
@@ -615,7 +615,7 @@ TEST_F(AlternateProtocolServerPropertiesTest, MRUOfGetAlternativeServiceInfos) {
 
   // GetAlternativeServices should reorder the AlternateProtocol map.
   it = map.begin();
-  EXPECT_TRUE(it->first.Equals(test_server1));
+  EXPECT_EQ(it->first, test_server1);
   ASSERT_EQ(1u, it->second.size());
   EXPECT_EQ(alternative_service1, it->second[0].alternative_service());
 }
@@ -802,7 +802,7 @@ TEST_F(AlternateProtocolServerPropertiesTest, AlternativeServiceWithScheme) {
 
   const net::AlternativeServiceMap& map = impl_.alternative_service_map();
   net::AlternativeServiceMap::const_iterator it = map.begin();
-  EXPECT_TRUE(it->first.Equals(http_server));
+  EXPECT_EQ(it->first, http_server);
   ASSERT_EQ(2u, it->second.size());
   EXPECT_EQ(alternative_service1, it->second[0].alternative_service());
   EXPECT_EQ(alternative_service2, it->second[1].alternative_service());
@@ -839,7 +839,7 @@ TEST_F(AlternateProtocolServerPropertiesTest, ClearAlternativeServices) {
 
   const net::AlternativeServiceMap& map = impl_.alternative_service_map();
   net::AlternativeServiceMap::const_iterator it = map.begin();
-  EXPECT_TRUE(it->first.Equals(test_server));
+  EXPECT_EQ(it->first, test_server);
   ASSERT_EQ(2u, it->second.size());
   EXPECT_EQ(alternative_service1, it->second[0].alternative_service());
   EXPECT_EQ(alternative_service2, it->second[1].alternative_service());
@@ -1381,13 +1381,13 @@ TEST_F(ServerNetworkStatsServerPropertiesTest, Set) {
   ASSERT_EQ(3u, map.size());
   ServerNetworkStatsMap::const_iterator map_it = map.begin();
 
-  EXPECT_TRUE(map_it->first.Equals(docs_server));
+  EXPECT_EQ(map_it->first, docs_server);
   EXPECT_EQ(new_stats_docs, map_it->second);
   ++map_it;
-  EXPECT_TRUE(map_it->first.Equals(google_server));
+  EXPECT_EQ(map_it->first, google_server);
   EXPECT_EQ(stats_google, map_it->second);
   ++map_it;
-  EXPECT_TRUE(map_it->first.Equals(mail_server));
+  EXPECT_EQ(map_it->first, mail_server);
   EXPECT_EQ(stats_mail, map_it->second);
 }
 

services/network/public/cpp/net_ipc_param_traits.cc

@@ -500,19 +500,20 @@ bool ParamTraits<url::Origin>::Read(const base::Pickle* m,
   uint16_t port;
   if (!ReadParam(m, iter, &unique) || !ReadParam(m, iter, &scheme) ||
       !ReadParam(m, iter, &host) || !ReadParam(m, iter, &port)) {
-    *p = url::Origin();
     return false;
   }
 
-  *p = unique ? url::Origin()
-              : url::Origin::UnsafelyCreateOriginWithoutNormalization(
-                    scheme, host, port);
+  if (unique) {
+    *p = url::Origin();
+  } else {
+    base::Optional<url::Origin> origin =
+        url::Origin::UnsafelyCreateTupleOriginWithoutNormalization(scheme, host,
+                                                                   port);
+    if (!origin.has_value())
+      return false;
 
-  // If a unique origin was created, but the unique flag wasn't set, then
-  // the values provided to 'UnsafelyCreateOriginWithoutNormalization' were
-  // invalid; kill the renderer.
-  if (!unique && p->unique())
-    return false;
+    *p = origin.value();
+  }
 
   return true;
 }

third_party/blink/common/feature_policy/feature_policy.cc

@@ -20,6 +20,7 @@ std::unique_ptr<FeaturePolicy::Allowlist> AllowlistFromDeclaration(
     result->AddAll();
   for (const auto& origin : parsed_declaration.origins)
     result->Add(origin);
+
   return result;
 }
 
@@ -82,7 +83,7 @@ bool FeaturePolicy::Allowlist::Contains(const url::Origin& origin) const {
   if (matches_all_origins_)
     return true;
   for (const auto& targetOrigin : origins_) {
-    if (targetOrigin.IsSameOriginWith(origin))
+    if (!origin.unique() && targetOrigin.IsSameOriginWith(origin))
       return true;
   }
   return false;
@@ -125,12 +126,8 @@ bool FeaturePolicy::IsFeatureEnabledForOrigin(
       feature_list_.at(feature);
   if (default_policy == FeaturePolicy::FeatureDefault::EnableForAll)
     return true;
-  if (default_policy == FeaturePolicy::FeatureDefault::EnableForSelf) {
-    // TODO(iclelland): Remove the pointer equality check once it is possible to
-    // compare opaque origins successfully against themselves.
-    // https://crbug.com/690520
-    return (&origin_ == &origin) || origin_.IsSameOriginWith(origin);
-  }
+  if (default_policy == FeaturePolicy::FeatureDefault::EnableForSelf)
+    return origin_.IsSameOriginWith(origin);
   return false;
 }
 
@@ -170,7 +167,14 @@ void FeaturePolicy::SetHeaderPolicy(const ParsedFeaturePolicy& parsed_header) {
 
 FeaturePolicy::FeaturePolicy(url::Origin origin,
                              const FeatureList& feature_list)
-    : origin_(origin), feature_list_(feature_list) {}
+    : origin_(std::move(origin)), feature_list_(feature_list) {
+  if (origin_.unique()) {
+    // FeaturePolicy was written expecting opaque Origins to be indistinct, but
+    // this has changed. Split out a new opaque origin here, to defend against
+    // origin-equality.
+    origin_ = origin_.DeriveNewOpaqueOrigin();
+  }
+}
 
 FeaturePolicy::~FeaturePolicy() = default;
 

third_party/blink/common/feature_policy/feature_policy_unittest.cc

@@ -1209,8 +1209,8 @@ TEST_F(FeaturePolicyTest, TestSandboxedPolicyCanBePropagated) {
   // However, it will not pass that on to any other origin
   std::unique_ptr<FeaturePolicy> policy1 =
       CreateFromParentPolicy(nullptr, origin_a_);
-  url::Origin sandboxed_origin_1 = url::Origin();
-  url::Origin sandboxed_origin_2 = url::Origin();
+  url::Origin sandboxed_origin_1 = origin_a_.DeriveNewOpaqueOrigin();
+  url::Origin sandboxed_origin_2 = sandboxed_origin_1.DeriveNewOpaqueOrigin();
   ParsedFeaturePolicy frame_policy_1 = {
       {{kDefaultSelfFeature, true, false, std::vector<url::Origin>()}}};
   std::unique_ptr<FeaturePolicy> policy2 = CreateFromParentWithFramePolicy(

third_party/blink/public/common/feature_policy/feature_policy.h

@@ -40,7 +40,7 @@ namespace blink {
 // ----------
 // Allowlists are collections of origins, although two special terms can be used
 // when declaring them:
-//   "self" refers to the orgin of the frame which is declaring the policy.
+//   "self" refers to the origin of the frame which is declaring the policy.
 //   "*" refers to all origins; any origin will match an allowlist which
 //   contains it.
 //

tools/ipc_fuzzer/fuzzer/fuzzer.cc

@@ -13,6 +13,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/memory/shared_memory_handle.h"
 #include "base/strings/string_util.h"
+#include "base/unguessable_token.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "ipc/ipc_message.h"
@@ -641,6 +642,20 @@ struct FuzzTraits<base::DictionaryValue> {
   }
 };
 
+template <>
+struct FuzzTraits<base::UnguessableToken> {
+  static bool Fuzz(base::UnguessableToken* p, Fuzzer* fuzzer) {
+    auto low = p->GetLowForSerialization();
+    auto high = p->GetHighForSerialization();
+    if (!FuzzParam(&low, fuzzer))
+      return false;
+    if (!FuzzParam(&high, fuzzer))
+      return false;
+    *p = base::UnguessableToken::Deserialize(high, low);
+    return true;
+  }
+};
+
 template <>
 struct FuzzTraits<viz::CompositorFrame> {
   static bool Fuzz(viz::CompositorFrame* p, Fuzzer* fuzzer) {
@@ -1509,21 +1524,48 @@ struct FuzzTraits<ui::LatencyInfo> {
 template <>
 struct FuzzTraits<url::Origin> {
   static bool Fuzz(url::Origin* p, Fuzzer* fuzzer) {
-    std::string scheme = p->scheme();
-    std::string host = p->host();
-    uint16_t port = p->port();
+    bool opaque = p->unique();
+    if (!FuzzParam(&opaque, fuzzer))
+      return false;
+    std::string scheme = p->GetTupleOrPrecursorTupleIfOpaque().scheme();
+    std::string host = p->GetTupleOrPrecursorTupleIfOpaque().host();
+    uint16_t port = p->GetTupleOrPrecursorTupleIfOpaque().port();
     if (!FuzzParam(&scheme, fuzzer))
       return false;
     if (!FuzzParam(&host, fuzzer))
       return false;
     if (!FuzzParam(&port, fuzzer))
       return false;
-    *p = url::Origin::UnsafelyCreateOriginWithoutNormalization(scheme, host,
-                                                               port);
 
-    // Force a unique origin 1% of the time:
-    if (RandInRange(100) == 1)
-      *p = url::Origin();
+    base::Optional<url::Origin> origin;
+    if (!opaque) {
+      origin = url::Origin::UnsafelyCreateTupleOriginWithoutNormalization(
+          scheme, host, port);
+    } else {
+      base::Optional<base::UnguessableToken> token =
+          p->GetNonceForSerialization();
+      if (!token)
+        token = base::UnguessableToken::Deserialize(RandU64(), RandU64());
+      if (!FuzzParam(&(*token), fuzzer))
+        return false;
+      origin = url::Origin::UnsafelyCreateOpaqueOriginWithoutNormalization(
+          scheme, host, port, url::Origin::Nonce(*token));
+    }
+
+    if (!origin) {
+      // This means that we produced non-canonical values that were rejected by
+      // UnsafelyCreate. Which is nice, except, those are arguably interesting
+      // values to be sending over the wire sometimes, to make sure they're
+      // rejected at the receiving end.
+      //
+      // We could potentially call CreateFromNormalizedTuple here to force their
+      // creation, except that could lead to invariant violations within the
+      // url::Origin we construct -- and potentially crash the fuzzer. What to
+      // do?
+      return false;
+    }
+
+    *p = std::move(origin).value();
     return true;
   }
 };

url/mojom/origin_mojom_traits.h

@@ -25,15 +25,14 @@ struct StructTraits<url::mojom::OriginDataView, url::Origin> {
       if (!data.ReadScheme(&scheme) || !data.ReadHost(&host))
         return false;
 
-      *out = url::Origin::UnsafelyCreateOriginWithoutNormalization(scheme, host,
-                                                                   data.port());
-    }
+      base::Optional<url::Origin> origin =
+          url::Origin::UnsafelyCreateTupleOriginWithoutNormalization(
+              scheme, host, data.port());
+      if (!origin.has_value())
+        return false;
 
-    // If a unique origin was created, but the unique flag wasn't set, then
-    // the values provided to 'UnsafelyCreateOriginWithoutNormalization' were
-    // invalid.
-    if (!data.unique() && out->unique())
-      return false;
+      *out = origin.value();
+    }
 
     return true;
   }

url/mojom/url_gurl_mojom_traits_unittest.cc

@@ -72,8 +72,9 @@ TEST(MojoGURLStructTraitsTest, Basic) {
   }
 
   // Test basic Origin serialization.
-  Origin non_unique = Origin::UnsafelyCreateOriginWithoutNormalization(
-      "http", "www.google.com", 80);
+  Origin non_unique = Origin::UnsafelyCreateTupleOriginWithoutNormalization(
+                          "http", "www.google.com", 80)
+                          .value();
   Origin output;
   EXPECT_TRUE(proxy->BounceOrigin(non_unique, &output));
   EXPECT_EQ(non_unique, output);