Closed Bug 1483631 Opened 2 years ago Closed 11 months ago

Restrict nested permission requests (camera/microphone/geolocation/screensharing) with Feature Policy

Categories

(Core :: DOM: Security, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox74 --- fixed

People

(Reporter: tjr, Assigned: tnguyen, NeedInfo)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

(4 keywords, Whiteboard: [domsecurity-active] [adv-main73-] [adv-main74-])

Attachments

(4 files, 3 obsolete files)

Currently, we allow iframes to request camera or microphone access.  This is both confusing for users (we display the origin of the iframe which doesn't match the address bar) and potentially dangerous (an ad network could start prompting for access and users are more inclined to access if it comes on a 'trusted' domain).

The best solution here would be to disallow this behavior unless the embedding website explicitly allows it with Feature Policy.

Testing with https://ritter.vg/misc/ff/gum-embed-withfp.html , both Safari and Chrome blocked iframes from requesting access to the camera/microphone unless they were allowed by Feature Policy. (I did not test Edge.)  Each behaved slightly different though:

In Chrome, if the you requested access in the iframe first, and then the main page, you would get two prompts. If you requested access in the main page and then the iframe, there would be no prompt for the iframe.  In Safari you would get two prompts regardless of order.
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
(In reply to Tom Ritter [:tjr] from comment #0)
> In Chrome, if the you requested access in the iframe first, and then the
> main page, you would get two prompts.

Are you sure? This does not match what they announced [0] and I can't reproduce the behavior you describe in Canary on https://joo.crater.uberspace.de/frame-permissions.html.

When I ask in the iframe first and accept, then permission is granted to the top level origin, not double keyed or anything.

[0] https://groups.google.com/a/chromium.org/forum/#!searchin/blink-dev/feature$20policy/blink-dev/mG6vL09JMOQ/VPVEH7P0BwAJ
(In reply to Johann Hofmann [:johannh] from comment #1)
> (In reply to Tom Ritter [:tjr] from comment #0)
> > In Chrome, if the you requested access in the iframe first, and then the
> > main page, you would get two prompts.
> 
> Are you sure? This does not match what they announced [0] and I can't
> reproduce the behavior you describe in Canary on
> https://joo.crater.uberspace.de/frame-permissions.html.
> 
> When I ask in the iframe first and accept, then permission is granted to the
> top level origin, not double keyed or anything.

I can replicate your results and negate my previous statement.

Nonetheless I'm pretty sure that's how it worked when I tested it, so I would chalk it up to a Canary bug :)
No longer blocks: 1390801
Depends on: 1390801
Depends on: 1560741
Blocks: 1572461
No longer blocks: 1450957
Assignee: nobody → tnguyen

I realize this is a bit off-topic, but users are complaining about this scenario starting with Firefox 69:

Want to make a Hangouts call from Gmail:

  • Already granted persistent permission for mail.google.com to use microphone
  • Already granted persistent permission for hangouts.google.com to use microphone
  • When microphone permission is requested from a hangouts.google.com iframe on mail.google.com, user needs to give permission every time

Will that be streamlined by this bug, or should it be filed as a different bug, or is that "working as intended" for now?

SUMO threads:

That should be fixed by this, but maybe file a bug that depends on this bug so we don't lose track of that problem in case something changes here.

Keywords: site-compat
Priority: P3 → P2
Whiteboard: [domsecurity-backlog1] → [domsecurity-active]

In post fission world, we should move the top principal and top window check
to parent, so we should pass browsingcontextId to allocate function and
get working with top in parent process.

Attachment #9109128 - Attachment description: Bug 1483631 - Using browsingcontextId when allocating media source instead of principal → Bug 1483631 - Using windowId when allocating media source instead of principal
Attachment #9113081 - Attachment is obsolete: true
Attachment #9113082 - Attachment is obsolete: true
Attachment #9113083 - Attachment is obsolete: true

Sorry, a moz-phab spill on my part. Apologies.

Pushed by tnguyen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a909dcbbea2b
Restrict nested permission requests in ContentPermissionPrompt with permission delegate r=baku,mccr8
https://hg.mozilla.org/integration/autoland/rev/7ef09193a7ef
Restrict nested permission requests in webrtc with permission delegate r=jib
https://hg.mozilla.org/integration/autoland/rev/c5e562c1d590
Prompt with both first party and third party origin if we are delegating permission with allows all feature policy. r=baku
https://hg.mozilla.org/integration/autoland/rev/f5bb5f6a148f
Using windowId when allocating media source instead of principal r=jib,farre

Backed out 4 changesets (bug 1483631) for failing at browser_temporary_permissions.js on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/69a59b68fe0ed227e5ea2aba2c1636775f42f1a0

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&searchStr=%28bc6%29&revision=f5bb5f6a148fac996020824fc0eedd36d91bc6ff&selectedJob=279399784

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=279399784&repo=autoland&lineNumber=3043

Log snippet:

[task 2019-12-03T19:34:08.867Z] 19:34:08 ERROR - TEST-UNEXPECTED-FAIL | ShutdownLeaks | process() called before end of test suite
[task 2019-12-03T19:34:08.868Z] 19:34:08 INFO - TEST-INFO | Confirming we saw 52 DOCSHELL created and 31 destroyed log strings.
[task 2019-12-03T19:34:08.868Z] 19:34:08 INFO - TEST-INFO | Confirming we saw 143 DOMWINDOW created and 95 destroyed log strings.
[task 2019-12-03T19:34:08.869Z] 19:34:08 INFO - Buffered messages logged at 19:33:45
[task 2019-12-03T19:34:08.870Z] 19:34:08 INFO - Entering test bound testTempPermissionChangeEvents
[task 2019-12-03T19:34:08.870Z] 19:34:08 INFO - Buffered messages logged at 19:33:46
[task 2019-12-03T19:34:08.871Z] 19:34:08 INFO - TEST-PASS | browser/base/content/test/permissions/browser_temporary_permissions.js | {"state":2,"scope":"{SitePermissions.SCOPE_TEMPORARY}"} deepEqual {"state":2,"scope":"{SitePermissions.SCOPE_TEMPORARY}"} -
[task 2019-12-03T19:34:08.871Z] 19:34:08 INFO - TEST-PASS | browser/base/content/test/permissions/browser_temporary_permissions.js | geo anchor should be visible - 16 != 0 -
[task 2019-12-03T19:34:08.872Z] 19:34:08 INFO - TEST-PASS | browser/base/content/test/permissions/browser_temporary_permissions.js | geo anchor should not be visible - 0 == 0 -
[task 2019-12-03T19:34:08.874Z] 19:34:08 INFO - Leaving test bound testTempPermissionChangeEvents
[task 2019-12-03T19:34:08.875Z] 19:34:08 INFO - Entering test bound testTempPermissionSubframes
[task 2019-12-03T19:34:08.875Z] 19:34:08 INFO - Buffered messages logged at 19:33:48
[task 2019-12-03T19:34:08.875Z] 19:34:08 INFO - TEST-PASS | browser/base/content/test/permissions/browser_temporary_permissions.js | "example.org" != "example.com" -
[task 2019-12-03T19:34:08.876Z] 19:34:08 INFO - Buffered messages finished
[task 2019-12-03T19:34:08.876Z] 19:34:08 ERROR - TEST-UNEXPECTED-FAIL | browser/base/content/test/permissions/browser_temporary_permissions.js | application terminated with exit code 11
[task 2019-12-03T19:34:08.876Z] 19:34:08 INFO - runtests.py | Application ran for: 0:01:05.449000
[task 2019-12-03T19:34:08.880Z] 19:34:08 INFO - zombiecheck | Reading PID log: /tmp/tmp9Y5SDypidlog

.......................................................

[task 2019-12-03T19:34:08.908Z] 19:34:08 INFO - mozcrash Copy/paste: /builds/worker/workspace/build/linux64-minidump_stackwalk /tmp/tmpMtrSKe.mozrunner/minidumps/1b2e203c-cc8d-5179-182b-4361483b347a.dmp /builds/worker/workspace/build/symbols
[task 2019-12-03T19:34:15.215Z] 19:34:15 INFO - mozcrash Saved minidump as /builds/worker/workspace/build/blobber_upload_dir/1b2e203c-cc8d-5179-182b-4361483b347a.dmp
[task 2019-12-03T19:34:15.216Z] 19:34:15 INFO - mozcrash Saved app info as /builds/worker/workspace/build/blobber_upload_dir/1b2e203c-cc8d-5179-182b-4361483b347a.extra
[task 2019-12-03T19:34:15.391Z] 19:34:15 INFO - PROCESS-CRASH | browser/base/content/test/permissions/browser_temporary_permissions.js | application crashed [@ nsContentPermissionRequestProxy::GetTopLevelPrincipal(nsIPrincipal**)]
[task 2019-12-03T19:34:15.391Z] 19:34:15 INFO - Crash dump filename: /tmp/tmpMtrSKe.mozrunner/minidumps/1b2e203c-cc8d-5179-182b-4361483b347a.dmp
[task 2019-12-03T19:34:15.392Z] 19:34:15 INFO - Operating system: Linux
[task 2019-12-03T19:34:15.392Z] 19:34:15 INFO - 0.0.0 Linux 4.4.0-1014-aws #14taskcluster1-Ubuntu SMP Tue Apr 3 10:27:00 UTC 2018 x86_64
[task 2019-12-03T19:34:15.393Z] 19:34:15 INFO - CPU: amd64
[task 2019-12-03T19:34:15.393Z] 19:34:15 INFO - family 6 model 85 stepping 4
[task 2019-12-03T19:34:15.393Z] 19:34:15 INFO - 2 CPUs
[task 2019-12-03T19:34:15.393Z] 19:34:15 INFO -
[task 2019-12-03T19:34:15.393Z] 19:34:15 INFO - GPU: UNKNOWN
[task 2019-12-03T19:34:15.394Z] 19:34:15 INFO -
[task 2019-12-03T19:34:15.394Z] 19:34:15 INFO - Crash reason: SIGSEGV /SEGV_MAPERR
[task 2019-12-03T19:34:15.394Z] 19:34:15 INFO - Crash address: 0x0
[task 2019-12-03T19:34:15.395Z] 19:34:15 INFO - Process uptime: not available
[task 2019-12-03T19:34:15.395Z] 19:34:15 INFO -
[task 2019-12-03T19:34:15.395Z] 19:34:15 INFO - Thread 0 (crashed)
[task 2019-12-03T19:34:15.395Z] 19:34:15 INFO - 0 libxul.so!nsContentPermissionRequestProxy::GetTopLevelPrincipal(nsIPrincipal**) [nsContentPermissionHelper.cpp:f5bb5f6a148fac996020824fc0eedd36d91bc6ff : 946 + 0x7]
[task 2019-12-03T19:34:15.396Z] 19:34:15 INFO - rax = 0x00007f6ec7b5bf20 rdx = 0x00007f6ee5b203da
[task 2019-12-03T19:34:15.396Z] 19:34:15 INFO - rcx = 0x0000000000000000 rbx = 0x00007f6ee7733aa0
[task 2019-12-03T19:34:15.396Z] 19:34:15 INFO - rsi = 0x00007fff26c54340 rdi = 0x0000000000000000
[task 2019-12-03T19:34:15.396Z] 19:34:15 INFO - rbp = 0x00007fff26c54160 rsp = 0x00007fff26c54160
[task 2019-12-03T19:34:15.396Z] 19:34:15 INFO - r8 = 0x0000000000000001 r9 = 0x0000000000000000
[task 2019-12-03T19:34:15.396Z] 19:34:15 INFO - r10 = 0x00007f6ee4eae8bc r11 = 0x0000000000000003
[task 2019-12-03T19:34:15.397Z] 19:34:15 INFO - r12 = 0x00007f6ec89d4fa0 r13 = 0x00007fff26c54178
[task 2019-12-03T19:34:15.397Z] 19:34:15 INFO - r14 = 0x00007fff26c54340 r15 = 0x00007fff26c54328
[task 2019-12-03T19:34:15.397Z] 19:34:15 INFO - rip = 0x00007f6ee02545a7
[task 2019-12-03T19:34:15.397Z] 19:34:15 INFO - Found by: given as instruction pointer in context
[task 2019-12-03T19:34:15.397Z] 19:34:15 INFO - 1 libxul.so!PermissionDelegateHandler::GetDelegatePrincipal(nsTSubstring<char> const&, nsIContentPermissionRequest*, nsIPrincipal**) [PermissionDelegateHandler.cpp:f5bb5f6a148fac996020824fc0eedd36d91bc6ff : 116 + 0xd]
[task 2019-12-03T19:34:15.404Z] 19:34:15 INFO - rbx = 0x00007f6ee7733aa0 rbp = 0x00007fff26c54240
[task 2019-12-03T19:34:15.404Z] 19:34:15 INFO - rsp = 0x00007fff26c54170 r12 = 0x00007f6ec89d4fa0
[task 2019-12-03T19:34:15.404Z] 19:34:15 INFO - r13 = 0x00007fff26c54178 r14 = 0x00007fff26c54340
[task 2019-12-03T19:34:15.405Z] 19:34:15 INFO - r15 = 0x00007fff26c54328 rip = 0x00007f6edfa91ae2
[task 2019-12-03T19:34:15.405Z] 19:34:15 INFO - Found by: call frame info
[task 2019-12-03T19:34:15.405Z] 19:34:15 INFO - 2 libxul.so!NS_InvokeByIndex + 0x8e
[task 2019-12-03T19:34:15.405Z] 19:34:15 INFO - rbx = 0x0000000000000001 rbp = 0x00007fff26c54270
[task 2019-12-03T19:34:15.405Z] 19:34:15 INFO - rsp = 0x00007fff26c54250 r12 = 0x00007f6ec89d4fa0
[task 2019-12-03T19:34:15.405Z] 19:34:15 INFO - r13 = 0x000000000000000d r14 = 0x00007fff26c542e0
[task 2019-12-03T19:34:15.405Z] 19:34:15 INFO - r15 = 0x00007fff26c54201 rip = 0x00007f6edf058a3e
[task 2019-12-03T19:34:15.405Z] 19:34:15 INFO - Found by: call frame info

Flags: needinfo?(tnguyen)

Thanks for reporting, that is the same fission failure with bug 1587743.

Flags: needinfo?(tnguyen)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla73 → ---
Pushed by tnguyen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/32943943ebf7
Restrict nested permission requests in ContentPermissionPrompt with permission delegate r=baku,mccr8
https://hg.mozilla.org/integration/autoland/rev/4cfb8575b83e
Restrict nested permission requests in webrtc with permission delegate r=jib
https://hg.mozilla.org/integration/autoland/rev/101323e2d295
Prompt with both first party and third party origin if we are delegating permission with allows all feature policy. r=baku
https://hg.mozilla.org/integration/autoland/rev/195319ed0bb4
Using windowId when allocating media source instead of principal r=jib,farre

This sounds like it's just a UX issue that doesn't require any work on MDN. If I'm mistaken, please let me know in what way. Thanks!

So this overall effort, tracked by bug 1572461, impacts how various APIs behave (permissions.query(), similar API for WebRTC; see dependencies), relies very much on <iframe allow> for delegation, and changes whether a bunch of permissions are available in third-party contexts. This bug is about that last bit and it seems like something we want to state when explaining, say, how geolocation works on the web?

Keywords: site-compat

It's probably time, then, to actually have an article on MDN specifically about allow and <iframe> and the way permissions are nested, inherited, and checked.

That brings me to the question -- is there any connection between Feature Policy and Permissions API? Does the latter consider the configuration of FP when returning results?

Is the gist of this that content iframes can now only get permission to do the things affected in this bug (camera, mic, geolocation, and screen sharing), if the parent already has permission to do, and then only after the user is prompted to okay it? And that if the parent has no permission already, the iframe can't get it, so the user isn't even asked?

(In reply to Eric Shepherd [:sheppy] from comment #22)

It's probably time, then, to actually have an article on MDN specifically about allow and <iframe> and the way permissions are nested, inherited, and checked.

That brings me to the question -- is there any connection between Feature Policy and Permissions API? Does the latter consider the configuration of FP when returning results?

Yes, see bug 1560570.

(In reply to Eric Shepherd [:sheppy] from comment #23)

Is the gist of this that content iframes can now only get permission to do the things affected in this bug (camera, mic, geolocation, and screen sharing), if the parent already has permission to do, and then only after the user is prompted to okay it? And that if the parent has no permission already, the iframe can't get it, so the user isn't even asked?

Not exactly, the idea is that if an iframe ask for permission, the user is asked to give the permission to the parent instead, which can delegate it to the iframe using the allow attribute. If an iframe tries to get permission without the allow attribute set, it will simply not prompt.

Whiteboard: [domsecurity-active] → [domsecurity-active] [adv-main73-]

OK, I've made what I feel are appropriate MDN content updates around this:

  • Created the new article Privacy, permissions, and information security. Designed to be a comprehensive article about ways to secure content, a lot of it is pending future work. the important thing is that the section Permission requests in <iframe> elements includes information about how the UX around this varies and how Firefox implements it in 73 and later.
  • Added links to the page above from appropriate places: the main <iframe> page, and a number of other pages throughout the security and privacy related content. I'm sure there are plenty of other appropriate places, but it's a good start.
  • Added the change to Firefox 73 for developers.

(In reply to Eric Shepherd [:sheppy] from comment #25)

  • Added the change to Firefox 73 for developers.

Thank your for the great work! However, we decided to delay this to 74, because of layoffs. Would you mind updating that? :)

Thanks!

Flags: needinfo?(eshepherd)
Whiteboard: [domsecurity-active] [adv-main73-] → [domsecurity-active] [adv-main73-] [adv-main74-]
You need to log in before you can comment on or make changes to this bug.