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

SRI: explain CORS requirement clearly #418

Closed
jonathanKingston opened this issue Jul 9, 2015 · 16 comments
Closed

SRI: explain CORS requirement clearly #418

jonathanKingston opened this issue Jul 9, 2015 · 16 comments
Labels
Milestone

Comments

@jonathanKingston
Copy link
Contributor

The CORS requirement in the specification I feel isn't clear enough for developers to understand why CORS was made as a requirement.

This comment clearly explains the reason it was added:
#338 (comment)

A clear example of an attacker being able to bypass firewall security by:

  • loading local URLs until they stop triggering hash errors
  • using the hash to calculate the content of the file

Clearly explaining that:
This attack bypasses the current security of the attacker only able to execute the code instead of being able to know all of its contents.

@joelweinberger
Copy link
Contributor

cc @devd @fmarier @mozfreddyb, and maybe of interest to @annevk and @mikewest

@jonathanKingston
Copy link
Contributor Author

This came about from a discussion raised on trying to get Ember to default SRI to be enabled by default which is risky if it is deployed over multiple origins.

The only safe thing I can get my addon to do is to only add integrities to relative URLs unless the user explicitly sets a crossorigin. (As mentioned in previous bugs this will harm implementations although a solution isn't clear either).

The only next best thing is to explain clearly to developers and library writers why CORS is required for assets on their servers.

@annevk
Copy link
Member

annevk commented Jul 10, 2015

I'm not sure this needs additional clarification. It's no different from cross-origin scripts not throwing useful exceptions unless CORS is used.

@annevk
Copy link
Member

annevk commented Jul 10, 2015

But also, SRI is more generic than just scripts.

@jonathanKingston
Copy link
Contributor Author

@annevk I would argue it could be overlooked by a new programmer being forced to implement SRI. Clearly explaining why things are harder than they might expect should lower the barrier to them giving up etc.

Is there was a more generic place like the CORS spec itself this one could cite that might be better for future specs also?

@annevk
Copy link
Member

annevk commented Jul 10, 2015

Hmm, https://fetch.spec.whatwg.org/#http-cors-protocol just has a basic explanation of what it protects. If anything <script> should elaborate on how it hacks around CORS by executing opaque responses.

@fmarier
Copy link
Member

fmarier commented Jul 10, 2015

IMHO, we should re-phrase section 5.3 under Security Considerations to cover what Jonathan is asking for. I've had to explain it to a few people already, so I don't think it will be non-obvious to a lot of readers of this spec.

Additionally, the last sentence of that section no longer makes sense:

User agents SHOULD mitigate the risk by refusing to fire error events on elements which loaded non-CORS cross-origin resources, but some side-channels will likely be difficult to avoid.

We don't allow non-CORS cross-origin loads with SRI.

@devd
Copy link
Contributor

devd commented Jul 12, 2015

filed #422 for removing the note.

The broader issue about CORS and why we need it: I think it is best fixed by browser warnings and blogging etc. Do developers read specs as much as stack overflow, html5rocks.com etc?

@jonathanKingston
Copy link
Contributor Author

Certainly some developers do read standards which use what they learn to trickle down to the wider community.

I would actually suggest formalising all the gotchas from:

These may be obvious to people who have worked in the industry a long time or have a vested interest in specs however as the whole internet security pins from it I think it would be useful to have a published note that was a centralised resource.

That way other specs could also reference different rationale in the note rather than rewriting it each time.

@fmarier fmarier added this to the SRI-v1-LC milestone Jul 13, 2015
@jonathanKingston
Copy link
Contributor Author

So as I discussed with @wseltzer it would be useful to have a from the security interest group a SOP note on how SOP works in the wild as a definitive source.

However this doesn't solve this clarity issue here as it shouldn't hold up rec on SRI.

As crossorigin isn't specified anywhere a small note may be useful.

@annevk
Copy link
Member

annevk commented Jul 14, 2015

What do you mean isn't specified?

@jonathanKingston
Copy link
Contributor Author

@annevk we are not linking to: https://html.spec.whatwg.org/multipage/infrastructure.html#cors-settings-attributes and also explaining clearly the integrity checking exploit why CORS is required for non SOP requests.

@mozfreddyb
Copy link
Contributor

With #422, we should add a sentence to (at least) the security considerations, that explain why we do CORS (i.e. very briefly re-iterate the danger of leaks). We should also link to the cors attributes, I'm sure.

@devd
Copy link
Contributor

devd commented Aug 11, 2015

hmm .. so I am confused. What do we want to do finally (in addition to #422)? I think regardless of what we do, this is gonna be a problem until there are articles and developer resources outside of the spec.

fmarier pushed a commit to fmarier/webappsec that referenced this issue Sep 23, 2015
fmarier pushed a commit to fmarier/webappsec that referenced this issue Sep 23, 2015
@fmarier
Copy link
Member

fmarier commented Sep 23, 2015

I took a stab at this in #485.

fmarier pushed a commit that referenced this issue Sep 28, 2015
SRI: clarify the CORS requirement in security considerations (fixes #418)
@FrankBWalsh
Copy link

This thread is brilliant! I was trying to wrap my head around why the relationship between SRI and CORS and without this thread I would have been left scratching my head!

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

No branches or pull requests

7 participants