[Blob URL] Allow contexts with a StorageAccessHandle to bypass Blob URL partitioning

Given that the RequestStorageAccess API specifically exposes the ability
to create unpartitioned Blob URLs by getting a StorageAccessHandle,
create an unpartitioned blob URL using the handle's CreateObjectURL
method, and then fetch it, it makes sense to ensure that contexts that
have been granted storage access can bypass the blob URL partitioning
check.

Bug: 399308041, 399769596
Change-Id: Ia47e815c819c39e6a6ce00aa0c8c6617173ea742
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6313600
Reviewed-by: Robert Flack <flackr@chromium.org>
Reviewed-by: Yann Dago <ydago@chromium.org>
Commit-Queue: Janice Liu <janiceliu@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Auto-Submit: Janice Liu <janiceliu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1436948}

chrome/browser/policy/test/partitioned_blob_url_browsertest.cc

@@ -4,12 +4,14 @@
 
 #include "base/test/scoped_feature_list.h"
 #include "base/values.h"
+#include "chrome/browser/content_settings/cookie_settings_factory.h"
 #include "chrome/browser/policy/policy_test_utils.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/policy/core/common/policy_map.h"
 #include "components/policy/core/common/policy_types.h"
@@ -215,6 +217,15 @@ IN_PROC_BROWSER_TEST_P(PartitionedBlobUrlUsagePolicyBrowserTestP,
   content::RenderFrameHost* rfh_b = content::ChildFrameAt(rfh_c, 0);
   content::RenderFrameHost* rfh_c_2 = content::ChildFrameAt(rfh_b, 0);
 
+  // The default cookie setting to BLOCK here to ensure that the
+  // cross-origin blob URL fetch will be blocked due to lack of storage
+  // access. If cookies are allowed, storage access might be granted, and the
+  // fetch would succeed even if the blob URL is cross-origin and
+  // kBlockCrossPartitionBlobUrlFetching is enabled.
+  content_settings::CookieSettings* settings =
+      CookieSettingsFactory::GetForProfile(browser()->profile()).get();
+  settings->SetDefaultCookieSetting(CONTENT_SETTING_BLOCK);
+
   bool fetch_results = FetchAndReadBlobUrl(rfh_c_2, blob_url);
 
   if (IsPolicyEnabled()) {

content/browser/blob_storage/blob_url_browsertest.cc

@@ -9,6 +9,7 @@
 #include "base/test/with_feature_override.h"
 #include "build/build_config.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
+#include "content/browser/permissions/permission_controller_impl.h"
 #include "content/browser/renderer_host/render_frame_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/common/content_switches.h"
@@ -43,6 +44,14 @@ class MockContentBrowserClient : public ContentBrowserTestContentBrowserClient {
               LogWebFeatureForCurrentPage,
               (content::RenderFrameHost*, blink::mojom::WebFeature),
               (override));
+
+  bool IsFullCookieAccessAllowed(
+      content::BrowserContext* browser_context,
+      content::WebContents* web_contents,
+      const GURL& url,
+      const blink::StorageKey& storage_key) override {
+    return false;
+  }
 };
 }  // namespace
 
@@ -251,8 +260,10 @@ IN_PROC_BROWSER_TEST_F(BlobUrlBrowserTest,
 class BlobUrlDevToolsIssueTest : public ContentBrowserTest {
  protected:
   BlobUrlDevToolsIssueTest() {
-    feature_list_.InitAndEnableFeature(
-        features::kBlockCrossPartitionBlobUrlFetching);
+    feature_list_.InitWithFeatures(
+        {features::kBlockCrossPartitionBlobUrlFetching,
+         blink::features::kEnforceNoopenerOnBlobURLNavigation},
+        {});
   }
 
   void SetUpOnMainThread() override {
@@ -260,8 +271,11 @@ class BlobUrlDevToolsIssueTest : public ContentBrowserTest {
     host_resolver()->AddRule("*", "127.0.0.1");
     SetupCrossSiteRedirector(embedded_test_server());
     ASSERT_TRUE(embedded_test_server()->Start());
+    client_ = std::make_unique<MockContentBrowserClient>();
   }
 
+  void TearDownOnMainThread() override { client_.reset(); }
+
   void WaitForIssueAndCheckUrl(const std::string& url,
                                TestDevToolsProtocolClient* client,
                                const std::string& expected_info_enum) {
@@ -300,7 +314,11 @@ class BlobUrlDevToolsIssueTest : public ContentBrowserTest {
     client->ClearNotifications();
   }
 
+ private:
   base::test::ScopedFeatureList feature_list_;
+
+ private:
+  std::unique_ptr<MockContentBrowserClient> client_;
 };
 
 IN_PROC_BROWSER_TEST_F(BlobUrlDevToolsIssueTest, PartitioningBlobUrlIssue) {
@@ -324,6 +342,12 @@ IN_PROC_BROWSER_TEST_F(BlobUrlDevToolsIssueTest, PartitioningBlobUrlIssue) {
   RenderFrameHost* rfh_b = ChildFrameAt(rfh_c, 0);
   RenderFrameHost* rfh_c_2 = ChildFrameAt(rfh_b, 0);
 
+  static_cast<PermissionControllerImpl*>(
+      rfh_c_2->GetBrowserContext()->GetPermissionController())
+      ->SetPermissionOverride(/*origin=*/std::nullopt,
+                              blink::PermissionType::STORAGE_ACCESS_GRANT,
+                              blink::mojom::PermissionStatus::DENIED);
+
   std::unique_ptr<content::TestDevToolsProtocolClient> client =
       std::make_unique<content::TestDevToolsProtocolClient>();
   client->AttachToFrameTreeHost(rfh_c_2);

content/browser/renderer_host/render_frame_host_impl.cc

@@ -156,6 +156,7 @@
 #include "content/browser/site_info.h"
 #include "content/browser/sms/webotp_service.h"
 #include "content/browser/speech/speech_synthesis_impl.h"
+#include "content/browser/storage_access/storage_access_handle.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/browser/url_loader_factory_params_helper.h"
 #include "content/browser/usb/web_usb_service_impl.h"
@@ -12666,6 +12667,12 @@ void RenderFrameHostImpl::ReportBlockingCrossPartitionBlobURL(
       std::move(details)));
 }
 
+void RenderFrameHostImpl::DoesDocumentHaveStorageAccess(
+    base::OnceCallback<void(bool)> callback) {
+  std::move(callback).Run(
+      StorageAccessHandle::DoesFrameHaveStorageAccess(this));
+}
+
 void RenderFrameHostImpl::BindBlobUrlStoreAssociatedReceiver(
     mojo::PendingAssociatedReceiver<blink::mojom::BlobURLStore> receiver) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -12677,6 +12684,8 @@ void RenderFrameHostImpl::BindBlobUrlStoreAssociatedReceiver(
       base::BindRepeating(
           &RenderFrameHostImpl::ReportBlockingCrossPartitionBlobURL,
           weak_ptr_factory_.GetWeakPtr()),
+      base::BindRepeating(&RenderFrameHostImpl::DoesDocumentHaveStorageAccess,
+                          weak_ptr_factory_.GetWeakPtr()),
       !(GetContentClient()->browser()->IsBlobUrlPartitioningEnabled(
           GetBrowserContext())));
 }

content/browser/renderer_host/render_frame_host_impl.h

@@ -4277,6 +4277,10 @@ class CONTENT_EXPORT RenderFrameHostImpl
       const GURL& blocked_url,
       std::optional<blink::mojom::PartitioningBlobURLInfo> info);
 
+  // This runs when fetches to cross-partition, same-origin Blob URL checks for
+  // storage access
+  void DoesDocumentHaveStorageAccess(base::OnceCallback<void(bool)> callback);
+
   // For frames and main thread worklets we use a navigation-associated
   // interface and bind `receiver` to a `BlobURLStore` instance, which
   // implements the Blob URL API in the browser process.

content/browser/storage_access/storage_access_handle.cc

@@ -173,12 +173,23 @@ void StorageAccessHandle::BindBlobStorage(
   static_cast<RenderFrameHostImpl&>(render_frame_host())
       .GetStoragePartition()
       ->GetBlobUrlRegistry()
-      ->AddReceiver(blink::StorageKey::CreateFirstParty(
-                        render_frame_host().GetStorageKey().origin()),
-                    render_frame_host().GetLastCommittedOrigin(),
-                    render_frame_host().GetProcess()->GetDeprecatedID(),
-                    std::move(receiver), base::DoNothing(),
-                    /*partitioning_disabled_by_policy=*/false);
+      ->AddReceiver(
+          blink::StorageKey::CreateFirstParty(
+              render_frame_host().GetStorageKey().origin()),
+          render_frame_host().GetLastCommittedOrigin(),
+          render_frame_host().GetProcess()->GetDeprecatedID(),
+          std::move(receiver),
+          /*partitioning_blob_url_closure=*/base::DoNothing(),
+          // In the case that a context is granted storage access, the
+          // StorageAccessHandle context still shouldn't bypass partitioning
+          // check. (eg. using a Blob URL created with URL.createObjectURL in
+          // the third-party context with the StorageAccessHandle's SharedWorker
+          // constructor.)
+          /*storage_access_check_callback= */
+          base::BindRepeating([](base::OnceCallback<void(bool)> callback) {
+            std::move(callback).Run(false);
+          }),
+          /*partitioning_disabled_by_policy=*/false);
 }
 
 void StorageAccessHandle::BindBroadcastChannel(

storage/browser/blob/blob_url_registry.cc

@@ -37,12 +37,15 @@ void BlobUrlRegistry::AddReceiver(
     base::RepeatingCallback<
         void(const GURL&, std::optional<blink::mojom::PartitioningBlobURLInfo>)>
         partitioning_blob_url_closure,
+    base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
+        storage_access_check_callback,
     bool partitioning_disabled_by_policy) {
   mojo::ReceiverId receiver_id = frame_receivers_.Add(
       std::make_unique<storage::BlobURLStoreImpl>(
           storage_key, renderer_origin, render_process_host_id, AsWeakPtr(),
           storage::BlobURLValidityCheckBehavior::DEFAULT,
           std::move(partitioning_blob_url_closure),
+          std::move(storage_access_check_callback),
           partitioning_disabled_by_policy),
       std::move(receiver));
 
@@ -62,6 +65,9 @@ void BlobUrlRegistry::AddReceiver(
       std::make_unique<storage::BlobURLStoreImpl>(
           storage_key, renderer_origin, render_process_host_id, AsWeakPtr(),
           validity_check_behavior, base::DoNothing(),
+          base::BindRepeating([](base::OnceCallback<void(bool)> callback) {
+            std::move(callback).Run(false);
+          }),
           partitioning_disabled_by_policy),
       std::move(receiver));
 }

storage/browser/blob/blob_url_registry.h

@@ -64,6 +64,8 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobUrlRegistry {
           void(const GURL&,
                std::optional<blink::mojom::PartitioningBlobURLInfo>)>
           partitioning_blob_url_closure,
+      base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
+          storage_access_check_callback,
       bool partitioning_disabled_by_policy = false);
 
   // Binds receivers corresponding to connections from renderer worker

storage/browser/blob/blob_url_store_impl.cc

@@ -9,6 +9,7 @@
 #include "base/functional/callback.h"
 #include "base/strings/strcat.h"
 #include "components/crash/core/common/crash_key.h"
+#include "mojo/public/cpp/bindings/callback_helpers.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "net/base/features.h"
 #include "storage/browser/blob/blob_impl.h"
@@ -75,6 +76,8 @@ BlobURLStoreImpl::BlobURLStoreImpl(
     base::RepeatingCallback<
         void(const GURL&, std::optional<blink::mojom::PartitioningBlobURLInfo>)>
         partitioning_blob_url_closure,
+    base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
+        storage_access_check_callback,
     bool partitioning_disabled_by_policy)
     : storage_key_(storage_key),
       renderer_origin_(renderer_origin),
@@ -82,6 +85,7 @@ BlobURLStoreImpl::BlobURLStoreImpl(
       registry_(std::move(registry)),
       validity_check_behavior_(validity_check_behavior),
       partitioning_blob_url_closure_(std::move(partitioning_blob_url_closure)),
+      storage_access_check_callback_(std::move(storage_access_check_callback)),
       partitioning_disabled_by_policy_(partitioning_disabled_by_policy) {}
 
 BlobURLStoreImpl::~BlobURLStoreImpl() {
@@ -131,7 +135,17 @@ void BlobURLStoreImpl::ResolveAsURLLoaderFactory(
     std::move(callback).Run(std::nullopt, std::nullopt);
     return;
   }
+  storage_access_check_callback_.Run(
+      base::BindOnce(&BlobURLStoreImpl::FinishResolveAsURLLoaderFactory,
+                     weak_ptr_factory_.GetWeakPtr(), url, std::move(receiver),
+                     std::move(callback)));
+}
 
+void BlobURLStoreImpl::FinishResolveAsURLLoaderFactory(
+    const GURL& url,
+    mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver,
+    ResolveAsURLLoaderFactoryCallback callback,
+    bool has_storage_access_handle) {
   if (registry_->IsUrlMapped(BlobUrlUtils::ClearUrlFragment(url),
                              storage_key_) ==
       BlobUrlRegistry::MappingStatus::kNotMappedCrossPartitionSameOrigin) {
@@ -139,7 +153,7 @@ void BlobURLStoreImpl::ResolveAsURLLoaderFactory(
         base::FeatureList::IsEnabled(
             features::kBlockCrossPartitionBlobUrlFetching) &&
         !partitioning_disabled_by_policy_;
-    if (feature_and_policy_check) {
+    if (feature_and_policy_check && !has_storage_access_handle) {
       partitioning_blob_url_closure_.Run(url,
                                          blink::mojom::PartitioningBlobURLInfo::
                                              kBlockedCrossPartitionFetching);
@@ -171,7 +185,20 @@ void BlobURLStoreImpl::ResolveAsBlobURLToken(
     std::move(callback).Run(std::nullopt);
     return;
   }
+  storage_access_check_callback_.Run(
+      mojo::WrapCallbackWithDefaultInvokeIfNotRun(
+          base::BindOnce(&BlobURLStoreImpl::FinishResolveAsBlobURLToken,
+                         weak_ptr_factory_.GetWeakPtr(), url, std::move(token),
+                         is_top_level_navigation, std::move(callback)),
+          false));
+}
 
+void BlobURLStoreImpl::FinishResolveAsBlobURLToken(
+    const GURL& url,
+    mojo::PendingReceiver<blink::mojom::BlobURLToken> token,
+    bool is_top_level_navigation,
+    ResolveAsBlobURLTokenCallback callback,
+    bool has_storage_access_handle) {
   if (!is_top_level_navigation &&
       (registry_->IsUrlMapped(BlobUrlUtils::ClearUrlFragment(url),
                               storage_key_) ==
@@ -180,7 +207,7 @@ void BlobURLStoreImpl::ResolveAsBlobURLToken(
         base::FeatureList::IsEnabled(
             features::kBlockCrossPartitionBlobUrlFetching) &&
         !partitioning_disabled_by_policy_;
-    if (feature_and_policy_check) {
+    if (feature_and_policy_check && !has_storage_access_handle) {
       partitioning_blob_url_closure_.Run(url,
                                          blink::mojom::PartitioningBlobURLInfo::
                                              kBlockedCrossPartitionFetching);

storage/browser/blob/blob_url_store_impl.h

@@ -39,6 +39,11 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLStoreImpl
                        const GURL&,
                        std::optional<blink::mojom::PartitioningBlobURLInfo>)>
                        partitioning_blob_url_closure = base::DoNothing(),
+                   base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
+                       storage_access_check_closure = base::BindRepeating(
+                           [](base::OnceCallback<void(bool)> callback) {
+                             std::move(callback).Run(false);
+                           }),
                    bool partitioning_disabled_by_policy = false);
 
   BlobURLStoreImpl(const BlobURLStoreImpl&) = delete;
@@ -71,6 +76,19 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLStoreImpl
   // `Revoke()`.
   bool BlobUrlIsValid(const GURL& url, const char* method) const;
 
+  void FinishResolveAsURLLoaderFactory(
+      const GURL& url,
+      mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver,
+      ResolveAsURLLoaderFactoryCallback callback,
+      bool has_storage_access_handle);
+
+  void FinishResolveAsBlobURLToken(
+      const GURL& url,
+      mojo::PendingReceiver<blink::mojom::BlobURLToken> token,
+      bool is_top_level_navigation,
+      ResolveAsBlobURLTokenCallback callback,
+      bool has_storage_access_handle);
+
   const blink::StorageKey storage_key_;
   // The origin used by the worker/document associated with this BlobURLStore on
   // the renderer side. This will almost always be the same as `storage_key_`'s
@@ -90,6 +108,9 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) BlobURLStoreImpl
       void(const GURL&, std::optional<blink::mojom::PartitioningBlobURLInfo>)>
       partitioning_blob_url_closure_;
 
+  base::RepeatingCallback<void(base::OnceCallback<void(bool)>)>
+      storage_access_check_callback_;
+
   const bool partitioning_disabled_by_policy_;
 
   base::WeakPtrFactory<BlobURLStoreImpl> weak_ptr_factory_{this};

storage/browser/blob/blob_url_store_impl_unittest.cc

@@ -74,18 +74,19 @@ class BlobURLStoreImplTestP
 
   bool BlockCrossPartitionBlobUrlFetchingEnabled() {
     switch (test_case_) {
-      case PartitionedBlobUrlTestCase::
-          kBlockCrossPartitionBlobUrlFetchingDisabled:
       case PartitionedBlobUrlTestCase::
           kBlockCrossPartitionBlobUrlFetchingEnabled:
         return true;
+      case PartitionedBlobUrlTestCase::
+          kBlockCrossPartitionBlobUrlFetchingDisabled:
       default:
         return false;
     }
   }
 
   bool StoragePartitioningEnabled() {
-    return test_case_ != PartitionedBlobUrlTestCase::kPartitioningDisabled;
+    return test_case_ == PartitionedBlobUrlTestCase::
+                             kBlockCrossPartitionBlobUrlFetchingEnabled;
   }
 
   void TearDown() override {

third_party/blink/web_tests/TestLists/chrome.filter

@@ -33,6 +33,7 @@ external/wpt/fedcm/fedcm-storage-access-api-autogrant.tentative.https.sub.html
 external/wpt/partitioned-popins/partitioned-popins.cookies-allowed.tentative.sub.https.window.html
 external/wpt/partitioned-popins/partitioned-popins.cookies-blocked.tentative.sub.https.window.html
 external/wpt/partitioned-popins/partitioned-popins.localStorage.tentative.sub.https.window.html
+virtual/cross-partition-blob-url/external/wpt/*
 virtual/partitioned-popins-disabled/external/wpt/partitioned-popins/partitioned-popins.cookies-allowed.tentative.sub.https.window.html
 virtual/partitioned-popins-disabled/external/wpt/partitioned-popins/partitioned-popins.cookies-blocked.tentative.sub.https.window.html
 virtual/partitioned-popins-disabled/external/wpt/partitioned-popins/partitioned-popins.localStorage.tentative.sub.https.window.html

third_party/blink/web_tests/VirtualTestSuites

@@ -1116,10 +1116,11 @@
     "bases": [
       "external/wpt/FileAPI/BlobURL/cross-partition.https.html",
       "external/wpt/FileAPI/BlobURL/cross-partition-worker-creation.https.html",
-      "external/wpt/FileAPI/BlobURL/cross-partition-navigation.https.html"
+      "external/wpt/FileAPI/BlobURL/cross-partition-navigation.https.html",
+      "external/wpt/storage-access-api/storage-access-beyond-cookies.blobStorage.sub.https.window.html"
     ],
     "args": ["--enable-features=BlockCrossPartitionBlobUrlFetching,EnforceNoopenerOnBlobURLNavigation"],
-    "expires": "Mar 1, 2025",
+    "expires": "Sep 1, 2025",
     "owners": ["janiceliu@chromium.org"]
   },