diff --git a/CHANGELOG.md b/CHANGELOG.md index ad8879c..312ac8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## Unreleased + +- In Cloudflare Workers and browsers (the WebCrypto entry), a refused `eventSigner` `sign` call, for example one with a `now` of `NaN` or a `null` message, no longer makes every later call on that signer reject. It now leaves the signer where it was, as the Node, Bun and Deno entries do. +- On the same entry, `chunk(data)` and `sign(message)` now read the buffer when they are called, not when their turn in the queue comes. A caller that reused its buffer straight after the call got a signature for the new bytes; it now gets the one the other entries give. + ## 1.0.4 - 2026-11-01 Documentation only: the code is unchanged since 1.0.2 (`src/` is the same tree and the built files are byte-identical). diff --git a/docs/guide.md b/docs/guide.md index d00fdeb..6a697de 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -124,6 +124,8 @@ A request that cannot be signed correctly throws instead of returning a signatur `chunk(data)` with an empty `data` returns it unchanged (zero bytes) and signs nothing, because a zero-length chunk ends the upload; call `end()` for that. Trailer names are lowercased before they are signed. +A call that is refused (a trailer with a line break, a `now` that is not a time, a message that is not a `Uint8Array`) throws, or on the promise-returning entry rejects, and leaves the signer where it was: the next call is signed with the signature before the refused one. The bytes you pass are read when you call, so you can reuse the buffer straight away. + ### How it reads the URL - **Paths.** For S3, percent-escapes are decoded and every byte except `A-Z a-z 0-9 - . _ ~ /` is encoded again, once. A raw `+` in the path is signed as a space (`%20`). That is this library's reading of how S3 treats it: the test corpus follows the same rule, botocore does not check it independently, and it has not been tried against AWS S3. Cloudflare R2 agrees: it accepted a raw `+` signed as `%20` and answered `SignatureDoesNotMatch` to the same URL signed as `%2B` (four read-only `GET` requests to a bucket that no longer exists, so each ended in an error after the signature check; which object a raw `+` then names was not tested). A local `s3s-fs` 0.17.0 server reads a raw `+` as a plus sign and rejected sigflint's signature for it. To mean a literal plus, write `%2B`: it is signed as a plus, and that server accepted it from sigflint and from botocore. `%20` is a space. An escape that is not valid (`%zz`, a lone `%`) is signed as a literal `%`. For every other service the path is normalised (empty segments dropped, `.` and `..` resolved) and encoded a second time, as the specification says. diff --git a/src/web.js b/src/web.js index 785ec22..405e5fb 100644 --- a/src/web.js +++ b/src/web.js @@ -273,6 +273,28 @@ export async function presignV4a(request, credentials) { */ const signWith = async (key, text) => toHex(await subtle.sign('HMAC', key, utf8.encode(text))); +/** + * A copy of the bytes as they are now. TypedArray's slice copies even for a Node Buffer, whose own slice returns a view, and it throws for + * anything that is not a typed array, as hashing it would. + * @param {Uint8Array} data + */ +const copyOf = (data) => Uint8Array.prototype.slice.call(data); + +/** + * Runs the signing steps of a stream one after the other. A step that throws rejects its own call only: the next one still runs, as on the + * synchronous entries, where a refused call throws and the signer stays usable. A step moves the chain only once it has succeeded. + * @returns {(step: () => Promise) => Promise} + */ +function inOrder() { + /** @type {Promise} */ + let turn = Promise.resolve(); + return (step) => { + const result = turn.then(step); + turn = result.catch(() => {}); + return result; + }; +} + /** * Signs an aws-chunked upload chunk by chunk; the same shape as the other entries, with promises. * @param {import('./base.js').AuthHeaders} authHeaders - the headers getAuthHeaders returned for the request @@ -289,19 +311,24 @@ export function chunkSigner(authHeaders, credentials) { seed.service, ); // chunks are signed one after the other: a call waits for the one before it - /** @type {Promise} */ - let turn = Promise.resolve(); + const inTurn = inOrder(); /** @param {Uint8Array} data */ const sign = (data) => - (turn = turn.then( + inTurn( async () => (previous = await signWith( await keyPromise, chunkStringToSign(seed.datetime, seed.scope, previous, await hexHash(data)), )), - )); + ); return { - chunk: async (data) => (data.length === 0 ? data : frameChunk(await sign(data), data)), + async chunk(data) { + if (data.length === 0) { + return data; + } + const own = copyOf(data); // the signature and the frame are made when the chunk's turn comes; the caller may reuse its buffer once this returns + return frameChunk(await sign(own), own); + }, async end(trailers) { const text = trailers === undefined ? undefined : canonicalTrailers(trailers); // before sign(), so a refused trailer leaves the chain where it was const signature = await sign(new Uint8Array(0)); @@ -329,24 +356,19 @@ export function chunkSigner(authHeaders, credentials) { export function eventSigner(authHeaders, credentials) { const seed = seedOf(authHeaders); let previous = seed.signature; - /** @type {Promise} */ - let turn = Promise.resolve(); + const inTurn = inOrder(); return { - sign: (message, now = Date.now()) => - (turn = turn.then(async () => { + async sign(message, now = Date.now()) { + const own = copyOf(message); // the caller may reuse its buffer once this returns, as on the synchronous entries + return inTurn(async () => { const { datetime, scope, date } = eventScope(now, seed.region, seed.service); const header = dateHeader(now); previous = await signWith( await getSigningKey(credentials.secretAccessKey, date, seed.region, seed.service), - eventStringToSign( - datetime, - scope, - previous, - await hexHash(header), - await hexHash(message), - ), + eventStringToSign(datetime, scope, previous, await hexHash(header), await hexHash(own)), ); - return frameEvent(header, previous, message); - })), + return frameEvent(header, previous, own); + }); + }, }; } diff --git a/test/hardening.test.js b/test/hardening.test.js index 49e7579..71c90ea 100644 --- a/test/hardening.test.js +++ b/test/hardening.test.js @@ -186,6 +186,62 @@ for (const [ assert.deepEqual(await signer.chunk(new Uint8Array([1, 2, 3])), reference); }); + test(`${name}: a refused chunk leaves the chunk signer usable and the chain where it was`, async () => { + const part = new Uint8Array([1, 2, 3]); + const fresh = await chunkSigner(await chunkedHeaders(), credentials).chunk(part); + const signer = chunkSigner(await chunkedHeaders(), credentials); + const refused = /** @type {Uint8Array} */ (/** @type {unknown} */ ({ length: 3 })); + await rejects(() => signer.chunk(refused), TypeError, /./); + assert.deepEqual(await signer.chunk(part), fresh); + // a call made straight after a refused one, without waiting for it, is not refused with it + const second = chunkSigner(await chunkedHeaders(), credentials); + const [first, next] = await Promise.allSettled([ + Promise.resolve().then(() => second.chunk(refused)), + Promise.resolve().then(() => second.chunk(part)), + ]); + assert.equal(first.status, 'rejected'); + assert.deepEqual(next.status === 'fulfilled' && next.value, fresh); + }); + + test(`${name}: a refused event leaves the event signer usable and the chain where it was`, async () => { + const message = new Uint8Array([1, 2, 3]); + const seed = () => getAuthHeaders({ ...request, service: 'transcribe' }, credentials); + const fresh = await eventSigner(await seed(), credentials).sign(message, now); + for (const [refusedMessage, at] of /** @type {[Uint8Array, number][]} */ ([ + [message, NaN], + [/** @type {Uint8Array} */ (/** @type {unknown} */ (null)), now], + ])) { + const signer = eventSigner(await seed(), credentials); + await rejects(() => signer.sign(refusedMessage, at), Error, /./); + assert.deepEqual(await signer.sign(message, now), fresh); + } + // a refused event takes its turn in the queue: the call made straight after it, without waiting, is not refused with it + const signer = eventSigner(await seed(), credentials); + const [first, next] = await Promise.allSettled([ + Promise.resolve().then(() => signer.sign(message, NaN)), + Promise.resolve().then(() => signer.sign(message, now)), + ]); + assert.equal(first.status, 'rejected'); + assert.deepEqual(next.status === 'fulfilled' && next.value, fresh); + }); + + test(`${name}: a chunk is signed as the caller passed it, even if the buffer is reused straight away`, async () => { + const buffer = new Uint8Array([1, 2, 3]); + const expected = await chunkSigner(await chunkedHeaders(), credentials).chunk(buffer.slice()); + const pending = chunkSigner(await chunkedHeaders(), credentials).chunk(buffer); + buffer.fill(9); + assert.deepEqual(await pending, expected); + }); + + test(`${name}: an event is signed as the caller passed it, even if the buffer is reused straight away`, async () => { + const buffer = new Uint8Array([1, 2, 3]); + const seed = () => getAuthHeaders({ ...request, service: 'transcribe' }, credentials); + const expected = await eventSigner(await seed(), credentials).sign(buffer.slice(), now); + const pending = eventSigner(await seed(), credentials).sign(buffer, now); + buffer.fill(9); + assert.deepEqual(await pending, expected); + }); + test(`${name}: an event signed at a fractional millisecond time is the same as at the whole millisecond`, async () => { const message = new Uint8Array([1, 2, 3]); const a = await eventSigner(