From 675f83657fd87fc3e6336cf03d33ed5956dc41dc Mon Sep 17 00:00:00 2001 From: forkwright Date: Fri, 21 Aug 2026 10:34:15 -0500 Subject: [PATCH] fix(kerykeion): stop the trial-decrypt loop swallowing AES init failures The loop skipped a channel on any `apply_aes_ctr` error without saying so. That was defensible while a bad-length key could reach it: init failure was then a routine outcome of guessing wrong, indistinguishable from a wrong-key decode. Since #436 it cannot be routine. `resolve_psk` yields `Key` only at 16 or 32 bytes, and `apply_aes_ctr` rejects every other length before touching the cipher -- so a failure here is a fault in the AES implementation or the machine under it, not a wrong guess about which channel a packet belongs to. It now warns with the error and the channel index. Still `continue` rather than `fail`: a later channel may decrypt, and refusing the whole packet over one channel's fault would turn a local problem into dropped traffic. What changes is that it can no longer happen quietly. Deliberately untested, and worth saying why rather than adding a test that passes for the wrong reason: with the length guarantee above, no input this crate accepts can reach the arm. A test would have to construct a key that `resolve_psk` cannot produce, which proves something about the test rather than about the code. The arm's value is precisely that it is loud if the impossible happens. Refs #229 --- crates/kerykeion/src/crypto.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/crates/kerykeion/src/crypto.rs b/crates/kerykeion/src/crypto.rs index f615a8a..ec0ad6d 100644 --- a/crates/kerykeion/src/crypto.rs +++ b/crates/kerykeion/src/crypto.rs @@ -219,7 +219,24 @@ pub fn decrypt( }; let mut candidate = ciphertext.to_vec(); - if apply_aes_ctr(&mut candidate, packet_id, from_node, &key).is_err() { + // WHY(#229) surfaced rather than skipped: this arm used to swallow the + // error, which was defensible while a bad-length key could reach here + // and fail init routinely. Since #436 it cannot — `resolve_psk` yields + // `Key` only at 16 or 32 bytes, and `apply_aes_ctr` rejects every other + // length before touching the cipher — so a failure here is a fault in + // the AES implementation or the machine under it, not a wrong guess + // about which channel this packet belongs to. + // + // Still `continue` rather than `fail`: a later channel may decrypt, and + // refusing the whole packet on one channel's fault would turn a local + // problem into dropped traffic. What changes is that it can no longer + // happen quietly. + if let Err(error) = apply_aes_ctr(&mut candidate, packet_id, from_node, &key) { + tracing::warn!( + channel = channel_idx, + %error, + "AES initialisation failed for a key of valid length; skipping channel" + ); continue; }