From 893f19930ede2c5d78c10c1181cdab30cd7d1a67 Mon Sep 17 00:00:00 2001 From: Karl Tomlinson Date: Thu, 29 Jul 2021 04:49:57 +0000 Subject: [PATCH] Bug 1720643 perform getUserMedia Permissions Policy checks even when "media.navigator.permission.disabled" is set r=jib "media.getusermedia.microphone.deny" and "media.getusermedia.camera.deny" now override "media.navigator.permission.disabled". User permission checks are removed because they are repeated in the app. Differential Revision: https://phabricator.services.mozilla.com/D120065 --- .../webrtc/browser_devices_get_user_media.js | 8 +-- ...ices_get_user_media_default_permissions.js | 57 +++++++++++-------- ...devices_get_user_media_in_xorigin_frame.js | 19 +++++-- .../browser_devices_get_user_media_screen.js | 12 ++-- dom/media/MediaManager.cpp | 43 +++++--------- .../test_getUserMedia_permission.html | 12 +++- ...ream-default-feature-policy.https.html.ini | 33 ----------- ...ions-policy-audio+video.https.sub.html.ini | 6 +- ...ermissions-policy-audio.https.sub.html.ini | 6 +- ...ermissions-policy-video.https.sub.html.ini | 6 +- 10 files changed, 92 insertions(+), 110 deletions(-) delete mode 100644 testing/web-platform/meta/mediacapture-streams/MediaStream-default-feature-policy.https.html.ini diff --git a/browser/base/content/test/webrtc/browser_devices_get_user_media.js b/browser/base/content/test/webrtc/browser_devices_get_user_media.js index cf1e84a240e2..eed38b74b64e 100644 --- a/browser/base/content/test/webrtc/browser_devices_get_user_media.js +++ b/browser/base/content/test/webrtc/browser_devices_get_user_media.js @@ -520,17 +520,15 @@ var gTests = [ } else { let expectedMessage = aExpectStream ? "ok" : permissionError; - let observerPromises = []; + let observerPromises = [expectObserverCalled("getUserMedia:request")]; if (expectedMessage == "ok") { - observerPromises.push(expectObserverCalled("getUserMedia:request")); - observerPromises.push( - expectObserverCalled("getUserMedia:response:allow") - ); observerPromises.push( + expectObserverCalled("getUserMedia:response:allow"), expectObserverCalled("recording-device-events") ); } else { observerPromises.push( + expectObserverCalled("getUserMedia:response:deny"), expectObserverCalled("recording-window-ended") ); } diff --git a/browser/base/content/test/webrtc/browser_devices_get_user_media_default_permissions.js b/browser/base/content/test/webrtc/browser_devices_get_user_media_default_permissions.js index 4c00c6e21991..e6464fd4aa74 100644 --- a/browser/base/content/test/webrtc/browser_devices_get_user_media_default_permissions.js +++ b/browser/base/content/test/webrtc/browser_devices_get_user_media_default_permissions.js @@ -16,24 +16,28 @@ var gTests = [ Services.prefs.setIntPref(CAMERA_PREF, SitePermissions.BLOCK); // Requesting audio+video shouldn't work. - let observerPromise = expectObserverCalled("recording-window-ended"); - let promise = promiseMessage(permissionError); - await promiseRequestDevice(true, true); - await promise; - await observerPromise; + await Promise.all([ + expectObserverCalled("getUserMedia:request"), + expectObserverCalled("getUserMedia:response:deny"), + expectObserverCalled("recording-window-ended"), + promiseMessage(permissionError), + promiseRequestDevice(true, true), + ]); await checkNotSharing(); // Requesting only video shouldn't work. - observerPromise = expectObserverCalled("recording-window-ended"); - promise = promiseMessage(permissionError); - await promiseRequestDevice(false, true); - await promise; - await observerPromise; + await Promise.all([ + expectObserverCalled("getUserMedia:request"), + expectObserverCalled("getUserMedia:response:deny"), + expectObserverCalled("recording-window-ended"), + promiseMessage(permissionError), + promiseRequestDevice(false, true), + ]); await checkNotSharing(); // Requesting audio should work. - observerPromise = expectObserverCalled("getUserMedia:request"); - promise = promisePopupNotificationShown("webRTC-shareDevices"); + const observerPromise = expectObserverCalled("getUserMedia:request"); + const promise = promisePopupNotificationShown("webRTC-shareDevices"); await promiseRequestDevice(true); await promise; await observerPromise; @@ -110,24 +114,27 @@ var gTests = [ Services.prefs.setIntPref(MICROPHONE_PREF, SitePermissions.BLOCK); // Requesting audio+video shouldn't work. - let observerPromise = expectObserverCalled("recording-window-ended"); - let promise = promiseMessage(permissionError); - await promiseRequestDevice(true, true); - await promise; - await observerPromise; + await Promise.all([ + expectObserverCalled("getUserMedia:request"), + expectObserverCalled("getUserMedia:response:deny"), + expectObserverCalled("recording-window-ended"), + promiseMessage(permissionError), + promiseRequestDevice(true, true), + ]); await checkNotSharing(); // Requesting only audio shouldn't work. - observerPromise = expectObserverCalled("recording-window-ended"); - promise = promiseMessage(permissionError); - await promiseRequestDevice(true); - await promise; - await observerPromise; - await checkNotSharing(); + await Promise.all([ + expectObserverCalled("getUserMedia:request"), + expectObserverCalled("getUserMedia:response:deny"), + expectObserverCalled("recording-window-ended"), + promiseMessage(permissionError), + promiseRequestDevice(true), + ]); // Requesting video should work. - observerPromise = expectObserverCalled("getUserMedia:request"); - promise = promisePopupNotificationShown("webRTC-shareDevices"); + const observerPromise = expectObserverCalled("getUserMedia:request"); + const promise = promisePopupNotificationShown("webRTC-shareDevices"); await promiseRequestDevice(false, true); await promise; await observerPromise; diff --git a/browser/base/content/test/webrtc/browser_devices_get_user_media_in_xorigin_frame.js b/browser/base/content/test/webrtc/browser_devices_get_user_media_in_xorigin_frame.js index b7ff04c5ca6a..0e9ef229a7b3 100644 --- a/browser/base/content/test/webrtc/browser_devices_get_user_media_in_xorigin_frame.js +++ b/browser/base/content/test/webrtc/browser_devices_get_user_media_in_xorigin_frame.js @@ -366,13 +366,20 @@ var gTests = [ await closeStream(false, aIframeId); } else if (aExpect == PromptResult.DENY) { - const observerPromise = expectObserverCalled( - "recording-window-ended" + const promises = []; + // frame3 disallows by feature Permissions Policy before request. + if (aIframeId != "frame3") { + promises.push( + expectObserverCalled("getUserMedia:request"), + expectObserverCalled("getUserMedia:response:deny") + ); + } + promises.push( + expectObserverCalled("recording-window-ended"), + promiseMessage(permissionError), + promiseRequestDevice(audio, video, aIframeId, screen) ); - const promise = promiseMessage(permissionError); - await promiseRequestDevice(audio, video, aIframeId, screen); - await promise; - await observerPromise; + await Promise.all(promises); } PermissionTestUtils.remove(uri, aRequestType); diff --git a/browser/base/content/test/webrtc/browser_devices_get_user_media_screen.js b/browser/base/content/test/webrtc/browser_devices_get_user_media_screen.js index 30278f7fffd0..24eed7425aca 100644 --- a/browser/base/content/test/webrtc/browser_devices_get_user_media_screen.js +++ b/browser/base/content/test/webrtc/browser_devices_get_user_media_screen.js @@ -757,11 +757,13 @@ var gTests = [ ); // Request screensharing again, expect an immediate failure. - observerPromise = expectObserverCalled("recording-window-ended"); - promise = promiseMessage(permissionError); - await promiseRequestDevice(false, true, null, "screen"); - await promise; - await observerPromise; + await Promise.all([ + expectObserverCalled("getUserMedia:request"), + expectObserverCalled("getUserMedia:response:deny"), + expectObserverCalled("recording-window-ended"), + promiseMessage(permissionError), + promiseRequestDevice(false, true, null, "screen"), + ]); // Now set the permission to allow and expect a prompt. SitePermissions.setForPrincipal( diff --git a/dom/media/MediaManager.cpp b/dom/media/MediaManager.cpp index e89ff131b9ac..8fa6c10cb87b 100644 --- a/dom/media/MediaManager.cpp +++ b/dom/media/MediaManager.cpp @@ -2640,45 +2640,32 @@ RefPtr MediaManager::GetUserMedia( auto placeholderListener = MakeRefPtr(); windowListener->Register(placeholderListener); - if (!privileged) { - // Check if this site has had persistent permissions denied. - RefPtr permDelegate = - doc->GetPermissionDelegateHandler(); - MOZ_RELEASE_ASSERT(permDelegate); - - uint32_t audioPerm = nsIPermissionManager::UNKNOWN_ACTION; + { // Check Permissions Policy. Reject if a requested feature is disabled. + bool disabled = !IsOn(c.mAudio) && !IsOn(c.mVideo); if (IsOn(c.mAudio)) { if (audioType == MediaSourceEnum::Microphone) { - if (Preferences::GetBool("media.getusermedia.microphone.deny", false)) { - audioPerm = nsIPermissionManager::DENY_ACTION; - } else { - rv = permDelegate->GetPermission("microphone"_ns, &audioPerm, true); - MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv)); + if (Preferences::GetBool("media.getusermedia.microphone.deny", false) || + !FeaturePolicyUtils::IsFeatureAllowed(doc, u"microphone"_ns)) { + disabled = true; } - } else { - rv = permDelegate->GetPermission("screen"_ns, &audioPerm, true); - MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv)); + } else if (!FeaturePolicyUtils::IsFeatureAllowed(doc, + u"display-capture"_ns)) { + disabled = true; } } - - uint32_t videoPerm = nsIPermissionManager::UNKNOWN_ACTION; if (IsOn(c.mVideo)) { if (videoType == MediaSourceEnum::Camera) { - if (Preferences::GetBool("media.getusermedia.camera.deny", false)) { - videoPerm = nsIPermissionManager::DENY_ACTION; - } else { - rv = permDelegate->GetPermission("camera"_ns, &videoPerm, true); - MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv)); + if (Preferences::GetBool("media.getusermedia.camera.deny", false) || + !FeaturePolicyUtils::IsFeatureAllowed(doc, u"camera"_ns)) { + disabled = true; } - } else { - rv = permDelegate->GetPermission("screen"_ns, &videoPerm, true); - MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv)); + } else if (!FeaturePolicyUtils::IsFeatureAllowed(doc, + u"display-capture"_ns)) { + disabled = true; } } - if ((!IsOn(c.mAudio) && !IsOn(c.mVideo)) || - (IsOn(c.mAudio) && audioPerm == nsIPermissionManager::DENY_ACTION) || - (IsOn(c.mVideo) && videoPerm == nsIPermissionManager::DENY_ACTION)) { + if (disabled) { placeholderListener->Stop(); return StreamPromise::CreateAndReject( MakeRefPtr(MediaMgrError::Name::NotAllowedError), diff --git a/dom/media/webrtc/tests/mochitests/test_getUserMedia_permission.html b/dom/media/webrtc/tests/mochitests/test_getUserMedia_permission.html index c392a23d26aa..74775554ea7a 100644 --- a/dom/media/webrtc/tests/mochitests/test_getUserMedia_permission.html +++ b/dom/media/webrtc/tests/mochitests/test_getUserMedia_permission.html @@ -59,9 +59,17 @@ runTest(async () => { const src = origin + path; is(await iframeGum({ src, sandbox: "allow-scripts" }), "NotAllowedError", "gUM fails in sandboxed iframe " + origin); - is(await iframeGum({ src, sandbox: "allow-scripts allow-same-origin" }), - "success", "gUM works in regular iframe" + origin); } + is(await iframeGum({ + src: path, + sandbox: "allow-scripts allow-same-origin", + }), + "success", "gUM works in regular same-origin iframe"); + is(await iframeGum({ + src: `https://test1.example.com${path}`, + sandbox: "allow-scripts allow-same-origin", + }), + "NotAllowedError", "gUM fails in regular cross-origin iframe"); // Test gUM in sandboxed vs regular srcdoc iframe diff --git a/testing/web-platform/meta/mediacapture-streams/MediaStream-default-feature-policy.https.html.ini b/testing/web-platform/meta/mediacapture-streams/MediaStream-default-feature-policy.https.html.ini deleted file mode 100644 index db13e89ae380..000000000000 --- a/testing/web-platform/meta/mediacapture-streams/MediaStream-default-feature-policy.https.html.ini +++ /dev/null @@ -1,33 +0,0 @@ -[MediaStream-default-feature-policy.https.sub.html] - [Default "microphone" feature policy ["self"\] disallows cross-origin iframes.] - expected: FAIL - - [Default "camera" feature policy ["self"\] disallows cross-origin iframes.] - expected: FAIL - - [Default "camera;microphone" feature policy ["self"\] disallows cross-origin iframes.] - expected: FAIL - - [Feature policy "microphone" can be enabled in cross-origin iframes using "allow" attribute.] - expected: FAIL - - [Feature policy "camera" can be enabled in cross-origin iframes using "allow" attribute.] - expected: FAIL - - -[MediaStream-default-feature-policy.https.html] - [Default "microphone" feature policy ["self"\] disallows cross-origin iframes.] - expected: FAIL - - [Default "camera" feature policy ["self"\] disallows cross-origin iframes.] - expected: FAIL - - [Default "camera;microphone" feature policy ["self"\] disallows cross-origin iframes.] - expected: FAIL - - [Feature policy "microphone" can be enabled in cross-origin iframes using "allow" attribute.] - expected: FAIL - - [Feature policy "camera" can be enabled in cross-origin iframes using "allow" attribute.] - expected: FAIL - diff --git a/testing/web-platform/meta/screen-capture/permissions-policy-audio+video.https.sub.html.ini b/testing/web-platform/meta/screen-capture/permissions-policy-audio+video.https.sub.html.ini index edabeaad1f24..d282d790e375 100644 --- a/testing/web-platform/meta/screen-capture/permissions-policy-audio+video.https.sub.html.ini +++ b/testing/web-platform/meta/screen-capture/permissions-policy-audio+video.https.sub.html.ini @@ -1,7 +1,8 @@ [permissions-policy-audio+video.https.sub.html] disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1722250 [Default "display-capture" permissions policy ["self"\] disallows cross-origin iframes.] - expected: FAIL + expected: + if os == "android": FAIL [Default "display-capture" permissions policy ["self"\] allows the top-level document.] expected: @@ -16,4 +17,5 @@ if os == "android": FAIL [permissions policy "display-capture" can be disabled in same-origin iframes using "allow" attribute.] - expected: FAIL + expected: + if os == "android": FAIL diff --git a/testing/web-platform/meta/screen-capture/permissions-policy-audio.https.sub.html.ini b/testing/web-platform/meta/screen-capture/permissions-policy-audio.https.sub.html.ini index bdbbef2086dd..feaf3546463c 100644 --- a/testing/web-platform/meta/screen-capture/permissions-policy-audio.https.sub.html.ini +++ b/testing/web-platform/meta/screen-capture/permissions-policy-audio.https.sub.html.ini @@ -1,7 +1,8 @@ [permissions-policy-audio.https.sub.html] disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1722250 [Default "display-capture" permissions policy ["self"\] disallows cross-origin iframes.] - expected: FAIL + expected: + if os == "android": FAIL [Default "display-capture" permissions policy ["self"\] allows the top-level document.] expected: @@ -16,4 +17,5 @@ if os == "android": FAIL [permissions policy "display-capture" can be disabled in same-origin iframes using "allow" attribute.] - expected: FAIL + expected: + if os == "android": FAIL diff --git a/testing/web-platform/meta/screen-capture/permissions-policy-video.https.sub.html.ini b/testing/web-platform/meta/screen-capture/permissions-policy-video.https.sub.html.ini index 5fbeaa14f421..ff790580274f 100644 --- a/testing/web-platform/meta/screen-capture/permissions-policy-video.https.sub.html.ini +++ b/testing/web-platform/meta/screen-capture/permissions-policy-video.https.sub.html.ini @@ -1,7 +1,8 @@ [permissions-policy-video.https.sub.html] disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1722250 [Default "display-capture" permissions policy ["self"\] disallows cross-origin iframes.] - expected: FAIL + expected: + if os == "android": FAIL [Default "display-capture" permissions policy ["self"\] allows the top-level document.] expected: @@ -16,4 +17,5 @@ if os == "android": FAIL [permissions policy "display-capture" can be disabled in same-origin iframes using "allow" attribute.] - expected: FAIL + expected: + if os == "android": FAIL