-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
"create navigation params by fetching" invokes "should navigation request of type be blocked by Content Security Policy?" without setting the policy container #10796
"create navigation params by fetching" invokes "should navigation request of type be blocked by Content Security Policy?" without setting the policy container #10796
Comments
@annevk, I think you might be best-positioned to help solve this? In particular, I'm unsure how to leave the resolution of "client" in Fetch, while still letting HTML do the manual redirect-following that it does. |
Since HTML has the need to create new environments as it encounters new URLs, the environment should be controllable by HTML and cannot just be cloned as a one-off and count for all subsequent fetches. Note that when we follow the redirect https://fetch.spec.whatwg.org/#http-redirect-fetch is invoked which invokes "main fetch" which doesn't even attempt to update the environment. I'm not sure what the best way to rejigger this is. Perhaps the client/environment should be owned by the fetch controller so it can be accessed by HTML. (It also strikes me that there's probably multiple environments in play. One that caused the navigation (e.g., responsible for referrer) and one that is responsible for the URL we are navigating to at the moment (e.g., responsible for service workers).) |
I took a bit more of a look at this. It's messier than it first appears. There's kind of a general conflict here. Our intent with the navigation spec these days is to snapshot source information synchronously, and then use it throughout. The current spec does not seem to attempt to do this. It captures the source snapshot params, which includes a fetch client and source policy container. But, it's possible the source policy container is still mutable. In particular, I think the intent of https://html.spec.whatwg.org/#attr-meta-http-equiv-content-security-policy is to mutate it. (But the "enforce the policy" algorithm that is supposed to do that work seems pretty underspecified.) From this perspective, a reasonable fix would be to change snapshot source snapshot params to use clone a policy container, which explicitly clones the CSP list. And then, modify create navigation params by fetching's interaction with CSP to use the snapshotted policy container, instead of just using request. (In particular, stop using request's policy container, which is currently left as "client", which is what's broken.) But I'm unsure how to square that with @annevk's
@annevk, are you saying that we should be using the policy container from the intermediate URLs in a redirect chain? That doesn't seem right to me. I think a snapshot of the source policy container would work well. Maybe you are confusing "client" and "reserved client"? /cc @domfarolino to get more brains in the room. |
I might well be confused. The end user is on A. They click a link to navigate to B. B redirects to C. For CSP we need the policy from A's environment. (Although I think there are unresolved issues with this form submission reporting, in particular around redirects.) For referrer it's also from A's environment. If B has a service worker, we need to get that from B (from its reserved environment perhaps?). If C has a service worker, we need to get that from C. Now if you have a different situation where K embeds L. And clicking a link/submitting a form in L navigates K to M. It's more complicated I think which of K and L hold the authority for the navigation and I'm not sure we ever really got to the bottom of this. I believe both might have to be tracked depending on the piece of state you care about? |
I agree with all this. This issue is mainly about CSP, and secondarily about the other contents of the policy container. So A is the only relevant thing for those, I believe. The service worker is not involved, and is handled via separate parts of the spec.
"Either, depending on which piece of state" sounds right. The distinction here in the spec is between "source" (L) and "target" (K). We currently consult source for everything except sandboxing flags. (For sandboxing flags we consult a sort of union of source and target, at one point, and just target for determining the origin.) The case of CSP currently consults "request's client", which is derived from source. So my suggestion above would be a way to make that more rigorous and properly snapshotted. Now, maybe CSP should be consulting target, instead of source? But assuming the intent of the current spec is reasonable, I think my suggestion above is good to implement. Do you agree? |
Yes, I think that would work. And thank you for reminding me of the source/target distinction, it seems we have made quite a bit of progress here over the years. 😊 |
Closes #10796, by passing along the intended snapshotted source CSP instead of attempting to look up the policy container from the request (which will not work when it's left as "client").
This allows HTML to pass in the correct CSP list, to help fix whatwg/html#10796. See also: whatwg/html#10949 and w3c#494. Closes w3c#692 by superseding it.
This allows HTML to pass in the correct CSP list, to help fix whatwg/html#10796. See also: whatwg/html#10949 and w3c#494. Closes w3c#692 by superseding it.
What is the issue with the HTML Standard?
The request's policy container is set only in 1, step 12. "should navigation request of type be blocked by Content Security Policy?" is called from 2, step 19.3. 1 is called from step 19.5 of 2. So during the first iteration of the while-loop at step 19 of 2, the request's policy container will be its default value, "client". 3 doesn't handle "client", though.
CC @antosart, as mentioned at w3c/webappsec-csp#692 (comment).
Footnotes
https://fetch.spec.whatwg.org/#concept-fetch ↩ ↩2
https://html.spec.whatwg.org/multipage/browsing-the-web.html#create-navigation-params-by-fetching ↩ ↩2 ↩3
https://www.w3.org/TR/CSP3/#should-block-navigation-request ↩
The text was updated successfully, but these errors were encountered: