Read TAG_FINAL from libsodium, detect partial chunks, cover the atomic write
All checks were successful
check / check (push) Successful in 22s

Rework against review findings on the branch.

Test runtime (B1): the suite's cost was never the 4 MiB fixture, it was
filling that fixture from the CSPRNG. sodium.randombytes_buf goes through
the wasm wrapper a byte at a time and takes ~20s for 4 MiB, against ~108ms
to encrypt the same buffer. Fixture content is not load-bearing anywhere in
the file — only length and tag are — so payloads now come from a seeded LCG
instead, which also makes them deterministic and reproducible as the README
asks. A seeded generator rather than a constant fill, so a downloader that
reordered or repeated chunks would still be caught. make test goes from
over the 30s cap in script/test (it was failing outright, then rerunning
verbose) back to 8.96s, against 10.02s on main.

Atomic write coverage (B2): every failure the suite injected originated in
streamDecrypt, which runs before anything is written, so no test observed a
temp file existing or being cleaned up and the catch block in writeAtomic
was dead code. rename is now intercepted in the test file, which adds two
cases per entry point: one asserting the staged file exists at rename time
and is a sibling of the destination, and one failing the rename itself so
the cleanup path runs with a temp file genuinely on disk. Deleting
writeAtomic in favour of a plain writeFile now turns the suite red.

TAG_FINAL (M1): STREAM_TAG_FINAL was a hardcoded 3 plus a test to detect
drift. The premise was right — libsodium attaches its constants inside
ready.then(...), so an eager module-level read binds undefined — but a lazy
read works, and decryptBlob was doing exactly that before. Replaced with
streamTagFinal(), which reads the library's own value at call time. The
drift test is repurposed to pin the accessor against the tag observed on a
real final chunk, which fails if it is ever made eager again.

Partial trailing chunk (M2): a transfer that stopped mid-chunk surfaced as
"authentication failed", which reads as corruption and sends the user after
the wrong problem. A final chunk that arrived in full always authenticates,
so trailing bytes that do not are reported as the truncation they almost
always are, with the authentication failure kept as the error's cause.
Poly1305 cannot separate a partial chunk from a corrupt one, so the message
names both possibilities; a corrupt whole chunk mid-stream is still
reported as an authentication failure, and both are now tested.

The pre-existing test that wrote its output to the process working
directory, i.e. the repo root, now writes into the test's temp directory.
This commit is contained in:
2026-08-09 02:23:39 +00:00
parent 99905277a3
commit 8a200be8a7
5 changed files with 313 additions and 57 deletions

View File

@@ -14,6 +14,6 @@ export {
pullStreamChunk,
STREAM_CHUNK_OVERHEAD,
STREAM_CHUNK_SIZE,
STREAM_TAG_FINAL,
streamTagFinal,
type StreamPullState,
} from "./stream.js";

View File

@@ -8,17 +8,17 @@ export const STREAM_CHUNK_SIZE = 4 * 1024 * 1024;
// 16 bytes of Poly1305 tag plus 1 byte of secretstream tag.
export const STREAM_CHUNK_OVERHEAD = 17;
// The secretstream tag that marks the last chunk of a stream, i.e.
// libsodium's crypto_secretstream_xchacha20poly1305_TAG_FINAL. Exported so
// callers (the download layer) can detect truncation without importing
// sodium themselves.
// libsodium's crypto_secretstream_xchacha20poly1305_TAG_FINAL: the tag that
// marks the last chunk of a stream. Exported so callers (the download layer)
// can detect truncation without importing sodium themselves.
//
// Written as a literal because libsodium only attaches its constants to the
// module object once `sodium.ready` has resolved, which is long after this
// module is evaluated. The value is fixed at 3 by the secretstream wire
// format; test/crypto/stream.test.ts pins it against libsodium's own
// constant so the two cannot drift apart unnoticed.
export const STREAM_TAG_FINAL = 3;
// This is a function rather than a constant because libsodium attaches its
// constants to the module object only once `sodium.ready` has resolved, which
// is long after this module is evaluated — a module-level read would bind
// `undefined`. Reading it at call time returns the library's own value, so
// there is no second copy of a protocol constant to keep in sync.
export const streamTagFinal = (): number =>
sodium.crypto_secretstream_xchacha20poly1305_TAG_FINAL;
// Encrypt a small blob as a single secretstream chunk with TAG_FINAL.
// Returns the header and ciphertext. Used for encrypting thumbnails
@@ -59,8 +59,11 @@ export const decryptBlob = (
): Uint8Array => {
const state = initStreamPull(header, key);
const { plaintext, tag } = pullStreamChunk(state, ciphertext);
if (tag !== STREAM_TAG_FINAL) {
throw new Error(`decryptBlob: expected TAG_FINAL (3), got tag ${tag}`);
const tagFinal = streamTagFinal();
if (tag !== tagFinal) {
throw new Error(
`decryptBlob: expected TAG_FINAL (${tagFinal}), got tag ${tag}`,
);
}
return plaintext;
};

View File

@@ -7,7 +7,7 @@ import {
pullStreamChunk,
STREAM_CHUNK_OVERHEAD,
STREAM_CHUNK_SIZE,
STREAM_TAG_FINAL,
streamTagFinal,
} from "../crypto/index.js";
import type { ApiClient } from "../api/client.js";
import type { EnteFile } from "../model/types.js";
@@ -53,11 +53,27 @@ const streamDecrypt = async (
if (done) {
if (buffer.length > 0) {
const { plaintext, tag } = pullStreamChunk(state, buffer);
plainChunks.push(plaintext);
totalPlain += plaintext.length;
// Whatever is left over once every whole chunk has been
// consumed must be the stream's final chunk, and a final
// chunk that actually arrived in full authenticates. If it
// does not, the body stopped part-way through a chunk — the
// ordinary shape of a dropped connection. Poly1305 cannot
// tell a partial chunk from a corrupt one, so this is
// reported as the truncation it almost always is, with the
// authentication failure kept as the error's cause.
let pulled;
try {
pulled = pullStreamChunk(state, buffer);
} catch (err) {
throw new Error(
`download: stream truncated: response body ended with ${buffer.length} trailing bytes that did not authenticate as a final chunk (transfer stopped mid-chunk, or the data is corrupt)`,
{ cause: err },
);
}
plainChunks.push(pulled.plaintext);
totalPlain += pulled.plaintext.length;
chunksPulled++;
lastTag = tag;
lastTag = pulled.tag;
}
break;
}
@@ -73,9 +89,10 @@ const streamDecrypt = async (
"download: stream truncated: response body contained no secretstream chunks",
);
}
if (lastTag !== STREAM_TAG_FINAL) {
const tagFinal = streamTagFinal();
if (lastTag !== tagFinal) {
throw new Error(
`download: stream truncated: last chunk tag ${lastTag}, expected TAG_FINAL (${STREAM_TAG_FINAL})`,
`download: stream truncated: last chunk tag ${lastTag}, expected TAG_FINAL (${tagFinal})`,
);
}