ECDSA signatures and public keys in CTAP (#358)

* ECDSA signatures and public keys in CTAP

* adds one constant usage

* documents pub functions in ECDSA

* typo: involved

* extends wrong length test

libraries/crypto/src/ec/point.rs

@@ -120,7 +120,6 @@ impl PointP256 {
     }
 
     // Computes n1*G + n2*self
-    #[cfg(feature = "std")]
     pub fn points_mul(&self, n1: &ExponentP256, n2: &ExponentP256) -> PointP256 {
         let p = self.to_affine();
         let p1 = PointProjective::scalar_base_mul(n1);

libraries/crypto/src/ecdsa.rs

@@ -21,11 +21,9 @@ use super::rng256::Rng256;
 use super::{Hash256, HashBlockSize64Bytes};
 use alloc::vec;
 use alloc::vec::Vec;
-#[cfg(test)]
-use arrayref::array_mut_ref;
 #[cfg(feature = "std")]
-use arrayref::array_ref;
+use arrayref::array_mut_ref;
-use arrayref::mut_array_refs;
+use arrayref::{array_ref, mut_array_refs};
 use core::marker::PhantomData;
 
 pub const NBYTES: usize = int256::NBYTES;
@@ -150,6 +148,7 @@ impl SecKey {
         }
     }
 
+    /// Creates a private key from the exponent's bytes, or None if checks fail.
     pub fn from_bytes(bytes: &[u8; 32]) -> Option<SecKey> {
         let k = NonZeroExponentP256::from_int_checked(Int256::from_bin(bytes));
         // The branching here is fine because all this reveals is whether the key was invalid.
@@ -160,12 +159,16 @@ impl SecKey {
         Some(SecKey { k })
     }
 
+    /// Writes a private key's exponent's bytes to the passed in array.
     pub fn to_bytes(&self, bytes: &mut [u8; 32]) {
         self.k.to_int().to_bin(bytes);
     }
 }
 
 impl Signature {
+    pub const BYTES_LENGTH: usize = 2 * int256::NBYTES;
+
+    /// Converts a signature to its ASN1 DER representation.
     pub fn to_asn1_der(&self) -> Vec<u8> {
         const DER_INTEGER_TYPE: u8 = 0x02;
         const DER_DEF_LENGTH_SEQUENCE: u8 = 0x30;
@@ -193,15 +196,12 @@ impl Signature {
         encoding
     }
 
-    #[cfg(feature = "std")]
+    /// Creates a signature from the exponents' bytes, or None if checks fail.
-    pub fn from_bytes(bytes: &[u8]) -> Option<Signature> {
+    pub fn from_bytes(bytes: &[u8; Signature::BYTES_LENGTH]) -> Option<Signature> {
-        if bytes.len() != 64 {
+        let r_bytes_ref = array_ref![bytes, 0, int256::NBYTES];
-            None
+        let r = NonZeroExponentP256::from_int_checked(Int256::from_bin(r_bytes_ref));
-        } else {
+        let s_bytes_ref = array_ref![bytes, int256::NBYTES, int256::NBYTES];
-            let r =
+        let s = NonZeroExponentP256::from_int_checked(Int256::from_bin(s_bytes_ref));
-                NonZeroExponentP256::from_int_checked(Int256::from_bin(array_ref![bytes, 0, 32]));
-            let s =
-                NonZeroExponentP256::from_int_checked(Int256::from_bin(array_ref![bytes, 32, 32]));
         if bool::from(r.is_none()) || bool::from(s.is_none()) {
             return None;
         }
@@ -209,12 +209,15 @@ impl Signature {
         let s = s.unwrap();
         Some(Signature { r, s })
     }
-    }
 
-    #[cfg(test)]
+    #[cfg(feature = "std")]
-    fn to_bytes(&self, bytes: &mut [u8; 64]) {
+    pub fn to_bytes(&self, bytes: &mut [u8; Signature::BYTES_LENGTH]) {
-        self.r.to_int().to_bin(array_mut_ref![bytes, 0, 32]);
+        self.r
-        self.s.to_int().to_bin(array_mut_ref![bytes, 32, 32]);
+            .to_int()
+            .to_bin(array_mut_ref![bytes, 0, int256::NBYTES]);
+        self.s
+            .to_int()
+            .to_bin(array_mut_ref![bytes, int256::NBYTES, int256::NBYTES]);
     }
 }
 
@@ -222,6 +225,12 @@ impl PubKey {
     #[cfg(feature = "with_ctap1")]
     const UNCOMPRESSED_LENGTH: usize = 1 + 2 * int256::NBYTES;
 
+    /// Creates a new PubKey from its coordinates on the elliptic curve.
+    pub fn from_coordinates(x: &[u8; NBYTES], y: &[u8; NBYTES]) -> Option<PubKey> {
+        PointP256::new_checked_vartime(Int256::from_bin(x), Int256::from_bin(y))
+            .map(|p| PubKey { p })
+    }
+
     #[cfg(feature = "std")]
     pub fn from_bytes_uncompressed(bytes: &[u8]) -> Option<PubKey> {
         PointP256::from_bytes_uncompressed_vartime(bytes).map(|p| PubKey { p })
@@ -252,12 +261,12 @@ impl PubKey {
         self.p.gety().to_int().to_bin(y);
     }
 
-    #[cfg(feature = "std")]
+    /// Verifies if the data's hash matches its signature.
-    pub fn verify_vartime<H>(&self, msg: &[u8], sign: &Signature) -> bool
+    ///
-    where
+    /// This function is not a constant time implementation, and does not resist side channel
-        H: Hash256,
+    /// attacks. Only use if all data involved is public knowledge.
-    {
+    pub fn verify_hash_vartime(&self, hash: &[u8; NBYTES], sign: &Signature) -> bool {
-        let m = ExponentP256::modn(Int256::from_bin(&H::hash(msg)));
+        let m = ExponentP256::modn(Int256::from_bin(hash));
 
         let v = sign.s.inv();
         let u = &m * v.as_exponent();
@@ -267,6 +276,14 @@ impl PubKey {
 
         ExponentP256::modn(u.to_int()) == *sign.r.as_exponent()
     }
+
+    #[cfg(feature = "std")]
+    pub fn verify_vartime<H>(&self, msg: &[u8], sign: &Signature) -> bool
+    where
+        H: Hash256,
+    {
+        self.verify_hash_vartime(&H::hash(msg), sign)
+    }
 }
 
 struct Rfc6979<H>
@@ -442,6 +459,21 @@ mod test {
         test_rfc6979(msg, k, r, s);
     }
 
+    /** Tests that sign and verify hashes are consistent **/
+    // Test that signed message hashes are correctly verified.
+    #[test]
+    fn test_sign_rfc6979_verify_hash_random() {
+        let mut rng = ThreadRng256 {};
+
+        for _ in 0..ITERATIONS {
+            let msg = rng.gen_uniform_u8x32();
+            let sk = SecKey::gensk(&mut rng);
+            let pk = sk.genpk();
+            let sign = sk.sign_rfc6979::<Sha256>(&msg);
+            assert!(pk.verify_hash_vartime(&Sha256::hash(&msg), &sign));
+        }
+    }
+
     /** Tests that sign and verify are consistent **/
     // Test that signed messages are correctly verified.
     #[test]
@@ -537,7 +569,8 @@ mod test {
             let sig_bytes = sig.as_ref();
 
             let pk = PubKey::from_bytes_uncompressed(public_key_bytes).unwrap();
-            let sign = Signature::from_bytes(sig_bytes).unwrap();
+            let sign =
+                Signature::from_bytes(array_ref![sig_bytes, 0, Signature::BYTES_LENGTH]).unwrap();
             assert!(pk.verify_vartime::<Sha256>(&msg_bytes, &sign));
         }
     }

src/ctap/data_formats.rs

@@ -17,6 +17,7 @@ use alloc::string::String;
 use alloc::vec::Vec;
 use arrayref::array_ref;
 use core::convert::TryFrom;
+use core::fmt;
 use crypto::{ecdh, ecdsa};
 #[cfg(test)]
 use enum_iterator::IntoEnumIterator;
@@ -722,12 +723,18 @@ impl TryFrom<cbor::Value> for CoseKey {
             } = extract_map(cbor_value)?;
         }
 
+        let algorithm = extract_integer(ok_or_missing(algorithm)?)?;
+        let nbytes = match algorithm {
+            CoseKey::ECDH_ALGORITHM => ecdh::NBYTES,
+            ES256_ALGORITHM => ecdsa::NBYTES,
+            _ => return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM),
+        };
         let x_bytes = extract_byte_string(ok_or_missing(x_bytes)?)?;
-        if x_bytes.len() != ecdh::NBYTES {
+        if x_bytes.len() != nbytes {
             return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
         }
         let y_bytes = extract_byte_string(ok_or_missing(y_bytes)?)?;
-        if y_bytes.len() != ecdh::NBYTES {
+        if y_bytes.len() != nbytes {
             return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
         }
         let curve = extract_integer(ok_or_missing(curve)?)?;
@@ -738,10 +745,6 @@ impl TryFrom<cbor::Value> for CoseKey {
         if key_type != CoseKey::EC2_KEY_TYPE {
             return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM);
         }
-        let algorithm = extract_integer(ok_or_missing(algorithm)?)?;
-        if algorithm != CoseKey::ECDH_ALGORITHM && algorithm != ES256_ALGORITHM {
-            return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM);
-        }
 
         Ok(CoseKey {
             x_bytes: *array_ref![x_bytes.as_slice(), 0, ecdh::NBYTES],
@@ -817,6 +820,87 @@ impl TryFrom<CoseKey> for ecdh::PubKey {
     }
 }
 
+impl TryFrom<CoseKey> for ecdsa::PubKey {
+    type Error = Ctap2StatusCode;
+
+    fn try_from(cose_key: CoseKey) -> Result<Self, Ctap2StatusCode> {
+        let CoseKey {
+            x_bytes,
+            y_bytes,
+            algorithm,
+        } = cose_key;
+
+        if algorithm != ES256_ALGORITHM {
+            return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM);
+        }
+        ecdsa::PubKey::from_coordinates(&x_bytes, &y_bytes)
+            .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)
+    }
+}
+
+/// Data structure for receiving a signature.
+///
+/// See https://datatracker.ietf.org/doc/html/rfc8152#appendix-C.1.1 for reference.
+///
+/// TODO derive Debug and PartialEq with compiler version 1.47
+#[derive(Clone)]
+pub struct CoseSignature {
+    pub algorithm: SignatureAlgorithm,
+    pub bytes: [u8; ecdsa::Signature::BYTES_LENGTH],
+}
+
+impl fmt::Debug for CoseSignature {
+    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        formatter
+            .debug_struct("CoseSignature")
+            .field("algorithm", &self.algorithm)
+            .field("bytes", &self.bytes.to_vec())
+            .finish()
+    }
+}
+
+impl PartialEq for CoseSignature {
+    fn eq(&self, other: &CoseSignature) -> bool {
+        self.algorithm == other.algorithm && self.bytes[..] == other.bytes[..]
+    }
+}
+
+impl TryFrom<cbor::Value> for CoseSignature {
+    type Error = Ctap2StatusCode;
+
+    fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
+        destructure_cbor_map! {
+            let {
+                "alg" => algorithm,
+                "signature" => bytes,
+            } = extract_map(cbor_value)?;
+        }
+
+        let algorithm = SignatureAlgorithm::try_from(ok_or_missing(algorithm)?)?;
+        let bytes = extract_byte_string(ok_or_missing(bytes)?)?;
+        if bytes.len() != ecdsa::Signature::BYTES_LENGTH {
+            return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
+        }
+
+        Ok(CoseSignature {
+            algorithm,
+            bytes: *array_ref![bytes.as_slice(), 0, ecdsa::Signature::BYTES_LENGTH],
+        })
+    }
+}
+
+impl TryFrom<CoseSignature> for ecdsa::Signature {
+    type Error = Ctap2StatusCode;
+
+    fn try_from(cose_signature: CoseSignature) -> Result<Self, Ctap2StatusCode> {
+        match cose_signature.algorithm {
+            SignatureAlgorithm::ES256 => ecdsa::Signature::from_bytes(&cose_signature.bytes)
+                .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER),
+            SignatureAlgorithm::Unknown => Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM),
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum PinUvAuthProtocol {
     V1 = 1,
@@ -1147,6 +1231,7 @@ mod test {
         cbor_text, cbor_unsigned,
     };
     use crypto::rng256::{Rng256, ThreadRng256};
+    use crypto::sha256::Sha256;
 
     #[test]
     fn test_extract_unsigned() {
@@ -1814,6 +1899,64 @@ mod test {
         assert_eq!(cose_key.algorithm, ES256_ALGORITHM);
     }
 
+    #[test]
+    fn test_from_into_cose_signature() {
+        let mut rng = ThreadRng256 {};
+        let sk = crypto::ecdsa::SecKey::gensk(&mut rng);
+        let dummy_signature = sk.sign_rfc6979::<Sha256>(&[]);
+        let mut bytes = [0; ecdsa::Signature::BYTES_LENGTH];
+        dummy_signature.to_bytes(&mut bytes);
+        let cbor_value = cbor_map! {
+            "alg" => ES256_ALGORITHM,
+            "signature" => bytes,
+        };
+        let cose_signature = CoseSignature::try_from(cbor_value).unwrap();
+        let created_signature = crypto::ecdsa::Signature::try_from(cose_signature).unwrap();
+        let mut created_bytes = [0; ecdsa::Signature::BYTES_LENGTH];
+        created_signature.to_bytes(&mut created_bytes);
+        assert_eq!(bytes[..], created_bytes[..]);
+    }
+
+    #[test]
+    fn test_cose_signature_wrong_algorithm() {
+        let mut rng = ThreadRng256 {};
+        let sk = crypto::ecdsa::SecKey::gensk(&mut rng);
+        let dummy_signature = sk.sign_rfc6979::<Sha256>(&[]);
+        let mut bytes = [0; ecdsa::Signature::BYTES_LENGTH];
+        dummy_signature.to_bytes(&mut bytes);
+        let cbor_value = cbor_map! {
+            "alg" => -1, // unused algorithm
+            "signature" => bytes,
+        };
+        let cose_signature = CoseSignature::try_from(cbor_value).unwrap();
+        let created_signature = crypto::ecdsa::Signature::try_from(cose_signature);
+        // Can not compare directly, since ecdsa::Signature does not implement Debug.
+        assert_eq!(
+            created_signature.err(),
+            Some(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM)
+        );
+    }
+
+    #[test]
+    fn test_cose_signature_wrong_signature_length() {
+        let cbor_value = cbor_map! {
+            "alg" => ES256_ALGORITHM,
+            "signature" => [0; ecdsa::Signature::BYTES_LENGTH - 1],
+        };
+        assert_eq!(
+            CoseSignature::try_from(cbor_value),
+            Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)
+        );
+        let cbor_value = cbor_map! {
+            "alg" => ES256_ALGORITHM,
+            "signature" => [0; ecdsa::Signature::BYTES_LENGTH + 1],
+        };
+        assert_eq!(
+            CoseSignature::try_from(cbor_value),
+            Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER)
+        );
+    }
+
     #[test]
     fn test_from_pin_uv_auth_protocol() {
         let cbor_protocol: cbor::Value = cbor_int!(0x01);

src/ctap/mod.rs

@@ -741,14 +741,11 @@ where
                 .attestation_certificate()?
                 .ok_or(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR)?;
             (
-                attestation_key.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data),
+                attestation_key.sign_rfc6979::<Sha256>(&signature_data),
                 Some(vec![attestation_certificate]),
             )
         } else {
-            (
+            (sk.sign_rfc6979::<Sha256>(&signature_data), None)
-                sk.sign_rfc6979::<crypto::sha256::Sha256>(&signature_data),
-                None,
-            )
         };
         let attestation_statement = PackedAttestationStatement {
             alg: SignatureAlgorithm::ES256 as i64,
@@ -829,7 +826,7 @@ where
         signature_data.extend(client_data_hash);
         let signature = credential
             .private_key
-            .sign_rfc6979::<crypto::sha256::Sha256>(&signature_data);
+            .sign_rfc6979::<Sha256>(&signature_data);
 
         let cred_desc = PublicKeyCredentialDescriptor {
             key_type: PublicKeyCredentialType::PublicKey,