attempt to write down `require-sri-for` directive as part of SRI #32
6 participants
| @@ -343,7 +354,47 @@ implementation detail. It is not an API that implementors | ||
| provide to web applications. It is used in this document | ||
| only to simplify the algorithm description. | ||
| -## Response verification algorithms ## {#verification-algorithms} | ||
| +## Request verification algorithms ## {#request-verification-algorithms} |
|
|
mozfreddyb
commented on an outdated diff
| @@ -343,7 +354,47 @@ implementation detail. It is not an API that implementors | ||
| provide to web applications. It is used in this document | ||
| only to simplify the algorithm description. | ||
| -## Response verification algorithms ## {#verification-algorithms} | ||
| +## Request verification algorithms ## {#request-verification-algorithms} | ||
| + | ||
| +### Opting-in | ||
| + | ||
| +Authors may opt a Document to requre SRI metadata be present for |
|
|
Please don't remove index.html in your branch.
Please don't remove index.html in your branch.
I thought approach here is the same as in CSP, where maintainer commits bikeshed output.
I amended commit to include index.bikeshed.html.
I haven't changed spec.markdown, which is the producer of index.html assuming development was switched to used Bikeshed. Should I port my changes to Markdown after we are done, to get updated index.html?
spec.markdown and index.html are SRI version 1, which we can't modify lightly.
require-sri-for should land in the next SRI version, so I'd prefer you only patch index.bikeshed.bs :)
Looks good from my side. Thank you for writing this down, Sergey!
paging other spec editors @metromoxie, @devd @fmarier
metromoxie
commented on an outdated diff
| @@ -343,7 +354,47 @@ implementation detail. It is not an API that implementors | ||
| provide to web applications. It is used in this document | ||
| only to simplify the algorithm description. | ||
| -## Response verification algorithms ## {#verification-algorithms} | ||
| +## Request verification algorithms ## {#request-verification-algorithms} | ||
| + | ||
| +### Opting-in {#opt-in-require-sri-for} | ||
| + | ||
| +Authors may opt a Document to require SRI metadata be present for |
|
|
metromoxie
and 1 other
commented on an outdated diff
| +## Request verification algorithms ## {#request-verification-algorithms} | ||
| + | ||
| +### Opting-in {#opt-in-require-sri-for} | ||
| + | ||
| +Authors may opt a Document to require SRI metadata be present for | ||
| +some resource types via a <dfn export>require-sri-for</dfn> <a>Content | ||
| +Security Policy</a> directive defined by the following ABNF grammar: | ||
| + | ||
| +<pre dfn-type="grammar" link-type="grammar"> | ||
| + directive-name = "require-sri-for" | ||
| + directive-value = <a grammar>token</a> *( <a>RWS</a> <a>token</a> ) | ||
| +</pre> | ||
| + | ||
| +The directive recognizes a number of potential token values: | ||
| + | ||
| + * `script` requires SRI for scripts |
metromoxie
These should be in the grammar above, I think. Something like: This section should then link to those token entries. 
shekyan
Uh. I proposed it in w3c/webappsec-csp#64 (comment), but following discussion convinced me that token values shouldn't be locked down by the grammar. Thoughts?
metromoxie
Haha, well @mikewest knows best, so feel free to ignore my comment :-) |
metromoxie
commented on an outdated diff
| + | ||
| +### Parsing `require-sri-for` ### {#parse-require-sri-for} | ||
| + | ||
| +To parse the |token| list, the user agent MUST use an algorithm equivalent to the following: | ||
| + | ||
| +1. Let the set of |protected resource types| that require SRI be the empty set. | ||
| + | ||
| +2. For each token returned by <a>splitting tokens on spaces</a>, | ||
| + if token matches the grammar for <a>require-sri-for</a>, | ||
| + add the token to the set of |protected resource types|. Otherwise, ignore the token. | ||
| + | ||
| +3. Return the set of |protected resource types|. | ||
| + | ||
| +### Apply |algorithm| to |request| ### {#apply-algorithm-to-request} | ||
| + | ||
| +1. Let |protected resource types| be the result of [[#parse-require-sri-for]]. |
|
metromoxie
Probably should read "...be the result of applying [[#parse-require-sri-for]] to the value of the <a>require-sri-for</a> directive." |
lgtm % nits/minor comments. Thanks a lot, @shekyan!
mikewest
commented on an outdated diff
| +### Parsing `require-sri-for` ### {#parse-require-sri-for} | ||
| + | ||
| +To parse the |token| list, the user agent MUST use an algorithm equivalent to the following: | ||
| + | ||
| +1. Let the set of |protected resource types| that require SRI be the empty set. | ||
| + | ||
| +2. For each token returned by <a>splitting tokens on spaces</a>, | ||
| + if token matches the grammar for <a>require-sri-for</a>, | ||
| + add the token to the set of |protected resource types|. Otherwise, ignore the token. | ||
| + | ||
| +3. Return the set of |protected resource types|. | ||
| + | ||
| +### Apply |algorithm| to |request| ### {#apply-algorithm-to-request} | ||
| + | ||
| +1. Let |protected resource types| be the result of applying [[#parse-require-sri-for]] | ||
| + to the value of the <a>require-sri-for</a> directive. |
mikewest
I think this needs to be clarified. A few questions come to mind:
|
mikewest
commented on an outdated diff
| + * `script` requires SRI for scripts | ||
| + * `style` requires SRI for style sheets | ||
| + | ||
| +### Parsing `require-sri-for` ### {#parse-require-sri-for} | ||
| + | ||
| +To parse the |token| list, the user agent MUST use an algorithm equivalent to the following: | ||
| + | ||
| +1. Let the set of |protected resource types| that require SRI be the empty set. | ||
| + | ||
| +2. For each token returned by <a>splitting tokens on spaces</a>, | ||
| + if token matches the grammar for <a>require-sri-for</a>, | ||
| + add the token to the set of |protected resource types|. Otherwise, ignore the token. | ||
| + | ||
| +3. Return the set of |protected resource types|. | ||
| + | ||
| +### Apply |algorithm| to |request| ### {#apply-algorithm-to-request} |
|
mikewest
When does this get called? It sounds like it ought to be defined as the directive's "pre-request check", which would take care of hooking it into Fetch... |
This looks like a good start, but I have some questions that I've left inline. Thanks!
@mikewest , please review if this looks sane when you have a chance.
mikewest
commented on an outdated diff
mikewest
commented on an outdated diff
| + directive-value = <a grammar>token</a> *( <a>RWS</a> <a>token</a> ) | ||
| +</pre> | ||
| + | ||
| +The directive recognizes a number of potential token values: | ||
| + | ||
| + * `script` requires SRI for scripts | ||
| + * `style` requires SRI for style sheets | ||
| + | ||
| +### Parsing `require-sri-for` ### {#parse-require-sri-for} | ||
| + | ||
| +To parse the |token| list, the user agent MUST use an algorithm equivalent to the following: | ||
| + | ||
| +1. Let the set of |protected resource types| that require SRI be the empty set. | ||
| + | ||
| +2. For each token returned by <a>splitting tokens on spaces</a>, | ||
| + if token matches the grammar for <a>require-sri-for</a>, |
|
mikewest
Tiny style nit: Line this up with the first sentence. |
mikewest
commented on an outdated diff
| @@ -343,7 +361,55 @@ implementation detail. It is not an API that implementors | ||
| provide to web applications. It is used in this document | ||
| only to simplify the algorithm description. | ||
| -## Response verification algorithms ## {#verification-algorithms} | ||
| +## Request verification algorithms ## {#request-verification-algorithms} | ||
| + | ||
| +### Opting-in ### {#opt-in-require-sri-for} | ||
| + | ||
| +Authors may opt a {{Document}} to require SRI metadata be present for | ||
| +some resource types via a <dfn export>require-sri-for</dfn> <a>Content | ||
| +Security Policy</a> directive defined by the following ABNF grammar: | ||
| + | ||
| +<pre dfn-type="grammar" link-type="grammar"> | ||
| + directive-name = "require-sri-for" | ||
| + directive-value = <a grammar>token</a> *( <a>RWS</a> <a>token</a> ) | ||
| +</pre> | ||
| + | ||
| +The directive recognizes a number of potential token values: |
|
mikewest
Can you add a definition here that we can link to below? Perhaps, " |
mikewest
commented on an outdated diff
| +some resource types via a <dfn export>require-sri-for</dfn> <a>Content | ||
| +Security Policy</a> directive defined by the following ABNF grammar: | ||
| + | ||
| +<pre dfn-type="grammar" link-type="grammar"> | ||
| + directive-name = "require-sri-for" | ||
| + directive-value = <a grammar>token</a> *( <a>RWS</a> <a>token</a> ) | ||
| +</pre> | ||
| + | ||
| +The directive recognizes a number of potential token values: | ||
| + | ||
| + * `script` requires SRI for scripts | ||
| + * `style` requires SRI for style sheets | ||
| + | ||
| +### Parsing `require-sri-for` ### {#parse-require-sri-for} | ||
| + | ||
| +To parse the |token| list, the user agent MUST use an algorithm equivalent to the following: |
|
mikewest
Nit: I'd rephrase this to something like: " |
mikewest
commented on an outdated diff
| + directive-name = "require-sri-for" | ||
| + directive-value = <a grammar>token</a> *( <a>RWS</a> <a>token</a> ) | ||
| +</pre> | ||
| + | ||
| +The directive recognizes a number of potential token values: | ||
| + | ||
| + * `script` requires SRI for scripts | ||
| + * `style` requires SRI for style sheets | ||
| + | ||
| +### Parsing `require-sri-for` ### {#parse-require-sri-for} | ||
| + | ||
| +To parse the |token| list, the user agent MUST use an algorithm equivalent to the following: | ||
| + | ||
| +1. Let the set of |protected resource types| that require SRI be the empty set. | ||
| + | ||
| +2. For each token returned by <a>splitting tokens on spaces</a>, |
|
mikewest
Nit: " |
mikewest
commented on an outdated diff
| +</pre> | ||
| + | ||
| +The directive recognizes a number of potential token values: | ||
| + | ||
| + * `script` requires SRI for scripts | ||
| + * `style` requires SRI for style sheets | ||
| + | ||
| +### Parsing `require-sri-for` ### {#parse-require-sri-for} | ||
| + | ||
| +To parse the |token| list, the user agent MUST use an algorithm equivalent to the following: | ||
| + | ||
| +1. Let the set of |protected resource types| that require SRI be the empty set. | ||
| + | ||
| +2. For each token returned by <a>splitting tokens on spaces</a>, | ||
| + if token matches the grammar for <a>require-sri-for</a>, | ||
| + add the token to the set of |protected resource types|. Otherwise, ignore the token. |
|
mikewest
Nit: Rather than "matches the grammar", perhaps something like " |
mikewest
commented on an outdated diff
| + | ||
| +2. For each token returned by <a>splitting tokens on spaces</a>, | ||
| + if token matches the grammar for <a>require-sri-for</a>, | ||
| + add the token to the set of |protected resource types|. Otherwise, ignore the token. | ||
| + | ||
| +3. Return the set of |protected resource types|. | ||
| + | ||
| +### Apply |algorithm| to |request| ### {#apply-algorithm-to-request} | ||
| + | ||
| +This directive’s <a>pre-request check</a> is as follows: | ||
| + | ||
| +Given a <a>request</a> (|request|) and a <a>policy</a> (|policy|): | ||
| + | ||
| +1. Let |protected resource types| be an empty set. | ||
| + | ||
| +2. If a directive whose name is "require-sri-for" is present in policy’s directive set, |
|
mikewest
This will only be called when such a directive is present. You can rephrase this a bit by replacing 1 and 2 with something like: |
Seems pretty reasonable to me % the comments I left. I'll leave the question of which files to touch to the SRI editors who know what's up there.
Again, thank you Sergey for bearing with our endless feedback :-)
(Also thanks to @mikewest for reviewing!)
I think this PR needs wording that suggests a missing integrity attribute will trigger a CSP violation report.
Addressed @mikewest and @mozfreddyb comments. I don't know if we want to allow this directive be delivered through header only or within <meta> element as well. Thoughts?
I see no harm in allowing it in <meta> other than our general push to allow fewer things there. I suppose if we extend the directive later, we could be sad, if, for example, we allow requiring certain algorithms, then there might be a path to downgrading what algorithms we enforce, but that does already require injection on the page.
@mikewest, what do you think?
The only things we've banned from <meta> were things that were dangerous (report-uri), or difficult to apply at runtime (sandbox and frame-ancestors). I don't think this directive would be difficult to apply (though it probably has some interaction with the preload scanner that we'll need to document somewhere).
mikewest
commented on an outdated diff
| +</pre> | ||
| + | ||
| +The following list contains the set of <dfn noexport>known tokens</dfn>: | ||
| + | ||
| + * `script` requires SRI for scripts | ||
| + * `style` requires SRI for style sheets | ||
| + | ||
| +### Parsing `require-sri-for` ### {#parse-require-sri-for} | ||
| + | ||
| +Given a string (|token list|), this algorithm returns a list of resource | ||
| +types which will require integrity checks: | ||
| + | ||
| +1. Let the set of |protected resource types| that require SRI be the empty set. | ||
| + | ||
| +2. For each |token| in the result of <a lt="split a string on spaces"> | ||
| + splitting |token list| on spaces</a> if token matches the grammar |
|
|
mikewest
commented on an outdated diff
| @@ -36,22 +36,47 @@ spec: ABNF; urlPrefix: https://tools.ietf.org/html/rfc5234 | ||
| text: VCHAR; url: appendix-B.1 | ||
| text: WSP; url: appendix-B.1 | ||
| +spec: CSP; urlPrefix: https://w3c.github.io/webappsec-csp/ | ||
| + type: dfn | ||
| + text: Content Security Policy; urlPrefix: # | ||
| + text: policy; url: policy | ||
| + text: directive; url: directives | ||
| + text: directive value; url: directive-value |
|
mikewest
I'd mark this up as |
mikewest
commented on an outdated diff
| @@ -36,22 +36,47 @@ spec: ABNF; urlPrefix: https://tools.ietf.org/html/rfc5234 | ||
| text: VCHAR; url: appendix-B.1 | ||
| text: WSP; url: appendix-B.1 | ||
| +spec: CSP; urlPrefix: https://w3c.github.io/webappsec-csp/ | ||
| + type: dfn | ||
| + text: Content Security Policy; urlPrefix: # | ||
| + text: policy; url: policy | ||
| + text: directive; url: directives | ||
| + text: directive value; url: directive-value | ||
| + text: pre-request check; url: directive-pre-request-check | ||
| + text: create a violation object for global; url: create-violation-for-global | ||
| + text: report violation; url: report-violation | ||
| + text: violation; url: violation | ||
| + text: violation-resource; url: violation-resource |
|
mikewest
Ditto here: |
mikewest
commented on an outdated diff
| + | ||
| +2. For each |token| in the result of <a lt="split a string on spaces"> | ||
| + splitting |token list| on spaces</a> if token matches the grammar | ||
| + for <a>require-sri-for</a>, add |token| to |protected resource types| | ||
| + if |token| is a <a>known token</a>. Otherwise, ignore the token. | ||
| + | ||
| +3. Return the set of |protected resource types|. | ||
| + | ||
| +### Apply |algorithm| to |request| ### {#apply-algorithm-to-request} | ||
| + | ||
| +This directive’s <a>pre-request check</a> is as follows: | ||
| + | ||
| +Given a <a>request</a> (|request|) and a <a>policy</a> (|policy|): | ||
| + | ||
| +1. Let |protected resource types| be the result of executing | ||
| + [[#parse-require-sri-for]] on this <a>directive</a>'s <a lt="directive value">value</a>. |
|
|
mikewest
and 2 others
commented on an outdated diff
| +This directive’s <a>pre-request check</a> is as follows: | ||
| + | ||
| +Given a <a>request</a> (|request|) and a <a>policy</a> (|policy|): | ||
| + | ||
| +1. Let |protected resource types| be the result of executing | ||
| + [[#parse-require-sri-for]] on this <a>directive</a>'s <a lt="directive value">value</a>. | ||
| + | ||
| +2. If |request|'s <a>destination</a> is a <a>ASCII case-insensitive match</a> for at least | ||
| + one token in |protected resource types|, and |request|'s integrity metadata | ||
| + is the empty string: | ||
| + | ||
| + 1. Let |violation| be the result of executing <a lt="create a violation object for global"> | ||
| + Create a violation object for global, policy, and directive</a> on |document|'s | ||
| + <a>global object</a>, |policy|, and "<a>`require-sri-for`</a>". | ||
| + | ||
| + 2. Set |violation|'s <a lt="violation-resource">resource</a> to "`inline`". |
|
mikewest
Is this
shekyan
Was not sure which one to choose. My thought was that parent document violates the policy by not having the integrity metadata. On the other hand, 
mozfreddyb
Thanks for raising this! |
LGTM % nits.
mikewest
and 1 other
commented on an outdated diff
| +3. Return the set of |protected resource types|. | ||
| + | ||
| +### Apply |algorithm| to |request| ### {#apply-algorithm-to-request} | ||
| + | ||
| +This directive’s <a>pre-request check</a> is as follows: | ||
| + | ||
| +Given a <a>request</a> (|request|) and a <a>policy</a> (|policy|): | ||
| + | ||
| +1. Let |protected resource types| be the result of executing | ||
| + [[#parse-require-sri-for]] on this <a>directive</a>'s <a lt="directive value">value</a>. | ||
| + | ||
| +2. If |request|'s <a>destination</a> is a <a>ASCII case-insensitive match</a> for at least | ||
| + one token in |protected resource types|, and |request|'s integrity metadata | ||
| + is the empty string: | ||
| + | ||
| + 1. Let |violation| be the result of executing <a lt="create a violation object for global"> |
|
mikewest
Looking at this again, I don't think you need to do the reporting here. It ought to be taken care of by https://w3c.github.io/webappsec-csp/#should-block-request (which eventually calls into the
shekyan
Eh, I agree, that algorithm covers the reporting. We'll need to remove it for base-uri as well then. Will wait for @mozfreddyb input before removing this.
mikewest
|
shekyan
and 1 other
commented on an outdated diff
| + splitting |token list| on spaces</a> if token matches the grammar | ||
| + for <a>require-sri-for</a>, add |token| to |protected resource types| | ||
| + if |token| is a <a>known token</a>. Otherwise, ignore the token. | ||
| + | ||
| +3. Return the set of |protected resource types|. | ||
| + | ||
| +### Apply |algorithm| to |request| ### {#apply-algorithm-to-request} | ||
| + | ||
| +This directive’s <a>pre-request check</a> is as follows: | ||
| + | ||
| +Given a <a>request</a> (|request|) and a <a>policy</a> (|policy|): | ||
| + | ||
| +1. Let |protected resource types| be the result of executing | ||
| + [[#parse-require-sri-for]] on this <a>directive</a>'s <a lt="directive value">value</a>. | ||
| + | ||
| +2. If |request|'s <a>destination</a> is a <a>ASCII case-insensitive match</a> for at least |
|
shekyan
@mikewest, @mozfreddyb, I realized that destination "script" or "style" also covers
mozfreddyb
We can't easily add Best find a way to leave it out for now :-/ |
Folks, there is currently a problem in @mozfreddyb's implementation around resources loaded from CORS-disabled servers. It needs to be reflected in the spec, so I'll start the discussion here:
require-sri-for matching algorithm should allow requests that are missing both integrity metadata and CORS setting attribute, Otherwise, require-sri-for usage is limited to pages that load resources only from CORS-enabled servers.
Example: Content-Security-Policy: require-sri-for script implies that all script elements should have integrity metadata present, but it is not possible to have integrity metadata on resources that are served by CORS-disabled servers, like https://www.google-analytics.com/analytics.js.
What do you think?
P.S. Is there a way to load a resource anonymously from CORS-unaware servers without violating SOP?
Do not see why crossorigin='anonymous' requires server to be CORS-aware.
My understanding is that this only affects scripts/styles inserted through HTML elements. (because that's what SRI is defined for right now). If so, should the tokens be named script-element rather than just script? e.g., what if a service worker is served with a CSP policy of require-sri-for? How will that work?
I think it's substantially cleaner to reuse the terms from Fetch (https://fetch.spec.whatwg.org/#concept-request-destination) rather than inventing new subsets. Doing so ensures that you'll actually have the detail necessary to block requests. Note, for instance, that Fetch doesn't have any information that would allow it to distinguish <script src='whatever'> and importScripts(whatever).
The argument that SRI doesn't yet support importScripts seems like a poor reason for introducing more complexity to script loading. It instead seems like a good reason to extend SRI to support more resource types, which I think y'all are planning to do anyway. :)
They'd break right away in the presence of this directive, since we'd be blocking the requests. They'd start working once we support those mechanisms. Sounds like a good reason to start widening our support. :)
require-sri-for matching algorithm should allow requests that are missing both integrity metadata and CORS setting attribute, Otherwise, require-sri-for usage is limited to pages that load resources only from CORS-enabled servers.
I thought that's the intention behind all this? That nobody can use scripts without using integrity (and thus requesting from a CORS-enabled server or a same-origin server).
To answer the other question: require-sri-for should work in <meta> CSP policies. Firefox Nightly does not this and that's a bug
I thought that's the intention behind all this? That nobody can use scripts without using integrity (and thus requesting from a CORS-enabled server or a same-origin server).
Yes, but if SRI does not provide a way to verify integrity of a resource from CORS-disabled server, then that resource should not be subject to require-sri-for, no?
Again, that sounds to me like an argument that SRI should be extended to support resource types it doesn't yet support, not that the current model is a bad one.
Yes, but if SRI does not provide a way to verify integrity of a resource from CORS-disabled server, then that resource should not be subject to require-sri-for, no?
I strongly disagree. If there's no way to verify integrity, then there's no way that resource is safe to use. The keyword quite literally says require integrity ;)
I strongly disagree. If there's no way to verify integrity, then there's no way that resource is safe to use. The keyword quite literally says require integrity ;)
Ultimately, I agree with you, e.g. SRI should be able to verify the integrity of all <script> elements, except if they should not be verified for security reasons.
Practically, require-sri-for would have very limited usage today, because, for example, Google Analytics serves it's code without Access-Control-Allow-Origin, so no way to load the resource anonymously, thus enable SRI on it.
I don't know if anything can be done on SRI land to fix this.
I'll try to tackle CORS spec authors to understand why it is not possible to send anonymous requests to CORS-disabled servers.
Oh man. I totally missed @mikewest comment. I am ok with that and this PR seems to be ready to go then if there is no other objections!
@devd, added a note.
1 check passed
thanks! this looks good. merging since everyone else ok'ed it.
Would y'all mind publishing this document somewhere? It's tough to skim through sections when reviewing patches, as I end up needing to come back to this PR rather than reading a nicely formatted doc. :)
done at https://w3c.github.io/webappsec-subresource-integrity/index.bikeshed.html
Since v1 is in the final stages of becoming a rec, I din't want to touch index.html. Once it is done, I will update index.html too.
Just now saw this was merged. Thanks so much for the effort
FYI, I started a discussion on the mailing list. It is about APIs that do not know about integrity metadata (e.g. CSS @import and importScript() and the new Worker constructor in JS) and whether they should be blocked by require-sri-for.
Please read the thread and send your feedback!
https://lists.w3.org/Archives/Public/public-webappsec/2016Sep/0027.html
Background: w3c/webappsec-csp#85.