Make web locks respect third party cookie restrictions

Bug: 1016355
Change-Id: Ie5b5a19735bb50738bc2e0ba7541a91f04b5cb12
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884824
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#712827}

chrome/browser/chrome_content_browser_client.cc

@@ -2526,6 +2526,15 @@ bool ChromeContentBrowserClient::AllowWorkerCacheStorage(
   return allow;
 }
 
+bool ChromeContentBrowserClient::AllowWorkerWebLocks(
+    const GURL& url,
+    content::BrowserContext* browser_context,
+    const std::vector<content::GlobalFrameRoutingId>& render_frames) {
+  Profile* profile = Profile::FromBrowserContext(browser_context);
+  auto cookie_settings = CookieSettingsFactory::GetForProfile(profile);
+  return cookie_settings->IsCookieAccessAllowed(url, url);
+}
+
 ChromeContentBrowserClient::AllowWebBluetoothResult
 ChromeContentBrowserClient::AllowWebBluetooth(
     content::BrowserContext* browser_context,

chrome/browser/chrome_content_browser_client.h

@@ -259,6 +259,10 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
       const GURL& url,
       content::BrowserContext* browser_context,
       const std::vector<content::GlobalFrameRoutingId>& render_frames) override;
+  bool AllowWorkerWebLocks(
+      const GURL& url,
+      content::BrowserContext* browser_context,
+      const std::vector<content::GlobalFrameRoutingId>& render_frames) override;
   AllowWebBluetoothResult AllowWebBluetooth(
       content::BrowserContext* browser_context,
       const url::Origin& requesting_origin,

chrome/browser/content_settings/content_settings_manager_impl.cc

@@ -145,6 +145,9 @@ void ContentSettingsManagerImpl::AllowStorageAccess(
       TabSpecificContentSettings::CacheStorageAccessed(
           render_process_id_, render_frame_id_, url, !allowed);
       break;
+    case StorageType::WEB_LOCKS:
+      // State not persisted, no need to record anything.
+      break;
   }
 
   std::move(callback).Run(allowed);

chrome/browser/net/cookie_policy_browsertest.cc

@@ -36,10 +36,9 @@ const std::vector<std::string> kStorageTypes{
     "IndexedDb", "WebSql",       "CacheStorage", "ServiceWorker",
 };
 
-// TODO(crbug.com/1016355): WebLocks can't be blocked yet.
 const std::vector<std::string> kCrossTabCommunicationTypes{
     "SharedWorker",
-    //"WebLock",
+    "WebLock",
 };
 
 class CookiePolicyBrowserTest : public InProcessBrowserTest {

chrome/common/content_settings_manager.mojom

@@ -21,7 +21,8 @@ interface ContentSettingsManager {
     SESSION_STORAGE,
     FILE_SYSTEM,
     INDEXED_DB,
-    CACHE
+    CACHE,
+    WEB_LOCKS,
   };
 
   // Sent by the renderer process to check whether access to a particular

chrome/renderer/content_settings_agent_impl.cc

@@ -245,16 +245,8 @@ void ContentSettingsAgentImpl::OnContentSettingsAgentRequest(
 }
 
 bool ContentSettingsAgentImpl::AllowDatabase() {
-  WebLocalFrame* frame = render_frame()->GetWebFrame();
-  if (IsUniqueFrame(frame))
-    return false;
-
-  bool result = false;
-  content_settings_manager_->AllowStorageAccess(
-      chrome::mojom::ContentSettingsManager::StorageType::DATABASE,
-      frame->GetSecurityOrigin(), frame->GetDocument().SiteForCookies(),
-      frame->GetDocument().TopFrameOrigin(), &result);
-  return result;
+  return AllowStorageAccess(
+      chrome::mojom::ContentSettingsManager::StorageType::DATABASE);
 }
 
 void ContentSettingsAgentImpl::RequestFileSystemAccessAsync(
@@ -293,29 +285,18 @@ bool ContentSettingsAgentImpl::AllowImage(bool enabled_per_settings,
 }
 
 bool ContentSettingsAgentImpl::AllowIndexedDB() {
-  WebLocalFrame* frame = render_frame()->GetWebFrame();
-  if (IsUniqueFrame(frame))
-    return false;
-
-  bool result = false;
-  content_settings_manager_->AllowStorageAccess(
-      chrome::mojom::ContentSettingsManager::StorageType::INDEXED_DB,
-      frame->GetSecurityOrigin(), frame->GetDocument().SiteForCookies(),
-      frame->GetDocument().TopFrameOrigin(), &result);
-  return result;
+  return AllowStorageAccess(
+      chrome::mojom::ContentSettingsManager::StorageType::INDEXED_DB);
 }
 
 bool ContentSettingsAgentImpl::AllowCacheStorage() {
-  WebLocalFrame* frame = render_frame()->GetWebFrame();
-  if (IsUniqueFrame(frame))
-    return false;
+  return AllowStorageAccess(
+      chrome::mojom::ContentSettingsManager::StorageType::CACHE);
+}
 
-  bool result = false;
-  content_settings_manager_->AllowStorageAccess(
-      chrome::mojom::ContentSettingsManager::StorageType::CACHE,
-      frame->GetSecurityOrigin(), frame->GetDocument().SiteForCookies(),
-      frame->GetDocument().TopFrameOrigin(), &result);
-  return result;
+bool ContentSettingsAgentImpl::AllowWebLocks() {
+  return AllowStorageAccess(
+      chrome::mojom::ContentSettingsManager::StorageType::WEB_LOCKS);
 }
 
 bool ContentSettingsAgentImpl::AllowScript(bool enabled_per_settings) {
@@ -635,3 +616,17 @@ bool ContentSettingsAgentImpl::IsWhitelistedForContentSettings(
   }
   return false;
 }
+
+bool ContentSettingsAgentImpl::AllowStorageAccess(
+    chrome::mojom::ContentSettingsManager::StorageType storage_type) {
+  WebLocalFrame* frame = render_frame()->GetWebFrame();
+  if (IsUniqueFrame(frame))
+    return false;
+
+  bool result = false;
+  content_settings_manager_->AllowStorageAccess(
+      storage_type, frame->GetSecurityOrigin(),
+      frame->GetDocument().SiteForCookies(),
+      frame->GetDocument().TopFrameOrigin(), &result);
+  return result;
+}

chrome/renderer/content_settings_agent_impl.h

@@ -84,6 +84,7 @@ class ContentSettingsAgentImpl
                   const blink::WebURL& image_url) override;
   bool AllowIndexedDB() override;
   bool AllowCacheStorage() override;
+  bool AllowWebLocks() override;
   bool AllowScript(bool enabled_per_settings) override;
   bool AllowScriptFromSource(bool enabled_per_settings,
                              const blink::WebURL& script_url) override;
@@ -160,6 +161,10 @@ class ContentSettingsAgentImpl
       const blink::WebSecurityOrigin& origin,
       const blink::WebURL& document_url);
 
+  // Common logic for AllowIndexedDB, AllowCacheStorage, etc.
+  bool AllowStorageAccess(
+      chrome::mojom::ContentSettingsManager::StorageType storage_type);
+
   mojo::Remote<chrome::mojom::ContentSettingsManager> content_settings_manager_;
 
 #if BUILDFLAG(ENABLE_EXTENSIONS)

chrome/renderer/worker_content_settings_client.cc

@@ -70,6 +70,11 @@ bool WorkerContentSettingsClient::AllowCacheStorage() {
       chrome::mojom::ContentSettingsManager::StorageType::CACHE);
 }
 
+bool WorkerContentSettingsClient::AllowWebLocks() {
+  return AllowStorageAccess(
+      chrome::mojom::ContentSettingsManager::StorageType::WEB_LOCKS);
+}
+
 bool WorkerContentSettingsClient::AllowRunningInsecureContent(
     bool allowed_per_settings,
     const blink::WebURL& url) {

chrome/renderer/worker_content_settings_client.h

@@ -32,6 +32,7 @@ class WorkerContentSettingsClient : public blink::WebContentSettingsClient {
   bool RequestFileSystemAccessSync() override;
   bool AllowIndexedDB() override;
   bool AllowCacheStorage() override;
+  bool AllowWebLocks() override;
   bool AllowRunningInsecureContent(bool allowed_per_settings,
                                    const blink::WebURL& url) override;
   bool AllowScriptFromSource(bool enabled_per_settings,

content/browser/service_worker/service_worker_content_settings_proxy_impl.cc

@@ -72,6 +72,27 @@ void ServiceWorkerContentSettingsProxyImpl::AllowCacheStorage(
           render_frames));
 }
 
+void ServiceWorkerContentSettingsProxyImpl::AllowWebLocks(
+    AllowWebLocksCallback callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // May be shutting down.
+  if (!context_wrapper_->browser_context()) {
+    std::move(callback).Run(false);
+    return;
+  }
+  if (origin_.opaque()) {
+    std::move(callback).Run(false);
+    return;
+  }
+  // |render_frames| is used to show UI for the frames affected by the
+  // content setting. However, service worker is not necessarily associated
+  // with frames or making the request on behalf of frames,
+  // so just pass an empty |render_frames|.
+  std::vector<GlobalFrameRoutingId> render_frames;
+  std::move(callback).Run(GetContentClient()->browser()->AllowWorkerWebLocks(
+      origin_.GetURL(), context_wrapper_->browser_context(), render_frames));
+}
+
 void ServiceWorkerContentSettingsProxyImpl::RequestFileSystemAccessSync(
     RequestFileSystemAccessSyncCallback callback) {
   mojo::ReportBadMessage(

content/browser/service_worker/service_worker_content_settings_proxy_impl.h

@@ -37,11 +37,11 @@ class ServiceWorkerContentSettingsProxyImpl final
   // blink::mojom::WorkerContentSettingsProxy implementation
   void AllowIndexedDB(AllowIndexedDBCallback callback) override;
   void AllowCacheStorage(AllowCacheStorageCallback callback) override;
+  void AllowWebLocks(AllowCacheStorageCallback callback) override;
   void RequestFileSystemAccessSync(
       RequestFileSystemAccessSyncCallback callback) override;
 
  private:
-
   const url::Origin origin_;
   scoped_refptr<ServiceWorkerContextWrapper> context_wrapper_;
   mojo::Receiver<blink::mojom::WorkerContentSettingsProxy> receiver_;

content/browser/worker_host/shared_worker_content_settings_proxy_impl.cc

@@ -41,6 +41,15 @@ void SharedWorkerContentSettingsProxyImpl::AllowCacheStorage(
   }
 }
 
+void SharedWorkerContentSettingsProxyImpl::AllowWebLocks(
+    AllowCacheStorageCallback callback) {
+  if (!origin_.opaque()) {
+    owner_->AllowWebLocks(origin_.GetURL(), std::move(callback));
+  } else {
+    std::move(callback).Run(false);
+  }
+}
+
 void SharedWorkerContentSettingsProxyImpl::RequestFileSystemAccessSync(
     RequestFileSystemAccessSyncCallback callback) {
   if (!origin_.opaque()) {

content/browser/worker_host/shared_worker_content_settings_proxy_impl.h

@@ -37,6 +37,7 @@ class CONTENT_EXPORT SharedWorkerContentSettingsProxyImpl
   // blink::mojom::WorkerContentSettingsProxy implementation.
   void AllowIndexedDB(AllowIndexedDBCallback callback) override;
   void AllowCacheStorage(AllowCacheStorageCallback callback) override;
+  void AllowWebLocks(AllowCacheStorageCallback callback) override;
   void RequestFileSystemAccessSync(
       RequestFileSystemAccessSyncCallback callback) override;
 

content/browser/worker_host/shared_worker_host.cc

@@ -334,6 +334,13 @@ void SharedWorkerHost::AllowCacheStorage(
           GetRenderFrameIDsForWorker()));
 }
 
+void SharedWorkerHost::AllowWebLocks(const GURL& url,
+                                     base::OnceCallback<void(bool)> callback) {
+  std::move(callback).Run(GetContentClient()->browser()->AllowWorkerWebLocks(
+      url, RenderProcessHost::FromID(worker_process_id_)->GetBrowserContext(),
+      GetRenderFrameIDsForWorker()));
+}
+
 void SharedWorkerHost::BindVideoDecodePerfHistory(
     mojo::PendingReceiver<media::mojom::VideoDecodePerfHistory> receiver) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);

content/browser/worker_host/shared_worker_host.h

@@ -109,6 +109,7 @@ class CONTENT_EXPORT SharedWorkerHost
   void AllowIndexedDB(const GURL& url, base::OnceCallback<void(bool)> callback);
   void AllowCacheStorage(const GURL& url,
                          base::OnceCallback<void(bool)> callback);
+  void AllowWebLocks(const GURL& url, base::OnceCallback<void(bool)> callback);
 
   void BindVideoDecodePerfHistory(
       mojo::PendingReceiver<media::mojom::VideoDecodePerfHistory> receiver);

content/public/browser/content_browser_client.cc

@@ -372,6 +372,13 @@ bool ContentBrowserClient::AllowWorkerCacheStorage(
   return true;
 }
 
+bool ContentBrowserClient::AllowWorkerWebLocks(
+    const GURL& url,
+    BrowserContext* browser_context,
+    const std::vector<GlobalFrameRoutingId>& render_frames) {
+  return true;
+}
+
 ContentBrowserClient::AllowWebBluetoothResult
 ContentBrowserClient::AllowWebBluetooth(
     content::BrowserContext* browser_context,

content/public/browser/content_browser_client.h

@@ -633,6 +633,13 @@ class CONTENT_EXPORT ContentBrowserClient {
       BrowserContext* browser_context,
       const std::vector<GlobalFrameRoutingId>& render_frames);
 
+  // Allow the embedder to control if access to Web Locks by a shared worker
+  // is allowed.
+  virtual bool AllowWorkerWebLocks(
+      const GURL& url,
+      BrowserContext* browser_context,
+      const std::vector<GlobalFrameRoutingId>& render_frames);
+
   // Allow the embedder to control if access to CacheStorage by a shared worker
   // is allowed.
   virtual bool AllowWorkerCacheStorage(

content/test/data/browsing_data/site_data.html

@@ -195,7 +195,7 @@
         domAutomationController.send(locks.held[0].name === "foo");
       else
         failure_();
-    });
+    }).catch(failure_);
   }
 </script>
 

third_party/blink/public/mojom/worker/worker_content_settings_proxy.mojom

@@ -22,6 +22,10 @@ interface WorkerContentSettingsProxy {
   [Sync]
   AllowCacheStorage() => (bool result);
 
+  // Returns whether the worker is allowed access to Web Locks.
+  [Sync]
+  AllowWebLocks() => (bool result);
+
   // Returns whether the worker is allowed access to the file system.
   [Sync]
   RequestFileSystemAccessSync() => (bool result);

third_party/blink/public/platform/web_content_settings_client.h

@@ -47,6 +47,9 @@ class WebContentSettingsClient {
   // Controls whether access to CacheStorage is allowed for this frame.
   virtual bool AllowCacheStorage() { return true; }
 
+  // Controls whether access to Web Locks is allowed for this frame.
+  virtual bool AllowWebLocks() { return true; }
+
   // Controls whether scripts are allowed to execute for this frame.
   virtual bool AllowScript(bool enabled_per_settings) {
     return enabled_per_settings;

third_party/blink/renderer/core/workers/shared_worker_content_settings_proxy.cc

@@ -27,6 +27,12 @@ bool SharedWorkerContentSettingsProxy::AllowCacheStorage() {
   return result;
 }
 
+bool SharedWorkerContentSettingsProxy::AllowWebLocks() {
+  bool result = false;
+  GetService()->AllowWebLocks(&result);
+  return result;
+}
+
 bool SharedWorkerContentSettingsProxy::RequestFileSystemAccessSync() {
   bool result = false;
   GetService()->RequestFileSystemAccessSync(&result);

third_party/blink/renderer/core/workers/shared_worker_content_settings_proxy.h

@@ -26,6 +26,7 @@ class SharedWorkerContentSettingsProxy : public WebContentSettingsClient {
   // WebContentSettingsClient overrides.
   bool AllowIndexedDB() override;
   bool AllowCacheStorage() override;
+  bool AllowWebLocks() override;
   bool RequestFileSystemAccessSync() override;
 
  private:

third_party/blink/renderer/modules/locks/lock_manager.cc

@@ -9,11 +9,16 @@
 #include "mojo/public/cpp/bindings/associated_receiver.h"
 #include "third_party/blink/public/common/browser_interface_broker_proxy.h"
 #include "third_party/blink/public/platform/task_type.h"
+#include "third_party/blink/public/platform/web_content_settings_client.h"
+#include "third_party/blink/public/platform/web_security_origin.h"
 #include "third_party/blink/renderer/bindings/core/v8/script_promise_resolver.h"
 #include "third_party/blink/renderer/bindings/modules/v8/v8_lock_granted_callback.h"
 #include "third_party/blink/renderer/core/dom/abort_signal.h"
+#include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/dom/dom_exception.h"
+#include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/frame/web_feature.h"
+#include "third_party/blink/renderer/core/workers/worker_global_scope.h"
 #include "third_party/blink/renderer/modules/locks/lock.h"
 #include "third_party/blink/renderer/modules/locks/lock_info.h"
 #include "third_party/blink/renderer/modules/locks/lock_manager_snapshot.h"
@@ -26,6 +31,7 @@
 #include "third_party/blink/renderer/platform/scheduler/public/frame_scheduler.h"
 #include "third_party/blink/renderer/platform/scheduler/public/scheduling_policy.h"
 #include "third_party/blink/renderer/platform/weborigin/security_origin.h"
+#include "third_party/blink/renderer/platform/wtf/casting.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 #include "third_party/blink/renderer/platform/wtf/vector.h"
 
@@ -223,7 +229,8 @@ ScriptPromise LockManager::request(ScriptState* script_state,
 
   // 5. If origin is an opaque origin, then reject promise with a
   // "SecurityError" DOMException.
-  if (!context->GetSecurityOrigin()->CanAccessLocks()) {
+  if (!context->GetSecurityOrigin()->CanAccessLocks() ||
+      !AllowLocks(script_state)) {
     exception_state.ThrowSecurityError(
         "Access to the Locks API is denied in this context.");
     return ScriptPromise();
@@ -343,7 +350,8 @@ ScriptPromise LockManager::query(ScriptState* script_state,
   ExecutionContext* context = ExecutionContext::From(script_state);
   DCHECK(context->IsContextThread());
 
-  if (!context->GetSecurityOrigin()->CanAccessLocks()) {
+  if (!context->GetSecurityOrigin()->CanAccessLocks() ||
+      !AllowLocks(script_state)) {
     exception_state.ThrowSecurityError(
         "Access to the Locks API is denied in this context.");
     return ScriptPromise();
@@ -412,4 +420,34 @@ void LockManager::OnLockReleased(Lock* lock) {
   held_locks_.erase(lock);
 }
 
+bool LockManager::AllowLocks(ScriptState* script_state) {
+  if (!cached_allowed_.has_value()) {
+    ExecutionContext* execution_context = ExecutionContext::From(script_state);
+    DCHECK(execution_context->IsContextThread());
+    SECURITY_DCHECK(execution_context->IsDocument() ||
+                    execution_context->IsWorkerGlobalScope());
+    if (auto* document = DynamicTo<Document>(execution_context)) {
+      LocalFrame* frame = document->GetFrame();
+      if (!frame) {
+        cached_allowed_ = false;
+      } else if (auto* settings_client = frame->GetContentSettingsClient()) {
+        // This triggers a sync IPC.
+        cached_allowed_ = settings_client->AllowWebLocks();
+      } else {
+        cached_allowed_ = true;
+      }
+    } else {
+      WebContentSettingsClient* content_settings_client =
+          To<WorkerGlobalScope>(execution_context)->ContentSettingsClient();
+      if (!content_settings_client) {
+        cached_allowed_ = true;
+      } else {
+        // This triggers a sync IPC.
+        cached_allowed_ = content_settings_client->AllowWebLocks();
+      }
+    }
+  }
+  return cached_allowed_.value();
+}
+
 }  // namespace blink

third_party/blink/renderer/modules/locks/lock_manager.h

@@ -6,6 +6,7 @@
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_LOCKS_LOCK_MANAGER_H_
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/blink/public/mojom/locks/lock_manager.mojom-blink-forward.h"
 #include "third_party/blink/renderer/bindings/core/v8/string_or_string_sequence.h"
@@ -64,11 +65,18 @@ class LockManager final : public ScriptWrappable,
   void RemovePendingRequest(LockRequestImpl*);
   bool IsPendingRequest(LockRequestImpl*);
 
+  // Query the ContentSettingsClient to ensure access is allowed from
+  // this context. The first call invokes a synchronous IPC call, but
+  // the result is cached for subsequent accesses.
+  bool AllowLocks(ScriptState* script_state);
+
   HeapHashSet<Member<LockRequestImpl>> pending_requests_;
   HeapHashSet<Member<Lock>> held_locks_;
 
   mojo::Remote<mojom::blink::LockManager> service_;
 
+  base::Optional<bool> cached_allowed_;
+
   DISALLOW_COPY_AND_ASSIGN(LockManager);
 };