c24f528 · skrepka · git.iko.soy

git.iko.soy/skrepka

Seventh-pass audit: zeroize encrypt inner on AEAD failure, fix stale comments

crypto.rs: the inner buffer in encrypt() (sender pubkey || signature || compressed_len || compressed || padding) was a plain Vec<u8> — zeroized on the success path but not on the AEAD failure path, where ? returns early and drops it without wiping. Now Zeroizing<Vec<u8>>, matching the pattern established for decrypt's inner/signed in the 4th audit and plaintext in the 6th. Every other sensitive intermediate in encrypt was already Zeroizing; inner was the sole exception. Also fixes: - CI smoke test comment: 336-hex-char → 346-hex-char (minBlobLen is 346 hex = 173 bytes, not 336/168) - server.knot genToken comment: claimed O(n) but acc ++ char copies the accumulator each call, making it O(n²) — corrected to say so

commitc24f528b1f42cda6fe5efddf5d3520c42f706d11 authoriko <mail@iko.soy> dateTue 14 Jul 2026, 00:21 (3 months ago) parent4e03722

4 files changed+30−5

compare with master

unifiedsplit

.github/workflows/build.yml+1−1 PROTOCOL-REVIEW.md+19 core/src/crypto.rs+7−1 server.knot+3−3

.github/workflows/build.yml+1−1

@@ -199,7 +199,7 @@ jobs:

199199 -X POST http://127.0.0.1:8080/federation/forward -H "$J" \

200200 -d "{\"toKey\":\"$PK\",\"encryptedBlob\":\"$BLOB\"}"

201201

202− # Below the 336-hex-char (168-byte) minimum envelope: rejected by BlobHex

202+ # Below the 346-hex-char (173-byte) minimum envelope: rejected by BlobHex

203203 # at the route boundary, before the federation gate.

204204 expect 400 "federation/forward (undersized blob rejected)" \

205205 -X POST http://127.0.0.1:8080/federation/forward -H "$J" \

PROTOCOL-REVIEW.md+19

@@ -395,3 +395,22 @@ and retries the flush after `FLUSH_WATCHDOG_MS` (90 s).

395395The `nickname` in `AddContact` was stored without any length bound. A user could

396396set a megabyte-long nickname, bloating the contacts kv blob (rewritten in full on

397397every change). `nickname.len()` is now checked against `MAX_DISPLAY_NAME_LEN`.

398+

399+---

400+

401+## Fixed in seventh-pass audit (2026-07-14)

402+

403+### ✅ P. `encrypt`'s `inner` buffer not zeroized on AEAD failure

404+

405+**Severity: low — fixed.**

406+

407+The `inner` buffer in `crypto::encrypt` (sender pubkey || signature ||

408+compressed_len || compressed || padding) was a plain `Vec<u8>`. It was

409+explicitly `zeroize()`d on the success path, but if `cipher.encrypt()`

410+returned `Err`, the `?` operator returned early and `inner` was dropped

411+without zeroization — the sender's identity, signature, and compressed

412+plaintext remained in freed memory. Every other sensitive intermediate in

413+`encrypt` (`compressed`, `signed`, `padding`, `eph_priv`, `raw_secret`,

414+`key`) was already `Zeroizing`; `inner` was the sole exception. Now

415+`Zeroizing<Vec<u8>>`, matching the pattern established for `decrypt`'s

416+`inner` and `signed` in the fourth-pass audit.

core/src/crypto.rs+7−1

@@ -327,7 +327,13 @@ pub fn encrypt(

327327 let signature = sender.signing_key().sign(&signed);

328328

329329 // inner = sender_pub(32) || sig(64) || compressed_len(4) || compressed || padding

330− let mut inner = Vec::with_capacity(32 + 64 + 4 + compressed.len() + padding.len());

330+ //

331+ // `Zeroizing` so the sender's pubkey, signature, and the compressed plaintext

332+ // are wiped from memory even on the AEAD failure path — the `?` below returns

333+ // early, and a plain `Vec` would be dropped without zeroization. Every other

334+ // sensitive intermediate here (`compressed`, `signed`, `padding`) is already

335+ // `Zeroizing`; `inner` holds copies of all of them plus the sender identity.

336+ let mut inner = Zeroizing::new(Vec::with_capacity(32 + 64 + 4 + compressed.len() + padding.len()));

331337 inner.extend_from_slice(&sender_pub);

332338 inner.extend_from_slice(&signature.to_bytes());

333339 inner.extend_from_slice(&compressed_len_bytes);

server.knot+3−3

@@ -526,9 +526,9 @@ capForwards = \fwds -> take maxForwardsTotal (sortBy (\f -> f.retries) fwds)

526526-- Session bearers, /auth challenges, and federation auth nonces all use n=48

527527-- (192 bits of entropy, well beyond birthday bound).

528528-- Tail-recursive: builds the string left-to-right via an accumulator (`acc`),

529−-- appending one hex char per call. Each `acc ++ ...` still copies the accumulator,

530−-- but the total work is O(n) rather than O(n²) because characters are appended

531−-- in forward order instead of prepended at every return.

529+-- appending one hex char per call. Each `acc ++ ...` copies the accumulator,

530+-- so the total work is O(n²) in the length — but n is 48 (192 bits), so this

531+-- is a fixed ~1 KB of copying and not worth a more complex builder.

532532genToken : Int -> Text -> IO {random} Text

533533genToken = \n acc -> if n <= 0

534534then yield acc