Skip to content

fix: Use Web Crypto compatible function for nonce generation - v1 #817

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
Merged
merged 1 commit into from
Feb 13, 2025
Merged

fix: Use Web Crypto compatible function for nonce generation - v1 #817

merged 1 commit into from
Feb 13, 2025

Conversation

andreituicu
Copy link
Collaborator

@andreituicu andreituicu commented Feb 12, 2025 •
edited
Loading

Currently trying to serve a page with nonce CSP results in 500 in cloudflare:
Worker: x-error: crypto_worker_default2.randomBytes is not a function

curl ${CLOUDFLARE_ENDPOINT}/preview/adobe/helix-labs-website/csp-nonce/

< HTTP/2 200 
< date: Thu, 13 Feb 2025 00:28:48 GMT
< content-type: text/html; charset=utf-8
< content-length: 12006
< last-modified: Thu, 13 Feb 2025 00:16:47 GMT
< content-security-policy: script-src 'nonce-MEp9pOJCGw6/rtKRGuKzuLKU' 'strict-dynamic'; base-uri 'self'; object-src 'none';
...
<!DOCTYPE html>
<html>
  <head>
    <title>Home | Admin Labs</title>
    ...
    <script nonce="MEp9pOJCGw6/rtKRGuKzuLKU" src="/scripts/aem.js" type="module"></script>
    <script nonce="MEp9pOJCGw6/rtKRGuKzuLKU" src="/scripts/scripts.js" type="module"></script>
    <link rel="stylesheet" href="/styles/styles.css">
  </head>
  <body>
  </body>
</html>

@codecov Codecov
Copy link

codecov bot commented Feb 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (3f4895f) to head (fcc73ca).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff            @@
##              main      #817   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           46        46           
  Lines         3914      3916    +2     
=========================================
+ Hits          3914      3916    +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@andreituicu andreituicu changed the title fix: Use Web Crypto compatible function for nonce generation fix: Use Web Crypto compatible function for nonce generation - v1 Feb 12, 2025
tripodsan
tripodsan reviewed Feb 13, 2025
View reviewed changes
src/steps/csp.js
Comment on lines +62 to +64
const array = new Uint8Array(18);
cryptoImpl.getRandomValues(array);
return btoa(String.fromCharCode(...array));
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this better than the uuid approach in v2

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@tripodsan I agree, but it has the esmock problem in the tests... that's the only reason I implemented v2.
The problem is that getRandomValues cannot be overriden in the module as simple as randomUUID, or other functions.

Other functions are simple module exports, while getRandomValues is done with:

ObjectDefineProperties(module.exports, {
  getRandomValues: {
    __proto__: null,
    configurable: false,
    enumerable: true,
    get: () => getRandomValues,
    set: undefined,
  },
});

Which means that when I try to do cryptoImpl.getRandomValues = () => mock, I get:

TypeError: Cannot set property getRandomValues of #<Object> which has only a getter

So I couldn't find another way except for using esmock, which makes the test setup quite complex.

@andreituicu andreituicu mentioned this pull request Feb 13, 2025
Closed
@andreituicu andreituicu merged commit cf654f8 into main Feb 13, 2025
9 checks passed
@andreituicu andreituicu deleted the fix-nonce branch February 13, 2025 18:43
github-actions bot pushed a commit that referenced this pull request Feb 13, 2025
@semantic-release-bot
chore(release): 6.20.1 [skip ci]
bb067b9 
## [6.20.1](v6.20.0...v6.20.1) (2025-02-13)

### Bug Fixes

* Use Web Crypto compatible function for nonce generation ([#817](#817)) ([cf654f8](cf654f8))
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version 6.20.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tripodsan tripodsan tripodsan approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andreituicu @tripodsan