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

XHR from content scripts should not be affected by page's CSP / Permissions-Policy (also, cross-browser inconsistencies) #730

Open
hackademix opened this issue Dec 4, 2024 · 6 comments
Labels
needs-triage: firefox Firefox needs to assess this issue for the first time needs-triage: safari Safari needs to assess this issue for the first time topic: csp Related to content security policy enforcement

Comments

@hackademix
Copy link

Try inject https://maone.net/test/docpolicy/ with a MV3 content script trying to perform sync XHR to any origin (including the page itself):

  1. On Chrome, synchronous XHR is blocked by Permissions-Policy: sync-xhr=()" (and by the deprecated Feature-Policy: sync-xhr 'none';`, likely for retro-compat)
  2. On Firefox, XHR from content scripts is blocked by Content-Security-Policy: connect-src

While n. 1 can be worked around, albeit in very hackish and expensive ways (here and there), n.2 cannot because of how CSP headers are merged in MV3 (no way to relax them narrowly and safely).

Of course we're talking about hacks to enable another hack (synchronous messaging through XHR), but so far proposed solutions (#539, #536, #284...) have been stalled mentioning the existence of this very hack (example, example) as an excuse, so please fix this issue at least.

@github-actions github-actions bot added needs-triage: chrome Chrome needs to assess this issue for the first time needs-triage: firefox Firefox needs to assess this issue for the first time needs-triage: safari Safari needs to assess this issue for the first time labels Dec 4, 2024
@hackademix hackademix changed the title XHR from content scripts should not be affected by page's CSP (also, cross-browser inconsistencies) XHR from content scripts should not be affected by page's CSP / Permissions-Policy (also, cross-browser inconsistencies) Dec 4, 2024
@xeenon xeenon added the topic: csp Related to content security policy enforcement label Dec 5, 2024
@tophf
Copy link

tophf commented Dec 5, 2024

SyncXHR for the extension's own blob URL should be allowed regardless of the policies/CSP i.e. blob:chrome-extension://id/aaa-aaaaa-aaaaa also because it's not a remote request, so it usually takes just a millisecond or a few, depending on the size of the data, essentially the same amount of time it takes to parse a JSON string from an API message, i.e. in this particular case syncXHR is not detrimental to the page load process.

@Rob--W Rob--W added the follow-up: chrome Needs a response from a Chrome representative label Dec 5, 2024
@Rob--W
Copy link
Member

Rob--W commented Dec 5, 2024

As noted in the meeting, Firefox's MV3 behavior matches exactly the web page's behavior. I'm aware that Chrome made an effort to use the content script-specific CSP for several DOM APIs, including XMLHttpRequest and fetch.

@oliverdunk is going to look into the kind of exceptions that Chrome has. From there we can decide whether we should enforce these exceptions more broadly across browsers.

@oliverdunk
Copy link
Member

I was able to follow-up on this internally. Chrome's stance is that we have a content script CSP and therefore should follow it as closely as possible whenever feasible. This includes using the content script CSP for XHR and fetch and not applying the page CSP.

Currently in Chrome, the content script CSP is always equivalent to the minimum MV3 CSP, regardless of any extension CSP specified in the manifest. We're open to supporting additional configuration in principle but aren't yet convinced this would be adopted.

I don't have a full list of exceptions we have but hopefully this is enough to figure out a direction :)

@oliverdunk oliverdunk removed follow-up: chrome Needs a response from a Chrome representative needs-triage: chrome Chrome needs to assess this issue for the first time labels Dec 5, 2024
@tophf
Copy link

tophf commented Dec 5, 2024

I see comments mentioning only CSP, but in Chrome it is not a problem as noted in the issue description. It's Permissions-Policy/Feature-Policy for sync-xhr. Is this what @oliverdunk meant by "additional configuration"? If so, regarding "aren't yet convinced this would be adopted", I guess Chrome wouldn't want to add an exception to support a deprecated feature, but 1) it's possible that implementing such an exception would be trivial or at least much simpler than implementing a proper API to provide config synchronously and 2) it'd be removed shortly after that new API appears anyway.

@oliverdunk
Copy link
Member

Thanks for asking that. The focus of our discussion in the meeting was if Firefox (and other browsers generally) should align with Chrome's behavior - I promised to follow-up on if this behavior is something we intended to keep so my response was based on that.

I don't have an answer for if we would change the behavior of Permissions-Policy. I can try to follow up on that too but wanted to prioritize the second point on CSP which was flagged as harder to workaround :)

@dotproto
Copy link
Member

dotproto commented Dec 6, 2024

Slightly tangiential, but Permissions-Policy, X-Frame-Options, and the Content-Security-Policy directives src-frame and frame-ancestors are all similar in that extensions need a way to bypass site-declared injection/embedding restrictions without having to directly modify the resource (whether HTTP header, HTML, or JS API) used by the site to declare it. Relevant issues discussing this are:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage: firefox Firefox needs to assess this issue for the first time needs-triage: safari Safari needs to assess this issue for the first time topic: csp Related to content security policy enforcement
Projects
None yet
Development

No branches or pull requests

6 participants