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

Content security policy hashes problem #220

Closed
frederikhors opened this issue Jan 8, 2023 · 4 comments · Fixed by #221
Closed

Content security policy hashes problem #220

frederikhors opened this issue Jan 8, 2023 · 4 comments · Fixed by #221

Comments

@frederikhors
Copy link

I'm using Content Security Policy to increase security and Lighthouse score.

I'm using many frameworks which generate html files like:

<!DOCTYPE html>
<html lang="en">
<head>
  <!-- ... -->
  <meta http-equiv="content-security-policy" content="default-src domain.com; img-src *; script-src domain.com 'sha256-fXiuOGTc9r/EgsIAcDthKaYGYUT3svq8f4NzsIwFHmE=' 'sha256-Vn0EF4j1eAcb5cTHq3jjTvL32M3ijgVackLAvIuZg9Q='">

  <link href="/some.js">

  <script>/*SOME MINIFIED SCRIPT HERE for first sha256 hash*/</script>
</head>

<body>
  <script type="module">/*OTHER MINIFIED SCRIPT HERE for second sha256 hash*/</script>
</body>

</html>

When I use htmlnano on it the <script></script> content obviously changes (is minified too along with html) and I get CSP errors like:

Refused to execute inline script because it violates the following Content Security Policy directive: "script-src domain.com 'sha256-fXiuOGTc9r/EgsIAcDthKaYGYUT3svq8f4NzsIwFHmE=' 'sha256-Vn0EF4j1eAcb5cTHq3jjTvL32M3ijgVackLAvIuZg9Q='". Either the 'unsafe-inline' keyword, a hash ('sha256-tcp8uK2Psib07uQY3U6+vLDwwqjQkUhn8M7KfM61Fg4='), or a nonce ('nonce-...') is required to enable inline execution.

because no generated (before htmlnano launch) hash matches.

I think htmlnano should re-calculate and change those sha256 values.

What do you think?
@SukkaW
Copy link
Contributor

SukkaW commented Jan 8, 2023

IMHO the current workaround is to disable htmlnano's built-in minifyJs feature to prevent manipulating <script /> tags.

Calculating new SRI in htmlnano is kinda risky anyway.

@frederikhors
Copy link
Author

Why risky?

@SukkaW
Copy link
Contributor

SukkaW commented Jan 8, 2023 •
edited

Why risky?

@frederikhors

If the <script /> has an SRI attribute, it means that the original <script /> could be trusted, and should not be altered anymore. htmlnano is exactly a MITM that SRI is designed to protect from.

Although htmlnano is trying its best not to be compromised. But what happens if the upstream package is compromised? What happens if the minifyJs feature introduces malicious code and calculates an SRI based on the malicious code?

So IMHO, if you do trust htmlnano, you should generate SRI after htmlnano modify the <script />. Otherwise, you should disable the minifyJs feature for now.

I will create a PR to let htmlnano skips <style /> and <script /> that have SRI attribute, so you can still minify <script /> that doesn't have SRI.

@maltsev
Copy link
Member

maltsev commented Jan 8, 2023

Good points! I agree with @SukkaW.

SukkaW added a commit to SukkaW/htmlnano that referenced this issue Jan 8, 2023
@SukkaW
fix(posthtml#220): should not minify <script> and <style> with SRI
5478092
SukkaW added a commit to SukkaW/htmlnano that referenced this issue Jan 8, 2023
@SukkaW
fix(posthtml#220): should not merge <style> or <script> with SRI
9681eaa
SukkaW added a commit to SukkaW/htmlnano that referenced this issue Jan 8, 2023
@SukkaW
fix(posthtml#220): should not minify json with SRI
447079d
SukkaW added a commit to SukkaW/htmlnano that referenced this issue Jan 8, 2023
@SukkaW
chore(posthtml#220): add comments, documenting why SRI should be skipped
e795aa2
@SukkaW SukkaW mentioned this issue Jan 8, 2023
Merged
maltsev added a commit that referenced this issue Jan 8, 2023
@maltsev
Merge pull request #221 from SukkaW/fix-220
688cf37 
fix(#220): skip <script> and <style> with SRI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(#220): skip <script> and <style> with SRI SukkaW/htmlnano
3 participants
@maltsev @SukkaW @frederikhors