From 6f9f833c0b6c6b94cc1a3d3ff75769d130ad0288 Mon Sep 17 00:00:00 2001 From: Fabian Kaczmarczyck Date: Fri, 8 Jan 2021 15:42:35 +0100 Subject: [PATCH] moves COSE related conversion from crypto to data_formats --- libraries/crypto/src/ecdh.rs | 8 +++- libraries/crypto/src/ecdsa.rs | 59 ++++++++++----------------- src/ctap/data_formats.rs | 75 +++++++++++++++++++++++++++-------- src/ctap/mod.rs | 10 ++--- 4 files changed, 88 insertions(+), 64 deletions(-) diff --git a/libraries/crypto/src/ecdh.rs b/libraries/crypto/src/ecdh.rs index c735d11..9645f66 100644 --- a/libraries/crypto/src/ecdh.rs +++ b/libraries/crypto/src/ecdh.rs @@ -62,8 +62,10 @@ impl SecKey { // - https://www.secg.org/sec1-v2.pdf } - // DH key agreement method defined in the FIDO2 specification, Section 5.5.4. "Getting - // sharedSecret from Authenticator" + /// Creates a shared key using the Diffie Hellman key agreement. + /// + /// The key agreement is defined in the FIDO2 specification, + /// Section 6.5.5.4. "Obtaining the Shared Secret" pub fn exchange_x_sha256(&self, other: &PubKey) -> [u8; 32] { let p = self.exchange_raw(other); let mut x: [u8; 32] = [Default::default(); 32]; @@ -83,11 +85,13 @@ impl PubKey { self.p.to_bytes_uncompressed(bytes); } + /// Creates a new PubKey from their coordinates. pub fn from_coordinates(x: &[u8; NBYTES], y: &[u8; NBYTES]) -> Option { PointP256::new_checked_vartime(Int256::from_bin(x), Int256::from_bin(y)) .map(|p| PubKey { p }) } + /// Writes the coordinates into the passed in arrays. pub fn to_coordinates(&self, x: &mut [u8; NBYTES], y: &mut [u8; NBYTES]) { self.p.getx().to_int().to_bin(x); self.p.gety().to_int().to_bin(y); diff --git a/libraries/crypto/src/ecdsa.rs b/libraries/crypto/src/ecdsa.rs index 52949e3..8fef458 100644 --- a/libraries/crypto/src/ecdsa.rs +++ b/libraries/crypto/src/ecdsa.rs @@ -21,12 +21,15 @@ 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, mut_array_refs}; -use cbor::{cbor_bytes, cbor_map_options}; +use arrayref::mut_array_refs; use core::marker::PhantomData; +pub const NBYTES: usize = int256::NBYTES; + #[derive(Clone, PartialEq)] #[cfg_attr(feature = "derive_debug", derive(Debug))] pub struct SecKey { @@ -38,6 +41,7 @@ pub struct Signature { s: NonZeroExponentP256, } +#[cfg_attr(feature = "derive_debug", derive(Clone))] pub struct PubKey { p: PointP256, } @@ -58,10 +62,11 @@ impl SecKey { } } - // ECDSA signature based on a RNG to generate a suitable randomization parameter. - // Under the hood, rejection sampling is used to make sure that the randomization parameter is - // uniformly distributed. - // The provided RNG must be cryptographically secure; otherwise this method is insecure. + /// Creates an ECDSA signature based on a RNG. + /// + /// Under the hood, rejection sampling is used to make sure that the + /// randomization parameter is uniformly distributed. The provided RNG must + /// be cryptographically secure; otherwise this method is insecure. pub fn sign_rng(&self, msg: &[u8], rng: &mut R) -> Signature where H: Hash256, @@ -77,8 +82,7 @@ impl SecKey { } } - // Deterministic ECDSA signature based on RFC 6979 to generate a suitable randomization - // parameter. + /// Creates a deterministic ECDSA signature based on RFC 6979. pub fn sign_rfc6979(&self, msg: &[u8]) -> Signature where H: Hash256 + HashBlockSize64Bytes, @@ -101,8 +105,10 @@ impl SecKey { } } - // Try signing a curve element given a randomization parameter k. If no signature can be - // obtained from this k, None is returned and the caller should try again with another value. + /// Try signing a curve element given a randomization parameter k. + /// + /// If no signature can be obtained from this k, None is returned and the + /// caller should try again with another value. fn try_sign(&self, k: &NonZeroExponentP256, msg: &ExponentP256) -> Option { let r = ExponentP256::modn(PointP256::base_point_mul(k.as_exponent()).getx().to_int()); // The branching here is fine because all this reveals is that k generated an unsuitable r. @@ -242,35 +248,10 @@ impl PubKey { representation } - // Encodes the key according to CBOR Object Signing and Encryption, defined in RFC 8152. - pub fn to_cose_key(&self) -> Option> { - const EC2_KEY_TYPE: i64 = 2; - const P_256_CURVE: i64 = 1; - let mut x_bytes = vec![0; int256::NBYTES]; - self.p - .getx() - .to_int() - .to_bin(array_mut_ref![x_bytes.as_mut_slice(), 0, int256::NBYTES]); - let x_byte_cbor: cbor::Value = cbor_bytes!(x_bytes); - let mut y_bytes = vec![0; int256::NBYTES]; - self.p - .gety() - .to_int() - .to_bin(array_mut_ref![y_bytes.as_mut_slice(), 0, int256::NBYTES]); - let y_byte_cbor: cbor::Value = cbor_bytes!(y_bytes); - let cbor_value = cbor_map_options! { - 1 => EC2_KEY_TYPE, - 3 => PubKey::ES256_ALGORITHM, - -1 => P_256_CURVE, - -2 => x_byte_cbor, - -3 => y_byte_cbor, - }; - let mut encoded_key = Vec::new(); - if cbor::write(cbor_value, &mut encoded_key) { - Some(encoded_key) - } else { - None - } + /// Writes the coordinates into the passed in arrays. + pub fn to_coordinates(&self, x: &mut [u8; NBYTES], y: &mut [u8; NBYTES]) { + self.p.getx().to_int().to_bin(x); + self.p.gety().to_int().to_bin(y); } #[cfg(feature = "std")] diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index 8081567..ac1fb73 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -18,7 +18,7 @@ use alloc::string::String; use alloc::vec::Vec; use arrayref::array_ref; use cbor::{cbor_array_vec, cbor_bytes_lit, cbor_map_options, destructure_cbor_map}; -use core::convert::TryFrom; +use core::convert::{TryFrom, TryInto}; use crypto::{ecdh, ecdsa}; #[cfg(test)] use enum_iterator::IntoEnumIterator; @@ -631,26 +631,39 @@ const ES256_ALGORITHM: i64 = -7; const EC2_KEY_TYPE: i64 = 2; const P_256_CURVE: i64 = 1; +impl TryFrom for CoseKey { + type Error = Ctap2StatusCode; + + fn try_from(cbor_value: cbor::Value) -> Result { + if let cbor::Value::Map(cose_map) = cbor_value { + Ok(CoseKey(cose_map)) + } else { + Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR) + } + } +} + +fn cose_key_from_bytes(x_bytes: [u8; ecdh::NBYTES], y_bytes: [u8; ecdh::NBYTES]) -> CoseKey { + let x_byte_cbor: cbor::Value = cbor_bytes_lit!(&x_bytes); + let y_byte_cbor: cbor::Value = cbor_bytes_lit!(&y_bytes); + // TODO(kaczmarczyck) do not write optional parameters, spec is unclear + let cose_cbor_value = cbor_map_options! { + 1 => EC2_KEY_TYPE, + 3 => ECDH_ALGORITHM, + -1 => P_256_CURVE, + -2 => x_byte_cbor, + -3 => y_byte_cbor, + }; + // Unwrap is safe here since we know it's a map. + cose_cbor_value.try_into().unwrap() +} + impl From for CoseKey { fn from(pk: ecdh::PubKey) -> Self { let mut x_bytes = [0; ecdh::NBYTES]; let mut y_bytes = [0; ecdh::NBYTES]; pk.to_coordinates(&mut x_bytes, &mut y_bytes); - let x_byte_cbor: cbor::Value = cbor_bytes_lit!(&x_bytes); - let y_byte_cbor: cbor::Value = cbor_bytes_lit!(&y_bytes); - // TODO(kaczmarczyck) do not write optional parameters, spec is unclear - let cose_cbor_value = cbor_map_options! { - 1 => EC2_KEY_TYPE, - 3 => ECDH_ALGORITHM, - -1 => P_256_CURVE, - -2 => x_byte_cbor, - -3 => y_byte_cbor, - }; - if let cbor::Value::Map(cose_map) = cose_cbor_value { - CoseKey(cose_map) - } else { - unreachable!(); - } + cose_key_from_bytes(x_bytes, y_bytes) } } @@ -696,6 +709,15 @@ impl TryFrom for ecdh::PubKey { } } +impl From for CoseKey { + fn from(pk: ecdsa::PubKey) -> Self { + let mut x_bytes = [0; ecdh::NBYTES]; + let mut y_bytes = [0; ecdh::NBYTES]; + pk.to_coordinates(&mut x_bytes, &mut y_bytes); + cose_key_from_bytes(x_bytes, y_bytes) + } +} + #[cfg_attr(any(test, feature = "debug_ctap"), derive(Clone, Debug, PartialEq))] #[cfg_attr(test, derive(IntoEnumIterator))] pub enum ClientPinSubCommand { @@ -1322,7 +1344,7 @@ mod test { } #[test] - fn test_from_into_cose_key() { + fn test_from_into_cose_key_ecdh() { let mut rng = ThreadRng256 {}; let sk = crypto::ecdh::SecKey::gensk(&mut rng); let pk = sk.genpk(); @@ -1331,6 +1353,25 @@ mod test { assert_eq!(created_pk, Ok(pk)); } + #[test] + fn test_into_cose_key_ecdsa() { + let mut rng = ThreadRng256 {}; + let sk = crypto::ecdsa::SecKey::gensk(&mut rng); + let pk = sk.genpk(); + let cose_key = CoseKey::from(pk); + let cose_map = cose_key.0; + let template = cbor_map! { + 1 => 0, + 3 => 0, + -1 => 0, + -2 => 0, + -3 => 0, + }; + for key in CoseKey::try_from(template).unwrap().0.keys() { + assert!(cose_map.contains_key(key)); + } + } + #[test] fn test_from_into_client_pin_sub_command() { let cbor_sub_command: cbor::Value = cbor_int!(0x01); diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index fe60e80..f9229ac 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -31,7 +31,7 @@ use self::command::{ MAX_CREDENTIAL_COUNT_IN_LIST, }; use self::data_formats::{ - AuthenticatorTransport, CredentialProtectionPolicy, GetAssertionHmacSecretInput, + AuthenticatorTransport, CoseKey, CredentialProtectionPolicy, GetAssertionHmacSecretInput, PackedAttestationStatement, PublicKeyCredentialDescriptor, PublicKeyCredentialParameter, PublicKeyCredentialSource, PublicKeyCredentialType, PublicKeyCredentialUserEntity, SignatureAlgorithm, @@ -534,11 +534,9 @@ where } auth_data.extend(vec![0x00, credential_id.len() as u8]); auth_data.extend(&credential_id); - let cose_key = match pk.to_cose_key() { - Some(cose_key) => cose_key, - None => return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR), - }; - auth_data.extend(cose_key); + if !cbor::write(cbor::Value::Map(CoseKey::from(pk).0), &mut auth_data) { + return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); + } if has_extension_output { let hmac_secret_output = if use_hmac_extension { Some(true) } else { None }; let extensions_output = cbor_map_options! {