Merge changes I5c955cba,Ie522f556 into main
* changes:
Use raw (R | S) format for COSE keys
Make ecdsa_verify_cose testable
diff --git a/libs/bssl/error/src/lib.rs b/libs/bssl/error/src/lib.rs
index 0bdb821..822e02d 100644
--- a/libs/bssl/error/src/lib.rs
+++ b/libs/bssl/error/src/lib.rs
@@ -88,6 +88,7 @@
EC_KEY_new_by_curve_name,
EC_KEY_set_public_key_affine_coordinates,
EC_POINT_get_affine_coordinates,
+ ECDSA_SIG_from_bytes,
ECDSA_SIG_new,
ECDSA_SIG_set0,
ECDSA_sign,
diff --git a/libs/bssl/src/ec_key.rs b/libs/bssl/src/ec_key.rs
index 9eb5e5c..3e2e382 100644
--- a/libs/bssl/src/ec_key.rs
+++ b/libs/bssl/src/ec_key.rs
@@ -23,9 +23,10 @@
use bssl_avf_error::{ApiName, Error, Result};
use bssl_sys::{
i2d_ECDSA_SIG, BN_bin2bn, BN_bn2bin_padded, BN_clear_free, BN_new, CBB_flush, CBB_len,
- ECDSA_SIG_free, ECDSA_SIG_new, ECDSA_SIG_set0, ECDSA_sign, ECDSA_size, ECDSA_verify,
- EC_GROUP_get_curve_name, EC_GROUP_new_by_curve_name, EC_KEY_check_key, EC_KEY_free,
- EC_KEY_generate_key, EC_KEY_get0_group, EC_KEY_get0_public_key, EC_KEY_marshal_private_key,
+ ECDSA_SIG_free, ECDSA_SIG_from_bytes, ECDSA_SIG_get0_r, ECDSA_SIG_get0_s, ECDSA_SIG_new,
+ ECDSA_SIG_set0, ECDSA_sign, ECDSA_size, ECDSA_verify, EC_GROUP_get_curve_name,
+ EC_GROUP_new_by_curve_name, EC_KEY_check_key, EC_KEY_free, EC_KEY_generate_key,
+ EC_KEY_get0_group, EC_KEY_get0_public_key, EC_KEY_marshal_private_key,
EC_KEY_new_by_curve_name, EC_KEY_parse_private_key, EC_KEY_set_public_key_affine_coordinates,
EC_POINT_get_affine_coordinates, NID_X9_62_prime256v1, NID_secp384r1, BIGNUM, ECDSA_SIG,
EC_GROUP, EC_KEY, EC_POINT,
@@ -165,11 +166,11 @@
check_int_result(ret, ApiName::ECDSA_verify)
}
- /// Verifies the NIST-encoded (R | S) ECDSA `signature` of the `digest` with the current
- /// `EcKey`.
+ /// Verifies the COSE-encoded (R | S, see RFC8152) ECDSA `signature` of the `digest` with the
+ /// current `EcKey`.
///
/// Returns Ok(()) if the verification succeeds, otherwise an error will be returned.
- pub fn ecdsa_verify_nist(&self, signature: &[u8], digest: &[u8]) -> Result<()> {
+ pub fn ecdsa_verify_cose(&self, signature: &[u8], digest: &[u8]) -> Result<()> {
let signature = ec_cose_signature_to_der(signature)?;
self.ecdsa_verify_der(&signature, digest)
}
@@ -177,7 +178,7 @@
/// Signs the `digest` with the current `EcKey` using ECDSA.
///
/// Returns the DER-encoded ECDSA signature.
- pub fn ecdsa_sign(&self, digest: &[u8]) -> Result<Vec<u8>> {
+ pub fn ecdsa_sign_der(&self, digest: &[u8]) -> Result<Vec<u8>> {
// The `type` argument should be 0 as required in the BoringSSL spec.
const TYPE: i32 = 0;
@@ -204,6 +205,15 @@
}
}
+ /// Signs the `digest` with the current `EcKey` using ECDSA.
+ ///
+ /// Returns the COSE-encoded (R | S, see RFC8152) ECDSA signature.
+ pub fn ecdsa_sign_cose(&self, digest: &[u8]) -> Result<Vec<u8>> {
+ let signature = self.ecdsa_sign_der(digest)?;
+ let coord_bytes = self.ec_group()?.affine_coordinate_size()?;
+ ec_der_signature_to_cose(&signature, coord_bytes)
+ }
+
/// Returns the maximum size of an ECDSA signature using the current `EcKey`.
fn ecdsa_size(&self) -> Result<usize> {
// SAFETY: This function only reads the `EC_KEY` that has been initialized
@@ -335,13 +345,19 @@
}
}
-/// Convert a R | S format NIST signature to a DER-encoded form.
+/// Convert a COSE format (R | S) ECDSA signature to a DER-encoded form.
fn ec_cose_signature_to_der(signature: &[u8]) -> Result<Vec<u8>> {
let mut ec_sig = EcSignature::new()?;
- ec_sig.load_from_nist(signature)?;
+ ec_sig.load_from_cose(signature)?;
ec_sig.to_der()
}
+/// Convert a DER-encoded signature to COSE format (R | S).
+fn ec_der_signature_to_cose(signature: &[u8], coord_bytes: usize) -> Result<Vec<u8>> {
+ let ec_sig = EcSignature::new_from_der(signature)?;
+ ec_sig.to_cose(coord_bytes)
+}
+
/// Wrapper for an `ECDSA_SIG` object representing an EC signature.
struct EcSignature(NonNull<ECDSA_SIG>);
@@ -356,8 +372,8 @@
Ok(Self(signature))
}
- /// Populate the signature parameters from a NIST encoding (R | S).
- fn load_from_nist(&mut self, signature: &[u8]) -> Result<()> {
+ /// Populate the signature parameters from a COSE encoding (R | S).
+ fn load_from_cose(&mut self, signature: &[u8]) -> Result<()> {
let coord_bytes = signature.len() / 2;
if signature.len() != 2 * coord_bytes {
return Err(Error::InternalError);
@@ -379,6 +395,44 @@
Ok(())
}
+ fn to_cose(&self, coord_bytes: usize) -> Result<Vec<u8>> {
+ let mut result = vec![0u8; coord_bytes.checked_mul(2).unwrap()];
+ let (r_bytes, s_bytes) = result.split_at_mut(coord_bytes);
+
+ // SAFETY: The ECDSA_SIG was properly allocated and not yet freed. Always returns a valid
+ // non-null, non-owning pointer.
+ let r = unsafe { ECDSA_SIG_get0_r(self.0.as_ptr()) };
+ check_int_result(
+ // SAFETY: The r pointer is known to be valid. Only writes within the destination
+ // slice.
+ unsafe { BN_bn2bin_padded(r_bytes.as_mut_ptr(), r_bytes.len(), r) },
+ ApiName::BN_bn2bin_padded,
+ )?;
+
+ // SAFETY: The ECDSA_SIG was properly allocated and not yet freed. Always returns a valid
+ // non-null, non-owning pointer.
+ let s = unsafe { ECDSA_SIG_get0_s(self.0.as_ptr()) };
+ check_int_result(
+ // SAFETY: The r pointer is known to be valid. Only writes within the destination
+ // slice.
+ unsafe { BN_bn2bin_padded(s_bytes.as_mut_ptr(), s_bytes.len(), s) },
+ ApiName::BN_bn2bin_padded,
+ )?;
+
+ Ok(result)
+ }
+
+ /// Populate the signature parameters from a DER encoding
+ fn new_from_der(signature: &[u8]) -> Result<Self> {
+ // SAFETY: Only reads within the bounds of the slice. Returns a pointer to a new ECDSA_SIG
+ // which we take ownership of, or null on error which we check.
+ let signature = unsafe { ECDSA_SIG_from_bytes(signature.as_ptr(), signature.len()) };
+
+ let signature = NonNull::new(signature)
+ .ok_or_else(|| to_call_failed_error(ApiName::ECDSA_SIG_from_bytes))?;
+ Ok(Self(signature))
+ }
+
/// Return the signature encoded as DER.
fn to_der(&self) -> Result<Vec<u8>> {
// SAFETY: The ECDSA_SIG was properly allocated and not yet freed. Null is a valid
diff --git a/libs/bssl/tests/eckey_test.rs b/libs/bssl/tests/eckey_test.rs
index 00ed6c5..0fc78a1 100644
--- a/libs/bssl/tests/eckey_test.rs
+++ b/libs/bssl/tests/eckey_test.rs
@@ -87,7 +87,7 @@
let digest = digester.digest(MESSAGE1)?;
assert_eq!(digest, sha256(MESSAGE1)?);
- let signature = ec_key.ecdsa_sign(&digest)?;
+ let signature = ec_key.ecdsa_sign_der(&digest)?;
ec_key.ecdsa_verify_der(&signature, &digest)?;
// Building a `PKey` from a temporary `CoseKey` should work as the lifetime
// of the `PKey` is not tied to the lifetime of the `CoseKey`.
@@ -102,7 +102,7 @@
let digester = Digester::sha384();
let digest = digester.digest(MESSAGE1)?;
- let signature = ec_key.ecdsa_sign(&digest)?;
+ let signature = ec_key.ecdsa_sign_der(&digest)?;
ec_key.ecdsa_verify_der(&signature, &digest)?;
let pkey = PKey::from_cose_public_key(&ec_key.cose_public_key()?)?;
pkey.verify(&signature, MESSAGE1, Some(digester))
@@ -113,7 +113,7 @@
let mut ec_key1 = EcKey::new_p256()?;
ec_key1.generate_key()?;
let digest = sha256(MESSAGE1)?;
- let signature = ec_key1.ecdsa_sign(&digest)?;
+ let signature = ec_key1.ecdsa_sign_der(&digest)?;
let mut ec_key2 = EcKey::new_p256()?;
ec_key2.generate_key()?;
@@ -134,7 +134,7 @@
let mut ec_key = EcKey::new_p256()?;
ec_key.generate_key()?;
let digest1 = sha256(MESSAGE1)?;
- let signature = ec_key.ecdsa_sign(&digest1)?;
+ let signature = ec_key.ecdsa_sign_der(&digest1)?;
let digest2 = sha256(MESSAGE2)?;
let err = ec_key.ecdsa_verify_der(&signature, &digest2).unwrap_err();
@@ -142,3 +142,42 @@
assert_eq!(expected_err, err);
Ok(())
}
+
+#[test]
+fn ecdsa_cose_signing_and_verification_succeed() -> Result<()> {
+ let digest = sha256(MESSAGE1)?;
+ let mut ec_key = EcKey::new_p256()?;
+ ec_key.generate_key()?;
+
+ let signature = ec_key.ecdsa_sign_cose(&digest)?;
+ ec_key.ecdsa_verify_cose(&signature, &digest)?;
+ assert_eq!(signature.len(), 64);
+ Ok(())
+}
+
+#[test]
+fn verifying_ecdsa_cose_signed_with_a_different_message_fails() -> Result<()> {
+ let digest = sha256(MESSAGE1)?;
+ let mut ec_key = EcKey::new_p256()?;
+ ec_key.generate_key()?;
+
+ let signature = ec_key.ecdsa_sign_cose(&digest)?;
+
+ let err = ec_key.ecdsa_verify_cose(&signature, &sha256(MESSAGE2)?).unwrap_err();
+ let expected_err = Error::CallFailed(ApiName::ECDSA_verify, EcdsaError::BadSignature.into());
+ assert_eq!(expected_err, err);
+ Ok(())
+}
+
+#[test]
+fn verifying_ecdsa_cose_signed_as_der_fails() -> Result<()> {
+ let digest = sha256(MESSAGE1)?;
+ let mut ec_key = EcKey::new_p256()?;
+ ec_key.generate_key()?;
+
+ let signature = ec_key.ecdsa_sign_cose(&digest)?;
+ let err = ec_key.ecdsa_verify_der(&signature, &digest).unwrap_err();
+ let expected_err = Error::CallFailed(ApiName::ECDSA_verify, EcdsaError::BadSignature.into());
+ assert_eq!(expected_err, err);
+ Ok(())
+}
diff --git a/service_vm/client_vm_csr/src/lib.rs b/service_vm/client_vm_csr/src/lib.rs
index 512ecaf..0babfff 100644
--- a/service_vm/client_vm_csr/src/lib.rs
+++ b/service_vm/client_vm_csr/src/lib.rs
@@ -100,7 +100,7 @@
sign(message, cdi_leaf_priv.as_array()).map(|v| v.to_vec())
})?
.try_add_created_signature(attestation_key_sig_headers, aad, |message| {
- ecdsa_sign(message, attestation_key)
+ ecdsa_sign_cose(message, attestation_key)
})?
.build();
Ok(signed_data)
@@ -113,12 +113,18 @@
CoseSignatureBuilder::new().protected(protected).build()
}
-fn ecdsa_sign(message: &[u8], key: &EcKeyRef<Private>) -> Result<Vec<u8>> {
+fn ecdsa_sign_cose(message: &[u8], key: &EcKeyRef<Private>) -> Result<Vec<u8>> {
let digest = sha256(message);
// Passes the digest to `ECDSA_do_sign` as recommended in the spec:
// https://commondatastorage.googleapis.com/chromium-boringssl-docs/ecdsa.h.html#ECDSA_do_sign
let sig = EcdsaSig::sign::<Private>(&digest, key)?;
- Ok(sig.to_der()?)
+ ecdsa_sig_to_cose(&sig)
+}
+
+fn ecdsa_sig_to_cose(signature: &EcdsaSig) -> Result<Vec<u8>> {
+ let mut result = signature.r().to_vec_padded(ATTESTATION_KEY_AFFINE_COORDINATE_SIZE)?;
+ result.extend_from_slice(&signature.s().to_vec_padded(ATTESTATION_KEY_AFFINE_COORDINATE_SIZE)?);
+ Ok(result)
}
fn get_affine_coordinates(key: &EcKeyRef<Private>) -> Result<(Vec<u8>, Vec<u8>)> {
@@ -175,29 +181,38 @@
let chain = dice::Chain::from_cbor(&session, &csr.dice_cert_chain)?;
let public_key = chain.leaf().subject_public_key();
cose_sign
- .verify_signature(0, aad, |signature, message| public_key.verify(signature, message))?;
+ .verify_signature(0, aad, |signature, message| public_key.verify(signature, message))
+ .context("Verifying CDI_Leaf_Priv signature")?;
// Checks the second signature is signed with attestation key.
let attestation_public_key = CoseKey::from_slice(&csr_payload.public_key).unwrap();
let ec_public_key = to_ec_public_key(&attestation_public_key)?;
- cose_sign.verify_signature(1, aad, |signature, message| {
- ecdsa_verify(signature, message, &ec_public_key)
- })?;
+ cose_sign
+ .verify_signature(1, aad, |signature, message| {
+ ecdsa_verify_cose(signature, message, &ec_public_key)
+ })
+ .context("Verifying attestation key signature")?;
// Verifies that private key and the public key form a valid key pair.
let message = b"test message";
- let signature = ecdsa_sign(message, &ec_private_key)?;
- ecdsa_verify(&signature, message, &ec_public_key)?;
+ let signature = ecdsa_sign_cose(message, &ec_private_key)?;
+ ecdsa_verify_cose(&signature, message, &ec_public_key)
+ .context("Verifying signature with attested key")?;
Ok(())
}
- fn ecdsa_verify(
+ fn ecdsa_verify_cose(
signature: &[u8],
message: &[u8],
ec_public_key: &EcKeyRef<Public>,
) -> Result<()> {
- let sig = EcdsaSig::from_der(signature)?;
+ let coord_bytes = signature.len() / 2;
+ assert_eq!(signature.len(), coord_bytes * 2);
+
+ let r = BigNum::from_slice(&signature[..coord_bytes])?;
+ let s = BigNum::from_slice(&signature[coord_bytes..])?;
+ let sig = EcdsaSig::from_private_components(r, s)?;
let digest = sha256(message);
if sig.verify(&digest, ec_public_key)? {
Ok(())
diff --git a/service_vm/requests/src/client_vm.rs b/service_vm/requests/src/client_vm.rs
index 00effd7..2aa7113 100644
--- a/service_vm/requests/src/client_vm.rs
+++ b/service_vm/requests/src/client_vm.rs
@@ -66,7 +66,7 @@
// Verifies the second signature with the public key in the CSR payload.
let ec_public_key = EcKey::from_cose_public_key_slice(&csr_payload.public_key)?;
cose_sign.verify_signature(ATTESTATION_KEY_SIGNATURE_INDEX, aad, |signature, message| {
- ecdsa_verify(&ec_public_key, signature, message)
+ ecdsa_verify_cose(&ec_public_key, signature, message)
})?;
let subject_public_key_info = PKey::try_from(ec_public_key)?.subject_public_key_info()?;
@@ -110,20 +110,20 @@
RequestProcessingError::FailedToDecryptKeyBlob
})?;
let ec_private_key = EcKey::from_ec_private_key(private_key.as_slice())?;
- let signature = ecdsa_sign(&ec_private_key, &tbs_cert.to_der()?)?;
+ let signature = ecdsa_sign_der(&ec_private_key, &tbs_cert.to_der()?)?;
let certificate = cert::build_certificate(tbs_cert, &signature)?;
Ok(certificate.to_der()?)
}
-fn ecdsa_verify(key: &EcKey, signature: &[u8], message: &[u8]) -> bssl_avf::Result<()> {
+fn ecdsa_verify_cose(key: &EcKey, signature: &[u8], message: &[u8]) -> bssl_avf::Result<()> {
// The message was signed with ECDSA with curve P-256 and SHA-256 at the signature generation.
let digest = sha256(message)?;
- key.ecdsa_verify_der(signature, &digest)
+ key.ecdsa_verify_cose(signature, &digest)
}
-fn ecdsa_sign(key: &EcKey, message: &[u8]) -> bssl_avf::Result<Vec<u8>> {
+fn ecdsa_sign_der(key: &EcKey, message: &[u8]) -> bssl_avf::Result<Vec<u8>> {
let digest = sha256(message)?;
- key.ecdsa_sign(&digest)
+ key.ecdsa_sign_der(&digest)
}
fn validate_service_vm_dice_chain_length(service_vm_dice_chain: &[Value]) -> Result<()> {
diff --git a/service_vm/requests/src/dice.rs b/service_vm/requests/src/dice.rs
index f4da7ef..247c34e 100644
--- a/service_vm/requests/src/dice.rs
+++ b/service_vm/requests/src/dice.rs
@@ -214,7 +214,7 @@
if !key_ops.is_empty()
&& !key_ops.contains(&KeyOperation::Assigned(iana::KeyOperation::Verify))
{
- error!("Public key does not support verification");
+ error!("Public key does not support verification - key_ops: {key_ops:?}");
return Err(RequestProcessingError::InvalidDiceChain);
}
Ok(Self(key))
@@ -231,7 +231,7 @@
/// PubKeyEd25519 / PubKeyECDSA256 / PubKeyECDSA384
///
/// The signature should be in the format defined by COSE in RFC 9053 section 2 for the
- /// specifric algorithm.
+ /// specific algorithm.
pub(crate) fn verify(&self, signature: &[u8], message: &[u8]) -> Result<()> {
match &self.0.kty {
KeyType::Assigned(iana::KeyType::EC2) => {
@@ -249,7 +249,7 @@
}
};
let digest = digester.digest(message)?;
- Ok(public_key.ecdsa_verify_nist(signature, &digest)?)
+ Ok(public_key.ecdsa_verify_cose(signature, &digest)?)
}
KeyType::Assigned(iana::KeyType::OKP) => {
let curve_type =