Repository navigation
fix(crypto): wipe AES key schedules, H and ChaCha state with the volatile wipe - #639
Conversation
…rypto-wipe # Conflicts: # tests/self/goldens/checked.txt
…rypto-wipe # Conflicts: # tests/self/goldens/checked.txt
…rypto-wipe # Conflicts: # tests/self/goldens/checked.txt
amritk
left a comment
amritk
left a comment
There was a problem hiding this comment.
Review round 1 of feature/secret-keys stage crypto-wipe, at bbc887f. There are 3 confirmed findings, each one checked by an independent verifier. Fix all three in one push.
Refuted, so no action is needed:
- Kernel tail not pinned. The zero read-back walks the whole
roundKeysarray, so a truncated wipe fails it. - New
h[...]indexes would trip NL9007. The performance gate innpm testholdsstd/to zero warnings, and CI is green.
Also not raised:
- Placing the fallback wipes inside the bitsliced bodies is what the plan intends.
- The stale
hHi/hLotext in AEAD-4 belongs to the secret-boundary stage. - A wiped key sealing under a zero schedule is documented behaviour.
Generated by Claude Code
| ["nish.aesGcmSeal", 2], | ||
| ["nish.aesGcmOpen", 2], |
There was a problem hiding this comment.
[1/3] The GCM path's key-derived working copies are left unwiped, and these exact counts lock that gap in. In std/crypto/aes.ts, aesCtrInto (~986-1010) allocates buf (AES_BATCH bytes) and q (8 × u64). aesEncryptBatch(rk, rounds, buf, q) leaves the CTR keystream and its bitsliced state in them, and nothing wipes them. aesGcmTag (~1045-1054) leaves E(K, J0) in mask after aesEncryptOne(key, j0, mask), also unwiped. These are the same class of working copy the stage already wipes in aesBlockInto, whose own doc says the block "may be H or E(K, J0)". The count of 2 for aesGcmSeal/aesGcmOpen comes only from aesEncryptOne.
Wipe buf, q and mask once they are dead, on the bitsliced path, without touching the seam calls. Add a root for aesCtrInto (or its caller) and raise the seal/open counts to match. Re-measure the AES-GCM figure in the Measured: trailer if it moves.
Generated by Claude Code
| // `hkdfExtractInto` inlines `hkdfWipe` into both hash branches, so it reaches | ||
| // 50 where its source writes 49). | ||
| { | ||
| name: "crypto_hmac crypto_hkdf: the wipes survive -O2 (HMAC's key blocks, hashed key, inner digest and hashers; HKDF's T(i) and PRK)", |
There was a problem hiding this comment.
[2/3] Two references still point to tests/run.js for this check, which now lives here. std/crypto/hmac.ts:34 (keeps (`tests/run.js`, "crypto_hmac crypto_hkdf: the wipes survive -O2")) and docs/security/crypto-k1.md:91 (`tests/run.js` ("crypto_hmac crypto_hkdf: the wipes survive -O2") counts...) both name tests/run.js. The string no longer appears in tests/run.js (git grep "crypto_hmac crypto_hkdf" -- tests/run.js is empty), so the grep CLAUDE.md's Security section asks for fails. Repoint both to tests/wipes/crypto.js and leave the check name untouched. These two pointer edits are allowed outside your Owns for this purpose. If the record row's anchor commit has to change, follow the record convention: name the new commit rather than moving the anchors silently.
Generated by Claude Code
| wipe(okm); | ||
| console.log("32-byte secret: accepted"); | ||
| const short: Secret<u8[]> = secret(new Array<u8>(31)); | ||
| const refused: Secret<u8[]> = hkdfExpandLabelSha256Secret(short, "key", [], len); |
There was a problem hiding this comment.
[3/3] The SHA-384 Secret form's short-PRK panic has no test. std/crypto/hkdf.ts:418 panics with hkdfExpandLabelSha384Secret: a secret shorter than 48 bytes, but only the SHA-256 form is driven into its panic here. The plain forms test both (crypto_hkdf_expand_label_short_secret and crypto_hkdf_expand_label_short_secret_sha384). Add a _sha384 twin of this test that expects the 48-byte message and exit code.
Generated by Claude Code
…; repoint the HMAC/HKDF pin to tests/wipes
amritk
left a comment
amritk
left a comment
There was a problem hiding this comment.
Review round 2 of feature/secret-keys stage crypto-wipe, at 07c940d. The round-1 fixes are in. A verifier confirmed 2 new findings; please fix both in one push.
Refuted, so no action is needed:
aesHeaderMaskleaves its block unwiped. That block is one AES output for a public sample, so it reveals nothing about the key and nothing reads it.- Counts ignore which buffer each wipe hits. The per-root count is the documented design, and targeting each wipe would need alias tracking out of proportion to the gain.
Not raised again: the items rejected in round 1.
Generated by Claude Code
| ["nish.poly1305", 2], | ||
| ["nish.poly1305KeyGen", 2], | ||
| ["nish.chacha20Poly1305Seal", 6], | ||
| ["nish.chacha20Poly1305Open", 7], |
There was a problem hiding this comment.
[1/2] This count does not pin the refusal path's wipe(state) (medium). chacha20Poly1305Open has if (match === 0) { wipe(state); return null } and a later wipe(state). The verifier built tests/link/crypto_aes_wipes against this head and ran LLVM 18's opt -O2. @nish.chacha20Poly1305Open held the same number of volatile memsets with and without the refusal-path wipe, because opt merges the two into one, as the comment above says. So deleting that wipe keeps this root at 7, every forged open leaves the ChaCha state unwiped, and the gate stays green.
Make each of the two wipes independently load-bearing for some checked count. For example, move the refusal path's wipe into a path opt cannot merge with the success path's. Or add a second pin on the IR before optimisation, which counts both calls. Then show it the way the PR body shows the others: delete the refusal-path wipe and record the count it drops to.
Generated by Claude Code
| ["nish.aesGcmSeal", 5], | ||
| ["nish.aesGcmOpen", 5], |
There was a problem hiding this comment.
[2/2] For a non-12-byte IV, J0 and its GHASH state are left unwiped (low). In std/crypto/aes.ts aesGcmJ0 (~946-960 at this head), y (u64[2]) holds GHASH_H(IV‖len) after ghashUpdate/ghashLengths. The returned j0 holds the same value. Neither aesGcmSeal (~1087) nor aesGcmOpen (~1116) wipes j0, and nothing wipes y.
With a 12-byte IV, J0 = IV‖1 and is public. With any other IV length, J0 is a polynomial in H over a known IV. A leaked J0 lets an attacker solve for H, the GCM authentication key that aesKeyWipe wipes, and so forge tags.
Wipe y inside aesGcmJ0 once J0 is stored. Wipe j0 in seal and open once the CTR pass and the tag no longer need it, on every path, including open's refusal. A 12-byte J0 is public, so wiping it costs nothing and keeps one code path. Raise these seal/open counts and keep Measured: honest if the figures move.
Generated by Claude Code
…rypto-wipe # Conflicts: # tests/self/goldens/checked.txt
…d TLS-5 Review round 2: the key bounds are awake time under CLOCK_MONOTONIC; an early rotation may end the previous key's tickets, which the design accepts; the strike register's fill cost is scoped to an attacker without a key, and a key-holder's is stated; the wipes no test can read are pinned in the IR after -O2; row 14 wins over the rows that never abort, and row 16 is placed for QUIC. TLS-5 records the ChaCha20 state that keeps a copy of its key, which #639 closes.
… in the K5 baseline Review round 2. The mixed additions are 64 x 25,383 x 11/12 = 1,489,136 Ir, so k·G projects to 1,663,216 and the signature to 3,136,541; the ratios do not move. The modules #639 touched are given by their own Ir in the profile, 111,174 (0.81%), with ChaCha20-Poly1305 off the handshake's path.
* docs(security): threat model for TLS 1.3 session resumption Records the design of server-side TLS 1.3 session resumption in `docs/security/tls.md` before any of it is written: the attacker and what a win is; ticket keys as caller-held `Secret`s (NL2430), sealing for 12 h, accepted as the previous key for one 2 h ticket lifetime and then wiped; single-use tickets through a bounded strike register that fails closed; `psk_dhe_ke` only, so a leaked ticket key exposes no recorded traffic; 0-RTT out of scope; and the fallback-versus-abort table, checked against RFC 8446's MUSTs, that the later stages are tested against. Refs: #481 * docs(security): decide the strike register's order, fill cost, clock and key bounds for resumption Review round 1: tickets are spent once their binder verifies, in one call with no gap between check and record; the register's fill cost is restated and counted; every time is monotonicNanos; the module enforces each key's issue window, 14-hour life and a 2^30 seal cap from a public TlsTicketKeyInfo; the rotation turn is the caller's; and an opened ticket leaves exposeWith as a fresh array the caller parses and wipes. * docs(security): qualify the resumption bounds in awake time and record TLS-5 Review round 2: the key bounds are awake time under CLOCK_MONOTONIC; an early rotation may end the previous key's tickets, which the design accepts; the strike register's fill cost is scoped to an attacker without a key, and a key-holder's is stated; the wipes no test can read are pinned in the IR after -O2; row 14 wins over the rows that never abort, and row 16 is placed for QUIC. TLS-5 records the ChaCha20 state that keeps a copy of its key, which #639 closes. * docs(security): close TLS-5, now that #639 wipes the ChaCha20 state * docs(security): bound strike entries by the server's lifetime, and count TLS-5 Review round 3: a strike entry lives no longer than one server lifetime after it is recorded, and a ticket whose sealed lifetime exceeds the server's or whose chain start follows its issue time fails to open, so a key holder cannot pin entries past rotation. The T2 result's open-finding sentence is brought up to date, and the security index counts TLS-5 as a fixed Low. * docs(security): pin the strike clamp through take, and bound a key holder's fill Review round 4: the register's own clamp is pinned by calling TlsTicketStrikes.take directly, since a ticket claiming a longer life no longer opens; and a leaked key's fill is bounded on both branches, at most about 16 awake hours after the draw, with what retiring a key that still opens as the previous one takes. * docs(security): state the strike entry's boundary and what take answers Review round 5: an entry is held through its expiry inclusive, matching when a ticket opens, and TlsTicketStrikes.take answers true when it records an id and false when the id is held or the register is full. * docs(security): pin the TLS resumption wipes, salt and binder order Round 7 of review on the resumption threat model. Each of `tlsTicketOpen` and `tlsTicketSeal` wipes its plaintext once, after its branch, since `opt -O2` merges per-path wipes. The record now covers the resumed handshake-secret salt, which is secret, and the PSK, PSK early secret and binder key on every path, the fail and close wipe included. It also covers the strike register's probe-chain scan and the time a leaked key's bytes stay in memory. A ClientHello that hits both a failed binder and a missing `signature_algorithms` gets `decrypt_error`. Refs: docs/security/tls.md * docs(security): pin the resumed salt's wipes and the codec's mode check The resumed handshake's salt is read and zeroed at the top of tlsExchange, beside the key's words, and its rebuilt bytes are wiped on every path; the net_tls_resume pin now covers a low-order share and the row 4 to 8 fallbacks. The wipe-count paragraph names its real roots and says what a textual count proves. A malformed psk_key_exchange_modes is a decode_error whether or not pre_shared_key is present. * docs(security): place the resumed master secret's wipe and pin the exchange salt A resumed handshake has no sign, so the master secret's derivation and wipe move into a post-Finished schedule step both paths call, counted by the wipe-count check. tlsExchange wipes its rebuilt salt array once, at a point every path reaches, and is a root too. The ticket roots carry their real counts, with the rotation turn's wipe noted as checker-enforced (NL2442), and a malformed psk_key_exchange_modes without pre_shared_key is pinned. * docs(security): spell out the master-secret step and the exchange's salt source The master-secret step split out of sign is named TlsServer.masterSecretStep and lists all it does: the application and exporter secrets, the expected client Finished absorbed into the transcript before the resumption master secret, the wait-Finished state, and the master's wipe. The caller always writes the salt's words, the constant's on a full handshake, so tlsExchange needs no flag and its one wipe is unconditional. * docs(security): point the master-secret step's fields at the slot's arrays TlsServer.masterSecretStep points the application, exporter and expected client Finished fields at the slot's arrays before its arena block opens, as sign does, so its derivations land where wipeHandshake wipes; it wipes the master before setting the wait-Finished state. Two stale references to sign and the roots sentence are reworded. * docs(security): pin both salt wipes and the master secret's by deletion The resumption design now says exchangeKeys writes the salt's words and, on a resumed handshake, wipes the derived salt's bytes before calling tlsExchange. It pins exchangeKeys at 7, two more than its 5, with each salt wipe shown counted on its own by deleting it. The master secret's secureZero in masterSecretStep is shown the same way, and sign and masterSecretStep together count at least sign's 5. --------- Co-authored-by: Claude <noreply@anthropic.com>
2 participants
Summary
Stage 1 (crypto-wipe) of the plan that moves TLS and QUIC key material onto
nish:secret. Part of #430. Part of #626.Proposed squash message (the changelog entry):
Changes
std/crypto/aes.tsAesKey.h: u64[](2 words) replaceshHi/hLo. The constructor allocates it, so the signature is unchanged.aesKeyUsableandaesKeyIntoalso requireh.length === 2.aesKeyWipe(key).aesScheduleInto:w,qandbuf.aesBlockInto:buf, and itsq(now a named local).aesKeyInto:h.aesCtrInto:bufandq.aesGcmTag:mask.aesGcmJ0:y.j0; open also the computedtag.aesGcmOpencomputes its answer under one branch and wipesj0once, after it.ghashBlocksIntostill takeshHi, hLo, now passed askey.h[0], key.h[1].std/crypto/chacha20poly1305.ts: per-call state wiped as described above. Every one-shot function now names its state, so it can wipe it.chacha20Poly1305Openwipesstateonce, after the branch on the tag.std/crypto/hkdf.ts: the two*Secretlabel helpers, built onhkdfLabelandhkdfExpandSha*Secret.tests/wipes/index.jsis the harness moved out oftests/run.js;run.jsnow callsrunWipeChecks(...).crypto.jsholds the HMAC/HKDF group, under its existing check name byte for byte, with the two new label roots (15 each). It also holds a new AES/ChaCha group.tls.js,quic.jsandquic-server.jsexport empty lists for stages 2, 3 and 5.std/crypto/hmac.ts:34anddocs/security/crypto-k1.md(K1-7's Tests) now point attests/wipes/crypto.jsfor the "crypto_hmac crypto_hkdf: the wipes survive -O2" check. The check name is unchanged. These are pointer edits only, asked for in review round 1.AesKey's shape changed:std/net/quic-packet.ts(quicPacketClearAes) andstd/net/tls/record.ts(tlsClearAesKey) now writeh[0]/h[1]instead ofhHi/hLo. Nothing else in them changed. Stages 2 and 3 replace both helpers withaesKeyWipe.hHi/hLo:tests/link/crypto_aes/suite.ts,tests/link/crypto_aes_into/checks.tsandtests/link/net_quic_memory/units.ts. Their output is unchanged.tests/link/crypto_aes_agree/expected.ir: only SSA numbers moved, at two seam calls:aesScheduleInto:%25, %26became%33, %34, from the newhlength check inaesKeyInto.ghashBlocksInto:%15, %17, %19became%23, %33, %35, from the bounds-checkedh[0]andh[1]loads.std/README.md: thecrypto/aes.ts,crypto/chacha20poly1305.tsandcrypto/hkdf.tsrows.docs/security/crypto-aead.md(AEAD-1's text and AEAD-4) anddocs/security/ct-verification.mdstill sayhHi/hLo. Each record is anchored at its own commit; stage 4 rewrites AEAD-4.New tests
tests/link/crypto_aes_wipesand its_f64twin:roundKeyswords and both words ofhread zero;roundsis kept; the wiped key opens nothing;aesKeyIntoreinstalls it and seals the same bytes.hthat is not two words is refused.cryptography; RFC 8439 §2.6.2's Poly1305 key and RFC 9001 A.5's header mask are also checked.NISH_AES=soft.tests/link/crypto_hkdf_expand_label_secretand its_f64twin: RFC 9001 A.1's whole chain held asSecrets, plus the SHA-384 vectors, each checked against the plain form.tests/link/crypto_hkdf_expand_label_secret_short: a 31-byte PRK panics (exit 1).tests/link/crypto_hkdf_expand_label_secret_short_sha384: a 47-byte PRK panics with the 48-byte message (exit 1).git diff origin/main -- 'tests/link/*/expected.out'lists only these six new directories.-O2 pins (
crypto: the wipes survive opt -O2 (…))aesKeyWipeaesKey,aesKeyIntoaesEncryptBlock,aesHeaderMaskaesGcmCtr(GCTR;optinlines the seam into its one caller)aesGcmJ0aesGcmSealaesGcmOpenchacha20XorIntochacha20Block,chacha20,chacha20HeaderMask,poly1305,poly1305KeyGenchacha20Poly1305Sealchacha20Poly1305OpenEvery wipe in both opens, both seals and J0 was deleted in turn, and its root's count dropped each time:
chacha20Poly1305Open:state,polyKeyandtag, each 7 to 6.chacha20Poly1305Seal:stateandpolyKey, each 6 to 5.aesGcmOpen:j0andtag, each 8 to 7.aesGcmSeal:j0, 7 to 6.aesGcmJ0:y, 1 to 0.aesGcmTag'smaskand GCTR'sbufturned intofill(0): 3 to 2 and 2 to 1.aesKeyInto'swipe(h)andaesKeyWipe'swipe(key.h)turned into ordinary stores: 6 to 5 and 2 to 1.Round 2 found that open's refusal-path
wipe(state)could be deleted without the count moving, becauseoptmerged the two path wipes into one store. Both opens now wipe once, after the branch.IR of the new wipe
aesKeyWipeand thewipeinstance it calls, as emitted fortests/link/crypto_aes_wipes(std/crypto/aes.ts):The new test programs are whole
tests/linkprograms. Their IR is the modules above plusnish/testing, andtests/wipes/crypto.jspins what matters of it after-O2.Testing
All on head
b97ce50b, which mergesmainat6729c7cc. The goldens were regenerated withnode tests/self/goldens.js --updateagainst a rebuiltbuild/nish;checked-self.txtis identical tomain's. The seed is 0.18.0.npm run check: passnpm run lint: exit 0.npm run lint:dead(knip): exit 0WASI_SYSROOT=/usr node tests/run.js: 18121 passed, 0 failed, 1 skipped, noDEGRADED:line. The skip issimd's aarch64 static link, which needs an aarch64crt1.othis container lacks.WASI_SYSROOT=/usr node tests/run.js wasi: 49 passed, 0 failed, 0 skippednode tests/run.js performance: 209 passed, 0 failednode tests/run.js survive: 23 passed, 0 failednode tests/differential/goldens.js: fresh, no changenode docs/check-links.mjs: passcopyOffromcrypto_x25519/plain, probeoptonce per run, state the area-name rule intests/wipes/index.js, and tidy two tests.aesKeyWipe, which stages 2 and 3 own.buf/qandaesGcmTag'smask, with a root for GCTR._sha384short-secret twin.Related issues
Part of #430
Part of #626