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

Permissions policy 'self' is at risk #169

Closed
ewilligers opened this issue Jul 31, 2020 · 35 comments · Fixed by #252
Closed

Permissions policy 'self' is at risk #169

ewilligers opened this issue Jul 31, 2020 · 35 comments · Fixed by #252
Assignees

Comments

@ewilligers
Copy link
Collaborator

Enforcing permissions policy, by default blocking third party content from sharing, won't ship in Blink until we have an understanding of how widespread this usage is (Intent thread).

@marcoscaceres
Copy link
Member

I'm currently sitting on this on the Gecko side (have a patch ready... just stuck in review), but I intend to ship it soon. I'd still encourage us to have this in the spec. Hopeful for support from WebKit. Cc @hober.

@mgiuca
Copy link
Collaborator

mgiuca commented Jul 31, 2020

I think it's fine to leave it in the spec for now, but Eric and I wanted to flag it as "at risk" pending some real-world usage investigation. We intend to follow through but if it turns out that a large set of sites will break because they are depending on WS within an iframe, we should hold back on it and work with those sites to add the permissions policy (which is fairly trivial to add, AIUI).

@marcoscaceres
Copy link
Member

Really appreciate the update and looking forward to hearing the outcome of the investigation (that sounds way cooler than it should).

@marcoscaceres
Copy link
Member

marcoscaceres commented Aug 27, 2020

Just noting that the Permission Policy for "web-share" landed in Gecko:
https://github.com/mozilla/gecko-dev/blob/cf64c38045d2e208625b574dfd834b1be53ded53/dom/base/Navigator.cpp#L1375

@marcoscaceres
Copy link
Member

@cdumez, WebKit also doesn't implement "web-share" Permission Policy.

Would that be something you'd consider adding? I'm happy to send a patch if so.

I'll file a WebKit bug just in case.

@marcoscaceres
Copy link
Member

Oh, looks like I already filed one... thanks, 2020-Marcos! https://bugs.webkit.org/show_bug.cgi?id=214448

@marcoscaceres
Copy link
Member

The "web-share" Permissions Policy is now implemented in WebKit and Firefox, and partially implemented in Chrome (it gets skipped, but recorded by a telemetry probe).

@marcoscaceres
Copy link
Member

@ewilligers any indication from the data you collected as to wether Chrome will enable it?

@marcoscaceres marcoscaceres changed the title Permissions Policy at risk Support "web-share" Permissions Policy in Chrome? Oct 5, 2021
@marcoscaceres
Copy link
Member

@mgiuca @ewilligers, would be great to get a conclusion on this one. If the introp pain would now be too much, we could consider a compromise with "*" now. It would be nice to get this in the spec (and implementations updated), as it's now affecting multiple engines.

@saschanaz
Copy link
Member

saschanaz commented May 24, 2022

I think one of the big issues is that YouTube is still using allowfullscreen only instead of allow="fullscreen *;web-share *". We'll have to convince them (and any other similar services) in order to make it default to self.

(Currently it's the biggest blocker against shipping Web Share on Windows on Gecko, IMO)

@marcoscaceres
Copy link
Member

marcoscaceres commented May 25, 2022

We'll have to convince them (and any other similar services) in order to make it default to self.

Might be a lost cause if they don't even seem to respond to Google folks reaching out to them internally.

Let's just cut out loses here and go with "*".

I'll send a pull request. @saschanaz, you ok with this change for Gecko? I'll can patch WebKit.

@marcoscaceres marcoscaceres changed the title Support "web-share" Permissions Policy in Chrome? Make "web-share" Permissions Policy be "*" (all) May 25, 2022
@saschanaz
Copy link
Member

Sounds okay to me. @annevk, do you have any opposition?

@marcoscaceres
Copy link
Member

Noting that it's not just YouTube... the problem is also JS widgets that embed YouTube all over the web. They are break (Safari/Firefox) because of 'self' all over the place. It also affects sites like jsbin and jsfiddle. I've filed bugs on them to add 'web-share' permission policy a few weeks ago, but still not fixed (e.g., jsbin/jsbin#3485).

@annevk
Copy link
Member

annevk commented May 25, 2022

This almost makes me think there should not be a Permissions Policy. Is this a feature that benefits from it? Does it even match the model we have for other permissions whereby authority is delegated from the top-level?

@marcoscaceres
Copy link
Member

This almost makes me think there should not be a Permissions Policy. Is this a feature that benefits from it?

In as far as that it can be disabled for third-parties, then "yes"? It's still a fairly powerful API.

Does it even match the model we have for other permissions whereby authority is delegated from the top-level?

No, but I'm not sure it applies here. The original purpose was only to allow sharing at the top-level.

@annevk
Copy link
Member

annevk commented May 25, 2022

I'm wondering if this should be similar to copy-and-paste, where Firefox and Safari do require an additional step of user consent for pasting, but it's not something that necessarily makes sense to be guarded by Permissions Policy since the user can achieve it through other means as well.

While Permissions Policy does allow for disabling, its main purpose is enabling embeddees to do something on behalf of the embedder.

Anyway, it doesn't matter too much I suppose.

@marcoscaceres
Copy link
Member

marcoscaceres commented May 25, 2022

While Permissions Policy does allow for disabling, its main purpose is enabling embeddees to do something on behalf of the embedder.

Yeah, that was the original intent... unfortunately, it didn't work out that way :( (hence this PR)

I'd still feel more comfortable that there is a means to disable it in third-party context. Its' a quick fix all round and it will solve the current web compatibly issues.

@marcoscaceres
Copy link
Member

@saschanaz, given @annevk's response, are you all good to make this change in Gecko? I'd like to move forward with this change in WebKit.

@saschanaz
Copy link
Member

saschanaz commented May 25, 2022

I also wonder what would be the real world use case of it if it defaults to * while Blink and websites never care. I wonder navigator.share() should open a permission prompt instead.

@marcoscaceres
Copy link
Member

I also wonder what would be the real world use case of it if it defaults to * while Blink and websites never care.

I'm not sure we can be presumptuous about what Blink will or won't do... that's up to @mgiuca or @ewilligers to answer.

The real use is allow sites to disable the API so they so choose, in case where they allow embedding of third party context (or as security measure on a first-party context). This is still an extremely powerful API, in that it has side effects that are sometimes unforeseen and uncontrollable at either and OS level or at an application level (again, remember what happened with the file:// password situation).

I feel pretty strongly it's right to give site authors a means to lock this down if they so choose.

I wonder navigator.share() should open a permission prompt instead.

No, that wouldn't work: the behavior is OS specific (see how macOS and iOS differ). The last thing we want is a permission prompt here.

@saschanaz
Copy link
Member

This is still an extremely powerful API, in that it has side effects that are sometimes unforeseen and uncontrollable at either and OS level or at an application level (again, remember what happened with the file:// password situation).

Defaulting to * means each webpage need to explicitly and consciously give self or none to prevent such potentially harmful situation, and I highly doubt that's currently the case. Websites are already broken and nobody is doing anything, can we really expect it will be different with *?

No, that wouldn't work: the behavior is OS specific (see how macOS and iOS differ). The last thing we want is a permission prompt here.

I thought this was totally a browser level thing, what does WebKit do for permission prompts? (That said, it's also true that malicious websites try hard to mock such prompts.)

@marcoscaceres
Copy link
Member

marcoscaceres commented May 26, 2022

Defaulting to * means each webpage need to explicitly and consciously give self or none to prevent such potentially harmful situation,

correct.

and I highly doubt that's currently the case. Websites are already broken and nobody is doing anything,

That claim is bogus because the only browsers that supports this (web share) today at scale is Chrome, which doesn’t support the permissions policy. We don’t have evidence either way.

can we really expect it will be different with *?

Yes. The point is to give them the option, so they choose. You seem to be taking a defeatist stance while denying site authors the ability to have control over this feature.

@saschanaz
Copy link
Member

saschanaz commented May 26, 2022

Yes. The point is to give them the option, so they choose.

I mean, that's sounds like 'opt-in to prevent harmful situation', I guess the web generally want to be secure by default, right? I feel like this is a no-go if this is just to enable the feature on YouTube on other engines.

You seem to be taking a defeatist stance while denying site authors the ability to have control over this feature.

Let's not assume my stance this way.

Edit: Technically browsers may be able to safelist YouTube (via webcompat extension in Gecko for example), but it's... meh.

@marcoscaceres
Copy link
Member

I'm really really sorry, @saschanaz, for really terrible choice of word ("defeatist"). I meant that we should not give up. I meant no offense and hope you forgive me.

Edit: Technically browsers may be able to safelist YouTube (via webcompat extension in Gecko for example), but it's... meh.

Yes, we it would mean playing whack-a-mole with every <iframe> that wants to use web share.

@saschanaz
Copy link
Member

saschanaz commented May 26, 2022

Okay, I and Marcos just did a quick friendly (🤗) chat with this. Given that we do have multiple broken websites and then we did observe a hack (#173), I agree that we need to keep some way to block it.

That said, currently the biggest blocker is still YouTube and the last trial to contact them happened two years ago. Probably worth contacting them again since YouTube now has a long allow list:

image

In a short term I prefer to add a site intervention for YouTube from Gecko side to keep it safe, and I want to wait before merging #234 to wait for YouTube's response again.

@saschanaz
Copy link
Member

Continuing from #234 (comment).

A previous version of this spec used to have this paragraph, which is removed by #243:

User agents that do not support sharing SHOULD NOT expose share() on the Navigator interface.

The above statement is designed to permit feature detection. If share() is present, there is a reasonable expectation that it will work and present the user with at least one share target. Clients can use the presence or absence of this method to determine whether to show UI that triggers its use.

How about reintroducing this in normative way for permissions policy, to achieve the same purpose? This can solve the YouTube failure raised here and can still keep permissions policy working.

@marcoscaceres
Copy link
Member

Sorry, I'm not following how that helps with the YouTube issue? Can you clarify a bit more?

If you are saying that we should allow arbitrary quirks, that doesn't sound great. The problem is not just with YouTube, there are tons of sites where this is a problem now (another one right now being Twitter, for instance)... but then there is also, JSBin, JSFiddle, and I'm sure many many more.

That's also not how WebKit handles the quirk... we currently disable the permissions policy specifically for YouTube and Twitter, but we still expose the API. However, it means that other sites I mentioned still don't work (i.e., this problem is larger than just YouTube).

@marcoscaceres
Copy link
Member

sorry, closed by accident.

@marcoscaceres marcoscaceres reopened this Jul 12, 2022
@marcoscaceres marcoscaceres changed the title Make "web-share" Permissions Policy be "*" (all) Permissions policy 'self' is at risk Jul 12, 2022
@saschanaz
Copy link
Member

saschanaz commented Jul 12, 2022

Hiding navigator.share mitigates the issue as YouTube checks its existence for feature detection, as the spec had been suggesting from that paragraph. Doing so allows YouTube to gracely fallback to the traditional popup, and that's way better than a silent failure.

Hiding by permissions policy should be way more general than a site intervention (i.e. fixing a specific website).

(This should not be new to you! #234 (comment))

@marcoscaceres
Copy link
Member

I believe the intent of the original paragraph was about not exposing the API at all, and not hide it for specific sites.

That's not to say that it's a bad approach should you take that approach in Gecko - but it's not the approach WebKit is taking as it doesn't scale: I don't want to be adding exceptions for every websites that doesn't add the permissions policy (and why I had to change it to "*").

The reality of the situation at the moment remains:

  • WebKit - Permission Policy "*"
  • Gecko - Permissions Policy "'self'"
  • Chromium - no Permissions Policy

Until that changes, there is not much more we can do. Realistically, we can only change to "self" only Chrome does, and Chrome probably won't until YouTube does the right thing... so we are kinda stuck.

@saschanaz
Copy link
Member

saschanaz commented Jul 12, 2022

but it's not the approach WebKit is taking as it doesn't scale: I don't want to be adding exceptions for every websites that doesn't add the permissions policy (and why I had to change it to "*").

I wouldn't say that as "adding exceptions", instead I'd like to ease the feature detection in general. That's what I'm working on for IndexedDB for a short term, and I'd like to see this be considered for other APIs too. (In other words, a general principle: If it doesn't work, then don't expose them.)

@marcoscaceres
Copy link
Member

I agree that a general solution is not to expose the API. Alternatively, the Permissions Policy spec does provide an API that could allows a document itself to check if it's allowed to use the feature before it call an API. That might work just as well as removing the API entirely (except in this legacy YouTube case, where it checks for the presence of navigator.share).

@ewilligers
Copy link
Collaborator Author

Is hiding the API based on permission policy something that is already done for other APIs? If that is a generally accepted approach for the web platform, that sounds like a great solution.

@saschanaz
Copy link
Member

saschanaz commented Jul 13, 2022

Is hiding the API based on permission policy something that is already done for other APIs? If that is a generally accepted approach for the web platform, that sounds like a great solution.

Sadly no precedents that I know. I only know that the previous WebExtensions spec draft wanted to add an Web IDL extended attribute for that, but the whole draft stalled.

Might be good to ping who knows well about general approach, maybe @smaug----? (A short summary of the issue: Exposing navigator.share without a permission breaks YouTube's (and more) feature detection and causes wrong behavior.)

@agreiner
Copy link

It looks like the spec is currently referencing the DXWG repo for this issue by mistake. It says "Permissions policy 'self' is at risk [DXWG] CR Request to DCAT v2 #169 - due to web compatibility issues.

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

Successfully merging a pull request may close this issue.

6 participants