Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable user media by default in cross-origin iframes #434

Closed
raymeskhoury opened this issue Feb 8, 2017 · 19 comments
Closed

Disable user media by default in cross-origin iframes #434

raymeskhoury opened this issue Feb 8, 2017 · 19 comments
Assignees

Comments

@raymeskhoury
Copy link

We would like to explore disabling user media (camera/mic/speakers) by default for cross-origin iframes. The idea is that it would be possible for the embedder to re-enable user media using the proposed Feature Policy mechanism.

getUserMedia already has a failure mode that occurs as a result of the user denying permission for hardware. This same failure mode can be reused but we probably still want to alter the spec to include the additional check to see if the feature is allowed by Feature Policy.

I think we probably want to add 3 features: camera, microphone and speakers, align with the permissions that exist in the permissions spec.

Note that this issue is mainly just intended to start the discussion about this change. We would need to nail down the Feature Policy spec before moving forward. Guidance is appreciated :).

The motivations for this change and a discussion of compatibility risk can be found here: https://docs.google.com/document/d/13dp9xWVyGM8THAQohDOT2mMOTSGLxEhSZEvgpmVLrxU/edit

@clelland @alvestrand

@alvestrand
Copy link
Contributor

The spec already says that this may be disabled, but after going back and forth a few times on it, we decided to leave the value of the permission to other specs (for a while, we had it as "disabled by default" in this spec, this being the consensus of the WG, but we were convinced by others that it was better left to other specs, to avoid monkey-patching).
A clarification was recently merged: #427 (title is obscure)

So I think that spec-wise, we're fine, but implementations are unlikely to conform until we have a mechanism in place for enabling the functionality that 1) is implemented and 2) people believe will stay consistently specified in the future.

Personally, I'd be very happy to see us close this opportunity for mischief.

@stefhak
Copy link
Contributor

stefhak commented Feb 8, 2017

I agree to Harald that specwise we're covering this already using allowusermedia combined with allowed-to-use from WhatWG's html spec (when it comes to microphone and camera at least).
A worry is that the permission spec (which we also reference) has not been updated since October last year, and in particular that a PR we want merged (w3c/permissions#131) just sits there.

@silviapfeiffer
Copy link
Member

Isn't that already implemented - at least in Chrome?

@raymeskhoury
Copy link
Author

I forgot to mention how we're thinking this will relate to allowusermedia (obviously this is up for discussion).

allowusermedia is in the spec, but AFAIK it isn't implemented anywhere yet. allowusermedia is coarse-grained in the sense that it gates the entire getUserMedia API. The direction that most browsers seem to be heading, (as well as the permissions API) is to have fine-grained permissions for camera/mic/speakers. It seems like it would be good to align the features that we add to Feature Policy with those permissions and have the failure modes be the same as if each of those features was denied.

So I'm proposing that we remove allowusermedia from the spec here and have 3 features added to Feature Policy instead for camera/mic/speakers. I think allowed-to-use will essentially still be the way to check if a feature is enabled but @clelland can verify that.

This is all still a bit up in the air but again just wanting to get the discussion going :)

@alvestrand
Copy link
Contributor

We need to pick one, for sure.

@clelland
Copy link

That sounds right, @raymeskhoury, we should still use allowed-to-use for cam/mik/spk. (And update that algorithm to check the document's policy when called)

I'd also like to see if we can remove allowusermedia from the spec; we can replace it with the feature policy <iframe allow="camera mic"> syntax, and allow fine-grained delegation of permissions that way.

@raymeskhoury
Copy link
Author

raymeskhoury commented Feb 16, 2017

Thanks @clelland!

Do we need to mention anything in the mediacapture spec at all besides the allowed-to-use bit and defining the feature policy feature names? Shouldn't the allow syntax be defined in the FP spec?

+1 to removing allowusermedia.

@stefhak stefhak self-assigned this Feb 16, 2017
@alvestrand
Copy link
Contributor

Awaiting instructions on what to do, if anything, in the mediacapture-main spec to accomplish this.

@clelland
Copy link

clelland commented Mar 2, 2017

Sorry for the wait, @alvestrand

The FP spec is up now at https://wicg.github.io/feature-policy. I've mentioned camera, mic, and speakers in it, but only as part of the appendix listing known features.

I think the thing to do in the usermedia spec is to

  • declare those three to be features, as defined in the FP spec
  • declare their corresponding keywords and default allowlists (should be ["self"] for all three, I believe)
  • define exactly what should happen if each feature is disabled in a frame.

Something like the wording in https://wicg.github.io/feature-policy/#camera-feature, although it may need to be more precise in terms of errors returned, exceptions raised, promises rejected, etc.

If it is defined as a feature, then it will be usable either in <iframe allow="..."> syntax, or in a Feature-Policy HTTP header.

@raymeskhoury
Copy link
Author

Regarding the behavior if the feature isn't available:

My preference is that it would be exactly the same as the behavior as if each of those permissions were denied, modulo perhaps returning a different error code. This makes the behavior sensible and allows secure implementation.

@stefhak
Copy link
Contributor

stefhak commented Mar 27, 2017

@raymeskhoury @clelland is there any example in another spec referencing FeaturePolicy we could look at? It looks to me like allow-to-use (https://html.spec.whatwg.org/multipage/embedded-content.html#allowed-to-use) is not really compatible with FeaturePolicy.

@stefhak
Copy link
Contributor

stefhak commented Mar 30, 2017

I've drafted a PR, PTAL.

@alvestrand
Copy link
Contributor

alvestrand commented Apr 6, 2017

FYI on the status of "feature policy":

Chrome is intending to ship this (target M59).
https://bugs.chromium.org/p/chromium/issues/detail?id=623682&desc=2

@aboba aboba removed the PR exists label Nov 23, 2017
@alvestrand
Copy link
Contributor

w3c/permissions#163 is a better approach, probably.

@alvestrand
Copy link
Contributor

Need to reference feature policy? That seems to be the place where it's currently specified.
@jan-ivar to find a proper place to reference (if needed).

@ewpreston
Copy link

is the allow attribute supported in Chrome 64 yet? I can't seem to get it to work. Referencing this document:

https://dev.chromium.org/Home/chromium-security/deprecating-permissions-in-cross-origin-iframes

@raymeskhoury
Copy link
Author

raymeskhoury commented Feb 11, 2018 via email

@ewpreston
Copy link

Sorry, the documentation is lacking. We do a redirect once we load the iframe. Have to add * or domain after the capability. The document above doesn't discuss that but I found it on a google forum.

@jan-ivar
Copy link
Member

jan-ivar commented Mar 8, 2018

allow is here https://wicg.github.io/feature-policy/#iframe-allow-attribute

Asssuming w3c/permissions#163 merges, we should be able to reference it from there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants