diff --git a/crates/ogar-auth/src/legacy.rs b/crates/ogar-auth/src/legacy.rs index 33dae2d..db8a497 100644 --- a/crates/ogar-auth/src/legacy.rs +++ b/crates/ogar-auth/src/legacy.rs @@ -23,8 +23,31 @@ //! **This crate bakes NO password table.** The 62-key (or however-many) //! table belongs to the consumer that owns the legacy corpus and is passed as //! a `&[&str]` argument. That is the whole reason this primitive is public and -//! agnostic while the table stays private: the *algorithm* is a documented -//! .NET convention; the *table* is a live secret. +//! agnostic while the table stays private: the *algorithm* is a *reconstruction* +//! of the .NET convention (see the UNVERIFIED note below); the *table* is a live +//! secret. +//! +//! ## ⚠ UNVERIFIED — the KDF likely does NOT match real .NET yet (Codex P1) +//! +//! The key derivation above is a **best-effort reconstruction, NOT proven +//! against production ciphertext.** A review (Codex, PR #186) correctly flags a +//! specific likely discrepancy: **.NET's `CryptDeriveKey("RC2","MD5",128,…)` +//! does not simply take the `PasswordDeriveBytes.GetBytes` 100-round PBKDF1 +//! stream plus a final `MD5`** — it feeds the password hash through the Windows +//! CryptoAPI `CryptDeriveKey` key-material transform (the `0x36`/`0x5C` +//! double-hash construction), producing a *different* 16-byte key. So real +//! legacy ciphertext will very likely fail the PKCS#7 / UTF-8 checks here, and +//! [`decrypt_prefixed`] currently round-trips only values this Rust helper +//! itself produced. +//! +//! **Consequence — treat this module as a SCAFFOLD, not a working verifier.** +//! Do NOT use it in a production migration until it is byte-parity-proven +//! against a real `(plaintext, ciphertext)` pair from the target corpus (the +//! canonical `Crypt.cs`). The `#[cfg(test)]` round-trips below prove *internal +//! self-consistency only* — never oracle parity. When the real vector arrives, +//! the fix is to replace [`derive_tdes_key_pbkdf1_md5`] with the actual +//! `CryptDeriveKey` transform and gate it with a `tests/legacy_vectors.rs` +//! byte-parity check. use base64::Engine; use base64::engine::general_purpose::STANDARD as BASE64; @@ -66,6 +89,11 @@ pub fn index_to_prefix(idx: usize) -> Option { /// `.NET PasswordDeriveBytes` over MD5 with empty salt and `iterations` /// rounds, then the `CryptDeriveKey("RC2","MD5",128,…)` step, expanded to the /// 24-byte TripleDES-EDE2 key (K1‖K2‖K1). +/// +/// **⚠ UNVERIFIED** (see the module-level note): this PBKDF1-fold is a +/// *reconstruction* and likely diverges from the real Windows `CryptDeriveKey` +/// transform, so it will not decrypt real .NET corpus ciphertext until proven +/// against an oracle vector. Round-trips with [`encrypt_prefixed`] only. #[must_use] pub fn derive_tdes_key_pbkdf1_md5(password: &[u8], iterations: usize) -> [u8; 24] { // Step 1 — initial hash of password (empty salt appended unchanged). diff --git a/crates/ogar-auth/src/totp.rs b/crates/ogar-auth/src/totp.rs index 14e5929..c0a64db 100644 --- a/crates/ogar-auth/src/totp.rs +++ b/crates/ogar-auth/src/totp.rs @@ -77,6 +77,17 @@ pub fn base32_decode(s: &str) -> AuthResult> { out.push(((bits >> n_bits) & 0xff) as u8); } } + // RFC 4648: a valid unpadded base32 group leaves 0..=4 stray bits, ALL of + // which MUST be zero (the encoder zero-pads). 5..=7 stray bits means an + // invalid length — a lone/partial symbol carrying real entropy that cannot + // complete a byte (e.g. "A" would otherwise decode to 0 bytes = an empty + // HMAC key). Fail CLOSED so a corrupted / imported enrollment secret is + // rejected, not silently truncated to a shorter (or zero-entropy) key. + if n_bits >= 5 || (bits & ((1u64 << n_bits) - 1)) != 0 { + return Err(AuthError::Encoding( + "totp secret has invalid trailing base32 bits", + )); + } Ok(out) } @@ -202,6 +213,24 @@ mod tests { assert!(base32_decode("not!base32").is_err()); } + #[test] + fn base32_rejects_invalid_trailing_bits() { + // Lengths that leave 5..=7 stray bits cannot complete a byte and would + // otherwise silently truncate — "A" -> empty HMAC key (zero entropy). + for bad in ["A", "ABC", "ABCDEF", "AAAAAA"] { + assert!( + base32_decode(bad).is_err(), + "{bad:?} has an invalid unpadded length and must be rejected" + ); + } + // Valid length but non-zero trailing bits (encoder always zero-pads, so + // this is a corrupted secret) must also fail closed. + assert!(base32_decode("AB").is_err(), "non-zero trailing bits"); + // Canonical zero-padded values still decode. + assert_eq!(base32_decode("AA").unwrap(), vec![0u8]); + assert!(base32_decode(&base32_encode(b"hi")).is_ok()); + } + #[test] fn verify_accepts_skew_window_and_rejects_outside() { let secret = generate_secret_base32().unwrap();