Encode the root-key in the canonicalized form requested in the spec
When serializing the identity to be sent to the other party, use the
helper method in coset to encode the root key in the requested
canonicalized form as per the spec.
Add a check in validation of Identity to ensure that the root key is
in accordance with the canonical form required by the spec.
Add relevant unit tests.
Bug: 319613231
Test: libauthgraph_boringssl_test
Change-Id: Iee95bad7ad2f7978d6c2b1951480c497cf9cb802
diff --git a/boringssl/src/test_device.rs b/boringssl/src/test_device.rs
index d777675..779a041 100644
--- a/boringssl/src/test_device.rs
+++ b/boringssl/src/test_device.rs
@@ -27,7 +27,7 @@
};
use authgraph_wire::{ErrorCode, SESSION_ID_LEN};
use core::cell::RefCell;
-use coset::iana;
+use coset::{iana, CborOrdering};
/// The struct implementing the Authgraph `Device` trait.
pub struct AgDevice {
@@ -92,8 +92,8 @@
/// DiceCertChainInitialPayload is an EC public key on P-256 curve in the default implementation
fn get_identity(&self) -> Result<(Option<EcSignKey>, Identity), Error> {
if self.identity.borrow().is_none() {
- let (priv_key, pub_key) = crate::ec::create_p256_key_pair(iana::Algorithm::ES256)?;
-
+ let (priv_key, mut pub_key) = crate::ec::create_p256_key_pair(iana::Algorithm::ES256)?;
+ pub_key.canonicalize(CborOrdering::Lexicographic);
let identity = Identity {
version: IDENTITY_VERSION,
cert_chain: CertChain {
diff --git a/boringssl/src/tests.rs b/boringssl/src/tests.rs
index 19c0449..217fccf 100644
--- a/boringssl/src/tests.rs
+++ b/boringssl/src/tests.rs
@@ -16,10 +16,16 @@
//! Tests for BoringSSL-based implementations of AuthGraph traits.
use alloc::rc::Rc;
-use authgraph_core::{key::Identity, keyexchange};
+use authgraph_core::{
+ key::{
+ CertChain, EcSignKey, EcVerifyKey, Identity, EXPLICIT_KEY_DICE_CERT_CHAIN_VERSION,
+ IDENTITY_VERSION,
+ },
+ keyexchange,
+};
use authgraph_core_test as ag_test;
use core::cell::RefCell;
-use coset::{iana, CborSerializable};
+use coset::{cbor::value::Value, iana, CborOrdering, CborSerializable, Label};
#[test]
fn test_rng() {
@@ -257,3 +263,58 @@
ag_test::test_key_exchange_finish(&mut source, &mut sink);
ag_test::test_key_exchange_auth_complete(&mut source, &mut sink);
}
+
+#[test]
+#[should_panic(expected = "root key is not in the required canonical form")]
+fn test_get_identity_with_root_key_in_incorrect_canonical_form() {
+ // Check that the `Identity` returned from `get_identity` in the `Device` trait fails the
+ // validation, given that the root key is in incorrect canonical form.
+ let test_device = crate::test_device::AgDevice::default();
+ let (priv_key, mut pub_key) = crate::ec::create_p256_key_pair(iana::Algorithm::ES256).unwrap();
+ let mut test_params = Vec::<(Label, Value)>::new();
+ for param in pub_key.params {
+ test_params.push(param);
+ }
+ test_params.push((Label::Int(23), Value::Text("test1".to_string())));
+ test_params.push((Label::Int(1234), Value::Text("test2".to_string())));
+ pub_key.params = test_params;
+ pub_key.canonicalize(CborOrdering::LengthFirstLexicographic);
+ let identity = Identity {
+ version: IDENTITY_VERSION,
+ cert_chain: CertChain {
+ version: EXPLICIT_KEY_DICE_CERT_CHAIN_VERSION,
+ root_key: EcVerifyKey::P256(pub_key),
+ dice_cert_chain: None,
+ },
+ policy: None,
+ };
+ test_device.set_identity((EcSignKey::P256(priv_key), identity), iana::Algorithm::ES256);
+ ag_test::test_get_identity(&test_device, &crate::BoringEcDsa);
+}
+
+#[test]
+fn test_get_identity_with_root_key_in_correct_canonical_form() {
+ // Check that the `Identity` returned from `get_identity` in the `Device` trait passes the
+ // validation, given that the root key is in correct canonical form.
+ let test_device = crate::test_device::AgDevice::default();
+ let (priv_key, mut pub_key) = crate::ec::create_p256_key_pair(iana::Algorithm::ES256).unwrap();
+ let mut test_params = Vec::<(Label, Value)>::new();
+ for param in pub_key.params {
+ test_params.push(param);
+ }
+ test_params.push((Label::Int(23), Value::Text("test1".to_string())));
+ test_params.push((Label::Int(1234), Value::Text("test2".to_string())));
+ pub_key.params = test_params;
+ pub_key.canonicalize(CborOrdering::Lexicographic);
+ let identity = Identity {
+ version: IDENTITY_VERSION,
+ cert_chain: CertChain {
+ version: EXPLICIT_KEY_DICE_CERT_CHAIN_VERSION,
+ root_key: EcVerifyKey::P256(pub_key),
+ dice_cert_chain: None,
+ },
+ policy: None,
+ };
+ test_device.set_identity((EcSignKey::P256(priv_key), identity), iana::Algorithm::ES256);
+ ag_test::test_get_identity(&test_device, &crate::BoringEcDsa);
+}
diff --git a/core/src/key.rs b/core/src/key.rs
index a5c5d13..e6c73ee 100644
--- a/core/src/key.rs
+++ b/core/src/key.rs
@@ -23,8 +23,8 @@
use alloc::{string::String, vec::Vec};
use authgraph_wire as wire;
use coset::{
- cbor, cbor::value::Value, iana, AsCborValue, CborSerializable, CoseError, CoseKey, CoseSign1,
- Label,
+ cbor, cbor::value::Value, iana, AsCborValue, CborOrdering, CborSerializable, CoseError,
+ CoseKey, CoseSign1, Label,
};
use wire::ErrorCode;
use zeroize::ZeroizeOnDrop;
@@ -192,6 +192,23 @@
}
}
+ /// Validate whether the CoseKey is in the expected canonical form as per the spec.
+ pub fn is_canonicalized(&self) -> bool {
+ let mut expected = self.clone();
+ expected.canonicalize_cose_key();
+ *self == expected
+ }
+
+ /// Order the labels of the Cose Key, in order to ensure canonical encoding in accordance with
+ /// Core Deterministic Encoding Requirements [RFC 8949 s4.2.1].
+ pub fn canonicalize_cose_key(&mut self) {
+ match self {
+ EcVerifyKey::Ed25519(k) | EcVerifyKey::P256(k) | EcVerifyKey::P384(k) => {
+ k.canonicalize(CborOrdering::Lexicographic);
+ }
+ }
+ }
+
/// Return the Cose signing algorithm corresponds to the given public signing key.
/// Assume that the `CoseKey` is checked for appropriate header parameters before it is used for
/// signature verification.
@@ -463,8 +480,10 @@
impl CertChain {
/// Perform the following validations on the decoded DICE cert chain:
/// 1. correctness of the `version`
- /// 2. correctness of Cose key parameters of the `root_key`
- /// 3. if dice_cert_chain is present, check for each DiceChainEntry,
+ /// 2. `root_key` is in accordance with Core Deterministic Encoding Requirements
+ /// [RFC 8949 s4.2.1]
+ /// 3. correctness of Cose key parameters of the `root_key`
+ /// 4. if dice_cert_chain is present, check for each DiceChainEntry,
/// i. Cose key parameters of `subject_pub_key`
/// ii. the signature is verified with the parent's `subject_pub_key` or with the `root_key`
/// for the first DiceChainEntry
@@ -475,6 +494,12 @@
if self.version != EXPLICIT_KEY_DICE_CERT_CHAIN_VERSION {
return Err(ag_err!(InvalidCertChain, "version mismatch"));
}
+ if !self.root_key.is_canonicalized() {
+ return Err(ag_err!(
+ InvalidCertChain,
+ "root key is not in the required canonical form"
+ ));
+ }
self.root_key.validate_cose_key_params()?;
match &self.dice_cert_chain {
None => Ok(self.root_key.clone()),
@@ -550,7 +575,6 @@
} else {
None
};
- // TODO: Confirm whether root key is a bstr or a map with a single entry.
let root_cose_key = match array.remove(1) {
Value::Bytes(root_key_encoded) => {
let cose_key = CoseKey::from_slice(&root_key_encoded)?;
@@ -573,10 +597,15 @@
})
}
- fn to_cbor_value(self) -> Result<Value, CoseError> {
+ fn to_cbor_value(mut self) -> Result<Value, CoseError> {
let mut array = Vec::<Value>::new();
array.try_reserve(2).map_err(|_| CoseError::EncodeFailed)?;
array.push(Value::Integer(self.version.into()));
+ // Prepare the root key to be encoded in accordance with
+ // Core Deterministic Encoding Requirements [RFC 8949 s4.2.1], as specified in
+ // hardware/interfaces/security/authgraph/aidl/android/hardware/security/authgraph/
+ // ExplicitKeyDiceCertChain.cddl
+ self.root_key.canonicalize_cose_key();
array.push(Value::Bytes(self.root_key.get_key().to_vec()?));
if let Some(dice_chain_entries) = self.dice_cert_chain {
let len = dice_chain_entries.len();
diff --git a/core/src/keyexchange.rs b/core/src/keyexchange.rs
index e6353ba..a6d03c7 100644
--- a/core/src/keyexchange.rs
+++ b/core/src/keyexchange.rs
@@ -149,6 +149,9 @@
&mut *self.crypto.rng,
)?;
let (_sign_key, identity) = self.device.borrow().get_identity()?;
+ // Validate the identity returned from the trait implementation during the first time it is
+ // retrieved from the AuthGraphParticipant.
+ identity.validate(&*self.crypto.ecdsa)?;
let ke_key =
Key { pub_key: Some(pub_key_for_ecdh.0.to_vec()?), arc_from_pbk: Some(priv_key_arc) };
let ke_digest = compute_ke_key_digest(&ke_key, &*self.crypto.sha256)?;
@@ -181,7 +184,9 @@
let ecdh_secret =
compute_shared_secret(&*self.crypto.ecdh, &key_for_ecdh.priv_key, &peer_pub_key)?;
let (sign_key, own_identity) = self.device.borrow().get_identity()?;
-
+ // Validate the identity returned from the trait implementation during the first time it is
+ // retrieved from the AuthGraphParticipant.
+ own_identity.validate(&*self.crypto.ecdsa)?;
let salt_input = SaltInput {
source_version: peer_version,
sink_ke_pub_key: key_for_ecdh.pub_key.clone(),
diff --git a/tests/src/lib.rs b/tests/src/lib.rs
index c5c7c3a..a95fa0e 100644
--- a/tests/src/lib.rs
+++ b/tests/src/lib.rs
@@ -6,7 +6,9 @@
CURVE25519_PRIV_KEY_LEN, EXPLICIT_KEY_DICE_CERT_CHAIN_VERSION, IDENTITY_VERSION,
};
use authgraph_core::keyexchange;
-use authgraph_core::traits::{AesGcm, EcDh, EcDsa, Hkdf, Hmac, MonotonicClock, Rng, Sha256};
+use authgraph_core::traits::{
+ AesGcm, Device, EcDh, EcDsa, Hkdf, Hmac, MonotonicClock, Rng, Sha256,
+};
use authgraph_core::{ag_err, error::Error};
use authgraph_wire::ErrorCode;
use coset::{
@@ -1222,3 +1224,10 @@
}
Err(ag_err!(InternalError, "this method supports the maximum length of 5 for a DICE chain"))
}
+
+/// Add a smoke test for `get_identity` in the `Device` trait, to ensure that the returned Identity
+/// passes a set of validation.
+pub fn test_get_identity<D: Device, E: EcDsa>(ag_device: &D, ecdsa: &E) {
+ let (_, identity) = ag_device.get_identity().unwrap();
+ identity.validate(ecdsa).unwrap();
+}