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

Fix 36168: document secure upgrades for source expressions #36198

Closed
wants to merge 5 commits into from

Conversation

wbamberg
Copy link
Collaborator

@wbamberg wbamberg commented Oct 5, 2024

Fixes #36168, and partly addresses #35947 (review).

This PR adds a gentler intro to the sources page, adding context and an example, to help address part of #35947 (review)..

It also describes the upgrade/don't trap people in HTTP changes added in w3c/webappsec-csp@0e81d81, that were partly documented before, to fix #36168, and removes the note in the connect-src page that is I think out of date .

I also moved the note about protocol upgrades that was added to scheme-source in #21015 - I believe this was a mistake and the correct fix was to add it to host-source, as #3899 tried to do.

But as usual I'd welcome careful review of all this!

@github-actions github-actions bot added Content:HTTP HTTP docs size/s [PR only] 6-50 LoC changed labels Oct 5, 2024
Copy link
Contributor

github-actions bot commented Oct 5, 2024

@wbamberg wbamberg marked this pull request as ready for review October 5, 2024 02:58
@wbamberg wbamberg requested a review from a team as a code owner October 5, 2024 02:58
@wbamberg wbamberg requested review from hamishwillee and removed request for a team October 5, 2024 02:58
Comment on lines -21 to -22
> **Note:** `connect-src 'self'` does not resolve to websocket
> schemes in all browsers, more info in this [issue](https://github.com/w3c/webappsec-csp/issues/7).
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Removing this makes sense - even if it were still correct I don't think it really gives a helpful or sufficient flavour for the problem/solution.
  2. I think your interpretation of what this was really about is correct (i.e. transparent upgrades of protocol levels). That is already captured in the sources docs, so we don't need more detail here.
  3. But this might require a BCD entry.

Comment on lines +20 to +21
- `'self'` means the resource can be loaded from the same origin as the document.
- `example.org` means the resource can be loaded from the `example.org` domain.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a specific resource type in the example, so we should possibly indicate "images". I only comment because when I read this as "the resource" it irritated me that this is actually restriction "all images", not just a singular.

Suggested change
- `'self'` means the resource can be loaded from the same origin as the document.
- `example.org` means the resource can be loaded from the `example.org` domain.
- `'self'` means that the resource type, in this case images, can be loaded from the same origin as the document.
- `example.org` means that the resource type can be loaded from the `example.org` domain.

For example, the following fetch directive, [`img-src`](/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/img-src), contains two source expressions, `'self'` and `example.org`:

```plain
img-src 'self' example.org
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can/should we make this the full header and directive with the HTTP header (and http for the block name)? At least here in the very first example?

Suggested change
img-src 'self' example.org
Content-Security-Policty: img-src 'self' example.org

You can also specify data schemes (not recommended).
- : A scheme such as `http:` or `https:`. The colon is required.

Secure upgrades are allowed, so `http:` will also match resources loaded using HTTPS, and `ws:` will also match resources loaded using WSS.

- `data:` Allows [`data:` URLs](/en-US/docs/Web/URI/Schemes/data) to be used as a content source.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should ws be in this list?

- : Refers to the origin from which the protected document is being served, including the same URL scheme and port number.
You must include the single quotes. Some browsers specifically exclude `blob` and `filesystem` from source directives.
Sites needing to allow these content types can specify them using the Data attribute.

Note that `https:` and `wss:` schemes are automatically matched even if the document's origin does not match that scheme, so for example:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is good. I assume the reverse is not true - i.e. wss does not match ws? Should we say that?

@wbamberg
Copy link
Collaborator Author

Closing in favour of #36792.

@wbamberg wbamberg closed this Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:HTTP HTTP docs size/s [PR only] 6-50 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSP: Document http->https, and wss being allowed in 'self'
2 participants