Tidy and add trait smoke test
- Add a smoke test for the RKP trait.
- Drop some TODOs that aren't relevant
- Fix some clippy warnings.
- Convert some comments to doc comments.
Bug: 258069484
Test: libkmr_cf_test
Change-Id: Ic2f1c4db59462c8f0869332e85c1f861e5a39afb
diff --git a/common/src/crypto/ec.rs b/common/src/crypto/ec.rs
index bad05c6..ac6cae4 100644
--- a/common/src/crypto/ec.rs
+++ b/common/src/crypto/ec.rs
@@ -352,14 +352,15 @@
/// Import an NIST EC key in SEC1 ECPrivateKey format.
pub fn import_sec1_private_key(data: &[u8]) -> Result<KeyMaterial, Error> {
let ec_key = sec1::EcPrivateKey::from_der(data)?;
- let ec_parameters = ec_key.parameters.ok_or(km_err!(
- InvalidArgument,
- "sec1 formatted EC private key didn't have a parameters field"
- ))?;
- let parameters_oid = ec_parameters.named_curve().ok_or(km_err!(
- InvalidArgument,
- "couldn't retrieve parameters oid from sec1 ECPrivateKey formatted ec key parameters"
- ))?;
+ let ec_parameters = ec_key.parameters.ok_or_else(|| {
+ km_err!(InvalidArgument, "sec1 formatted EC private key didn't have a parameters field")
+ })?;
+ let parameters_oid = ec_parameters.named_curve().ok_or_else(|| {
+ km_err!(
+ InvalidArgument,
+ "couldn't retrieve parameters oid from sec1 ECPrivateKey formatted ec key parameters"
+ )
+ })?;
let algorithm =
AlgorithmIdentifier { oid: X509_NIST_OID, parameters: Some(AnyRef::from(¶meters_oid)) };
let pkcs8_key = pkcs8::PrivateKeyInfo::new(algorithm, data);
diff --git a/ta/src/device.rs b/ta/src/device.rs
index 0e62897..410d02c 100644
--- a/ta/src/device.rs
+++ b/ta/src/device.rs
@@ -167,10 +167,9 @@
/// implementation of IRemotelyProvisionedComponent (IRPC) HAL.
/// Note: The devices only supporting IRPC V3+ may ignore the optional IRPC V2 specific types in
/// the method signatures.
-/// TODO (b/258069484): Add smoke tests to this device trait.
pub trait RetrieveRpcArtifacts {
- // Retrieve secret bytes (of the given output length) derived from a hardware backed key.
- // For a given context, the output is deterministic.
+ /// Retrieve secret bytes (of the given output length) derived from a hardware backed key.
+ /// For a given context, the output is deterministic.
fn derive_bytes_from_hbk(
&self,
hkdf: &dyn crypto::Hkdf,
@@ -178,7 +177,7 @@
output_len: usize,
) -> Result<Vec<u8>, Error>;
- // Compute HMAC_SHA256 over the given input using a key derived from hardware.
+ /// Compute HMAC_SHA256 over the given input using a key derived from hardware.
fn compute_hmac_sha256(
&self,
hmac: &dyn crypto::Hmac,
@@ -189,16 +188,16 @@
crypto::hmac_sha256(hmac, &secret, input)
}
- // Retrieve the information about the DICE chain belonging to the IRPC HAL implementation.
+ /// Retrieve the information about the DICE chain belonging to the IRPC HAL implementation.
fn get_dice_info(&self, test_mode: rpc::TestMode) -> Result<DiceInfo, Error>;
- // Sign the input data with the CDI leaf private key of the IRPC HAL implementation. In IRPC V2,
- // the `data` to be signed is the [`SignedMac_structure`] in ProtectedData.aidl, when signing
- // the ephemeral MAC key used to authenticate the public keys. In IRPC V3, the `data` to be
- // signed is the [`SignedDataSigStruct`].
- // If a particular implementation would like to return the signature in a COSE_Sign1 message,
- // they can mark this unimplemented and override the default implementation in the
- // `sign_data_in_cose_sign1` method below.
+ /// Sign the input data with the CDI leaf private key of the IRPC HAL implementation. In IRPC V2,
+ /// the `data` to be signed is the [`SignedMac_structure`] in ProtectedData.aidl, when signing
+ /// the ephemeral MAC key used to authenticate the public keys. In IRPC V3, the `data` to be
+ /// signed is the [`SignedDataSigStruct`].
+ /// If a particular implementation would like to return the signature in a COSE_Sign1 message,
+ /// they can mark this unimplemented and override the default implementation in the
+ /// `sign_data_in_cose_sign1` method below.
fn sign_data(
&self,
ec: &dyn crypto::Ec,
@@ -206,9 +205,9 @@
rpc_v2: Option<RpcV2Req>,
) -> Result<Vec<u8>, Error>;
- // Sign the payload and return a COSE_Sign1 message. In IRPC V2, the `payload` is the MAC Key.
- // In IRPC V3, the `payload` is the `Data` that the `SignedData` is parameterized with (i.e. a
- // CBOR array containing `challenge` and `CsrPayload`).
+ /// Sign the payload and return a COSE_Sign1 message. In IRPC V2, the `payload` is the MAC Key.
+ /// In IRPC V3, the `payload` is the `Data` that the `SignedData` is parameterized with (i.e. a
+ /// CBOR array containing `challenge` and `CsrPayload`).
fn sign_data_in_cose_sign1(
&self,
ec: &dyn crypto::Ec,
diff --git a/ta/src/keys.rs b/ta/src/keys.rs
index e7bcef6..50418f7 100644
--- a/ta/src/keys.rs
+++ b/ta/src/keys.rs
@@ -501,7 +501,8 @@
return Err(km_err!(InvalidArgument, "invalid version in Secure Key Wrapper."));
}
- // Decrypt the masked transport key.
+ // Decrypt the masked transport key, using an RSA key. (Only RSA wrapping keys are supported
+ // by the spec, as RSA is the only algorithm supporting asymmetric decryption.)
let masked_transport_key = match key_material {
KeyMaterial::Rsa(key) => {
// Check the requirements on the wrapping key characterisitcs
@@ -515,7 +516,6 @@
crypto_op.as_mut().update(secure_key_wrapper.encrypted_transport_key)?;
crypto_op.finish()?
}
- // TODO: For now, we consider wrapping keys to be RSA keys only.
_ => {
return Err(km_err!(InvalidArgument, "invalid key algorithm for transport key"));
}
diff --git a/ta/src/lib.rs b/ta/src/lib.rs
index c0a6f62..de047c7 100644
--- a/ta/src/lib.rs
+++ b/ta/src/lib.rs
@@ -106,12 +106,12 @@
/// Attestation ID information, fixed forever for a device, but retrieved on first use.
attestation_id_info: RefCell<Option<Rc<AttestationIdInfo>>>,
- // Public DICE artifacts (UDS certs and the DICE chain) included in the certificate signing
- // requests (CSR) and the algorithm used to sign the CSR for IRemotelyProvisionedComponent
- // (IRPC) HAL. Fixed for a device. Retrieved on first use.
- //
- // Note: This information is cached only in the implementations of IRPC HAL V3 and
- // IRPC HAL V2 in production mode.
+ /// Public DICE artifacts (UDS certs and the DICE chain) included in the certificate signing
+ /// requests (CSR) and the algorithm used to sign the CSR for IRemotelyProvisionedComponent
+ /// (IRPC) HAL. Fixed for a device. Retrieved on first use.
+ ///
+ /// Note: This information is cached only in the implementations of IRPC HAL V3 and
+ /// IRPC HAL V2 in production mode.
dice_info: RefCell<Option<Rc<DiceInfo>>>,
/// Whether the device is still in early-boot.
diff --git a/ta/src/rkp.rs b/ta/src/rkp.rs
index 5424463..cfb0b7d 100644
--- a/ta/src/rkp.rs
+++ b/ta/src/rkp.rs
@@ -186,7 +186,7 @@
return Err(rpc_err!(Removed, "generate_cert_req is not supported in IRPC V3+ HAL."));
}
let _device_info = self.rpc_device_info()?;
- Err(km_err!(Unimplemented, "TODO: GenerateCertificateRequest"))
+ Err(km_err!(Unimplemented, "GenerateCertificateRequest is only required for RKP before v3"))
}
pub(crate) fn generate_cert_req_v2(
diff --git a/tests/src/lib.rs b/tests/src/lib.rs
index acad50a..032fab9 100644
--- a/tests/src/lib.rs
+++ b/tests/src/lib.rs
@@ -7,7 +7,7 @@
};
use kmr_common::{keyblob, keyblob::SlotPurpose};
use kmr_ta::device::{SigningAlgorithm, SigningKey, SigningKeyType};
-use kmr_wire::keymint::Digest;
+use kmr_wire::{keymint::Digest, rpc};
use std::collections::HashMap;
use x509_cert::der::{Decode, Encode};
@@ -576,3 +576,21 @@
}
}
}
+
+/// Simple smoke test for an `RetrieveRpcArtifacts` trait implementation.
+pub fn test_retrieve_rpc_artifacts<T: kmr_ta::device::RetrieveRpcArtifacts>(
+ rpc: T,
+ hmac: &dyn Hmac,
+ hkdf: &dyn Hkdf,
+) {
+ assert!(rpc.get_dice_info(rpc::TestMode(false)).is_ok());
+
+ let context = b"abcdef";
+ let data1 = rpc.derive_bytes_from_hbk(hkdf, context, 16).expect("failed to derive from HBK");
+ let data2 = rpc.derive_bytes_from_hbk(hkdf, context, 16).expect("failed to derive from HBK");
+ assert_eq!(data1, data2, "derive_bytes_from_hbk() method should be deterministic");
+
+ let data1 = rpc.compute_hmac_sha256(hmac, hkdf, context).expect("failed to perform HMAC");
+ let data2 = rpc.compute_hmac_sha256(hmac, hkdf, context).expect("failed to perform HMAC");
+ assert_eq!(data1, data2, "compute_hmac_sha256() method should be deterministic");
+}
