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.

Sign up for GitHub

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

Jump to bottom

Add nonce to feature detection iframe #290

Merged
merged 1 commit into from May 11, 2022
Merged

Add nonce to feature detection iframe #290

merged 1 commit into from May 11, 2022

Conversation

iainbeeston
Copy link
Contributor

es-module-shims creates an iframe to do feature
detection. However, if a site is using CSP then
this iframe is blocked unless the CSP config allows
frame-src to use blobs, which prevents es-module-shims
from running. In chrome that error looks like:

Refused to frame 'blob:http://localhost:3000/477a88e6-fb4c-48fd-a677-2ba7fb69fc45'
because it violates the following Content Security Policy directive:
"default-src 'self' https:". Note that 'frame-src' was not explicitly set,
so 'default-src' is used as a fallback.

In safari it looks like:

Refused to load blob:https://cors-test.herokuapp.com/12cecc46-329e-4791-a73b-4824852d1ad1
because it appears in neither the child-src directive nor the default-src
directive of the Content Security Policy.

To prevent this, I've updated feature.js to set a nonce
value on the iframe, and this fixes the error.

@iainbeeston
Add nonce to feature detection iframe
1c41224 
es-module-shims creates an iframe to do feature
detection. However, if a site is using CSP then
this iframe is blocked unless the CSP config allows
frame-src to use blobs, which prevents es-module-shims
from running. In chrome that error looks like:

    Refused to frame 'blob:http://localhost:3000/477a88e6-fb4c-48fd-a677-2ba7fb69fc45'
    because it violates the following Content Security Policy directive:
    "default-src 'self' https:". Note that 'frame-src' was not explicitly set,
    so 'default-src' is used as a fallback.

In safari it looks like:

    Refused to load blob:https://cors-test.herokuapp.com/12cecc46-329e-4791-a73b-4824852d1ad1
    because it appears in neither the child-src directive nor the default-src
    directive of the Content Security Policy.

To prevent this, I've updated `feature.js` to set a nonce
value on the iframe, and this fixes the error.
@iainbeeston
Copy link
Contributor Author

iainbeeston commented May 11, 2022 •
edited

This PR is part of my attempt to fix rails/rails#45057 - there are more details there and you can see the issue in action (with v1.3.4 of es-module-shims) here

@guybedford
Copy link
Owner

Thanks for the PR, happy to merge. Any ideas why the existing CSP tests aren't catching this?

@guybedford guybedford merged commit 256435d into guybedford:main May 11, 2022
@iainbeeston
Copy link
Contributor Author

@guybedford I've investigated why the csp tests weren't catching this and I've discovered a few things:

  1. I'm using v1.3.4 which has this bug and adding the nonce fixes it
  2. The reason the tests don't pick this up is because there is no default-src rule defined in the csp <meta> tag and so blobs are allowed with any source (I'd recommend adding default-src: 'none' or maybe default-src: 'self' as that will detect this kind of error in future)
  3. This fix is actually unnecessary in the latest version of es-module-shims because it no longer uses a blob for the <iframe> source created in features.js, and so the script-src rule defined in the <meta> works for the <script> tag in the <iframe>

Based on that I'd suggest reverting this PR but adding default-src: 'none' to test-css.html. What do you think?

@iainbeeston
Copy link
Contributor Author

The change from blob to script was in #265, which means the original issue should have been fixed from v1.4.7 onwards

iainbeeston added a commit to iainbeeston/es-module-shims that referenced this pull request May 16, 2022
@iainbeeston
Set default csp policy
2ecb9c8 
Previously there was no default set so
any files could be loaded from anywhere
(except for scripts which were allow-listed).

This changes the csp so that by default
only localhost can be used. This should
detect issues like guybedford#290.
iainbeeston added a commit to iainbeeston/es-module-shims that referenced this pull request May 16, 2022
@iainbeeston
Set default csp policy
79e2922 
Previously there was no default set so
any files could be loaded from anywhere
(except for scripts which were allow-listed).

This changes the csp so that by default
only localhost can be used. This should
detect issues like guybedford#290.
@iainbeeston iainbeeston mentioned this pull request May 16, 2022
Merged
@iainbeeston
Copy link
Contributor Author

I've added #293 to update test-csp.html

iainbeeston added a commit to iainbeeston/importmap-rails that referenced this pull request May 16, 2022
@iainbeeston
Upgraded es-module-shims to v1.5.5
29ea3b5 
The current version of es-module-shims provided
by importmap-rails (v1.4.6) requires allowing
`frame-src: blob:` in an app's CSP policy (it
creates an iframe with a blob as it's source
in order to perform feature detection). This is
quite a permissive thing to allow in csp and
I wouldn't recommend it if there is an alternative.

Since v1.4.7 es-module-shims no longer uses a
blob and it will work with `script-src: 'self'`
instead. I've upgraded to the latest version,
which is v1.5.5. For more details about the
CSP issue and it's fix see:

* guybedford/es-module-shims#265
* guybedford/es-module-shims#290
@iainbeeston iainbeeston mentioned this pull request May 16, 2022
Merged
dhh pushed a commit to rails/importmap-rails that referenced this pull request May 22, 2022
@iainbeeston
Upgraded es-module-shims to v1.5.5 (#128)
9696db2 
The current version of es-module-shims provided
by importmap-rails (v1.4.6) requires allowing
`frame-src: blob:` in an app's CSP policy (it
creates an iframe with a blob as it's source
in order to perform feature detection). This is
quite a permissive thing to allow in csp and
I wouldn't recommend it if there is an alternative.

Since v1.4.7 es-module-shims no longer uses a
blob and it will work with `script-src: 'self'`
instead. I've upgraded to the latest version,
which is v1.5.5. For more details about the
CSP issue and it's fix see:

* guybedford/es-module-shims#265
* guybedford/es-module-shims#290
smartech7 pushed a commit to smartech7/ruby-import-maps that referenced this pull request Aug 4, 2023
@iainbeeston
Upgraded es-module-shims to v1.5.5 (#128)
57963a8 
The current version of es-module-shims provided
by importmap-rails (v1.4.6) requires allowing
`frame-src: blob:` in an app's CSP policy (it
creates an iframe with a blob as it's source
in order to perform feature detection). This is
quite a permissive thing to allow in csp and
I wouldn't recommend it if there is an alternative.

Since v1.4.7 es-module-shims no longer uses a
blob and it will work with `script-src: 'self'`
instead. I've upgraded to the latest version,
which is v1.5.5. For more details about the
CSP issue and it's fix see:

* guybedford/es-module-shims#265
* guybedford/es-module-shims#290
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@iainbeeston @guybedford