diff --git a/libraries/crypto/src/ecdh.rs b/libraries/crypto/src/ecdh.rs index 9645f66..a1e3736 100644 --- a/libraries/crypto/src/ecdh.rs +++ b/libraries/crypto/src/ecdh.rs @@ -85,7 +85,7 @@ impl PubKey { self.p.to_bytes_uncompressed(bytes); } - /// Creates a new PubKey from their coordinates. + /// Creates a new PubKey from its coordinates on the elliptic curve. 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 }) diff --git a/libraries/crypto/src/ecdsa.rs b/libraries/crypto/src/ecdsa.rs index 8fef458..b6a1708 100644 --- a/libraries/crypto/src/ecdsa.rs +++ b/libraries/crypto/src/ecdsa.rs @@ -220,7 +220,6 @@ impl Signature { } impl PubKey { - pub const ES256_ALGORITHM: i64 = -7; #[cfg(feature = "with_ctap1")] const UNCOMPRESSED_LENGTH: usize = 1 + 2 * int256::NBYTES; diff --git a/src/ctap/command.rs b/src/ctap/command.rs index 0a86093..6f0aa72 100644 --- a/src/ctap/command.rs +++ b/src/ctap/command.rs @@ -317,7 +317,7 @@ impl TryFrom for AuthenticatorClientPinParameters { let pin_protocol = extract_unsigned(ok_or_missing(pin_protocol)?)?; let sub_command = ClientPinSubCommand::try_from(ok_or_missing(sub_command)?)?; - let key_agreement = key_agreement.map(extract_map).transpose()?.map(CoseKey); + let key_agreement = key_agreement.map(CoseKey::try_from).transpose()?; let pin_auth = pin_auth.map(extract_byte_string).transpose()?; let new_pin_enc = new_pin_enc.map(extract_byte_string).transpose()?; let pin_hash_enc = pin_hash_enc.map(extract_byte_string).transpose()?; @@ -423,8 +423,8 @@ mod test { }; use super::super::ES256_CRED_PARAM; use super::*; - use alloc::collections::BTreeMap; use cbor::{cbor_array, cbor_map}; + use crypto::rng256::ThreadRng256; #[test] fn test_from_cbor_make_credential_parameters() { @@ -534,10 +534,15 @@ mod test { #[test] fn test_from_cbor_client_pin_parameters() { + let mut rng = ThreadRng256 {}; + let sk = crypto::ecdh::SecKey::gensk(&mut rng); + let pk = sk.genpk(); + let cose_key = CoseKey::from(pk); + let cbor_value = cbor_map! { 1 => 1, 2 => ClientPinSubCommand::GetPinRetries, - 3 => cbor_map!{}, + 3 => cbor::Value::from(cose_key.clone()), 4 => vec! [0xBB], 5 => vec! [0xCC], 6 => vec! [0xDD], @@ -552,7 +557,7 @@ mod test { let expected_pin_protocol_parameters = AuthenticatorClientPinParameters { pin_protocol: 1, sub_command: ClientPinSubCommand::GetPinRetries, - key_agreement: Some(CoseKey(BTreeMap::new())), + key_agreement: Some(cose_key), pin_auth: Some(vec![0xBB]), new_pin_enc: Some(vec![0xCC]), pin_hash_enc: Some(vec![0xDD]), diff --git a/src/ctap/data_formats.rs b/src/ctap/data_formats.rs index ac1fb73..87bc6b4 100644 --- a/src/ctap/data_formats.rs +++ b/src/ctap/data_formats.rs @@ -17,12 +17,23 @@ use alloc::collections::BTreeMap; 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, TryInto}; +use cbor::{cbor_array_vec, cbor_map, cbor_map_options, destructure_cbor_map}; +use core::convert::TryFrom; use crypto::{ecdh, ecdsa}; #[cfg(test)] use enum_iterator::IntoEnumIterator; +// This is the algorithm specifier that is supposed to be used in a COSE key +// map in ECDH. CTAP requests -25 which represents ECDH-ES + HKDF-256 here: +// https://www.iana.org/assignments/cose/cose.xhtml#algorithms +const ECDH_ALGORITHM: i64 = -25; +// This is the identifier used for ECDSA and ECDH in OpenSSH. +const ES256_ALGORITHM: i64 = -7; +// The COSE key parameter behind map key 1. +const EC2_KEY_TYPE: i64 = 2; +// The COSE key parameter behind map key -1. +const P_256_CURVE: i64 = 1; + // https://www.w3.org/TR/webauthn/#dictdef-publickeycredentialrpentity #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] pub struct PublicKeyCredentialRpEntity { @@ -322,17 +333,17 @@ impl TryFrom for GetAssertionHmacSecretInput { fn try_from(cbor_value: cbor::Value) -> Result { destructure_cbor_map! { let { - 1 => cose_key, + 1 => key_agreement, 2 => salt_enc, 3 => salt_auth, } = extract_map(cbor_value)?; } - let cose_key = extract_map(ok_or_missing(cose_key)?)?; + let key_agreement = CoseKey::try_from(ok_or_missing(key_agreement)?)?; let salt_enc = extract_byte_string(ok_or_missing(salt_enc)?)?; let salt_auth = extract_byte_string(ok_or_missing(salt_auth)?)?; Ok(Self { - key_agreement: CoseKey(cose_key), + key_agreement, salt_enc, salt_auth, }) @@ -432,7 +443,7 @@ impl From for cbor::Value { #[derive(PartialEq)] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug))] pub enum SignatureAlgorithm { - ES256 = ecdsa::PubKey::ES256_ALGORITHM as isize, + ES256 = ES256_ALGORITHM as isize, // This is the default for all numbers not covered above. // Unknown types should be ignored, instead of returning errors. Unknown = 0, @@ -449,7 +460,7 @@ impl TryFrom for SignatureAlgorithm { fn try_from(cbor_value: cbor::Value) -> Result { match extract_integer(cbor_value)? { - ecdsa::PubKey::ES256_ALGORITHM => Ok(SignatureAlgorithm::ES256), + ES256_ALGORITHM => Ok(SignatureAlgorithm::ES256), _ => Ok(SignatureAlgorithm::Unknown), } } @@ -614,85 +625,31 @@ impl PublicKeyCredentialSource { } } -// TODO(kaczmarczyck) we could decide to split this data type up -// It depends on the algorithm though, I think. -// So before creating a mess, this is my workaround. +// The COSE key is used for both ECDH and ECDSA public keys for transmission. #[derive(Clone)] #[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))] -pub struct CoseKey(pub BTreeMap); - -// This is the algorithm specifier that is supposed to be used in a COSE key -// map. The CTAP specification says -25 which represents ECDH-ES + HKDF-256 -// here: https://www.iana.org/assignments/cose/cose.xhtml#algorithms -// In fact, this is just used for compatibility with older specification versions. -const ECDH_ALGORITHM: i64 = -25; -// This is the identifier used by OpenSSH. To be compatible, we accept both. -const ES256_ALGORITHM: i64 = -7; -const EC2_KEY_TYPE: i64 = 2; -const P_256_CURVE: i64 = 1; +pub struct CoseKey { + x_bytes: [u8; ecdh::NBYTES], + y_bytes: [u8; ecdh::NBYTES], + algorithm: i64, +} +// This conversion accepts both ECDH and ECDSA. 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); - cose_key_from_bytes(x_bytes, y_bytes) - } -} - -impl TryFrom for ecdh::PubKey { - type Error = Ctap2StatusCode; - - fn try_from(cose_key: CoseKey) -> Result { destructure_cbor_map! { let { + // This is sorted correctly, negative encoding is bigger. 1 => key_type, 3 => algorithm, -1 => curve, -2 => x_bytes, -3 => y_bytes, - } = cose_key.0; + } = extract_map(cbor_value)?; } - let key_type = extract_integer(ok_or_missing(key_type)?)?; - if key_type != EC2_KEY_TYPE { - return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); - } - let algorithm = extract_integer(ok_or_missing(algorithm)?)?; - if algorithm != ECDH_ALGORITHM && algorithm != ES256_ALGORITHM { - return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); - } - let curve = extract_integer(ok_or_missing(curve)?)?; - if curve != P_256_CURVE { - return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); - } let x_bytes = extract_byte_string(ok_or_missing(x_bytes)?)?; if x_bytes.len() != ecdh::NBYTES { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); @@ -701,11 +658,55 @@ impl TryFrom for ecdh::PubKey { if y_bytes.len() != ecdh::NBYTES { return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER); } + let curve = extract_integer(ok_or_missing(curve)?)?; + if curve != P_256_CURVE { + return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); + } + let key_type = extract_integer(ok_or_missing(key_type)?)?; + if key_type != EC2_KEY_TYPE { + return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); + } + let algorithm = extract_integer(ok_or_missing(algorithm)?)?; + if algorithm != ECDH_ALGORITHM && algorithm != ES256_ALGORITHM { + return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); + } - let x_array_ref = array_ref![x_bytes.as_slice(), 0, ecdh::NBYTES]; - let y_array_ref = array_ref![y_bytes.as_slice(), 0, ecdh::NBYTES]; - ecdh::PubKey::from_coordinates(x_array_ref, y_array_ref) - .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) + Ok(CoseKey { + x_bytes: *array_ref![x_bytes.as_slice(), 0, ecdh::NBYTES], + y_bytes: *array_ref![y_bytes.as_slice(), 0, ecdh::NBYTES], + algorithm, + }) + } +} + +impl From for cbor::Value { + fn from(cose_key: CoseKey) -> Self { + let CoseKey { + x_bytes, + y_bytes, + algorithm, + } = cose_key; + + cbor_map! { + 1 => EC2_KEY_TYPE, + 3 => algorithm, + -1 => P_256_CURVE, + -2 => x_bytes, + -3 => y_bytes, + } + } +} + +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); + CoseKey { + x_bytes, + y_bytes, + algorithm: ECDH_ALGORITHM, + } } } @@ -714,7 +715,32 @@ impl From for CoseKey { 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) + CoseKey { + x_bytes, + y_bytes, + algorithm: ES256_ALGORITHM, + } + } +} + +impl TryFrom for ecdh::PubKey { + type Error = Ctap2StatusCode; + + fn try_from(cose_key: CoseKey) -> Result { + let CoseKey { + x_bytes, + y_bytes, + algorithm, + } = cose_key; + + // Since algorithm can be used for different COSE key types, we check + // whether the current type is correct for ECDH. For an OpenSSH bugfix, + // the algorithm ES256_ALGORITHM is allowed here too. + if algorithm != ECDH_ALGORITHM && algorithm != ES256_ALGORITHM { + return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM); + } + ecdh::PubKey::from_coordinates(&x_bytes, &y_bytes) + .ok_or(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) } } @@ -827,8 +853,8 @@ mod test { use super::*; use alloc::collections::BTreeMap; use cbor::{ - cbor_array, cbor_bool, cbor_bytes, cbor_false, cbor_int, cbor_map, cbor_null, cbor_text, - cbor_unsigned, + cbor_array, cbor_bool, cbor_bytes, cbor_bytes_lit, cbor_false, cbor_int, cbor_null, + cbor_text, cbor_unsigned, }; use crypto::rng256::{Rng256, ThreadRng256}; @@ -1151,7 +1177,7 @@ mod test { #[test] fn test_from_into_signature_algorithm() { - let cbor_signature_algorithm: cbor::Value = cbor_int!(ecdsa::PubKey::ES256_ALGORITHM); + let cbor_signature_algorithm: cbor::Value = cbor_int!(ES256_ALGORITHM); let signature_algorithm = SignatureAlgorithm::try_from(cbor_signature_algorithm.clone()); let expected_signature_algorithm = SignatureAlgorithm::ES256; assert_eq!(signature_algorithm, Ok(expected_signature_algorithm)); @@ -1225,7 +1251,7 @@ mod test { fn test_from_into_public_key_credential_parameter() { let cbor_credential_parameter = cbor_map! { "type" => "public-key", - "alg" => ecdsa::PubKey::ES256_ALGORITHM, + "alg" => ES256_ALGORITHM, }; let credential_parameter = PublicKeyCredentialParameter::try_from(cbor_credential_parameter.clone()); @@ -1279,7 +1305,7 @@ mod test { let cose_key = CoseKey::from(pk); let cbor_extensions = cbor_map! { "hmac-secret" => cbor_map! { - 1 => cbor::Value::Map(cose_key.0.clone()), + 1 => cbor::Value::from(cose_key.clone()), 2 => vec![0x02; 32], 3 => vec![0x03; 16], }, @@ -1343,6 +1369,83 @@ mod test { assert_eq!(created_cbor, cbor_packed_attestation_statement); } + #[test] + fn test_from_into_cose_key_cbor() { + for algorithm in &[ECDH_ALGORITHM, ES256_ALGORITHM] { + let cbor_value = cbor_map! { + 1 => EC2_KEY_TYPE, + 3 => algorithm, + -1 => P_256_CURVE, + -2 => [0u8; 32], + -3 => [0u8; 32], + }; + let cose_key = CoseKey::try_from(cbor_value.clone()).unwrap(); + let created_cbor_value = cbor::Value::from(cose_key); + assert_eq!(created_cbor_value, cbor_value); + } + + let cbor_value = cbor_map! { + 1 => EC2_KEY_TYPE, + // unknown algorithm + 3 => 0, + -1 => P_256_CURVE, + -2 => [0u8; 32], + -3 => [0u8; 32], + }; + assert_eq!( + CoseKey::try_from(cbor_value), + Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM) + ); + let cbor_value = cbor_map! { + // unknown type + 1 => 0, + 3 => ECDH_ALGORITHM, + -1 => P_256_CURVE, + -2 => [0u8; 32], + -3 => [0u8; 32], + }; + assert_eq!( + CoseKey::try_from(cbor_value), + Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM) + ); + let cbor_value = cbor_map! { + 1 => EC2_KEY_TYPE, + 3 => ECDH_ALGORITHM, + // unknown curve + -1 => 0, + -2 => [0u8; 32], + -3 => [0u8; 32], + }; + assert_eq!( + CoseKey::try_from(cbor_value), + Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM) + ); + let cbor_value = cbor_map! { + 1 => EC2_KEY_TYPE, + 3 => ECDH_ALGORITHM, + -1 => P_256_CURVE, + // wrong length + -2 => [0u8; 31], + -3 => [0u8; 32], + }; + assert_eq!( + CoseKey::try_from(cbor_value), + Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) + ); + let cbor_value = cbor_map! { + 1 => EC2_KEY_TYPE, + 3 => ECDH_ALGORITHM, + -1 => P_256_CURVE, + -2 => [0u8; 32], + // wrong length + -3 => [0u8; 33], + }; + assert_eq!( + CoseKey::try_from(cbor_value), + Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER) + ); + } + #[test] fn test_from_into_cose_key_ecdh() { let mut rng = ThreadRng256 {}; @@ -1359,17 +1462,7 @@ mod test { 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)); - } + assert_eq!(cose_key.algorithm, ES256_ALGORITHM); } #[test] diff --git a/src/ctap/mod.rs b/src/ctap/mod.rs index 676f4c2..8895605 100644 --- a/src/ctap/mod.rs +++ b/src/ctap/mod.rs @@ -534,7 +534,7 @@ where } auth_data.extend(vec![0x00, credential_id.len() as u8]); auth_data.extend(&credential_id); - if !cbor::write(cbor::Value::Map(CoseKey::from(pk).0), &mut auth_data) { + if !cbor::write(cbor::Value::from(CoseKey::from(pk)), &mut auth_data) { return Err(Ctap2StatusCode::CTAP2_ERR_VENDOR_INTERNAL_ERROR); } if has_extension_output { diff --git a/src/ctap/response.rs b/src/ctap/response.rs index 3ebab3b..f40dc21 100644 --- a/src/ctap/response.rs +++ b/src/ctap/response.rs @@ -192,7 +192,7 @@ impl From for cbor::Value { } = client_pin_response; cbor_map_options! { - 1 => key_agreement.map(|cose_key| cbor_map_btree!(cose_key.0)), + 1 => key_agreement.map(cbor::Value::from), 2 => pin_token, 3 => retries, }