From 663f5823826ec9f37f2de785a1c315cf4dd8800d Mon Sep 17 00:00:00 2001 From: Piotr Zaczkowski Date: Tue, 15 Apr 2025 13:39:28 +0200 Subject: [PATCH 1/7] PoC: decode ecdsa key when encoded key size is smaller than expected default --- ssh-key/src/private/ecdsa.rs | 11 +++++++++-- ssh-key/tests/private_key.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/ssh-key/src/private/ecdsa.rs b/ssh-key/src/private/ecdsa.rs index 5b64d7ff..de104c5a 100644 --- a/ssh-key/src/private/ecdsa.rs +++ b/ssh-key/src/private/ecdsa.rs @@ -39,7 +39,8 @@ impl Decode for EcdsaPrivateKey { fn decode(reader: &mut impl Reader) -> Result { reader.read_prefixed(|reader| { - if reader.remaining_len() == SIZE.checked_add(1).ok_or(encoding::Error::Length)? { + let len = reader.remaining_len(); + if len == SIZE.checked_add(1).ok_or(encoding::Error::Length)? { // Strip leading zero // TODO(tarcieri): make sure leading zero was necessary if u8::decode(reader)? != 0 { @@ -48,7 +49,13 @@ impl Decode for EcdsaPrivateKey { } let mut bytes = [0u8; SIZE]; - reader.read(&mut bytes)?; + if len < SIZE { + let mut temp = vec![0u8; len]; + reader.read(&mut temp)?; + bytes[0..len].copy_from_slice(&temp); + } else { + reader.read(&mut bytes)?; + } Ok(Self { bytes }) }) } diff --git a/ssh-key/tests/private_key.rs b/ssh-key/tests/private_key.rs index 912b7505..611f9916 100644 --- a/ssh-key/tests/private_key.rs +++ b/ssh-key/tests/private_key.rs @@ -713,3 +713,30 @@ fn encoding_integration_test(private_key: PrivateKey) { // Ensure ssh-keygen successfully parsed our public key assert_eq!(&public_key, private_key.public_key()); } + +#[test] +fn paramiko_ecdsa_key() { + let key_data = r#"-----BEGIN OPENSSH PRIVATE KEY----- +b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAArAAAABNlY2RzYS +1zaGEyLW5pc3RwNTIxAAAACG5pc3RwNTIxAAAAhQQBf1mOQFzzGhb+5d4dEzPRbrHrrV+G +ODWR5lOkdlhXpljXb/aPfFGD1RcsrD+WMo510xKHUN4MblVCa//W24EAwbAApVKdO2tte1 +72+L2JfNuaJWTU+7QfSWnCDqNFg+XIQSL3UN8nbgOl8Uqd+vF/Z6NqIhyVACLmsm5h3Z3c +blUsh0gAAAEQpYZBIaWGQSEAAAATZWNkc2Etc2hhMi1uaXN0cDUyMQAAAAhuaXN0cDUyMQ +AAAIUEAX9ZjkBc8xoW/uXeHRMz0W6x661fhjg1keZTpHZYV6ZY12/2j3xRg9UXLKw/ljKO +ddMSh1DeDG5VQmv/1tuBAMGwAKVSnTtrbXte9vi9iXzbmiVk1Pu0H0lpwg6jRYPlyEEi91 +DfJ24DpfFKnfrxf2ejaiIclQAi5rJuYd2d3G5VLIdIAAAAQQ9lo6iDFKZNqcdQtVY5teUy +2uAhY8gEm4tacIWp4k+PLPuz7l7fLe+V9JgZ6D32zoaVskogcJOXKw5fdF0D7VDUAAAAE3 +phY3prb3dzQGwtemFjemtvd3M= +-----END OPENSSH PRIVATE KEY-----"#; + + let key = PrivateKey::from_openssh(key_data); + assert!(key.is_ok()); + let key = key.unwrap(); + assert!(!key.is_encrypted()); + assert_eq!( + key.algorithm(), + Algorithm::Ecdsa { + curve: ssh_key::EcdsaCurve::NistP521 + } + ); +} From a7386bb2c970d6b72e90e83b7b8070553a7b3ab8 Mon Sep 17 00:00:00 2001 From: Piotr Zaczkowski Date: Tue, 15 Apr 2025 14:05:58 +0200 Subject: [PATCH 2/7] Simplify --- ssh-key/src/private/ecdsa.rs | 4 +--- ssh-key/tests/private_key.rs | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ssh-key/src/private/ecdsa.rs b/ssh-key/src/private/ecdsa.rs index de104c5a..59c1530e 100644 --- a/ssh-key/src/private/ecdsa.rs +++ b/ssh-key/src/private/ecdsa.rs @@ -50,9 +50,7 @@ impl Decode for EcdsaPrivateKey { let mut bytes = [0u8; SIZE]; if len < SIZE { - let mut temp = vec![0u8; len]; - reader.read(&mut temp)?; - bytes[0..len].copy_from_slice(&temp); + reader.read(&mut bytes[0..len])?; } else { reader.read(&mut bytes)?; } diff --git a/ssh-key/tests/private_key.rs b/ssh-key/tests/private_key.rs index 611f9916..22cc5bcb 100644 --- a/ssh-key/tests/private_key.rs +++ b/ssh-key/tests/private_key.rs @@ -714,6 +714,7 @@ fn encoding_integration_test(private_key: PrivateKey) { assert_eq!(&public_key, private_key.public_key()); } +#[cfg(all(feature = "alloc", feature = "p521"))] #[test] fn paramiko_ecdsa_key() { let key_data = r#"-----BEGIN OPENSSH PRIVATE KEY----- From de74cb6af0298cbdcef084550cd267d7ab46f11c Mon Sep 17 00:00:00 2001 From: Piotr Zaczkowski Date: Wed, 16 Apr 2025 21:21:56 +0200 Subject: [PATCH 3/7] Simplify --- ssh-key/src/private/ecdsa.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ssh-key/src/private/ecdsa.rs b/ssh-key/src/private/ecdsa.rs index 59c1530e..21bb6a1a 100644 --- a/ssh-key/src/private/ecdsa.rs +++ b/ssh-key/src/private/ecdsa.rs @@ -49,11 +49,7 @@ impl Decode for EcdsaPrivateKey { } let mut bytes = [0u8; SIZE]; - if len < SIZE { - reader.read(&mut bytes[0..len])?; - } else { - reader.read(&mut bytes)?; - } + reader.read(&mut bytes[0..std::cmp::min(len, SIZE)])?; Ok(Self { bytes }) }) } From b858b508ac8cb79b5708e9fd0379bac6e737cd9d Mon Sep 17 00:00:00 2001 From: Piotr Zaczkowski Date: Wed, 16 Apr 2025 21:23:37 +0200 Subject: [PATCH 4/7] Ups --- ssh-key/src/private/ecdsa.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssh-key/src/private/ecdsa.rs b/ssh-key/src/private/ecdsa.rs index 21bb6a1a..15b71b7c 100644 --- a/ssh-key/src/private/ecdsa.rs +++ b/ssh-key/src/private/ecdsa.rs @@ -49,7 +49,7 @@ impl Decode for EcdsaPrivateKey { } let mut bytes = [0u8; SIZE]; - reader.read(&mut bytes[0..std::cmp::min(len, SIZE)])?; + reader.read(&mut bytes[0..std::cmp::min(len, SIZE)])?; Ok(Self { bytes }) }) } From 67454aba58ee198c65a222108d3987a035fbd505 Mon Sep 17 00:00:00 2001 From: Piotr Zaczkowski Date: Wed, 16 Apr 2025 21:26:05 +0200 Subject: [PATCH 5/7] 0 is superfluous --- ssh-key/src/private/ecdsa.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssh-key/src/private/ecdsa.rs b/ssh-key/src/private/ecdsa.rs index 15b71b7c..4aada02d 100644 --- a/ssh-key/src/private/ecdsa.rs +++ b/ssh-key/src/private/ecdsa.rs @@ -49,7 +49,7 @@ impl Decode for EcdsaPrivateKey { } let mut bytes = [0u8; SIZE]; - reader.read(&mut bytes[0..std::cmp::min(len, SIZE)])?; + reader.read(&mut bytes[..std::cmp::min(len, SIZE)])?; Ok(Self { bytes }) }) } From 624f4260d4f947be0b7aee14bbbf4f43b6f04363 Mon Sep 17 00:00:00 2001 From: Piotr Zaczkowski Date: Wed, 16 Apr 2025 21:32:58 +0200 Subject: [PATCH 6/7] std is not needed obviously --- ssh-key/src/private/ecdsa.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssh-key/src/private/ecdsa.rs b/ssh-key/src/private/ecdsa.rs index 4aada02d..66e831c5 100644 --- a/ssh-key/src/private/ecdsa.rs +++ b/ssh-key/src/private/ecdsa.rs @@ -49,7 +49,7 @@ impl Decode for EcdsaPrivateKey { } let mut bytes = [0u8; SIZE]; - reader.read(&mut bytes[..std::cmp::min(len, SIZE)])?; + reader.read(&mut bytes[..core::cmp::min(len, SIZE)])?; Ok(Self { bytes }) }) } From 9f4742a63500f79501d65eae2870f2408ef73ef9 Mon Sep 17 00:00:00 2001 From: Piotr Zaczkowski Date: Fri, 18 Apr 2025 18:08:10 +0200 Subject: [PATCH 7/7] Add a reason for change --- ssh-key/src/private/ecdsa.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ssh-key/src/private/ecdsa.rs b/ssh-key/src/private/ecdsa.rs index 66e831c5..26441815 100644 --- a/ssh-key/src/private/ecdsa.rs +++ b/ssh-key/src/private/ecdsa.rs @@ -49,7 +49,17 @@ impl Decode for EcdsaPrivateKey { } let mut bytes = [0u8; SIZE]; - reader.read(&mut bytes[..core::cmp::min(len, SIZE)])?; + if SIZE == 66 { + // https://stackoverflow.com/questions/50002149/why-p-521-public-key-x-y-some-time-is-65-bytes-some-time-is-66-bytes + // although lower keys than 64 are vanishingly possible, but lets stop here + if len > 63 { + reader.read(&mut bytes[..core::cmp::min(len, SIZE)])?; + } else { + return Err(encoding::Error::Length.into()); + } + } else { + reader.read(&mut bytes)?; + } Ok(Self { bytes }) }) }