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 SRI Reports to inform site operators of integrity check failures #122

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ScottHelme
Copy link

@ScottHelme ScottHelme commented Apr 25, 2023

This PR defines SRI Reports, a mechanism to provide site operators with the ability to be notified about integrity check failures for resources they are loading. It will use the Reporting API to dispatch reports.

If assets your site depends on have been modified and fail the integrity check, there is currently no reliable way for a site operator to know. This is raised as a concern on a somewhat regular basis to me as the founder of Report URI where customers wish to implement SRI, but are surprised to learn that there is no feedback mechanism for failures. We have explored several methods to achieve this with JavaScript, but this is undesirable for various reasons, including the requirement to deploy more JavaScript and the difficulty of reliably detecting integrity check failures. This same problem was also raised in a recent whitepaper [1] where the authors had the following to say:

Takeaways: It is imperative for new standards, particular security
standards, to possess built-in reporting mechanisms. Surprisingly,
it was possible for SRI misconfigurations of first-party scripts to
remain unnoticed for multiple months.

There is further discussion in #20 about the benefits of such a mechanism and how to avoid concerns around security and privacy impacts, which have been considered for this proposal.

Fix #20

[1] The More Things Change, the More They Stay the Same: Integrity of Modern JavaScript


Preview | Diff

@annevk
Copy link
Member

annevk commented Apr 25, 2023

As explained in #20 I'm opposed to this. Definitely without adequate layering being sorted out first. Also, this particular PR doesn't do anything to assuage the security concerns, despite the claims in OP to the contrary.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r- per above.

@ScottHelme
Copy link
Author

Could you articulate what security concerns remain?

Per the comment by @mozfreddyb: #20 (comment)

Given that SRI requires crossorigin=anonymous, we can assume any code on that web page could fetch() the resource themselves, compute the hash and see if the failure has been an integrity failure. What's the danger in exposing this through an event?

The change also requires that reports:

must only be sent as the result of a failed integrity check and not any other error

We can achieve the same with JavaScript on the page but that is undesirable for the reasons mentioned above and more. This doesn't expose any information beyond what is already readily available, but allows the observation of failures without unnecessary overheads.

@ScottHelme
Copy link
Author

I've created a simple JS demo to show this how this is already possible, you can find it here: https://report-uri-demo.com/temp-sri-demo.html

The SRI Report proposed above suggests less information than that which is gathered in the JS demo. For a resource to be integrity checked, it has to be requested in crossorigin mode so there's nothing stopping this from being possible, it's just not a clean way to do it. There's the added overhead of loading the JS, the necessary checks on failed resources, requesting the resource again, etc... A native reporting mechanism using the Reporting API would be a far better solution and not yield anything more than that which is already available.

Of course, there may well be some security or privacy concern that I haven't considered, in which case, please do share that concern and we'll see how it can be addressed @annevk.

@annevk
Copy link
Member

annevk commented Apr 26, 2023

must only be sent as the result of a failed integrity check and not any other error

As I said, this is not an adequate way of specifying this. You'll need to actually refine Fetch's "network error" concept to pass this information along.

And that illustrates the security issue as well as this would be the first time the "network process" has to reveal this kind of information to the "website process". I'm not persuaded this alone is sufficient reason to start exposing that kind of information across this boundary.

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

Successfully merging this pull request may close these issues.

Consider integrity check violation reporting
2 participants