diff --git a/TODO.md b/TODO.md index 9efc22a..f1e961d 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,10 @@ downloads, cover it with mock-server tests, and update the README TODO checkbox. # Completed Steps +- 2026-08-09: Downloads verify the secretstream terminated on `TAG_FINAL` and + write output atomically: a truncated body is rejected instead of landing on + disk as a short file, and plaintext is staged in a sibling temp file and + renamed into place, so a failed download leaves the destination untouched. - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, Makefile shims, README Entrypoints section - 2026-06-10: Decrypted collections shared by other users (sealed-box keys); diff --git a/src/crypto/index.ts b/src/crypto/index.ts index ef129f7..3cd569a 100644 --- a/src/crypto/index.ts +++ b/src/crypto/index.ts @@ -14,5 +14,6 @@ export { pullStreamChunk, STREAM_CHUNK_OVERHEAD, STREAM_CHUNK_SIZE, + STREAM_TAG_FINAL, type StreamPullState, } from "./stream.js"; diff --git a/src/crypto/stream.ts b/src/crypto/stream.ts index f255c59..7799cdd 100644 --- a/src/crypto/stream.ts +++ b/src/crypto/stream.ts @@ -8,6 +8,18 @@ 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. +// +// 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; + // Encrypt a small blob as a single secretstream chunk with TAG_FINAL. // Returns the header and ciphertext. Used for encrypting thumbnails // and metadata before upload. @@ -37,9 +49,6 @@ export const initStreamPull = ( ): StreamPullState => sodium.crypto_secretstream_xchacha20poly1305_init_pull(header, key); -// Decrypt one ciphertext chunk. Returns the plaintext and the secretstream -// tag (0=MESSAGE, 1=PUSH, 2=REKEY, 3=FINAL). The caller should verify the -// stream ended on TAG_FINAL to detect truncation. // Decrypt a small blob that was encrypted as a single secretstream chunk // with TAG_FINAL. Ente uses this form ("blob") for file metadata and // magic metadata — anything under ~1 MiB that isn't chunked. @@ -50,12 +59,15 @@ export const decryptBlob = ( ): Uint8Array => { const state = initStreamPull(header, key); const { plaintext, tag } = pullStreamChunk(state, ciphertext); - if (tag !== sodium.crypto_secretstream_xchacha20poly1305_TAG_FINAL) { + if (tag !== STREAM_TAG_FINAL) { throw new Error(`decryptBlob: expected TAG_FINAL (3), got tag ${tag}`); } return plaintext; }; +// Decrypt one ciphertext chunk. Returns the plaintext and the secretstream +// tag (0=MESSAGE, 1=PUSH, 2=REKEY, 3=FINAL). The caller must verify the +// stream ended on TAG_FINAL to detect truncation. export const pullStreamChunk = ( state: StreamPullState, ciphertext: Uint8Array, diff --git a/src/download/index.ts b/src/download/index.ts index 10610d7..0d6dd30 100644 --- a/src/download/index.ts +++ b/src/download/index.ts @@ -1,10 +1,13 @@ -import { writeFile } from "node:fs/promises"; +import { randomUUID } from "node:crypto"; +import { rename, rm, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; import { fromBase64, initStreamPull, pullStreamChunk, STREAM_CHUNK_OVERHEAD, STREAM_CHUNK_SIZE, + STREAM_TAG_FINAL, } from "../crypto/index.js"; import type { ApiClient } from "../api/client.js"; import type { EnteFile } from "../model/types.js"; @@ -26,6 +29,8 @@ const streamDecrypt = async ( let buffer = new Uint8Array(0); const plainChunks: Uint8Array[] = []; let totalPlain = 0; + let chunksPulled = 0; + let lastTag = -1; for (;;) { const { done, value } = await reader.read(); @@ -39,21 +44,41 @@ const streamDecrypt = async ( while (buffer.length >= ENC_CHUNK_SIZE) { const encChunk = buffer.slice(0, ENC_CHUNK_SIZE); buffer = buffer.slice(ENC_CHUNK_SIZE); - const { plaintext } = pullStreamChunk(state, encChunk); + const { plaintext, tag } = pullStreamChunk(state, encChunk); plainChunks.push(plaintext); totalPlain += plaintext.length; + chunksPulled++; + lastTag = tag; } if (done) { if (buffer.length > 0) { - const { plaintext } = pullStreamChunk(state, buffer); + const { plaintext, tag } = pullStreamChunk(state, buffer); plainChunks.push(plaintext); totalPlain += plaintext.length; + chunksPulled++; + lastTag = tag; } break; } } + // Only the last chunk of a secretstream carries TAG_FINAL. Everything a + // dropped connection did deliver still decrypts and authenticates, so the + // absence of TAG_FINAL is the only evidence that the body was cut short. + // Returning a short plaintext here would put a corrupt file on disk that + // later backup runs would treat as complete. + if (chunksPulled === 0) { + throw new Error( + "download: stream truncated: response body contained no secretstream chunks", + ); + } + if (lastTag !== STREAM_TAG_FINAL) { + throw new Error( + `download: stream truncated: last chunk tag ${lastTag}, expected TAG_FINAL (${STREAM_TAG_FINAL})`, + ); + } + const result = new Uint8Array(totalPlain); let offset = 0; for (const chunk of plainChunks) { @@ -63,6 +88,29 @@ const streamDecrypt = async ( return result; }; +// Write `plaintext` to `destination` atomically: stage it in a temporary +// sibling file (same directory, so the rename cannot cross a filesystem +// boundary) and rename it into place. Callers therefore never observe a +// partially written destination, and a pre-existing file at that path is +// replaced only once the new contents are complete on disk. +const writeAtomic = async ( + destination: string, + plaintext: Uint8Array, +): Promise => { + // The random suffix keeps concurrent downloads of the same destination + // from stepping on each other's temporary file. + const tmpPath = join(dirname(destination), `.quak-${randomUUID()}.tmp`); + try { + await writeFile(tmpPath, plaintext); + await rename(tmpPath, destination); + } catch (err) { + // Best-effort cleanup. A failure to remove the temporary file must + // never replace the error that actually explains what went wrong. + await rm(tmpPath, { force: true }).catch(() => undefined); + throw err; + } +}; + export const downloadFile = async ( api: ApiClient, file: EnteFile, @@ -72,7 +120,7 @@ export const downloadFile = async ( const stream = await api.getFileStream(file.id); const header = fromBase64(file.file.decryptionHeader); const plaintext = await streamDecrypt(stream, header, file.key); - await writeFile(resolvedPath, plaintext); + await writeAtomic(resolvedPath, plaintext); return { path: resolvedPath, bytesWritten: plaintext.length }; }; @@ -85,6 +133,6 @@ export const downloadThumbnail = async ( const stream = await api.getThumbnailStream(file.id); const header = fromBase64(file.thumbnail.decryptionHeader); const plaintext = await streamDecrypt(stream, header, file.key); - await writeFile(resolvedPath, plaintext); + await writeAtomic(resolvedPath, plaintext); return { path: resolvedPath, bytesWritten: plaintext.length }; }; diff --git a/test/download/download.test.ts b/test/download/download.test.ts index 2209857..579991a 100644 --- a/test/download/download.test.ts +++ b/test/download/download.test.ts @@ -45,6 +45,7 @@ import { } from "node:fs"; import { join } from "node:path"; import { tmpdir } from "node:os"; +import { createHash } from "node:crypto"; import sodium from "libsodium-wrappers-sumo"; import { beforeAll, afterAll, describe, expect, it } from "vitest"; import { init, toBase64, STREAM_CHUNK_SIZE } from "../../src/crypto/index.js"; @@ -188,6 +189,42 @@ const mockFetchForBody = (body: Uint8Array) => { return fake as typeof globalThis.fetch; }; +/** + * Encrypt a body consisting of one chunk that is *not* tagged TAG_FINAL. + * + * This is the cheap way to present a stream that ended without its final + * chunk: the downloader pulls it, authenticates it, and finds the stream + * over on a TAG_MESSAGE chunk — the same terminal condition as a large file + * whose last chunk was lost, without paying for a 4 MiB fixture. The + * multi-chunk fixture above covers the realistic wire shape; this one is + * used where the test is really about what happens on disk afterwards. + */ +const encryptNonFinalBody = ( + plaintext: Uint8Array, + key: Uint8Array, +): { header: Uint8Array; ciphertext: Uint8Array } => { + const push = sodium.crypto_secretstream_xchacha20poly1305_init_push(key); + const ciphertext = sodium.crypto_secretstream_xchacha20poly1305_push( + push.state, + plaintext, + null, + sodium.crypto_secretstream_xchacha20poly1305_TAG_MESSAGE, + ); + return { header: push.header, ciphertext }; +}; + +/** + * Compare file contents by digest rather than with `toEqual`. Vitest's deep + * equality walks multi-megabyte buffers byte by byte, which costs seconds on + * the 4 MiB fixtures; a digest comparison is exact and effectively free. + */ +const expectSameBytes = (actual: Uint8Array, expected: Uint8Array): void => { + expect(actual.length).toBe(expected.length); + expect(createHash("sha256").update(actual).digest("hex")).toBe( + createHash("sha256").update(expected).digest("hex"), + ); +}; + /** * A multi-chunk fixture shared by the truncation tests. Building it costs a * few MiB of encryption, so it is built once: one full 4 MiB `TAG_MESSAGE` @@ -291,7 +328,7 @@ describe("downloadFile", () => { const result = await downloadFile(api, file, outPath); expect(result.bytesWritten).toBe(100_000); - expect(readFileSync(outPath)).toEqual(Buffer.from(plaintext)); + expectSameBytes(readFileSync(outPath), plaintext); }); it("decrypts a body that spans several secretstream chunks", async () => { @@ -311,9 +348,7 @@ describe("downloadFile", () => { const result = await downloadFile(api, file, outPath); expect(result.bytesWritten).toBe(multiChunk.plaintext.length); - expect(readFileSync(outPath)).toEqual( - Buffer.from(multiChunk.plaintext), - ); + expectSameBytes(readFileSync(outPath), multiChunk.plaintext); }); }); @@ -402,15 +437,18 @@ describe.each(entryPoints)( // The caller's contract: if the promise rejects, the destination // path does not exist. Nothing downstream should ever have to guess // whether a leftover file is complete. - const truncatedBody = multiChunk.body.slice( - 0, - multiChunk.finalChunkOffset, - ); - const { api, file } = fixtureFor( - multiChunkKey, - multiChunk.header, - truncatedBody, + // + // The body here is a single chunk that was never tagged TAG_FINAL, + // which puts the downloader in exactly the terminal state a lost + // last chunk produces, without the cost of a 4 MiB fixture. What + // this test is really about is the state of the filesystem after + // the rejection. + const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + const { header, ciphertext } = encryptNonFinalBody( + sodium.randombytes_buf(256), + key, ); + const { api, file } = fixtureFor(key, header, ciphertext); const dir = freshDir(); const outPath = join(dir, "absent.bin"); @@ -458,15 +496,12 @@ describe.each(entryPoints)( const existing = new TextEncoder().encode( "previously downloaded, known-good contents", ); - const truncatedBody = multiChunk.body.slice( - 0, - multiChunk.finalChunkOffset, - ); - const { api, file } = fixtureFor( - multiChunkKey, - multiChunk.header, - truncatedBody, + const key = sodium.crypto_secretstream_xchacha20poly1305_keygen(); + const { header, ciphertext } = encryptNonFinalBody( + sodium.randombytes_buf(256), + key, ); + const { api, file } = fixtureFor(key, header, ciphertext); const dir = freshDir(); const outPath = join(dir, "existing.bin"); writeFileSync(outPath, existing);