Extend CSP script-src hashes #784
Conversation
There was a problem hiding this comment.
Hey Carlos, thanks for the PR. I took a quick pass through and have a few questions.
That said, as we move CSP into a more stable maintenance model, we're going to be a little stricter about what we land in the spec to ensure that we have support from implementers, reasonably solid tests, etc. Can you help me understand the current state of the feature from that perspective?
|
|
||
| 3. Return the result of executing the <a>URL serializer</a> on |result|. | ||
|
|
||
| <h3 id="strip-https-url" algorithm>Strip HTTP(S) URL</h3> |
There was a problem hiding this comment.
| <h3 id="strip-https-url" algorithm>Strip HTTP(S) URL</h3> | |
| <h3 id="strip-https-url" algorithm>Strip HTTP(S) URL</h3> | |
| this algorithm returns a {{String}} that represents the relative path of | ||
| |resourceURL| according to |documentURL| if they are same origin. | ||
|
|
||
| 1. If |documentURL|'s <a for="url">scheme</a> is not |
There was a problem hiding this comment.
Nit: Here and elsewhere, it would be ideal to prefer the shorthand notation. I know the doc isn't consistent about it, but it's what we should be aiming for when landing new changes:
| 1. If |documentURL|'s <a for="url">scheme</a> is not | |
| 1. If |documentURL|'s [=url/scheme=] is not |
|
|
||
| 6. Let |documentURLTokens| and |resourceURLPathTokens| be the result of | ||
| <a lt="strictly split a string">strictly splitting</a> |documentURLPath| | ||
| and |resourceURLPath| respectively on the U+002F SOLIDUS character (`/`). |
There was a problem hiding this comment.
I don't understand the point of serializing and then splitting. Could you more simply access the URL's path, which is already either a single segment or a list of segments? I think that dropping the last item off the list (assuming it's non-empty) would address 5, and get you something similar to what you get with the serializing/splitting.
|
|
||
| Given a {{URL}} |resourceURL| and a {{URL}} |documentURL|, | ||
| this algorithm returns a {{String}} that represents the relative path of | ||
| |resourceURL| according to |documentURL| if they are same origin. |
There was a problem hiding this comment.
Can you help me understand the value of the check here? I think the goal is to specify what to hash in a way that's consistent across pages, but I don't understand why, for example, it's better to build a relative path than to hash the absolute path. Is the deployment story impacted one way or the other?
| 15. [=list/iterate|For each=] |index| of [=the exclusive range=] 0 to |resultTokens|' [=list/size=] - 1: | ||
|
|
||
| 1. Append |resultTokens|[|index|] to |result|. | ||
|
|
||
| 2. Append U+002F SOLIDUS character (`/`) to |result|. |
There was a problem hiding this comment.
It might make sense to rely on https://infra.spec.whatwg.org/#string-concatenate to construct |result|, which would let you consolidate 15.* and 16.
| MegaCorp, Inc. now wants to deploy a more strict policy using <a grammar>`url-hash-source`</a>s: | ||
|
|
||
| <pre> | ||
| <a http-header>Content-Security-Policy</a>: <a>script-src</a> https: 'unsafe-inline' 'strict-dynamic-url' 'url-hash-EAaArVRs5qV39C9S3zO0z9ynVoWeZkuNfeMpsVDQnOk=' |
There was a problem hiding this comment.
What is this hash referring to? It's not script.js, /script.js, or https://example.com/script.js, so I feel like I'm missing something.
|
|
||
| 1. If the result of executing [[#match-request-to-source-list]] on | ||
| |request|, |directive|'s <a for="directive">value</a>, and | ||
| |policy|, is "`Does Not Match`", return "`Blocked`". |
There was a problem hiding this comment.
I'm not sure I understands this step. If a url-hash-source expression is present, won't it be checked exhaustively in step 3, returning Allowed if it matches? We only get here when that expression doesn't match, in which case I'm not sure I understand why we ignore the rest of the source list checks. Do these expressions exclude other source list checks (non-URL hashes, for instance)?
|
From Mike's comment up top (#784 (review))
Part of that is that we need to have an issue in this repo, not TAG, where we can discuss and track the proposal before we ever get to landing a PR for it. |
This implements the work described in w3ctag/design-reviews#1128
Preview | Diff