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

Consider adding web package mime types to CORB #402

Open
jyasskin opened this issue Feb 28, 2019 · 13 comments
Open

Consider adding web package mime types to CORB #402

jyasskin opened this issue Feb 28, 2019 · 13 comments

Comments

@jyasskin
Copy link
Member

CORB seems like a good thing to enable for more mime types. Since there are very few users of our mime types, we should see if it makes sense to turn on CORB for them. We're trying to discourage personalized data in signed exchanges, but unsigned bundles could certainly have it.

@anforowicz for any thoughts.

@anforowicz
Copy link

Covering web package mime types by CORB sounds good to me (*). Could you please point out in whatwg/fetch#860 which mime types should be covered (anything beside "application/signed-exchange"?).

(*) Ideally we would figure out a more systematic protection for no-cors resources (e.g. credential-less or cors-only fetches by default), but current ideas seem likely to break existing websites and therefore CORB seems like a necessary short-term approach. (cc @annevk @csreis)

@anforowicz
Copy link

@annevk points out (in whatwg/fetch#721 (comment)) that for other new types (e.g. for JavaScript modules) we successfully started requiring CORS right from the beginning. Mentioning this, in case this is something we should consider for web packages (but maybe web package fetches are never initiated from a web page - requiring CORS doesn't seem to make sense for navigation-like fetches?).

@shhnjk
Copy link

shhnjk commented Mar 1, 2019

@annevk points out (in whatwg/fetch#721 (comment)) that for other new types (e.g. for JavaScript modules) we successfully started requiring CORS right from the beginning. Mentioning this, in case this is something we should consider for web packages (but maybe web package fetches are never initiated from a web page - requiring CORS doesn't seem to make sense for navigation-like fetches?).

Requiring CORS doesn't mitigate risks of info leak through Spectre or renderer compromise. i.e. any website should still be able to do no-cors requests (e.g. <img src="//victim.tld/secret.sxg">) and information will be loaded into renderer. So CORB makes sense right now. But once 347 comes in, requiring CORS for subresource SXG files might be a good idea. Otherwise we probably need inner mime type based CORB blocking for SXG files.

@jyasskin
Copy link
Member Author

jyasskin commented Mar 1, 2019

We should think about banning no-cors requests even for <img src="//victim.tld/secret.sxg">, since nobody yet uses .sxg images, but if we can't do that, I agree we should use @kinu's suggestion of CORB filtering based on the inner content type.

@anforowicz
Copy link

@shhnjk , the proposal is to disallow no-cors requests altogether (e.g. even <img src="//victim.tld/secret.sxg"> would require CORS).

@jyasskin and @kinu - CORB is needed to prevent a compromised renderer to just asking a NetworkService for the contents of the whole SXG. I don't understand why in addition to blocking whole SXG, CORB would also want to look into SXG - maybe I am missing some details / scenarios where this is important (e.g. I don't really understand how SXG enters the picture and maybe intercepts image and/or XHR requests)?

@jyasskin
Copy link
Member Author

jyasskin commented Mar 1, 2019

@anforowicz (With the caveat that I know very little about the CORB landscape) If we allow no-cors requests that find an application/signed-exchange resource, that resource might contain either an image or an HTML file. If it's an image, then the compromised renderer should be able to retrieve it, just like it could retrieve an image that's not wrapped into a signed exchange. If it's HTML, the renderer should be blocked, just like it's blocked from retrieving a non-wrapped HTML file.

I'd rather just block SXG responses to no-cors requests if we can and if that's sufficient, because that sounds simpler.

@shhnjk
Copy link

shhnjk commented Mar 1, 2019

@shhnjk , the proposal is to disallow no-cors requests altogether (e.g. even <img src="//victim.tld/secret.sxg"> would require CORS).

@anforowicz
How will you compare CORS against navigation requests?
Which CORS header will you compare? Outer or Inner?
I don't think CORS will makes sense for SXGs which will be used for navigation requests. Yes, it makes sense to require CORS for subresource requests to SXG files (so that CORB application/signed-exchange by default would still work).

@anforowicz
Copy link

I'd rather just block SXG responses to no-cors requests if we can and if that's sufficient

Maybe it is sufficient to restrict SXG, no-cors requests to only image/, audio/, video/*, application/javascript, text/css content types?

But then - where are the SXG contents "opened" / "extracted"? We can't block things in the renderer (it is already too late there because of Spectre and/or risk of compromised renderers).

@kinu
Copy link
Collaborator

kinu commented Mar 2, 2019

@anforowicz -- SXG is basically considered at network layer concept, in Chromium implementation it's unwrapped way before the content hits the renderer so sniffing the inner type for CORB should work. (To add a little more technical detail-- it's currently done in the browser process, we've discussed unwrapping in Network Service but it hasn't happened because of lack of strong pros vs cons, but that part can be discussed separately)

Given that outer body wouldn't hit the renderer process and renderer will only see the inner response, I'm not fully convinced why they (e.g. jpg image delivered in SXG) need to be handled differently from others (e.g. regular jpg image)-- while, to be clear, I'm generally in favor of pushing the word towards no-no-cors/cors-only so can understand the motivation.

@shhnjk
Copy link

shhnjk commented Mar 2, 2019

Given that outer body wouldn't hit the renderer process and renderer will only see the inner response, I'm not fully convinced why they (e.g. jpg image delivered in SXG) need to be handled differently from others (e.g. regular jpg image)

Maybe I didn't understand how SXG works. So in current implementation, if there's a image request to SXG file (from non-SXG page) which contains HTML (i.e. <img src="//other.tld/html.sxg">), will it be parsed in the browser process to serve inner content or will it be passed to renderer's image parser?

My understanding was that subresource request from normal HTML page to SXG file wouldn't trigger SXG parser in the browser. Is that right?

@kinu
Copy link
Collaborator

kinu commented Mar 2, 2019

@shhnjk Well current implementation doesn't support subresources, so we need talk about hypothetical code base, and I started to realize that this has more subtlety than I thought but let me try. (I suppose you're familiar with the chrome architecture)

If there's a image request where SXG for HTML is returned, it will be surely parsed in the browser process (or possibly in Network Service if we make the change) at least in our currenmt thinking. For #347 this is not that hard because we can know what subresource requests may be fulfilled as SXGs before the renderer starts to consume the main resource. If we start to support regular subresource requests for SXGs things will become a bit trickier for saner cross-origin isolation, and we probably need to move the parsing code into the Network Service due to the architectural reasons. But all in all unwrapping wouldn't happen in the renderer process in subresource cases either.

@shhnjk
Copy link

shhnjk commented Mar 2, 2019

Right, so the concern is that until there is a support for subresource requests in SXG, subresource requests to SXG files will be handled by other parsers (which means outer body will hit the renderer). Which we want to avoid because renderer process can then get cross-origin information.

But since unwrapping happens inside browser process, content type sniffing is possible. So until there is a subresource support in SXG, blocking SXG mime type as a whole should be safe. Once subresource supports comes in, inner content type based CORB blocking could happen (either by safe list or unsafe list). Require CORS for all SXG subresource is a nightmare++ :D

@kinu
Copy link
Collaborator

kinu commented Mar 2, 2019

Okay-- fair enough. Currently I don't think SXG spec says anything about main resource vs subresource (so it's just that impl has a gap), could we say something like "the CORB protection for SXG should be also applied unless implementation supports SXG handling for subresources" ?

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

No branches or pull requests

4 participants