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

Define behavior of preload when CSP was changed between fetching and consuming #7686

Closed
Tracked by #38
noamr opened this issue Mar 7, 2022 · 15 comments · Fixed by whatwg/fetch#1411
Closed
Tracked by #38
Labels
integration Better coordination across standards needed

Comments

@noamr
Copy link
Contributor

noamr commented Mar 7, 2022

Consider the following scenario:

  • A document with <link rel=preload as=image href="https://some-domain.com/image.png">

The document starts loading the image
Then a scripts adds CSP, before the image is discovered:
<meta http-equiv="Content-Security-Policy" content="default-src 'self'"> (or anything that doesn't allow the image)

And then an image
<img src="https://some-domain.com/image.png" />

Should the response that has already been loaded be consumed by the img element, or should CSP be applied again, denying the response from being used by the image element?
The current specs & WPT don't exactly cover this case. The way the spec is written, there is no additional CSP check upon consumption.

@noamr
Copy link
Contributor Author

noamr commented Mar 8, 2022

I've added a test for this: web-platform-tests/wpt#33109

In Firefox the behavior is to not block the image, and in Chromium/Safari it is. What should be the right behavior?
@annevk @mikewest @achristensen07 @yoavweiss WDYT?

I personally am forming an opinion that dynamically added CSP is a problematic feature - can create a false sense of security, and then fail to protect in certain edge scenarios.

@mikewest
Copy link
Member

mikewest commented Mar 8, 2022

My understanding is that the <img> tag is specced as a second fetch, which will hit the cache that was populated via the <link> tag. In that case, Chrome/Safari's behavior seems correct to me: we'll run through main fetch for the <link>, grab the image and throw it into the cache. Then we'll run through main fetch again, checking CSP in step 6 before we go rooting around in the cache (way down in HTTP-network-or-cache fetch).

https://w3c.github.io/webappsec-csp/#meta-element does call out <meta> as being less-effective than a header-delivered policy, specifically mentioning <link> as a potential problem. I agree that it substantially complicates the story, and I don't think I'd have argued for it knowing what I know today, but I do think it's pretty clearly defined.

@noamr
Copy link
Contributor Author

noamr commented Mar 8, 2022

My understanding is that the <img> tag is specced as a second fetch, which will hit the cache that was populated via the <link> tag. In that case, Chrome/Safari's behavior seems correct to me: we'll run through main fetch for the <link>, grab the image and throw it into the cache. Then we'll run through main fetch again, checking CSP in step 6 before we go rooting around in the cache (way down in HTTP-network-or-cache fetch).

It's spec'ed as a second fetch that doesn't go through all the checks (consume is called before main fetch). I wrote that part and it can be fixed :)
Preload cache is defined as something separate from HTTP caching. It's equivalent to "loading the resource and using it later". Also in implementations it's somehow closer to the "resource loader" than to the network layer.

I'm a bit concerned about memory cache and implementations though - they are not clearly defined and don't always go via fetch.

There could be a scenario where:

  • an image is loaded into the memory cache (or "the list of available images")
  • The image is removed from the document, but remains in the cache for performance reasons - it is not specified when an image is removed from that cache
  • Meanwhile, CSP is changed to disallow images
  • The image URL, which is still in the cache, is set to a new img element (or CSS etc.)
  • Should the image load be allowed? That would mean memory caches would have to re-route themselves to do CSP checks.

noamr added a commit to noamr/html that referenced this issue Mar 8, 2022
@mikewest
Copy link
Member

mikewest commented Mar 8, 2022

It's spec'ed as a second fetch that doesn't go through all the checks (consume is called before main fetch). I wrote that part and it can be fixed :)

It's been a while since I looked at any of this, but I think (thought?) that https://html.spec.whatwg.org/multipage/images.html#update-the-image-data was the path through which we'd end up going for <img> and <picture>, which (after a lot) just Fetches in step 23. It's quite likely there's context there I've missed, but it doesn't look to me as though "consume a preloaded resource" is actually used anywhere in HTML.

More practically, though: I don't think there would be security harm to adopting Firefox's behavior. The request already happened, so exfiltration risks can no longer be mitigated. There's some conceptual weirdness about being able to display an image you can't request, but it's not security-relevant weirdness.

Assuming that "consume" is in fact used somewhere, I could imagine us resolving this one of two ways:

  1. Add some pre-consume CSP check to <img>'s processing that looks at the URL and punts early.
  2. Adopt Firefox's behavior in Chromium/WebKit.

The former seems like more work in the spec, the latter is more work in implementations. 🤷

One other question, though: how does this work with redirects? Would blocking the original URL but allowing the redirect target to load be possible via the "consume" algorithm, thereby potentially revealing redirect endpoints?

@noamr
Copy link
Contributor Author

noamr commented Mar 9, 2022

It's spec'ed as a second fetch that doesn't go through all the checks (consume is called before main fetch). I wrote that part and it can be fixed :)

It's been a while since I looked at any of this, but I think (thought?) that https://html.spec.whatwg.org/multipage/images.html#update-the-image-data was the path through which we'd end up going for <img> and <picture>, which (after a lot) just Fetches in step 23.

Of course, but I meant the case where something is already in the list of available images. And there is no mentioned of when an image is removed from that list.

It's quite likely there's context there I've missed, but it doesn't look to me as though "consume a preloaded resource" is actually used anywhere in HTML.

It is called from the fetch algo at section 10.

More practically, though: I don't think there would be security harm to adopting Firefox's behavior. The request already happened, so exfiltration risks can no longer be mitigated. There's some conceptual weirdness about being able to display an image you can't request, but it's not security-relevant weirdness.

Right. I think it's more about an expectation around what happens when you add a CSP dynamically. I think it is a security-relevant weirdness if someone added a CSP at some phase in the lifetime of a document to completely disallow remote scripts, and then a remote script is able to execute because it happens to have remained in the cache (preload cache, memory cache, or anything else above fetch).

Assuming that "consume" is in fact used somewhere, I could imagine us resolving this one of two ways:

  1. Add some pre-consume CSP check to <img>'s processing that looks at the URL and punts early.
  2. Adopt Firefox's behavior in Chromium/WebKit.

The former seems like more work in the spec, the latter is more work in implementations. 🤷

I don't think the spec work is big, I can take it on if Firefox wants to go down this route. My issue is that it doesn't end at preload - preload is a symptom for "memory cache", they usually share an implementation, which is unspecified and can result in scenarios where things are arbitrarily allowed or denied based on the capacity of the cache.

One other question, though: how does this work with redirects? Would blocking the original URL but allowing the redirect target to load be possible via the "consume" algorithm, thereby potentially revealing redirect endpoints?

The redirects are "resolved" during the preload. I was thinking we need to call block request on the pre-consume request and block response on the response received from preload. If any intermediate redirect had happened already it shouldn't be blocked.

For now I posted an editorial note PR that describes the status quo.

@mikewest
Copy link
Member

mikewest commented Mar 9, 2022

It is called from the fetch algo at section 10.

Got it, thanks for the pointer!

I think it is a security-relevant weirdness if someone added a CSP at some phase in the lifetime of a document to completely disallow remote scripts, and then a remote script is able to execute because it happens to have remained in the cache (preload cache, memory cache, or anything else above fetch).

Yes, I agree. Now that I understand that the preloaded resource consumption mechanism bypasses CSP for things other than images, I'm more interested in Firefox changing its behavior to match Chromium/Safari.

My mental model has been that CSP aims to gate access to resource execution on the one hand, and exfiltration on the other (though I'm certainly guilty of caring much more about the former than the latter). We defined it in terms of Fetch as that seemed like the most straightforward way to represent both gatekeeping mechanisms. It would be ideal for us to define access to various caches in ways that respect the rules that developers aim to apply via CSP.

I think the approach you suggest for the pre- and post-consume algorithms makes sense, though I'd note that it might also need to address resource upgrading/mixed content checks, which are a little complicated as they happen inbetween CSP reporting and CSP enforcement in order to give developers the ability to find them on their pages.

@noamr
Copy link
Contributor Author

noamr commented Mar 9, 2022

I think the approach you suggest for the pre- and post-consume algorithms makes sense, though I'd note that it might also need to address resource upgrading/mixed content checks, which are a little complicated as they happen inbetween CSP reporting and CSP enforcement in order to give developers the ability to find them on their pages.

OK, so this is to deal with a scenario where mixed content is allowed at first, but later a CSP meta is added that disallows it?

@mikewest
Copy link
Member

mikewest commented Mar 9, 2022

OK, so this is to deal with a scenario where mixed content is allowed at first, but later a CSP meta is added that disallows it?

The mixed content bits shouldn't change during the lifetime of a page (though I think they technically can in Chrome, it's an edge case well-worth ignoring, as it would involve either enterprise policy or the user poking at settings pages; either way, forcing a reload is probably fine). The upgrade bits would, I think, cause mismatches between the requested resource and the cache. Consider <img src="http://non-secure.site/img.png">: that can be upgraded to https://non-secure.site/img.png via https://w3c.github.io/webappsec-mixed-content/level2.html#upgrade-algorithm or via https://w3c.github.io/webappsec-upgrade-insecure-requests/#upgrade-request. Either way, it should probably match resources preloaded via <link rel=preload as=image href="http://non-secure.site/img.png"> and <link rel=preload as=image href="https://non-secure.site/img.png">, which only happens if the preloaded URL matches.

@noamr
Copy link
Contributor Author

noamr commented Mar 9, 2022

OK,

OK, so this is to deal with a scenario where mixed content is allowed at first, but later a CSP meta is added that disallows it?

The mixed content bits shouldn't change during the lifetime of a page (though I think they technically can in Chrome, it's an edge case well-worth ignoring, as it would involve either enterprise policy or the user poking at settings pages; either way, forcing a reload is probably fine). The upgrade bits would, I think, cause mismatches between the requested resource and the cache. Consider <img src="http://non-secure.site/img.png">: that can be upgraded to https://non-secure.site/img.png via https://w3c.github.io/webappsec-mixed-content/level2.html#upgrade-algorithm or via https://w3c.github.io/webappsec-upgrade-insecure-requests/#upgrade-request. Either way, it should probably match resources preloaded via <link rel=preload as=image href="http://non-secure.site/img.png"> and <link rel=preload as=image href="https://non-secure.site/img.png">, which only happens if the preloaded URL matches.

OK, if it can't change during the lifetime of the document then it doesn't apply here - the URLs would either be the same or consume would fail.
The only cases we need to tend to are policies that can be enforced late by dynamically adding meta with Content-Security-Policy.

@mikewest
Copy link
Member

mikewest commented Mar 9, 2022

OK, if it can't change during the lifetime of the document then it doesn't apply here - the URLs would either be the same or consume would fail.

The URLs wouldn't be the same: assuming the preloaded resource triggered by <link rel=preload as=image href="http://non-secure.site/img.png"> went through Fetch and was upgraded to HTTPS, we'd store the HTTPS URL in the cache. This wouldn't match the as-yet-unupgraded URL in <img src="http://non-secure.site/img.png">.

I guess there wouldn't be web-visible impact to that, insofar as we'd simply proceed with Fetch and pull the resource out of the disk cache after upgrading its URL?

@noamr
Copy link
Contributor Author

noamr commented Mar 9, 2022

OK, if it can't change during the lifetime of the document then it doesn't apply here - the URLs would either be the same or consume would fail.

The URLs wouldn't be the same: assuming the preloaded resource triggered by <link rel=preload as=image href="http://non-secure.site/img.png"> went through Fetch and was upgraded to HTTPS, we'd store the HTTPS URL in the cache. This wouldn't match the as-yet-unupgraded URL in <img src="http://non-secure.site/img.png">.

I guess there wouldn't be web-visible impact to that, insofar as we'd simply proceed with Fetch and pull the resource out of the disk cache after upgrading its URL?

Yes, exactly.

@annevk annevk added the integration Better coordination across standards needed label Mar 10, 2022
@annevk
Copy link
Member

annevk commented Mar 10, 2022

FWIW, I agree with Mike that CSP (and other policies) should be enforced when "consuming". Thanks for catching this!

@noamr
Copy link
Contributor Author

noamr commented Mar 10, 2022

FWIW, I agree with Mike that CSP (and other policies) should be enforced when "consuming". Thanks for catching this!

OK, so I guess I can merge the WPT and post a PR; Or do we need anyone else from Mozilla to be on board?

@annevk
Copy link
Member

annevk commented Mar 11, 2022

I'll double check, but I suspect those responsible for CSP will agree.

@noamr
Copy link
Contributor Author

noamr commented Mar 17, 2022

Reopening, should not have been auto-closed until whatwg/fetch#1411 is merged

@noamr noamr reopened this Mar 17, 2022
annevk pushed a commit to whatwg/fetch that referenced this issue Mar 17, 2022
mfreed7 pushed a commit to mfreed7/html that referenced this issue Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration Better coordination across standards needed
Development

Successfully merging a pull request may close this issue.

3 participants