From f37bd0fcc412c517a14273438af94f2f0e4a2e2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=83=A1=E9=A3=9E?= <1835698775@qq.com> Date: Wed, 23 Sep 2026 14:24:11 +0800 Subject: [PATCH] ssh-encoding: normalize non-canonical mpint encodings when decoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RFC 4251 § 5 requires redundant leading `0x00` bytes to be omitted, but some SSH implementations send them anyway: older Huawei network devices, for example, place a redundant leading zero in the RSA exponent or modulus of their `ssh-rsa` host key, which previously made the whole key unparseable. OpenSSH tolerates this on the receiving side (`sshbuf_get_bignum2_bytes_direct` trims leading zeros rather than rejecting the message), so normalize the value in `Decode for Mpint` instead of failing. Negative values and already-canonical encodings are preserved verbatim. `Mpint::from_bytes` still rejects redundant leading zeros, so locally constructed values keep being held to RFC 4251. Signed-off-by: 胡飞 <1835698775@qq.com> --- ssh-encoding/src/mpint.rs | 110 +++++++++++++++++++++++++++++++++++- ssh-key/tests/public_key.rs | 46 +++++++++++++++ 2 files changed, 155 insertions(+), 1 deletion(-) diff --git a/ssh-encoding/src/mpint.rs b/ssh-encoding/src/mpint.rs index 2060d09..0f37f51 100644 --- a/ssh-encoding/src/mpint.rs +++ b/ssh-encoding/src/mpint.rs @@ -55,6 +55,10 @@ impl Mpint { /// /// # Errors /// Returns [`Error::MpintEncoding`] in the event of an unnecessary leading `0`. + /// + /// This matches the encoding rules of RFC 4251 § 5. [`Decode::decode`] is + /// deliberately more lenient, as it must be able to read non-canonical + /// encodings which other SSH implementations produce. pub fn from_bytes(bytes: &[u8]) -> Result { bytes.try_into() } @@ -116,6 +120,46 @@ impl Mpint { pub fn is_positive(&self) -> bool { self.as_positive_bytes().is_some() } + + /// Normalize a big endian-encoded integer as read from the wire. + /// + /// RFC 4251 § 5 requires redundant leading `0x00` bytes to be omitted, but + /// several SSH implementations send them anyway (e.g. in `ssh-rsa` host + /// keys, as observed with older Huawei network devices). OpenSSH tolerates + /// this when reading a peer's message: `sshbuf_get_bignum2_bytes_direct` + /// trims leading zeros instead of failing, so do the same here. + /// + /// Redundant leading zero bytes are stripped, and a single `0x00` is + /// re-added if the remaining value is positive with its MSB set, i.e. the + /// result is always canonically encoded. Values which are already canonical + /// (including negative values) are returned unchanged. + fn normalize_decode(bytes: Box<[u8]>) -> Self { + let Some(first_nonzero) = bytes.iter().position(|byte| *byte != 0) else { + // The value is zero, which RFC 4251 § 5 encodes as an empty string. + return Self { + inner: Box::default(), + }; + }; + + if first_nonzero == 0 { + return Self { inner: bytes }; + } + + let rest = &bytes[first_nonzero..]; + + let inner = match rest.first() { + // Positive, but the MSB is set: exactly one `0x00` prefix is needed. + Some(byte) if *byte >= 0x80 => { + let mut inner = Vec::with_capacity(rest.len().saturating_add(1)); + inner.push(0); + inner.extend_from_slice(rest); + inner.into_boxed_slice() + } + _ => Vec::from(rest).into_boxed_slice(), + }; + + Self { inner } + } } impl AsRef<[u8]> for Mpint { @@ -145,7 +189,9 @@ impl Decode for Mpint { type Error = Error; fn decode(reader: &mut impl Reader) -> Result { - Vec::decode(reader)?.into_boxed_slice().try_into() + Ok(Self::normalize_decode( + Vec::decode(reader)?.into_boxed_slice(), + )) } } @@ -257,8 +303,20 @@ impl TryFrom<&Mpint> for Uint { #[cfg(test)] mod tests { use super::Mpint; + use crate::Decode; + use alloc::vec::Vec; use hex_literal::hex; + /// Decode a raw `mpint` payload as it would be read off the wire, i.e. with a + /// 4-byte length prefix, so the [`Decode`] implementation itself is exercised. + fn decode(bytes: &[u8]) -> Mpint { + let len = u32::try_from(bytes.len()).unwrap().to_be_bytes(); + let mut prefixed = Vec::with_capacity(bytes.len().saturating_add(4)); + prefixed.extend_from_slice(&len); + prefixed.extend_from_slice(bytes); + Mpint::decode(&mut &prefixed[..]).unwrap() + } + #[test] fn decode_0() { let n = Mpint::from_bytes(b"").unwrap(); @@ -272,6 +330,56 @@ mod tests { assert!(Mpint::from_bytes(&hex!("00 01")).is_err()); } + /// Decoding from the wire tolerates and normalizes non-canonical encodings. + /// + /// Mirrors OpenSSH's `sshbuf_get_bignum2_bytes_direct()`, which trims leading + /// zero bytes instead of rejecting the message. + #[test] + fn decode_tolerates_extra_leading_zeroes() { + assert_eq!(decode(&hex!("00 01")).as_bytes(), &hex!("01")); + assert_eq!(decode(&hex!("00 00 01")).as_bytes(), &hex!("01")); + // A leading zero must be re-added: the MSB of the remaining value is set. + assert_eq!(decode(&hex!("00 00 80 01")).as_bytes(), &hex!("00 80 01")); + assert_eq!(decode(&hex!("00 00 00 80")).as_bytes(), &hex!("00 80")); + } + + /// The value zero is normalized to its canonical empty encoding. + #[test] + fn decode_normalizes_zero() { + assert_eq!(decode(b"").as_bytes(), b""); + assert_eq!(decode(&hex!("00")).as_bytes(), b""); + assert_eq!(decode(&hex!("00 00")).as_bytes(), b""); + } + + /// Canonical encodings and negative values are preserved verbatim. + #[test] + fn decode_preserves_canonical_encodings() { + assert_eq!(decode(&hex!("00 80")).as_bytes(), &hex!("00 80")); + assert_eq!( + decode(&hex!("09 a3 78 f9 b2 e3 32 a7")).as_bytes(), + &hex!("09 a3 78 f9 b2 e3 32 a7") + ); + // NOTE: negative values must not be re-signed by normalization. + assert_eq!(decode(&hex!("ed cc")).as_bytes(), &hex!("ed cc")); + assert_eq!( + decode(&hex!("ff 21 52 41 11")).as_bytes(), + &hex!("ff 21 52 41 11") + ); + assert!(decode(&hex!("ed cc")).as_positive_bytes().is_none()); + } + + /// A normalized integer is indistinguishable from its canonical encoding. + #[test] + fn decode_normalizes_to_canonical_value() { + let normalized = decode(&hex!("00 00 80 01")); + let canonical = Mpint::from_bytes(&hex!("00 80 01")).unwrap(); + assert_eq!(normalized.as_bytes(), canonical.as_bytes()); + assert_eq!( + normalized.as_positive_bytes().unwrap(), + canonical.as_positive_bytes().unwrap() + ); + } + #[test] fn decode_9a378f9b2e332a7() { assert!(Mpint::from_bytes(&hex!("09 a3 78 f9 b2 e3 32 a7")).is_ok()); diff --git a/ssh-key/tests/public_key.rs b/ssh-key/tests/public_key.rs index 3e77739..585395f 100644 --- a/ssh-key/tests/public_key.rs +++ b/ssh-key/tests/public_key.rs @@ -454,3 +454,49 @@ fn write_openssh_file() { key.write_openssh_file(&path).unwrap(); assert_eq!(example_key, fs::read_to_string(&path).unwrap()); } + +/// Non-canonical `mpint` encodings inside a host key are normalized, not rejected. +/// +/// Some SSH servers (e.g. older Huawei devices) send redundant leading zero bytes in +/// the RSA modulus/exponent of their `ssh-rsa` host keys. OpenSSH trims leading zeros +/// when reading a peer's message, so such keys must remain parseable. +#[cfg(feature = "alloc")] +#[test] +fn rsa_non_canonical_mpint_is_normalized() { + use ssh_key::encoding::Encode; + use ssh_key::public::KeyData; + + /// Encode an `ssh-rsa` public key blob from raw `mpint` payloads. + fn rsa_blob(e: &[u8], n: &[u8]) -> Vec { + let mut blob = Vec::new(); + "ssh-rsa".encode(&mut blob).unwrap(); + Vec::from(e).encode(&mut blob).unwrap(); + Vec::from(n).encode(&mut blob).unwrap(); + blob + } + + // e = 0x010001, n = 0x8001, canonically encoded. + let canonical = rsa_blob(&hex!("01 00 01"), &hex!("00 80 01")); + + // The same values, with redundant leading zero bytes prepended. + let non_canonical = rsa_blob(&hex!("00 01 00 01"), &hex!("00 00 80 01")); + + let key = + PublicKey::from_bytes(&non_canonical).expect("non-canonical mpint should be accepted"); + assert_eq!(key.algorithm(), Algorithm::Rsa { hash: None }); + + let KeyData::Rsa(rsa) = key.key_data() else { + panic!("expected an RSA public key"); + }; + + // The redundant leading zeroes are stripped, the value is preserved. + assert_eq!(rsa.e().as_positive_bytes().unwrap(), &hex!("01 00 01")); + assert_eq!(rsa.n().as_positive_bytes().unwrap(), &hex!("80 01")); + + // A normalized key is indistinguishable from its canonical encoding. + let canonical_key = PublicKey::from_bytes(&canonical).unwrap(); + assert_eq!( + key.to_openssh().unwrap(), + canonical_key.to_openssh().unwrap() + ); +}