Verify secretstream TAG_FINAL and write downloads atomically (closes #1)
Some checks failed
check / check (push) Failing after 1m19s
Some checks failed
check / check (push) Failing after 1m19s
streamDecrypt discarded the secretstream tag, so a download cut short by a dropped connection decrypted cleanly up to the last whole chunk and was returned as a success. downloadFile and downloadThumbnail then wrote straight to the destination, and runBackup skips any existing non-empty file, so a truncated original was treated as complete on every subsequent run and never repaired. streamDecrypt now tracks the tag of each chunk it pulls and throws if the stream ended on anything other than TAG_FINAL, or if the body carried no chunks at all — Ente always emits at least one chunk, as encryptBlob shows by producing a TAG_FINAL chunk even for zero-length plaintext, so an empty body is a failed transfer rather than an empty file. Both error messages say the stream was truncated. Plaintext is now staged in a temporary sibling file (same directory, so the rename cannot cross a filesystem boundary; random UUID suffix, so concurrent downloads cannot collide) and renamed into place only after the whole stream has decrypted and verified. On any error the temporary file is removed and the original error is rethrown unchanged, so a cleanup failure never masks the real diagnosis. A failed download therefore leaves the destination exactly as it was. Public signatures and the DownloadResult shape are unchanged. The download layer keeps its no-direct-sodium-import shape: TAG_FINAL is re-exported from src/crypto as STREAM_TAG_FINAL, which decryptBlob now uses too. Also moves the pullStreamChunk doc comment off decryptBlob, where it had been sitting. Retry and backoff remain out of scope; they stay the Next Step in TODO.md and are tracked separately.
This commit is contained in:
4
TODO.md
4
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);
|
||||
|
||||
@@ -14,5 +14,6 @@ export {
|
||||
pullStreamChunk,
|
||||
STREAM_CHUNK_OVERHEAD,
|
||||
STREAM_CHUNK_SIZE,
|
||||
STREAM_TAG_FINAL,
|
||||
type StreamPullState,
|
||||
} from "./stream.js";
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<void> => {
|
||||
// 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 };
|
||||
};
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user