Repository: google/OpenSK
File: src/ctap/data_formats.rs (Rust)

Code before:
#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))]
pub struct CoseKey(pub BTreeMap<cbor::KeyType, cbor::Value>);

// 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;

impl TryFrom<cbor::Value> for CoseKey {
    type Error = Ctap2StatusCode;

    fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
        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<ecdh::PubKey> 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<CoseKey> for ecdh::PubKey {
    type Error = Ctap2StatusCode;

Review comment (suggestion):
Might be worth explaining that this function is used to convert both `ecdh::PubKey` and `ecdsa::PubKey` into a `CoseKey`.

Code after:
#[derive(Clone)]
#[cfg_attr(any(test, feature = "debug_ctap"), derive(Debug, PartialEq))]
pub struct CoseKey {
    x_bytes: [u8; ecdh::NBYTES],
    y_bytes: [u8; ecdh::NBYTES],
    algorithm: i64,
}

impl CoseKey {
    // This is the algorithm specifier for 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;
    // The parameter behind map key 1.
    const EC2_KEY_TYPE: i64 = 2;
    // The parameter behind map key -1.
    const P_256_CURVE: i64 = 1;
}

// This conversion accepts both ECDH and ECDSA.
impl TryFrom<cbor::Value> for CoseKey {
    type Error = Ctap2StatusCode;

    fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
        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,
            } = extract_map(cbor_value)?;
        }

        let x_bytes = extract_byte_string(ok_or_missing(x_bytes)?)?;
        if x_bytes.len() != ecdh::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 {
            return Err(Ctap2StatusCode::CTAP1_ERR_INVALID_PARAMETER);
        }
        let curve = extract_integer(ok_or_missing(curve)?)?;
        if curve != CoseKey::P_256_CURVE {
            return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM);
        }
        let key_type = extract_integer(ok_or_missing(key_type)?)?;
        if key_type != CoseKey::EC2_KEY_TYPE {
            return Err(Ctap2StatusCode::CTAP2_ERR_UNSUPPORTED_ALGORITHM);
        }