Prevent permission delegation for "notifications".

TL;DR: Only browsing contexts that are same-origin with the top-level
context should be able to request the "notifications" permission; but
once an origin has obtained the permission, it should see its state as
granted, and be permitted to use the related APIs both in first and
third party contexts (in the latter case, regardless of the permission
state for the top-level origin).

In more detail, a cross-origin subframe that had not been granted the
NOTIFICATIONS permission previously (in a first-party context) should
see the permission as "denied" through all of `Notification.permission`,
`pushManager.permissionState`, and `permissions.query`; not be able to
prompt for the permission; and not be able to register Push API
subscriptions or use the Web Notification API.

A cross-origin subframe that had been granted the NOTIFICATIONS
permission previously (in a first-party context) should see it as
"granted", through all of the above-mentioned API surfaces; and be able
to register Push API subscriptions and use the Web Notification API.

This CL achieves this by explicitly turning off permission delegation
for CONTENT_SETTINGS_TYPE_NOTIFICATIONS.

Bug: 987654
Change-Id: Ie7d7dd7da7c9a4630b09a54ba215c883511feb50
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1720831
Commit-Queue: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Raymes Khoury <raymes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#682221}

chrome/browser/media/webrtc/media_stream_devices_controller.cc

@@ -516,7 +516,8 @@ void MediaStreamDevicesController::UpdateTabSpecificContentSettings(
 
   content_settings_->OnMediaStreamPermissionSet(
       PermissionManager::Get(profile_)->GetCanonicalOrigin(
-          request_.security_origin, web_contents_->GetLastCommittedURL()),
+          CONTENT_SETTINGS_TYPE_MEDIASTREAM_CAMERA, request_.security_origin,
+          web_contents_->GetLastCommittedURL()),
       microphone_camera_state, selected_audio_device, selected_video_device,
       requested_audio_device, requested_video_device);
 }

chrome/browser/permissions/permission_manager.cc

@@ -346,7 +346,8 @@ void PermissionManager::Shutdown() {
   }
 }
 
-GURL PermissionManager::GetCanonicalOrigin(const GURL& requesting_origin,
+GURL PermissionManager::GetCanonicalOrigin(ContentSettingsType permission,
+                                           const GURL& requesting_origin,
                                            const GURL& embedding_origin) const {
   if (embedding_origin.GetOrigin() ==
       GURL(chrome::kChromeUINewTabURL).GetOrigin()) {
@@ -359,6 +360,12 @@ GURL PermissionManager::GetCanonicalOrigin(const GURL& requesting_origin,
     }
   }
 
+  // TODO(crbug.com/987654): Generalize this to other "background permissions",
+  // that is, permissions that can be used by a service worker. This includes
+  // durable storage, background sync, etc.
+  if (permission == CONTENT_SETTINGS_TYPE_NOTIFICATIONS)
+    return requesting_origin;
+
   if (base::FeatureList::IsEnabled(features::kPermissionDelegation)) {
     // Once permission delegation is enabled by default, it may be possible to
     // remove "embedding_origin" as a parameter from all function calls in
@@ -405,17 +412,16 @@ int PermissionManager::RequestPermissions(
   content::WebContents* web_contents =
       content::WebContents::FromRenderFrameHost(render_frame_host);
 
-  GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin();
-  GURL canonical_requesting_origin =
-      GetCanonicalOrigin(requesting_origin, embedding_origin);
-
   int request_id = pending_requests_.Add(std::make_unique<PendingRequest>(
       render_frame_host, permissions, std::move(callback)));
 
   const PermissionRequestID request(render_frame_host, request_id);
+  const GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin();
 
   for (size_t i = 0; i < permissions.size(); ++i) {
     const ContentSettingsType permission = permissions[i];
+    const GURL canonical_requesting_origin =
+        GetCanonicalOrigin(permission, requesting_origin, embedding_origin);
 
     auto response_callback =
         std::make_unique<PermissionResponseCallback>(this, request_id, i);
@@ -533,7 +539,7 @@ void PermissionManager::ResetPermission(PermissionType permission,
   if (!context)
     return;
   context->ResetPermission(
-      GetCanonicalOrigin(requesting_origin, embedding_origin),
+      GetCanonicalOrigin(type, requesting_origin, embedding_origin),
       embedding_origin.GetOrigin());
 }
 
@@ -551,7 +557,7 @@ PermissionStatus PermissionManager::GetPermissionStatus(
   PermissionContextBase* context = GetPermissionContext(type);
   if (context) {
     result = context->UpdatePermissionStatusWithDeviceStatus(
-        result, GetCanonicalOrigin(requesting_origin, embedding_origin),
+        result, GetCanonicalOrigin(type, requesting_origin, embedding_origin),
         embedding_origin);
   }
 
@@ -563,9 +569,9 @@ PermissionStatus PermissionManager::GetPermissionStatusForFrame(
     content::RenderFrameHost* render_frame_host,
     const GURL& requesting_origin) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  ContentSettingsType type = PermissionTypeToContentSetting(permission);
   PermissionResult result =
-      GetPermissionStatusForFrame(PermissionTypeToContentSetting(permission),
-                                  render_frame_host, requesting_origin);
+      GetPermissionStatusForFrame(type, render_frame_host, requesting_origin);
 
   // TODO(benwells): split this into two functions, GetPermissionStatus and
   // GetPermissionStatusForPermissionsAPI.
@@ -576,7 +582,7 @@ PermissionStatus PermissionManager::GetPermissionStatusForFrame(
         content::WebContents::FromRenderFrameHost(render_frame_host);
     GURL embedding_origin = web_contents->GetLastCommittedURL().GetOrigin();
     result = context->UpdatePermissionStatusWithDeviceStatus(
-        result, GetCanonicalOrigin(requesting_origin, embedding_origin),
+        result, GetCanonicalOrigin(type, requesting_origin, embedding_origin),
         embedding_origin);
   }
 
@@ -618,7 +624,7 @@ int PermissionManager::SubscribePermissionStatusChange(
 
   subscription->permission = content_type;
   subscription->requesting_origin =
-      GetCanonicalOrigin(requesting_origin, embedding_origin);
+      GetCanonicalOrigin(content_type, requesting_origin, embedding_origin);
   subscription->callback =
       base::BindRepeating(&SubscriptionCallbackWrapper, std::move(callback));
 
@@ -707,7 +713,7 @@ PermissionResult PermissionManager::GetPermissionStatusHelper(
     const GURL& requesting_origin,
     const GURL& embedding_origin) {
   GURL canonical_requesting_origin =
-      GetCanonicalOrigin(requesting_origin, embedding_origin);
+      GetCanonicalOrigin(permission, requesting_origin, embedding_origin);
   auto status =
       GetPermissionOverrideForDevTools(canonical_requesting_origin, permission);
   if (status != CONTENT_SETTING_DEFAULT)

chrome/browser/permissions/permission_manager.h

@@ -41,7 +41,8 @@ class PermissionManager : public KeyedService,
   // GetPermissionStatus, take the actual origin and do the canonicalization
   // internally. You only need to call this directly if you do something else
   // with the origin, such as display it in the UI.
-  GURL GetCanonicalOrigin(const GURL& requesting_origin,
+  GURL GetCanonicalOrigin(ContentSettingsType permission,
+                          const GURL& requesting_origin,
                           const GURL& embedding_origin) const;
 
   // Callers from within chrome/ should use the methods which take the

chrome/browser/permissions/permission_manager_unittest.cc

@@ -600,27 +600,35 @@ TEST_F(PermissionManagerTest, GetCanonicalOriginSearch) {
   const GURL top_level_ntp(chrome::kChromeUINewTabURL);
 
   // "Normal" URLs are not affected by GetCanonicalOrigin.
-  EXPECT_EQ(google_com, GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                            google_com, google_com));
-  EXPECT_EQ(google_de, GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                           google_de, google_de));
-  EXPECT_EQ(other_url, GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                           other_url, other_url));
-  EXPECT_EQ(google_base, GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                             google_base, google_base));
+  EXPECT_EQ(google_com,
+            GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                CONTENT_SETTINGS_TYPE_GEOLOCATION, google_com, google_com));
+  EXPECT_EQ(google_de,
+            GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                CONTENT_SETTINGS_TYPE_GEOLOCATION, google_de, google_de));
+  EXPECT_EQ(other_url,
+            GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                CONTENT_SETTINGS_TYPE_GEOLOCATION, other_url, other_url));
+  EXPECT_EQ(google_base,
+            GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                CONTENT_SETTINGS_TYPE_GEOLOCATION, google_base, google_base));
 
   // The local NTP URL gets mapped to the Google base URL.
-  EXPECT_EQ(google_base, GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                             local_ntp, top_level_ntp));
+  EXPECT_EQ(google_base,
+            GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                CONTENT_SETTINGS_TYPE_GEOLOCATION, local_ntp, top_level_ntp));
   // However, other chrome-search:// URLs, including the remote NTP URL, are
   // not affected.
-  EXPECT_EQ(remote_ntp, GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                            remote_ntp, top_level_ntp));
-  EXPECT_EQ(google_com, GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                            google_com, top_level_ntp));
+  EXPECT_EQ(remote_ntp,
+            GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                CONTENT_SETTINGS_TYPE_GEOLOCATION, remote_ntp, top_level_ntp));
+  EXPECT_EQ(google_com,
+            GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                CONTENT_SETTINGS_TYPE_GEOLOCATION, google_com, top_level_ntp));
   EXPECT_EQ(other_chrome_search,
             GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                other_chrome_search, top_level_ntp));
+                CONTENT_SETTINGS_TYPE_GEOLOCATION, other_chrome_search,
+                top_level_ntp));
 }
 
 TEST_F(PermissionManagerTest, GetCanonicalOriginPermissionDelegation) {
@@ -636,9 +644,11 @@ TEST_F(PermissionManagerTest, GetCanonicalOriginPermissionDelegation) {
     // be returned.
     EXPECT_EQ(requesting_origin,
               GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                  requesting_origin, embedding_origin));
+                  CONTENT_SETTINGS_TYPE_GEOLOCATION, requesting_origin,
+                  embedding_origin));
     EXPECT_EQ(extensions_requesting_origin,
               GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                  CONTENT_SETTINGS_TYPE_GEOLOCATION,
                   extensions_requesting_origin, embedding_origin));
   }
 
@@ -646,13 +656,20 @@ TEST_F(PermissionManagerTest, GetCanonicalOriginPermissionDelegation) {
     base::test::ScopedFeatureList scoped_feature_list;
     scoped_feature_list.InitAndEnableFeature(features::kPermissionDelegation);
     // With permission delegation, the embedding origin should be returned
-    // except in the case of extensions.
+    // except in the case of extensions; and except for notifications, for which
+    // permission delegation is always off.
     EXPECT_EQ(embedding_origin,
               GetPermissionControllerDelegate()->GetCanonicalOrigin(
-                  requesting_origin, embedding_origin));
+                  CONTENT_SETTINGS_TYPE_GEOLOCATION, requesting_origin,
+                  embedding_origin));
     EXPECT_EQ(extensions_requesting_origin,
               GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                  CONTENT_SETTINGS_TYPE_GEOLOCATION,
                   extensions_requesting_origin, embedding_origin));
+    EXPECT_EQ(requesting_origin,
+              GetPermissionControllerDelegate()->GetCanonicalOrigin(
+                  CONTENT_SETTINGS_TYPE_NOTIFICATIONS, requesting_origin,
+                  embedding_origin));
   }
 }
 

chrome/browser/push_messaging/push_messaging_service_impl.cc

@@ -546,8 +546,8 @@ void PushMessagingServiceImpl::SubscribeFromDocument(
 
   // Push does not allow permission requests from iframes.
   PermissionManager::Get(profile_)->RequestPermission(
-      CONTENT_SETTINGS_TYPE_NOTIFICATIONS, web_contents->GetMainFrame(),
-      requesting_origin, user_gesture,
+      CONTENT_SETTINGS_TYPE_NOTIFICATIONS, render_frame_host, requesting_origin,
+      user_gesture,
       base::BindOnce(&PushMessagingServiceImpl::DoSubscribe,
                      weak_factory_.GetWeakPtr(), app_identifier,
                      std::move(options), std::move(callback)));

chrome/test/data/push_messaging/framed_test.html

+<iframe src="/cross-site/requester.com/push_messaging/test.html"></iframe>

chrome/test/data/push_messaging/push_test.js

@@ -176,7 +176,7 @@ function GetP256dh() {
   }).catch(sendErrorToTest);
 }
 
-function permissionState() {
+function pushManagerPermissionState() {
   navigator.serviceWorker.ready.then(function(swRegistration) {
     return swRegistration.pushManager.permissionState({userVisibleOnly: true})
         .then(function(permission) {
@@ -185,6 +185,17 @@ function permissionState() {
   }).catch(sendErrorToTest);
 }
 
+function notificationPermissionState() {
+  sendResultToTest('permission status - ' + Notification.permission);
+}
+
+function notificationPermissionAPIState() {
+  navigator.permissions.query({name: 'notifications'}).then(
+      permission_status => {
+    sendResultToTest('permission status - ' + permission_status.state);
+  }).catch(sendErrorToTest);
+}
+
 function isControlled() {
   if (navigator.serviceWorker.controller) {
     sendResultToTest('true - is controlled');

content/browser/notifications/blink_notification_service_impl.cc

@@ -179,6 +179,9 @@ void BlinkNotificationServiceImpl::CloseNonPersistentNotification(
 blink::mojom::PermissionStatus
 BlinkNotificationServiceImpl::CheckPermissionStatus() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // TOOD(crbug.com/987654): It is odd that a service instance can be created
+  // for cross-origin subframes, yet the instance is completely oblivious of
+  // whether it is serving a top-level browsing context or an embedded one.
   return BrowserContext::GetPermissionController(browser_context_)
       ->GetPermissionStatus(PermissionType::NOTIFICATIONS, origin_.GetURL(),
                             origin_.GetURL());