From bb69db4874d31eeba7864e0fcb7e7d6e7264a749 Mon Sep 17 00:00:00 2001 From: Arnaud Brousseau Date: Mon, 23 Dec 2024 18:49:53 +0100 Subject: [PATCH 1/3] Restrict P256EncryptPublic::from_bytes to uncompressed format only --- src/qos_p256/src/encrypt.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qos_p256/src/encrypt.rs b/src/qos_p256/src/encrypt.rs index 58621aa7..6799ffeb 100644 --- a/src/qos_p256/src/encrypt.rs +++ b/src/qos_p256/src/encrypt.rs @@ -220,6 +220,13 @@ impl P256EncryptPublic { /// Deserialize from a SEC1 encoded point, not compressed. pub fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > PUB_KEY_LEN_UNCOMPRESSED as usize { + return Err(P256Error::EncodedPublicKeyTooLong); + } + if bytes.len() < PUB_KEY_LEN_UNCOMPRESSED as usize { + return Err(P256Error::EncodedPublicKeyTooShort); + } + Ok(Self { public: PublicKey::from_sec1_bytes(bytes) .map_err(|_| P256Error::FailedToReadPublicKey)?, From 92ffc540d7da176135e3dd4c47d28faedb3b6279 Mon Sep 17 00:00:00 2001 From: Arnaud Brousseau Date: Mon, 23 Dec 2024 18:56:43 +0100 Subject: [PATCH 2/3] Restrict P256SignPublic::from_bytes to uncompressed format only --- src/qos_p256/src/sign.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/qos_p256/src/sign.rs b/src/qos_p256/src/sign.rs index 096899b8..ef58a79a 100644 --- a/src/qos_p256/src/sign.rs +++ b/src/qos_p256/src/sign.rs @@ -7,7 +7,7 @@ use p256::ecdsa::{ use rand_core::OsRng; use zeroize::ZeroizeOnDrop; -use crate::P256Error; +use crate::{P256Error, PUB_KEY_LEN_UNCOMPRESSED}; /// Sign private key pair. #[derive(ZeroizeOnDrop)] @@ -86,6 +86,13 @@ impl P256SignPublic { /// Deserialize from a SEC1 encoded point, not compressed. pub fn from_bytes(bytes: &[u8]) -> Result { + if bytes.len() > PUB_KEY_LEN_UNCOMPRESSED as usize { + return Err(P256Error::EncodedPublicKeyTooLong); + } + if bytes.len() < PUB_KEY_LEN_UNCOMPRESSED as usize { + return Err(P256Error::EncodedPublicKeyTooShort); + } + Ok(Self { public: VerifyingKey::from_sec1_bytes(bytes) .map_err(|_| P256Error::FailedToReadPublicKey)?, From ff696167f5a0af0cf1550ba30ea714fb901e3390 Mon Sep 17 00:00:00 2001 From: Arnaud Brousseau Date: Mon, 23 Dec 2024 22:16:33 +0100 Subject: [PATCH 3/3] Add test coverage to show new behavior --- src/qos_p256/src/encrypt.rs | 24 ++++++++++++++++++++++++ src/qos_p256/src/sign.rs | 24 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/qos_p256/src/encrypt.rs b/src/qos_p256/src/encrypt.rs index 6799ffeb..1d5a6d1f 100644 --- a/src/qos_p256/src/encrypt.rs +++ b/src/qos_p256/src/encrypt.rs @@ -558,6 +558,30 @@ mod test_asymmetric { assert_eq!(decrypted, plaintext); } + #[test] + fn encoded_public_keys_bytes_are_validated() { + let too_short = vec![0u8; 64]; + let too_long = vec![0u8; 66]; + + // Uncompressed public key with a compressed prefix (0x02) + let bad_prefix = qos_hex::decode("02bf772379de68fed2e81a47a141210c827c31fadc5b24ed3dafa84a9d19896172cb1b53ee6ecb38ca5c4be4d664b63f034886b764ad520c301fe542dfdf4002e4").unwrap(); + let just_right = qos_hex::decode("04bf772379de68fed2e81a47a141210c827c31fadc5b24ed3dafa84a9d19896172cb1b53ee6ecb38ca5c4be4d664b63f034886b764ad520c301fe542dfdf4002e4").unwrap(); + + assert!(matches!( + P256EncryptPublic::from_bytes(&too_short), + Err(P256Error::EncodedPublicKeyTooShort) + )); + assert!(matches!( + P256EncryptPublic::from_bytes(&too_long), + Err(P256Error::EncodedPublicKeyTooLong) + )); + assert!(matches!( + P256EncryptPublic::from_bytes(&bad_prefix), + Err(P256Error::FailedToReadPublicKey), + )); + assert!(matches!(P256EncryptPublic::from_bytes(&just_right), Ok(_),)); + } + #[test] fn private_key_roundtrip_bytes() { let pair = P256EncryptPair::generate(); diff --git a/src/qos_p256/src/sign.rs b/src/qos_p256/src/sign.rs index ef58a79a..45224e83 100644 --- a/src/qos_p256/src/sign.rs +++ b/src/qos_p256/src/sign.rs @@ -164,4 +164,28 @@ mod tests { assert_eq!(raw_secret1, raw_secret2); } + + #[test] + fn encoded_public_keys_bytes_are_validated() { + let too_short = vec![0u8; 64]; + let too_long = vec![0u8; 66]; + + // Uncompressed public key with a compressed prefix (0x02) + let bad_prefix = qos_hex::decode("02bf772379de68fed2e81a47a141210c827c31fadc5b24ed3dafa84a9d19896172cb1b53ee6ecb38ca5c4be4d664b63f034886b764ad520c301fe542dfdf4002e4").unwrap(); + let just_right = qos_hex::decode("04bf772379de68fed2e81a47a141210c827c31fadc5b24ed3dafa84a9d19896172cb1b53ee6ecb38ca5c4be4d664b63f034886b764ad520c301fe542dfdf4002e4").unwrap(); + + assert!(matches!( + P256SignPublic::from_bytes(&too_short), + Err(P256Error::EncodedPublicKeyTooShort) + )); + assert!(matches!( + P256SignPublic::from_bytes(&too_long), + Err(P256Error::EncodedPublicKeyTooLong) + )); + assert!(matches!( + P256SignPublic::from_bytes(&bad_prefix), + Err(P256Error::FailedToReadPublicKey), + )); + assert!(matches!(P256SignPublic::from_bytes(&just_right), Ok(_),)); + } }