From 3a4603ee77376086ef0b7eac2a5906893e897ca6 Mon Sep 17 00:00:00 2001 From: Tony Arcieri Date: Mon, 30 Mar 2026 14:49:36 -0600 Subject: [PATCH] Revert "srp: add HomeKit-compatible padding options for u and M1/M2" This reverts commit 5b33c78652d47a991facfd0cd6fb74615895b276 (#272) This is causing sporadic test failures which weren't caught in CI. This shows we need improved testing (e.g. `proptest`) --- srp/src/client.rs | 1 - srp/src/server.rs | 19 ++---------------- srp/src/utils.rs | 49 ++++------------------------------------------- 3 files changed, 6 insertions(+), 63 deletions(-) diff --git a/srp/src/client.rs b/srp/src/client.rs index b474d33..6b6f537 100644 --- a/srp/src/client.rs +++ b/srp/src/client.rs @@ -228,7 +228,6 @@ impl Client { let m1 = compute_m1_rfc5054::( &self.g, - false, username, salt, &a_pub_bytes, diff --git a/srp/src/server.rs b/srp/src/server.rs index 80fbdd3..40e531b 100644 --- a/srp/src/server.rs +++ b/srp/src/server.rs @@ -82,7 +82,6 @@ pub type ServerG4096 = Server; #[derive(Debug)] pub struct Server { g: BoxedMontyForm, - g_no_pad: bool, d: PhantomData<(G, D)>, } @@ -91,19 +90,6 @@ impl Server { #[must_use] pub fn new() -> Self { Self { - g: G::generator(), - g_no_pad: false, - d: PhantomData, - } - } - - /// Create a new SRP server instance. - /// - /// Set `g_no_pad` to `false` for Apple's HomeKit compatibility. - #[must_use] - pub fn new_with_options(g_no_pad: bool) -> Self { - Self { - g_no_pad, g: G::generator(), d: PhantomData, } @@ -170,8 +156,7 @@ impl Server { // Safeguard against malicious A self.validate_a_pub(&a_pub)?; - // [RFC5054]: Section 2.6. - let u = compute_u_padded::(&self.g, a_pub_bytes, &b_pub_bytes); + let u = compute_u::(a_pub_bytes, &b_pub_bytes); let premaster_secret = self .compute_premaster_secret(&a_pub, &v, &u, &b) @@ -181,13 +166,13 @@ impl Server { let m1 = compute_m1_rfc5054::( &self.g, - self.g_no_pad, username, salt, a_pub_bytes, &b_pub_bytes, session_key.as_slice(), ); + let m2 = compute_m2::(a_pub_bytes, &m1, session_key.as_slice()); Ok(ServerVerifier { diff --git a/srp/src/utils.rs b/srp/src/utils.rs index 15f13a3..0e1d80d 100644 --- a/srp/src/utils.rs +++ b/srp/src/utils.rs @@ -5,7 +5,7 @@ use bigint::{ }; use digest::{Digest, Output}; -/// `u = H(A | B)` +/// `u = H(PAD(A) | PAD(B))` #[must_use] pub fn compute_u(a_pub: &[u8], b_pub: &[u8]) -> BoxedUint { let mut u = D::new(); @@ -14,23 +14,6 @@ pub fn compute_u(a_pub: &[u8], b_pub: &[u8]) -> BoxedUint { BoxedUint::from_be_slice_vartime(&u.finalize()) } -/// `u = H(PAD(A) | PAD(B))` -#[must_use] -pub fn compute_u_padded(g: &BoxedMontyForm, a_pub: &[u8], b_pub: &[u8]) -> BoxedUint { - let n = g.params().modulus().to_be_bytes(); - let mut buf_a = vec![0u8; n.len()]; - let mut buf_b = vec![0u8; n.len()]; - let l_a = n.len() - a_pub.len(); - let l_b = n.len() - b_pub.len(); - buf_a[l_a..].copy_from_slice(a_pub); - buf_b[l_b..].copy_from_slice(b_pub); - - let mut u = D::new(); - u.update(&buf_a); - u.update(&buf_b); - BoxedUint::from_be_slice_vartime(&u.finalize()) -} - /// `k = H(N | PAD(g))` #[must_use] pub fn compute_k(g: &BoxedMontyForm) -> BoxedUint { @@ -48,7 +31,7 @@ pub fn compute_k(g: &BoxedMontyForm) -> BoxedUint { /// `H(N) XOR H(PAD(g))` #[must_use] -pub fn compute_hash_n_xor_hash_pad_g(g: &BoxedMontyForm) -> Vec { +pub fn compute_hash_n_xor_hash_g(g: &BoxedMontyForm) -> Vec { let n = g.params().modulus().to_be_bytes(); let g_bytes = g.retrieve().to_be_bytes(); let mut buf = vec![0u8; n.len()]; @@ -64,24 +47,6 @@ pub fn compute_hash_n_xor_hash_pad_g(g: &BoxedMontyForm) -> Vec { .collect() } -/// `H(N) XOR H(g)` -#[must_use] -pub fn compute_hash_n_xor_hash_g(g: &BoxedMontyForm) -> Vec { - let n = g.params().modulus().to_be_bytes(); - let g_bytes = g.retrieve().to_be_bytes(); - let first = g_bytes - .iter() - .position(|&b| b != 0) - .unwrap_or(g_bytes.len().saturating_sub(1)); - let g_bytes = &g_bytes[first..]; - - // H(N) and H(g) as byte strings - let h_n = compute_hash::(&n); - let h_g = compute_hash::(g_bytes); - - h_n.iter().zip(h_g.iter()).map(|(&a, &b)| a ^ b).collect() -} - #[must_use] pub fn compute_hash(data: &[u8]) -> Output { let mut d = D::new(); @@ -89,12 +54,10 @@ pub fn compute_hash(data: &[u8]) -> Output { d.finalize() } -/// `M1 = H(H(N) XOR H(PAD(g)) | H(U) | s | A | B | K)` following RFC5054 -/// Or `M1 = H(H(N) XOR H(g) | H(U) | s | A | B | K)` with `g_no_pad` set to true. +/// `M1 = H(H(N) XOR H(g) | H(U) | s | A | B | K)` following RFC5054 #[must_use] pub fn compute_m1_rfc5054( g: &BoxedMontyForm, - g_no_pad: bool, username: &[u8], salt: &[u8], a_pub: &[u8], @@ -102,11 +65,7 @@ pub fn compute_m1_rfc5054( key: &[u8], ) -> Output { let mut d = D::new(); - if g_no_pad { - d.update(compute_hash_n_xor_hash_g::(g)); - } else { - d.update(compute_hash_n_xor_hash_pad_g::(g)); - } + d.update(compute_hash_n_xor_hash_g::(g)); d.update(compute_hash::(username)); d.update(salt); d.update(a_pub);