Skip to content

Add Integrity-Policy for scripts #133

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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Add Integrity-Policy for scripts #133

wants to merge 19 commits into from

Conversation

yoavweiss
Copy link

@yoavweiss yoavweiss commented Apr 15, 2025

As discussed, this is replacing #129

It is defining two new headers: Integrity-Policy and Integrity-Policy-Report-Only that would enable developers to enforce integrity on scripts (in the immediate) and on more request destinations in the future.

Integrity-Policy header

Subresource-Integrity (SRI) enables developers to make sure the assets they intend to load are indeed the assets they are loading. But there's no current way for developers to be sure that all of their scripts are validated using SRI.

The Integrity-Policy header gives developers the ability to assert that every resource of a given type needs to be integrity-checked. If a resource of that type is attempted to be loaded without integrity metadata, that attempt will fail and trigger a violation report.

The Integrity-Policy header is a structured field Dictionary, where every member value is an inner list of tokens.
Possible keys are: blocked-destinations

Example usage

A developer that wants to validate that all of their script resources have integrity checks will be able to add a header similar to:

Integrity-Policy: blocked-destinations=(script), endpoints=(integrity-endpoint)

From that point, any external script that is fetched without a valid integrity attribute (that is, one that translates into non-empty integrity metadata) or with a "no-cors" request mode, will not be loaded.
It will also trigger a violation report.

The header also has a sources key. It's only possible value (as well as its default value) is "inline". It's presence would enable future-compatible additions of other integrity sources, such as headers.


Preview | Diff

@yoavweiss
Copy link
Author

@mozfreddyb - this is still missing the reporting parts, but I'd appreciate your feedback on the overall direction.

Copy link
Collaborator

@mozfreddyb mozfreddyb left a comment

Choose a reason for hiding this comment

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

Great start, thanks for kicking this off.

@yoavweiss yoavweiss requested a review from mozfreddyb April 16, 2025 09:07
@mozfreddyb
Copy link
Collaborator

I have yet to look at the reporting bits, but looks good otherwise.

@annevk: Can you review the integration bits (HTML & fetch) or defer to someone who can?

@yoavweiss
Copy link
Author

Points raised at WebAppSec:

  • We may want a future where we have different enforcement for different destinations. Maybe we can enable that through parameters? (e.g. IntegrityPolicy: destinations=(script;block image;warn))
  • We should report if the violation is a report-only one or an enforced one
  • No-cors assets should be blocked. The current algorithm doesn't cover it

@yoavweiss
Copy link
Author

Another thought: An event here is redundant with a ReportingObserver. There's no reason to copy those over from CSP.

@mozfreddyb - do you agree?

@mozfreddyb
Copy link
Collaborator

Neither Firefox nor Webkit ship the Reporting API (or Reporting Observer). We should get clarity on whether there are plans to do so eventually or if this ends up being a hindrance. I'm going to ask around for Firefox.

@yoavweiss
Copy link
Author

yoavweiss commented Apr 17, 2025

Neither Firefox nor Webkit ship the Reporting API (or Reporting Observer)

WebKit ships it (tests)

@yoavweiss
Copy link
Author

Neither Firefox nor Webkit ship the Reporting API (or Reporting Observer)

WebKit ships it (tests)

Seems like it's implemented (but not shipped) in Firefox. Maybe y'all should just ship it? :)

Removed the event as it seemed spurious to what Reporting already does.

@yoavweiss
Copy link
Author

  • We should report if the violation is a report-only one or an enforced one
  • No-cors assets should be blocked. The current algorithm doesn't cover it

Done

@yoavweiss
Copy link
Author

  • We may want a future where we have different enforcement for different destinations. Maybe we can enable that through parameters? (e.g. IntegrityPolicy: destinations=(script;block image;warn))

I'm not sure what to do about this one - if that syntax works, maybe we should add a processing step such that any parameter that's not "block" would cause the value to get dropped?

Also, if we're going with this, do we need an "enforcement" key?

@camillelamy @mozfreddyb - I'd love your opinions

@tomrittervg
Copy link

  • We may want a future where we have different enforcement for different destinations. Maybe we can enable that through parameters? (e.g. IntegrityPolicy: destinations=(script;block image;warn))

I'm not sure what to do about this one - if that syntax works, maybe we should add a processing step such that any parameter that's not "block" would cause the value to get dropped?

Also, if we're going with this, do we need an "enforcement" key?

@camillelamy @mozfreddyb - I'd love your opinions

Freddy is out and I know you're trying to keep momentum; I was not at the meeting but I looking at our original notes, we did have a separate list of destinations for different enforcement levels, so I think this is still desirable.

Maybe instead of destinations and enforcement, we could have destinations-block which would be a list of destinations, and any other unrecognized key would be ignored (which I think is the behavior now?)

Then in the future we could add a destinations-warn list. Would that work?

@yoavweiss
Copy link
Author

Maybe instead of destinations and enforcement, we could have destinations-block which would be a list of destinations, and any other unrecognized key would be ignored (which I think is the behavior now?)

Then in the future we could add a destinations-warn list. Would that work?

Yeah, that would work for me. (maybe calling it blocked-destinations instead..)

@camillelamy
Copy link
Member

Having destinations-block and extending with other kinds of destinations if needed seems good to me.

@yoavweiss yoavweiss requested a review from annevk May 1, 2025 17:31
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 6, 2025
This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
aarongable pushed a commit to chromium/chromium that referenced this pull request May 6, 2025
This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408111
Reviewed-by: Camille Lamy <[email protected]>
Commit-Queue: Yoav Weiss (@Shopify) <[email protected]>
Reviewed-by: Antonio Sartori <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1456383}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 6, 2025
This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408111
Reviewed-by: Camille Lamy <[email protected]>
Commit-Queue: Yoav Weiss (@Shopify) <[email protected]>
Reviewed-by: Antonio Sartori <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1456383}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 6, 2025
This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408111
Reviewed-by: Camille Lamy <[email protected]>
Commit-Queue: Yoav Weiss (@Shopify) <[email protected]>
Reviewed-by: Antonio Sartori <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1456383}
@yoavweiss
Copy link
Author

@mozfreddyb @annevk - Friendly ping on this review :)

@yoavweiss yoavweiss requested a review from annevk May 14, 2025 13:17
index.bs Outdated
Comment on lines 544 to 546
1. If |headers| <a for="header list">contains</a> a <a>header</a>
whose <a>byte-lowercased</a> <a>header name</a>'s <a>isomorphic decode</a>
is "`integrity-policy`",
Copy link
Member

Choose a reason for hiding this comment

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

No. https://fetch.spec.whatwg.org/#header-list-contains already does the case-insensitive match. You just have to write it as `Integrity-Policy` (bit annoying with backslashes if you want to stay full Markdown). This will have to be redone.

Copy link
Author

@yoavweiss yoavweiss May 14, 2025

Choose a reason for hiding this comment

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

Landed on "If |headers| contains ``integrity-policy``". Does that work?


A <dfn>source</dfn> is a string. The only possible value for it is "`inline`".

A <dfn>destination</dfn> is a string. The only possible value for it is "`script`".
Copy link
Member

Choose a reason for hiding this comment

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

A way to satisfy mt and I would be to define the actual type in Fetch. A middle ground might be a note. But this seems okay.

Copy link
Author

Choose a reason for hiding this comment

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

I can do that as part of the Fetch integration as a followup

lando-prod-mozilla bot pushed a commit to mozilla-firefox/firefox that referenced this pull request May 14, 2025
Automatic update from web-platform-tests
Integrity-Policy for script destinations

This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408111
Reviewed-by: Camille Lamy <[email protected]>
Commit-Queue: Yoav Weiss (@Shopify) <[email protected]>
Reviewed-by: Antonio Sartori <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1456383}

--

wpt-commits: 58c8754f2d64eda3c04c7afecca2be6799484f5b
wpt-pr: 52360

Differential Revision: https://phabricator.services.mozilla.com/D249270
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this pull request May 15, 2025
Automatic update from web-platform-tests
Integrity-Policy for script destinations

This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408111
Reviewed-by: Camille Lamy <[email protected]>
Commit-Queue: Yoav Weiss (@Shopify) <[email protected]>
Reviewed-by: Antonio Sartori <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1456383}

--

wpt-commits: 58c8754f2d64eda3c04c7afecca2be6799484f5b
wpt-pr: 52360

Differential Revision: https://phabricator.services.mozilla.com/D249270
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 15, 2025
Automatic update from web-platform-tests
Integrity-Policy for script destinations

This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408111
Reviewed-by: Camille Lamy <[email protected]>
Commit-Queue: Yoav Weiss (@Shopify) <[email protected]>
Reviewed-by: Antonio Sartori <[email protected]>
Reviewed-by: Adam Rice <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1456383}

--

wpt-commits: 58c8754f2d64eda3c04c7afecca2be6799484f5b
wpt-pr: 52360

Differential Revision: https://phabricator.services.mozilla.com/D249270
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request May 16, 2025
Automatic update from web-platform-tests
Integrity-Policy for script destinations

This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408111
Reviewed-by: Camille Lamy <clamychromium.org>
Commit-Queue: Yoav Weiss (Shopify) <yoavweisschromium.org>
Reviewed-by: Antonio Sartori <antoniosartorichromium.org>
Reviewed-by: Adam Rice <riceachromium.org>
Cr-Commit-Position: refs/heads/main{#1456383}

--

wpt-commits: 58c8754f2d64eda3c04c7afecca2be6799484f5b
wpt-pr: 52360

Differential Revision: https://phabricator.services.mozilla.com/D249270

UltraBlame original commit: b2f20b0522428084cd0d1f68900d6843d2fb1b22
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request May 16, 2025
Automatic update from web-platform-tests
Integrity-Policy for script destinations

This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408111
Reviewed-by: Camille Lamy <clamychromium.org>
Commit-Queue: Yoav Weiss (Shopify) <yoavweisschromium.org>
Reviewed-by: Antonio Sartori <antoniosartorichromium.org>
Reviewed-by: Adam Rice <riceachromium.org>
Cr-Commit-Position: refs/heads/main{#1456383}

--

wpt-commits: 58c8754f2d64eda3c04c7afecca2be6799484f5b
wpt-pr: 52360

Differential Revision: https://phabricator.services.mozilla.com/D249270

UltraBlame original commit: b2f20b0522428084cd0d1f68900d6843d2fb1b22
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request May 16, 2025
Automatic update from web-platform-tests
Integrity-Policy for script destinations

This adds support for Integrity-Policy instead of `require-sri-for`,
based on [1].

[1] w3c/webappsec-subresource-integrity#133

I2S: https://groups.google.com/a/chromium.org/g/blink-dev/c/Q304_OkDAZA/m/b3Bnyab9DgAJ

Change-Id: I9599280eb94045951351368d2531d25c32c15681
Bug: 412588111
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6408111
Reviewed-by: Camille Lamy <clamychromium.org>
Commit-Queue: Yoav Weiss (Shopify) <yoavweisschromium.org>
Reviewed-by: Antonio Sartori <antoniosartorichromium.org>
Reviewed-by: Adam Rice <riceachromium.org>
Cr-Commit-Position: refs/heads/main{#1456383}

--

wpt-commits: 58c8754f2d64eda3c04c7afecca2be6799484f5b
wpt-pr: 52360

Differential Revision: https://phabricator.services.mozilla.com/D249270

UltraBlame original commit: b2f20b0522428084cd0d1f68900d6843d2fb1b22
@yoavweiss yoavweiss requested a review from annevk May 16, 2025 12:55
@w3c w3c deleted a comment May 19, 2025
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.

6 participants