crypto: chunk randomFillSync to exceed Web Crypto 64 KiB quota#6762
Open
younggglcy wants to merge 2 commits into
Open
crypto: chunk randomFillSync to exceed Web Crypto 64 KiB quota#6762younggglcy wants to merge 2 commits into
younggglcy wants to merge 2 commits into
Conversation
Node's crypto.randomBytes/randomFill have no per-call size limit other than kMaxPossibleLength (~2 GiB), while crypto.getRandomValues mirrors Web Crypto and is capped at 65536 bytes. The Node compatibility polyfill in randomFillSync routed the whole buffer through crypto.getRandomValues in one call, causing QuotaExceededError for any size > 64 KiB. Fill in 64 KiB chunks so randomBytes/randomFill match Node behavior while leaving the Web Crypto path untouched. Fixes cloudflare#6749
|
All contributors have signed the CLA ✍️ ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
- Move kWebCryptoMaxRandomBytes constant above its sole consumer
randomFillSync for readability (matches the RAND_MAX style at L165).
- Test additions:
* randomBytes(65536) — exact 64 KiB boundary, off-by-one guard
* randomFillSync(new ArrayBuffer(70000)) — exercises the
isAnyArrayBuffer branch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6749.
Summary
crypto.randomBytes(n)(andrandomFill/randomFillSync) undernodejs_compatthrowsQuotaExceededError: getRandomValues() only accepts buffers of size <= 64K but provided <N> bytesfor anyn > 65536. Node.js has no such limit.Root cause
randomFillSyncinsrc/node/internal/crypto_random.tsroutes the entire requested span through a single call tocrypto.getRandomValues(). The C++ implementation atsrc/workerd/api/crypto/crypto.c++(Crypto::getRandomValues) correctly enforces the Web Crypto spec's 64 KiB per-call cap (buffer.size() <= 0x10000). Node'srandomBytes/randomFillare a different API and do not share that limit.Why Node has no 64 KiB cap (upstream evidence)
All references pinned to nodejs/node @
a159b57:The only size cap in the JS layer is
kMaxPossibleLength = min(kMaxLength, 2**31 - 1)— about 2 GiB:lib/internal/crypto/random.js#L70randomBytes's only size check:#L100-L101randomFillSync(no further size cap; wholesizepassed toRandomBytesJob):#L120-L148The C++ binding forwards the entire buffer to
ncrypto::CSPRNG:src/crypto/crypto_random.cc#L68-L74CSPRNGcalls OpenSSLRAND_bytes_ex(OpenSSL ≥ 3) or chunks withRAND_bytesatINT_MAX(~2 GiB) for older OpenSSL — never at 65536:deps/ncrypto/ncrypto.cc#L559-L593So in Node, the 64 KiB cap applies only to
crypto.getRandomValues(which is a separate Web Crypto API surface), not tocrypto.randomBytes/randomFill.Fix
Chunk the fill in
randomFillSyncso eachcrypto.getRandomValues()call is ≤ 65 536 bytes. The cap and behavior ofcrypto.getRandomValues()itself are unchanged.This was preferred over a new C++
cryptoImpl.randomBytesbinding because it (a) requires no schema change, (b) keepsIoContext::getEntropySource()as the single entropy source (matters for workerd's predictable-test mode), and (c) is the same pattern used by Node's own browser polyfills (e.g.randombyteson npm).Test plan
randomBytesAboveWebCryptoQuotaTestinsrc/workerd/api/node/tests/crypto_random-test.jscovers:randomBytes(65537)— the original repro from 🐛crypto.randomByteserrors for values above 65536 #6749randomBytes(200000)— multi-chunk path (3 chunks)randomFillSync(buf, offset=100, size=65500)— chunk-spanning offset + bytes outside the range untouchedrandomFill(buf, cb)— async pathjust stream-test //src/workerd/api/node/tests:crypto_random-test@@all-compat-flagsand@all-autogatesvariants