diff --git a/ssh-key/src/private.rs b/ssh-key/src/private.rs index 311a1fe..8a85018 100644 --- a/ssh-key/src/private.rs +++ b/ssh-key/src/private.rs @@ -176,7 +176,7 @@ const DEFAULT_RSA_KEY_SIZE: usize = 4096; const MAX_BLOCK_SIZE: usize = 16; /// Padding bytes to use. -const PADDING_BYTES: [u8; MAX_BLOCK_SIZE - 1] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15]; +const PADDING_BYTES: [u8; MAX_BLOCK_SIZE] = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16]; /// Unix file permissions for SSH private keys. #[cfg(all(unix, feature = "std"))] @@ -354,10 +354,12 @@ impl PrivateKey { let mut buffer = Zeroizing::new(ciphertext.to_vec()); self.cipher.decrypt(&key, &iv, &mut buffer, self.auth_tag)?; + #[allow(clippy::arithmetic_side_effects)] // block sizes are constants Self::decode_privatekey_comment_pair( &mut &**buffer, self.public_key.key_data.clone(), self.cipher.block_size(), + self.cipher.block_size() - 1, ) } @@ -548,8 +550,10 @@ impl PrivateKey { reader: &mut impl Reader, public_key: public::KeyData, block_size: usize, + max_padding_size: usize, ) -> Result { debug_assert!(block_size <= MAX_BLOCK_SIZE); + debug_assert!(max_padding_size <= MAX_BLOCK_SIZE); // Ensure input data is padding-aligned if reader.remaining_len().checked_rem(block_size) != Some(0) { @@ -575,7 +579,7 @@ impl PrivateKey { let padding_len = reader.remaining_len(); - if padding_len >= block_size { + if padding_len > max_padding_size { return Err(encoding::Error::Length.into()); } @@ -733,7 +737,25 @@ impl Decode for PrivateKey { } reader.read_prefixed(|reader| { - Self::decode_privatekey_comment_pair(reader, public_key, cipher.block_size()) + // PuTTYgen uses a non-standard block size of 16 + // and _always_ adds a padding even if data length + // is divisible by 16 - for unencrypted keys + // in the OpenSSH format. + // We're only relaxing the exact length check, but will + // still validate that the contents of the padding area. + // In all other cases there can be up to (but not including) + // `block_size` padding bytes as per `PROTOCOL.key`. + let max_padding_size = match cipher { + Cipher::None => 16, + #[allow(clippy::arithmetic_side_effects)] // block sizes are constants + _ => cipher.block_size() - 1, + }; + Self::decode_privatekey_comment_pair( + reader, + public_key, + cipher.block_size(), + max_padding_size, + ) }) } } diff --git a/ssh-key/tests/dot_ssh.rs b/ssh-key/tests/dot_ssh.rs index e5ce090..13d4b85 100644 --- a/ssh-key/tests/dot_ssh.rs +++ b/ssh-key/tests/dot_ssh.rs @@ -20,7 +20,7 @@ fn path_round_trip() { #[test] fn private_keys() { let dot_ssh = dot_ssh(); - assert_eq!(dot_ssh.private_keys().unwrap().count(), 20); + assert_eq!(dot_ssh.private_keys().unwrap().count(), 21); } #[test] diff --git a/ssh-key/tests/examples/puttygen_overpadded b/ssh-key/tests/examples/puttygen_overpadded new file mode 100644 index 0000000..56f1bfa --- /dev/null +++ b/ssh-key/tests/examples/puttygen_overpadded @@ -0,0 +1,8 @@ +-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtz +c2gtZWQyNTUxOQAAACA+af4QkC9p+OQHgC8EQ1xT+4Ykkf0SYPmEF85tb57WMwAA +ALBRB2JGUQdiRgAAAAtzc2gtZWQyNTUxOQAAACA+af4QkC9p+OQHgC8EQ1xT+4Yk +kf0SYPmEF85tb57WMwAAAEBGxdSjfrbFQ17/N6WcP1EmN6ymf3qRR3NGSGh6zCtm +JD5p/hCQL2n45AeALwRDXFP7hiSR/RJg+YQXzm1vntYzAAAAHWVkZHNhLWtleS0y +MDI0MTIyN2ExMjM0NTY3ODkwAQIDBAUGBwgJCgsMDQ4PEA== +-----END OPENSSH PRIVATE KEY----- diff --git a/ssh-key/tests/private_key.rs b/ssh-key/tests/private_key.rs index a6b3eb2..abfb83e 100644 --- a/ssh-key/tests/private_key.rs +++ b/ssh-key/tests/private_key.rs @@ -54,6 +54,11 @@ const OPENSSH_OPAQUE_EXAMPLE: &str = include_str!("examples/id_opaque"); #[cfg(feature = "ecdsa")] const OPENSSH_PADLESS_WONDER_EXAMPLE: &str = include_str!("examples/padless_wonder"); +/// OpenSSH-formatted private key generated by PuTTYgen that showcases its +/// incorrect 16-byte "block size" +#[cfg(feature = "ed25519")] +const OPENSSH_OVERPADDED_PUTTYGEN_EXAMPLE: &str = include_str!("examples/puttygen_overpadded"); + /// Get a path into the `tests/scratch` directory. #[cfg(feature = "std")] pub fn scratch_path(filename: &str) -> PathBuf { @@ -155,6 +160,19 @@ fn decode_padless_wonder_openssh() { assert_eq!("", key.comment()); } +#[cfg(feature = "ed25519")] +#[test] +fn decode_overpadded_puttygen_openssh() { + let key = PrivateKey::from_openssh(OPENSSH_OVERPADDED_PUTTYGEN_EXAMPLE).unwrap(); + assert_eq!(Algorithm::Ed25519, key.algorithm()); + assert_eq!(Cipher::None, key.cipher()); + assert_eq!(KdfAlg::None, key.kdf().algorithm()); + assert!(key.kdf().is_none()); + + #[cfg(feature = "alloc")] + assert_eq!("eddsa-key-20241227a1234567890", key.comment()); +} + #[cfg(feature = "ecdsa")] #[test] fn decode_ecdsa_p384_openssh() {