Blob: Check blob URL whose origin is opaque using SecurityOrigin::Nonce

> Introduction

When PlzDedicatedWorker (off-the-main-thread dedicated worker script loading) is
enabled, top-level dedicated worker script is fetched on a worker thread, not on
the main (or parent) thread. This causes a problem when a dedicated worker is
created from blob URL registered in a sandboxed iframe.

> Problem

A sandboxed iframe has an opaque origin. Blob URL registered in the frame also
has the same opaque origin.

When sending a network request to the blob URL, requester's SecurityOrigin takes
the origin from the blob URL, and then conducts security check (see
SecurityOrigin::CanRequest()). However, it's not straightforward on the opaque
origin because the opaque origin is serialized into the blob URL (KURL) as
"null" string like "blob:null/{UUID}", and there is no direct way to rebuild the
origin from the KURL. To deal with the case, in the current implementation, the
pair of the blob URL and the opaque origin is stored in thread-specific
BlobURLNullOriginMap, and then the security check refers to the map.

This mechanism works well when PlzDedicatedWorker is disabled, but not work when
PlzDedicatedWorker is enabled. This is because the pair of the blob URL and the
opaque origin is stored in the thread-specific map on the main (parent) thread,
but the network request and the security check are conducted on the worker
thread. As a result, the security check always fails.

> Solution

Making BlobURLNullOriginMap thread-safe could be one of solutions, but actually
it's quite hard and not practical. This is because SecurityOrigin is not
thread-safe, and widely used with an assumption that it's not shared among
threads.

Alternatively, this CL makes SecurityOrigin::CanRequest() check the opaque
origin of the blob URL using SecurityOrigin::Nonce. Nonce is uniquely assigned
to SecurityOrigin when it's constructed as an opaque origin, and SecurityOrigin
instances (isolated-)copied from the same opaque origin share the same nonce.
The nonce is base::UnguessableToken, and shareable among thread unlike
SecurityOrigin.

This CL introduces BlobURLNullOriginNonceMap that is the process-global
singleton object to contain pairs of blob URL and opaque origins's nonce. The
map is protected by Mutex, so any threads can access it.
SecurityOrigin::CanRequest() compares the nonce stored in the map to its own
nonce in addition to the existing checks. This fixes the problem.

> Test

This enables to pass some tests that were skipped because of timeout. Several
test cases are still failing even after this change, but it should be caused by
https://crbug.com/1017034.

Bug: 987130
Change-Id: I6bf2212aa4f4b1a37035c41866b3faeffe68c2b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1873831
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712503}

third_party/blink/renderer/platform/blob/blob_url_null_origin_map.cc

@@ -23,11 +23,14 @@ void BlobURLNullOriginMap::Add(const KURL& blob_url, SecurityOrigin* origin) {
   DCHECK(!blob_url.HasFragmentIdentifier());
   DCHECK(origin->SerializesAsNull());
   blob_url_null_origin_map_.insert(blob_url.GetString(), origin);
+  if (origin->IsOpaque())
+    BlobURLOpaqueOriginNonceMap::GetInstance().Add(blob_url, origin);
 }
 
 void BlobURLNullOriginMap::Remove(const KURL& blob_url) {
   DCHECK(blob_url.ProtocolIs("blob"));
   DCHECK_EQ(BlobURL::GetOrigin(blob_url), "null");
+  BlobURLOpaqueOriginNonceMap::GetInstance().Remove(blob_url);
   blob_url_null_origin_map_.erase(blob_url.GetString());
 }
 
@@ -39,4 +42,39 @@ SecurityOrigin* BlobURLNullOriginMap::Get(const KURL& blob_url) {
   return blob_url_null_origin_map_.at(blob_url_without_fragment.GetString());
 }
 
+BlobURLOpaqueOriginNonceMap& BlobURLOpaqueOriginNonceMap::GetInstance() {
+  DEFINE_THREAD_SAFE_STATIC_LOCAL(BlobURLOpaqueOriginNonceMap, map, ());
+  return map;
+}
+
+void BlobURLOpaqueOriginNonceMap::Add(const KURL& blob_url,
+                                      SecurityOrigin* origin) {
+  MutexLocker lock(mutex_);
+  DCHECK(blob_url.ProtocolIs("blob"));
+  DCHECK_EQ(BlobURL::GetOrigin(blob_url), "null");
+  DCHECK(!blob_url.HasFragmentIdentifier());
+  DCHECK(origin->IsOpaque());
+  DCHECK(origin->GetNonceForSerialization());
+  auto result = blob_url_opaque_origin_nonce_map_.insert(
+      blob_url.GetString(), *origin->GetNonceForSerialization());
+  // The blob URL must be registered only once within the process.
+  SECURITY_CHECK(result.is_new_entry);
+}
+
+void BlobURLOpaqueOriginNonceMap::Remove(const KURL& blob_url) {
+  MutexLocker lock(mutex_);
+  DCHECK(blob_url.ProtocolIs("blob"));
+  blob_url_opaque_origin_nonce_map_.erase(blob_url.GetString());
+}
+
+base::UnguessableToken BlobURLOpaqueOriginNonceMap::Get(const KURL& blob_url) {
+  MutexLocker lock(mutex_);
+  DCHECK(blob_url.ProtocolIs("blob"));
+  DCHECK_EQ(BlobURL::GetOrigin(blob_url), "null");
+  KURL blob_url_without_fragment = blob_url;
+  blob_url_without_fragment.RemoveFragmentIdentifier();
+  return blob_url_opaque_origin_nonce_map_.at(
+      blob_url_without_fragment.GetString());
+}
+
 }  // namespace blink

third_party/blink/renderer/platform/blob/blob_url_null_origin_map.h

@@ -5,10 +5,13 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_PLATFORM_BLOB_BLOB_URL_NULL_ORIGIN_MAP_H_
 #define THIRD_PARTY_BLINK_RENDERER_PLATFORM_BLOB_BLOB_URL_NULL_ORIGIN_MAP_H_
 
+#include "base/thread_annotations.h"
+#include "base/unguessable_token.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
 #include "third_party/blink/renderer/platform/wtf/hash_map.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_hash.h"
 #include "third_party/blink/renderer/platform/wtf/thread_specific.h"
+#include "third_party/blink/renderer/platform/wtf/threading_primitives.h"
 
 namespace blink {
 
@@ -49,6 +52,58 @@ class PLATFORM_EXPORT BlobURLNullOriginMap {
   HashMap<String, scoped_refptr<SecurityOrigin>> blob_url_null_origin_map_;
 };
 
+// BlobURLOpaqueOriginNonceMap contains pairs of blob URL and opaque security
+// origin's nonce. This is used for comparing opaque origins in a thread-safe
+// way. An instance of this class is singleton, and can safely be accessed from
+// any threads.
+//
+// BlobURLNullOriginMap above does not work for the case where the blob URL is
+// registered in an opaque origin, and then a network request is sent to the URL
+// from a different thread because the map contains non-thread-safe
+// SecurityOrigin. For example, this happens on dedicated worker construction
+// that loads a top-level worker script on a worker thread.
+//
+// To handle the case, BlobURLOpaqueOriginNonceMap keeps SecurityOrigin::Nonce
+// instead of SecurityOrigin. The nonce is uniquely assigned to SecurityOrigin
+// when it is constructed as an opaque origin, and SecurityOrigin instances
+// (isolated-)copied from the same opaque origin share the same nonce. The nonce
+// is thread-safe, so it is feasible to compare opaque origins over threads.
+//
+// TODO(nhiroki): Unify BlobURLNullOriginMap and BlobURLOpaqueOriginNonceMap.
+// Making BlobURLNullOriginMap thread-safe could be possible solution, but
+// actually it should be quite hard and not practical. This is because
+// SecurityOrigin is not thread-safe, and widely used with an assumption that it
+// is not shared among threads. Instead, we could stop using
+// BlobURLNullOriginMap, and use BlobURLNullOriginMap in any case.
+class PLATFORM_EXPORT BlobURLOpaqueOriginNonceMap {
+ public:
+  // Returns the singleton instance of this class. The instance is created when
+  // this function is called for the first time.
+  static BlobURLOpaqueOriginNonceMap& GetInstance();
+
+  // Returns an opaque origin's nonce keyed with |blob_url| from the map.
+  // |blob_url| must have the opaque origin.
+  base::UnguessableToken Get(const KURL& blob_url) LOCKS_EXCLUDED(mutex_);
+
+ private:
+  friend class BlobURLNullOriginMap;
+
+  // Adds a pair of |blob_url| and |origin|'s nonce to the map. |blob_url| and
+  // |origin| must have the same opaque origin. Only called from
+  // BlobURLNullOriginMap::Add().
+  void Add(const KURL& blob_url, SecurityOrigin* origin) LOCKS_EXCLUDED(mutex_);
+
+  // Removes an opaque origin's nonce keyed with |blob_url| from the map.
+  // |blob_url| must have the opaque origin. Only called from
+  // BlobURLNullOriginMap::Remove().
+  void Remove(const KURL& blob_url) LOCKS_EXCLUDED(mutex_);
+
+  HashMap<String, base::UnguessableToken> blob_url_opaque_origin_nonce_map_
+      GUARDED_BY(mutex_);
+
+  Mutex mutex_;
+};
+
 }  // namespace blink
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_PLATFORM_BLOB_BLOB_URL_NULL_ORIGIN_MAP_H_

third_party/blink/renderer/platform/weborigin/security_origin.cc

@@ -406,12 +406,17 @@ bool SecurityOrigin::CanRequest(const KURL& url) const {
   if (SerializesAsNull()) {
     // Allow the request if the URL is blob and it has the same "null" origin
     // with |this|.
-    // TODO(nhiroki): Probably we should check the equality by
-    // SecurityOrigin::IsSameSchemeHostPort().
-    if (url.ProtocolIs("blob") && BlobURL::GetOrigin(url) == "null" &&
-        BlobURLNullOriginMap::GetInstance()->Get(url) == this) {
+    if (!url.ProtocolIs("blob") || BlobURL::GetOrigin(url) != "null")
+      return false;
+    if (BlobURLNullOriginMap::GetInstance()->Get(url) == this)
+      return true;
+    // BlobURLNullOriginMap doesn't work for cross-thread blob URL loading
+    // (e.g., top-level worker script loading) because SecurityOrigin and
+    // BlobURLNullOriginMap are thread-specific. For the case, check
+    // BlobURLOpaqueOriginNonceMap.
+    base::Optional<base::UnguessableToken> nonce = GetNonceForSerialization();
+    if (nonce && BlobURLOpaqueOriginNonceMap::GetInstance().Get(url) == nonce)
       return true;
-    }
     return false;
   }
 

third_party/blink/renderer/platform/weborigin/security_origin.h

@@ -340,6 +340,9 @@ class PLATFORM_EXPORT SecurityOrigin : public RefCounted<SecurityOrigin> {
   friend struct mojo::UrlOriginAdapter;
   friend struct blink::SecurityOriginHash;
 
+  // For calling GetNonceForSerialization().
+  friend class BlobURLOpaqueOriginNonceMap;
+
   // Creates a new opaque SecurityOrigin using the supplied |precursor| origin
   // and |nonce|.
   static scoped_refptr<SecurityOrigin> CreateOpaque(
@@ -365,8 +368,9 @@ class PLATFORM_EXPORT SecurityOrigin : public RefCounted<SecurityOrigin> {
   bool PassesFileCheck(const SecurityOrigin*) const;
   void BuildRawString(StringBuilder&) const;
 
-  // Get the nonce associated with this origin, if it is unique. This should be
-  // used only when trying to send an Origin across an IPC pipe.
+  // Get the nonce associated with this origin, if it is opaque. This should be
+  // used only when trying to send an Origin across an IPC pipe or comparing
+  // blob URL's opaque origins in the thread-safe way.
   base::Optional<base::UnguessableToken> GetNonceForSerialization() const;
 
   const String protocol_ = g_empty_string;

third_party/blink/web_tests/TestExpectations

@@ -3926,12 +3926,6 @@ crbug.com/655458 virtual/omt-worker-fetch/external/wpt/workers/semantics/multipl
 
 crbug.com/910709 navigator_language/worker_navigator_language.html [ Timeout ]
 
-# Off-the-main-thread classic worker script fetch.
-# This fails because running a worker on an opaque origin is blocked.
-crbug.com/835717 virtual/omt-worker-fetch/external/wpt/workers/opaque-origin.html [ Timeout ]
-# Needs investigation. These are timing out.
-crbug.com/835717 virtual/omt-worker-fetch/external/wpt/service-workers/service-worker/sandboxed-iframe-fetch-event.https.html [ Skip ]
-crbug.com/835717 virtual/omt-worker-fetch/external/wpt/service-workers/service-worker/worker-in-sandboxed-iframe-by-csp-fetch-event.https.html [ Skip ]
 crbug.com/1002377 [ Win ] virtual/omt-service-worker-startup/external/wpt/service-workers/service-worker/update-bytecheck.https.html [ Timeout ]
 
 crbug.com/435547 http/tests/cachestorage/serviceworker/ignore-search-with-credentials.html [ Skip ]

third_party/blink/web_tests/virtual/omt-worker-fetch/external/wpt/service-workers/service-worker/sandboxed-iframe-fetch-event.https-expected.txt

+This is a testharness.js-based test.
+PASS Prepare a service worker.
+PASS Prepare a normal iframe.
+PASS Prepare an iframe sandboxed by <iframe sandbox="allow-scripts">.
+PASS Prepare an iframe sandboxed by <iframe sandbox="allow-scripts allow-same-origin">.
+PASS Prepare an iframe sandboxed by CSP HTTP header with allow-scripts.
+PASS Prepare an iframe sandboxed by CSP HTTP header with allow-scripts and allow-same-origin.
+PASS Fetch request from a normal iframe
+FAIL Fetch request from a worker in a normal iframe assert_equals: The fetch request should be handled by SW. expected 1 but got 0
+PASS Request for an iframe in the normal iframe
+PASS Request for an sandboxed iframe with allow-scripts flag in the normal iframe
+PASS Request for an sandboxed iframe with allow-scripts and allow-same-origin flag in the normal iframe
+PASS Fetch request from iframe sandboxed by an attribute with allow-scripts flag
+PASS Fetch request from a worker in iframe sandboxed by an attribute with allow-scripts flag
+PASS Request for an iframe in the iframe sandboxed by an attribute with allow-scripts flag
+PASS Request for an sandboxed iframe with allow-scripts flag in the iframe sandboxed by an attribute with allow-scripts flag
+PASS Request for an sandboxed iframe with allow-scripts and allow-same-origin flag in the iframe sandboxed by an attribute with allow-scripts flag
+PASS Fetch request from iframe sandboxed by an attribute with allow-scripts and allow-same-origin flag
+FAIL Fetch request from a worker in iframe sandboxed by an attribute with allow-scripts and allow-same-origin flag assert_equals: The fetch request should be handled by SW. expected 1 but got 0
+PASS Request for an iframe in the iframe sandboxed by an attribute with allow-scripts and allow-same-origin flag
+PASS Request for an sandboxed iframe with allow-scripts flag in the iframe sandboxed by attribute with allow-scripts and allow-same-origin flag
+PASS Request for an sandboxed iframe with allow-scripts and allow-same-origin flag in the iframe sandboxed by attribute with allow-scripts and allow-same-origin flag
+PASS Fetch request from iframe sandboxed by CSP HTTP header with allow-scripts flag
+PASS Request for an iframe in the iframe sandboxed by CSP HTTP header with allow-scripts flag
+PASS Request for an sandboxed iframe with allow-scripts flag in the iframe sandboxed by CSP HTTP header with allow-scripts flag
+PASS Request for an sandboxed iframe with allow-scripts and allow-same-origin flag in the iframe sandboxed by CSP HTTP header with allow-scripts flag
+PASS Fetch request from iframe sandboxed by CSP HTTP header with allow-scripts and allow-same-origin flag
+PASS Request for an iframe in the iframe sandboxed by CSP HTTP header with allow-scripts and allow-same-origin flag
+PASS Request for an sandboxed iframe with allow-scripts flag in the iframe sandboxed by CSP HTTP header with allow-scripts and allow-same-origin flag
+PASS Request for an sandboxed iframe with allow-scripts and allow-same-origin flag in the iframe sandboxed by CSP HTTP header with allow-scripts and allow-same-origin flag
+Harness: the test ran to completion.
+

third_party/blink/web_tests/virtual/omt-worker-fetch/external/wpt/service-workers/service-worker/worker-in-sandboxed-iframe-by-csp-fetch-event.https-expected.txt

+This is a testharness.js-based test.
+PASS Prepare a service worker.
+PASS Prepare an iframe sandboxed by CSP HTTP header with allow-scripts.
+PASS Prepare an iframe sandboxed by CSP HTTP header with allow-scripts and allow-same-origin.
+PASS Fetch request from a worker in iframe sandboxed by CSP HTTP header allow-scripts flag
+FAIL Fetch request from a worker in iframe sandboxed by CSP HTTP header with allow-scripts and allow-same-origin flag assert_equals: The request should be handled by SW. expected 1 but got 0
+Harness: the test ran to completion.
+