test(crypto): CRP-928 Add tests of ECDSA invalid public keys

Author: randombit

rs/crypto/ecdsa_secp256k1/src/lib.rs

@@ -334,6 +334,8 @@ impl PublicKey {
     ///
     /// This is just the encoding of the point. Both compressed and uncompressed
     /// points are accepted
+    ///
+    /// See SEC1 <https://www.secg.org/sec1-v2.pdf> section 2.3.3 for details of the format
     pub fn deserialize_sec1(bytes: &[u8]) -> Result<Self, KeyDecodingError> {
         let key = k256::ecdsa::VerifyingKey::from_sec1_bytes(bytes)
             .map_err(|e| KeyDecodingError::InvalidKeyEncoding(format!("{:?}", e)))?;
@@ -362,6 +364,8 @@ impl PublicKey {
     /// Serialize a public key in SEC1 format
     ///
     /// The point can optionally be compressed
+    ///
+    /// See SEC1 <https://www.secg.org/sec1-v2.pdf> section 2.3.3 for details
     pub fn serialize_sec1(&self, compressed: bool) -> Vec<u8> {
         self.key.to_encoded_point(compressed).as_bytes().to_vec()
     }

rs/crypto/ecdsa_secp256k1/tests/tests.rs

@@ -126,6 +126,64 @@ fn should_reject_high_s_in_signature_unless_malleable() -> Result<(), KeyDecodin
     Ok(())
 }
 
+#[test]
+fn should_reject_invalid_public_keys() {
+    struct InvalidKey {
+        reason: &'static str,
+        key: Vec<u8>,
+    }
+
+    impl InvalidKey {
+        fn new(reason: &'static str, key_hex: &'static str) -> Self {
+            let key = hex::decode(key_hex).expect("Invalid key_hex param");
+            Self { reason, key }
+        }
+    }
+
+    let invalid_keys = [
+        InvalidKey::new("empty", ""),
+        InvalidKey::new("too short", "02"),
+        InvalidKey::new(
+            "valid compressed point with uncompressed header",
+            "04F599CDA3A05987498A716E820651AC96A4EEAA3AD9B7D6F244A83CC3381CABC4",
+        ),
+        InvalidKey::new(
+            "invalid x, header 02",
+            "02F599CDA3A05987498A716E820651AC96A4EEAA3AD9B7D6F244A83CC3381CABC3",
+        ),
+        InvalidKey::new(
+            "invalid x, header 03",
+            "03F599CDA3A05987498A716E820651AC96A4EEAA3AD9B7D6F244A83CC3381CABC3",
+        ),
+        InvalidKey::new(
+            "valid uncompressed point with header 02",
+            "02F599CDA3A05987498A716E820651AC96A4EEAA3AD9B7D6F244A83CC3381CABC4C300A1369821A5A86D4D9BA74FF68817C4CAEA4BAC737A7B00A48C4835F28DB4"
+        ),
+        InvalidKey::new(
+            "valid uncompressed point with header 03",
+            "03F599CDA3A05987498A716E820651AC96A4EEAA3AD9B7D6F244A83CC3381CABC4C300A1369821A5A86D4D9BA74FF68817C4CAEA4BAC737A7B00A48C4835F28DB4"
+        ),
+        InvalidKey::new(
+            "invalid uncompressed point (y off by one)",
+            "04F599CDA3A05987498A716E820651AC96A4EEAA3AD9B7D6F244A83CC3381CABC4C300A1369821A5A86D4D9BA74FF68817C4CAEA4BAC737A7B00A48C4835F28DB5"
+        ),
+        InvalidKey::new(
+            "valid P256 point",
+            "04EB2D21CD969E68C767B091E91900863E7699826C3466F15B956BBB6CBAEDB09A5A16ED621975EC1BCB81A41EE5DCF719021B12A95CC858A735A266135EFD2E4E"
+        ),
+    ];
+
+    for invalid_key in &invalid_keys {
+        let result = PublicKey::deserialize_sec1(&invalid_key.key);
+
+        assert!(
+            result.is_err(),
+            "Accepted invalid key ({})",
+            invalid_key.reason
+        );
+    }
+}
+
 #[test]
 fn should_serialization_and_deserialization_round_trip_for_private_keys(
 ) -> Result<(), KeyDecodingError> {

rs/crypto/ecdsa_secp256r1/src/lib.rs

@@ -318,6 +318,8 @@ impl PublicKey {
     ///
     /// This is just the encoding of the point. Both compressed and uncompressed
     /// points are accepted
+    ///
+    /// See SEC1 <https://www.secg.org/sec1-v2.pdf> section 2.3.3 for details of the format
     pub fn deserialize_sec1(bytes: &[u8]) -> Result<Self, KeyDecodingError> {
         let key = p256::ecdsa::VerifyingKey::from_sec1_bytes(bytes)
             .map_err(|e| KeyDecodingError::InvalidKeyEncoding(format!("{:?}", e)))?;
@@ -346,6 +348,8 @@ impl PublicKey {
     /// Serialize a public key in SEC1 format
     ///
     /// The point can optionally be compressed
+    ///
+    /// See SEC1 <https://www.secg.org/sec1-v2.pdf> section 2.3.3 for details of the format
     pub fn serialize_sec1(&self, compressed: bool) -> Vec<u8> {
         self.key.to_encoded_point(compressed).to_bytes().to_vec()
     }

rs/crypto/ecdsa_secp256r1/tests/tests.rs

@@ -161,6 +161,64 @@ fn should_serialization_and_deserialization_round_trip_for_public_keys(
     Ok(())
 }
 
+#[test]
+fn should_reject_invalid_public_keys() {
+    struct InvalidKey {
+        reason: &'static str,
+        key: Vec<u8>,
+    }
+
+    impl InvalidKey {
+        fn new(reason: &'static str, key_hex: &'static str) -> Self {
+            let key = hex::decode(key_hex).expect("Invalid key_hex param");
+            Self { reason, key }
+        }
+    }
+
+    let invalid_keys = [
+        InvalidKey::new("empty", ""),
+        InvalidKey::new("too short", "02"),
+        InvalidKey::new(
+            "valid compressed point with uncompressed header",
+            "04EB2D21CD969E68C767B091E91900863E7699826C3466F15B956BBB6CBAEDB09A",
+        ),
+        InvalidKey::new(
+            "invalid x, header 02",
+            "02EB2D21CD969E68C767B091E91900863E7699826C3466F15B956BBB6CBAEDB09C",
+        ),
+        InvalidKey::new(
+            "invalid x, header 03",
+            "03EB2D21CD969E68C767B091E91900863E7699826C3466F15B956BBB6CBAEDB09C",
+        ),
+        InvalidKey::new(
+            "valid uncompressed point with header 02",
+            "02EB2D21CD969E68C767B091E91900863E7699826C3466F15B956BBB6CBAEDB09A5A16ED621975EC1BCB81A41EE5DCF719021B12A95CC858A735A266135EFD2E4E"
+        ),
+        InvalidKey::new(
+            "valid uncompressed point with header 03",
+            "03EB2D21CD969E68C767B091E91900863E7699826C3466F15B956BBB6CBAEDB09A5A16ED621975EC1BCB81A41EE5DCF719021B12A95CC858A735A266135EFD2E4E"
+        ),
+        InvalidKey::new(
+            "invalid uncompressed point (y off by one)",
+            "04EB2D21CD969E68C767B091E91900863E7699826C3466F15B956BBB6CBAEDB09A5A16ED621975EC1BCB81A41EE5DCF719021B12A95CC858A735A266135EFD2E4F",
+        ),
+        InvalidKey::new(
+            "valid secp256k1 point",
+            "04F599CDA3A05987498A716E820651AC96A4EEAA3AD9B7D6F244A83CC3381CABC4C300A1369821A5A86D4D9BA74FF68817C4CAEA4BAC737A7B00A48C4835F28DB4"
+        ),
+    ];
+
+    for invalid_key in &invalid_keys {
+        let result = PublicKey::deserialize_sec1(&invalid_key.key);
+
+        assert!(
+            result.is_err(),
+            "Accepted invalid key ({})",
+            invalid_key.reason
+        );
+    }
+}
+
 #[test]
 fn should_insecure_keygen_for_testing_be_deterministic() {
     assert_eq!(