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

Code before:
        if bytes.len() != 32 {
            return None;
        }
        if let Ok(secret) = ed25519_dalek::SecretKey::from_bytes(bytes) {
            let public = ed25519_dalek::PublicKey::from (&secret);
            Some(Self::Ed25519(ed25519_dalek::Keypair{secret, public}))
        } else {
            None
        }
    }

    /// Returns the corresponding public key.
    pub fn get_pub_key(&self) -> CoseKey {
        match self {
            PrivateKey::Ecdsa(ecdsa_key) => CoseKey::from(ecdsa_key.genpk()),
            #[cfg(feature = "with_ed25519")]
            PrivateKey::Ed25519(ed25519_keypair) => CoseKey::from(ed25519_keypair.public),
        }
    }

    /// Returns the encoded signature for a given message.
    pub fn sign_and_encode(&self, message: &[u8]) -> Vec<u8> {
        match self {
            PrivateKey::Ecdsa(ecdsa_key) => ecdsa_key.sign_rfc6979::<Sha256>(message).to_asn1_der(),
            #[cfg(feature = "with_ed25519")]
            PrivateKey::Ed25519(ed25519_keypair) => ed25519_keypair.try_sign(message).unwrap().to_bytes().to_vec(),
        }
    }

    /// The associated COSE signature algorithm identifier.
    pub fn signature_algorithm(&self) -> SignatureAlgorithm {
        match self {
            PrivateKey::Ecdsa(_) => SignatureAlgorithm::ES256,
            #[cfg(feature = "with_ed25519")]
            PrivateKey::Ed25519(_) => SignatureAlgorithm::EDDSA,
        }
    }

    /// Writes the key bytes.
    pub fn to_bytes(&self) -> Vec<u8> {
        match self {
            PrivateKey::Ecdsa(ecdsa_key) => {
                let mut key_bytes = vec![0u8; 32];
                ecdsa_key.to_bytes(array_mut_ref!(key_bytes, 0, 32));
                key_bytes
            }
            #[cfg(feature = "with_ed25519")]
            PrivateKey::Ed25519(ed25519_keypair) => ed25519_keypair.secret.to_bytes().to_vec(),
        }
    }
}

Review comment (bug):
Under what conditions can signing fail? I'd prefer either making this function return an error, or retrying the signing operation if it's probabilistic. `unwrap` would panic.

Code after:
    pub fn signature_algorithm(&self) -> SignatureAlgorithm {
        match self {
            PrivateKey::Ecdsa(_) => SignatureAlgorithm::ES256,
            #[cfg(feature = "ed25519")]
            PrivateKey::Ed25519(_) => SignatureAlgorithm::EDDSA,
        }
    }

    /// Writes the key bytes.
    pub fn to_bytes(&self) -> Vec<u8> {
        match self {
            PrivateKey::Ecdsa(ecdsa_key) => {
                let mut key_bytes = vec![0u8; 32];
                ecdsa_key.to_bytes(array_mut_ref!(key_bytes, 0, 32));
                key_bytes
            }
            #[cfg(feature = "ed25519")]
            PrivateKey::Ed25519(ed25519_key) => ed25519_key.seed().to_vec(),
        }
    }
}

impl From<PrivateKey> for cbor::Value {
    fn from(private_key: PrivateKey) -> Self {
        cbor_array![
            cbor_int!(private_key.signature_algorithm() as i64),
            cbor_bytes!(private_key.to_bytes()),
        ]
    }
}

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

    fn try_from(cbor_value: cbor::Value) -> Result<Self, Ctap2StatusCode> {
        let mut array = extract_array(cbor_value)?;
        if array.len() != 2 {
            return Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR);
        }
        let key_bytes = extract_byte_string(array.pop().unwrap())?;
        match SignatureAlgorithm::try_from(array.pop().unwrap())? {
            SignatureAlgorithm::ES256 => PrivateKey::new_ecdsa_from_bytes(&key_bytes)
                .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
            #[cfg(feature = "ed25519")]
            SignatureAlgorithm::EDDSA => PrivateKey::new_ed25519_from_bytes(&key_bytes)
                .ok_or(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
            _ => Err(Ctap2StatusCode::CTAP2_ERR_INVALID_CBOR),
        }
    }
}