Make ecdsa_verify_cose testable
Add ecdsa_sign_cose (and rename the existing ecdsa_sign to
ecdsa_sign_der). Implement DER to COSE signature conversion to support
this.
Fix terminology - COSE format rather than NIST format.
Add tests to ensure that signing & verification are compatible.
Note that while I'm adding code to the production bssl library, none
of it is currently used other than in the new tests, so this shouldn't
break anything.
Bug: 338745127
Test: atest libbssl_avf_nostd.test
Change-Id: Ie522f556f90c4515b4f738a23a42c02ff48b073e
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/requests/src/client_vm.rs b/service_vm/requests/src/client_vm.rs
index 00effd7..4f04ced 100644
--- a/service_vm/requests/src/client_vm.rs
+++ b/service_vm/requests/src/client_vm.rs
@@ -123,7 +123,7 @@
fn ecdsa_sign(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..7e0118e 100644
--- a/service_vm/requests/src/dice.rs
+++ b/service_vm/requests/src/dice.rs
@@ -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 =