Integrity check should be strict and throw by default

[LiviaMedeiros]

What is the issue with Subresource Integrity?

SRI looks like a security feature but is too loose to fulfill its purpose as such.

I see two main uses of it:

1. As owner of a resource (e.g. website) that uses third party subresources (e.g. libraries hosted on external servers), I can set integrity to ensure that my users will get correct files.
2. As user, I can get SRI string from third party that provides subresource (e.g. library hosted on external server) and see it as easily verifyable hash guaranteeing that their resource will not suddenly change.

In both cases, if anything bad (malicious behaviour on third-party side, compromised server, domain hijacking, MITM, etc.) happens and the subresource is altered, I expect it to be blocked.
Unfortunately, this is not always the case.

1. If SRI is not implemented on the client side (and it has reason to be not implemented, since it requires reading and hashing whole subresource before allowing said resource to be consumed in lazy and memory-efficient way), it's simply ignored. There's no mechanism to enforce it, and the attacker can use it: for example, providing malicious data if user's User-Agent matches known version that ignores it.
2. If the algorithm is unknown or malformed, the 3.3.2. Parse metadata tells to just ignore the item completely. The attacker can use it: for example, providing malformed string with correct hash and incorrect algorithm, which effectively disables the integrity check. Here's a repro to show how malicious script can be loaded with integrity looking like a legit well-known script: https://liviamedeiros.github.io/SRI-vulnerability-repro/

Proposed solutions for these particular problems:

For the algorithm parsing, make 3.3.4. Do bytes match metadataList? return false when metadata was not empty but parsedMetadata set is empty. Unknown algorithms can be skipped as long as at least one is known, but if integrity is set but not a single algorithm can be used, security must be prioritized hence the subresource must be blocked.

For the enforcing SRI check in general, it's trickier because proper security would hurt compatibility and be unlikely to be implemented by everyone.
Nevertheless, I think it wouldn't hurt if the spec had a note suggesting implementors to block resources by default if integrity is set but the spec isn't implemented yet.
In practice, for example, deno can reject fetch(url, { integrity: ... }) until the spec is implemented, and have an option to override it to be permissive at the cost of security (e.g. --no-validate-sri runtime flag or env variable).

---

In general, as described in 3.7. Handling integrity violations, in case of integrity violation a catchable error must occur. Thanks to that, more strict behaviour doesn't necessarily break compatibility: developers always can (and should, if their goal is to have compatibility, e.g. in case if the set of supported algorithms is changed in the future) catch errors and handle them in any way they need (logging, rejecting and asking to upgrade, fetching and checking the hash manually, etc.). AFAICT existing implementations follow this: fetch rejects with meaningful errors, and <script src="..." integrity="...-..." onerror="myErrorHandler.call(this, event)" /> also works.
So, I think the spec must enforce strict security: if integrity is set, subresource should be loaded only if it's parsed successfully, the algorithm is recognized, and the hash matches. If anything goes wrong, the implementation must throw and block the subresource instead of ignoring some errors.

[quantumpacket]

Isn't that the point of the Integrity-Policy HTTP header https://w3c.github.io/webappsec-subresource-integrity/#integrity-policy-section to enforce integrity checks?

[mozfreddyb]

In the demo, the integrity value is supplied by the attacker. I don't think you are fully (or even automatically) relying on the attacker to write your HTML source code, would you?

If you want to make integrity mandatory and rely on it to be in HTML, you can use the Integrity-Policy header as it is implemented and specified right now.

If you, at some point, want to make integrity mandatory and rely on it to be NOT in HTML, we are going to work on an integrity manifest that as well.

[mozfreddyb]

Not sure this is an issue, but happy to talk more

[LiviaMedeiros]

Isn't that the point of the Integrity-Policy HTTP header https://w3c.github.io/webappsec-subresource-integrity/#integrity-policy-section to enforce integrity checks?

Partially - yes. However:

• The issue is about what is done by default. The header must be added explicitly, and sometimes (e.g. GitHub Pages) we are unable to.
• At least in the current form, it seems to work with <style>s and <script>s (and by proxy, import), but not with fetch. Including the non-browser environments (e.g. Node.js&undici).
• Out of scope of this issue, but it's inconvenient to use if we have scripts with different sources. One page may need stable third-party scripts from third-party CDN, and dynamic scripts from own server; making us want to enforce check for everything except explicit exceptions.

In the demo, the integrity value is supplied by the attacker. I don't think you are fully (or even automatically) relying on the attacker to write your HTML source code, would you?

The demo showcases integrity that looks like correct one exactly because of this.
Example: site hosted on GitHub Pages or any similar service. Attacker (controlling the third-party script source) sends PR titled "update ${libname} from 3.6.0 to 3.6.1" with cyrillic а in sha or zero-width space. Owner can read the script, compare versions, even compute the hash; everything will look fine. PR gets merged, integrity check gets skipped, then attacker can append arbitrary malicious code at any time.
Pretty sure there are other ways to exploit it even with HTML, also the SRI spec is being used outside of web stack (for example, it's adopted by npm and integrity appears in package-lock.json).

If you, at some point, want to make integrity mandatory and rely on it to be NOT in HTML, we are going to work on an integrity manifest that as well.

Alternative to Integrity-Policy header is nice, but this issue is not about making integrity mandatory, it's about integrity being strict and robust in terms of security, no matter if it's mandatory or not. As long as integrity is non-empty, it must be strict.

---

With Integrity-Policy, cryptographic agility and better DX in mind, I would suggest adding a mechanism to explicitly skip integrity check for specific subresources. Something like this:

<?php if (enforceIntegrityPolicy) {
  header('Integrity-Policy: blocked-destinations=(script)');
} ?>

<!-- loaded only if implementation recognizes valid algorithm, computes the hash, and it matches the string -->
<script
  src="//third.party.cdn/lib-1.0.0.js"
  integrity="sha384-123...xyz=="
></script>

<!-- optional: loaded unconditionally -->
<script
  src="//location.hostname/frequently-updated-code.js?t=<?= filemtime('/path/to/frequently-updated-code.js') ?>"
  integrity="ignore"
></script>

<!-- loaded if `enforceIntegrityPolicy` is falsy, blocked otherwise -->
<script
  src="//arbitrary.host/any-script.js"
></script>

In the spec, making the check strict roughly translates to two changes to 3.3.4. Do bytes match metadataList?:

1. Before step 1 (parsing metadata), add early return: if metadataList is empty, return true.
2. In step 2, if parsedMetadata is empty set, return false rather than true.

And adding keyword to explicitly disable check (which is optional, is technically out of scope of this issue, and may or may not be added independently) can be done by:

1. Before step 1, add one more early return: if medatataList string equals "ignore", return true.

Is there anything that doesn't allow the spec to be changed this way?

[quantumpacket]

Correct me if I am misunderstanding this. You want it to be enforced by default, and then have a mechanism to opt-out of the default integrity checks? If so, I understand the reasoning for this to be secure by default, but it would likely break a lot of the internet as many sites either do not have integrity checks or are for the most part not going to have the proper opt-out in place.

[LiviaMedeiros]

No, I want it to be strict (not allowing unknown/malformed/discontinued algorithm) by default, not enforced (not allowing empty integrity) by default.
Opt-out for Integrity-Policy is just an optional addition.