Migrate ComposKeyTestCase to unit tests
Refactor SigningKey to allow it to be decoupled from Dice for
testing. Add unit tests with the same coverage as ComposKeyTestCase,
then remove the latter. (This does mean we are no longer testing the
code inside a VM, but that's covered by ComposTestCase and the end to
end tests.)
As a result we no longer need compos_key_cmd, so remove that. This is
a win as it was becoming a maintenance burden.
Bug: 213891964
Test: atest compsvc_device_tests
Change-Id: If863abcb4e89eeb97a4be6a4a958b691aa1446be
diff --git a/compos/src/artifact_signer.rs b/compos/src/artifact_signer.rs
index e1b0efa..b5eb8cb 100644
--- a/compos/src/artifact_signer.rs
+++ b/compos/src/artifact_signer.rs
@@ -20,7 +20,7 @@
#![allow(dead_code)] // Will be used soon
use crate::fsverity;
-use crate::signing_key::Signer;
+use crate::signing_key::DiceSigner;
use anyhow::{anyhow, Context, Result};
use odsign_proto::odsign_info::OdsignInfo;
use protobuf::Message;
@@ -63,7 +63,7 @@
/// Consume this ArtifactSigner and write details of all its artifacts to the given path,
/// with accompanying sigature file.
- pub fn write_info_and_signature(self, signer: Signer, info_path: &Path) -> Result<()> {
+ pub fn write_info_and_signature(self, signer: DiceSigner, info_path: &Path) -> Result<()> {
let mut info = OdsignInfo::new();
info.mut_file_hashes().extend(self.file_digests.into_iter());
let bytes = info.write_to_bytes()?;
diff --git a/compos/src/compilation.rs b/compos/src/compilation.rs
index 7e3834a..48ba4a6 100644
--- a/compos/src/compilation.rs
+++ b/compos/src/compilation.rs
@@ -27,7 +27,7 @@
use std::process::Command;
use crate::artifact_signer::ArtifactSigner;
-use crate::signing_key::Signer;
+use crate::signing_key::DiceSigner;
use authfs_aidl_interface::aidl::com::android::virt::fs::{
AuthFsConfig::{
AuthFsConfig, InputDirFdAnnotation::InputDirFdAnnotation,
@@ -101,7 +101,7 @@
odrefresh_path: &Path,
context: OdrefreshContext,
authfs_service: Strong<dyn IAuthFsService>,
- signer: Signer,
+ signer: DiceSigner,
) -> Result<ExitCode> {
// Mount authfs (via authfs_service). The authfs instance unmounts once the `authfs` variable
// is out of scope.
diff --git a/compos/src/compsvc.rs b/compos/src/compsvc.rs
index 9d754a7..3ec15dd 100644
--- a/compos/src/compsvc.rs
+++ b/compos/src/compsvc.rs
@@ -27,7 +27,8 @@
use std::sync::RwLock;
use crate::compilation::{odrefresh, OdrefreshContext};
-use crate::signing_key::{Signer, SigningKey};
+use crate::dice::Dice;
+use crate::signing_key::{DiceSigner, DiceSigningKey};
use authfs_aidl_interface::aidl::com::android::virt::fs::IAuthFsService::IAuthFsService;
use compos_aidl_interface::aidl::com::android::compos::{
CompOsKeyData::CompOsKeyData,
@@ -44,7 +45,7 @@
pub fn new_binder() -> Result<Strong<dyn ICompOsService>> {
let service = CompOsService {
odrefresh_path: PathBuf::from(ODREFRESH_PATH),
- signing_key: SigningKey::new()?,
+ signing_key: DiceSigningKey::new(Dice::new()?),
key_blob: RwLock::new(Vec::new()),
};
Ok(BnCompOsService::new_binder(service, BinderFeatures::default()))
@@ -52,12 +53,12 @@
struct CompOsService {
odrefresh_path: PathBuf,
- signing_key: SigningKey,
+ signing_key: DiceSigningKey,
key_blob: RwLock<Vec<u8>>,
}
impl CompOsService {
- fn new_signer(&self) -> BinderResult<Signer> {
+ fn new_signer(&self) -> BinderResult<DiceSigner> {
let key = &*self.key_blob.read().unwrap();
if key.is_empty() {
Err(new_binder_exception(ExceptionCode::ILLEGAL_STATE, "Key is not initialized"))
diff --git a/compos/src/dice.rs b/compos/src/dice.rs
index 9f66b5e..25148ab 100644
--- a/compos/src/dice.rs
+++ b/compos/src/dice.rs
@@ -20,6 +20,7 @@
use android_security_dice::binder::{wait_for_interface, Strong};
use anyhow::{Context, Result};
+#[derive(Clone)]
pub struct Dice {
node: Strong<dyn IDiceNode>,
}
diff --git a/compos/src/signing_key.rs b/compos/src/signing_key.rs
index 175a11b..90beecf 100644
--- a/compos/src/signing_key.rs
+++ b/compos/src/signing_key.rs
@@ -28,13 +28,26 @@
signature,
};
-pub struct SigningKey {
- _unused: (), // Prevent construction other than by new()
+pub type DiceSigningKey = SigningKey<Dice>;
+pub type DiceSigner = Signer<Dice>;
+
+pub struct SigningKey<T: SecretStore> {
+ secret_store: T,
}
-impl SigningKey {
- pub fn new() -> Result<Self> {
- Ok(Self { _unused: () })
+pub trait SecretStore: Clone {
+ fn get_secret(&self) -> Result<Vec<u8>>;
+}
+
+impl SecretStore for Dice {
+ fn get_secret(&self) -> Result<Vec<u8>> {
+ self.get_sealing_cdi()
+ }
+}
+
+impl<T: SecretStore> SigningKey<T> {
+ pub fn new(secret_store: T) -> Self {
+ Self { secret_store }
}
pub fn generate(&self) -> Result<CompOsKeyData> {
@@ -43,7 +56,8 @@
bail!("Failed to generate key pair: {}", key_result.error);
}
- let encrypted = encrypt_private_key(&Dice::new()?, &key_result.private_key)?;
+ let encrypted =
+ encrypt_private_key(&self.secret_store.get_secret()?, &key_result.private_key)?;
Ok(CompOsKeyData { publicKey: key_result.public_key, keyBlob: encrypted })
}
@@ -63,19 +77,19 @@
Ok(())
}
- pub fn new_signer(&self, key_blob: &[u8]) -> Result<Signer> {
- Ok(Signer { key_blob: key_blob.to_owned(), dice: Dice::new()? })
+ pub fn new_signer(&self, key_blob: &[u8]) -> Result<Signer<T>> {
+ Ok(Signer { key_blob: key_blob.to_owned(), secret_store: self.secret_store.clone() })
}
}
-pub struct Signer {
+pub struct Signer<T: SecretStore> {
key_blob: Vec<u8>,
- dice: Dice,
+ secret_store: T,
}
-impl Signer {
+impl<T: SecretStore> Signer<T> {
pub fn sign(self, data: &[u8]) -> Result<Vec<u8>> {
- let private_key = decrypt_private_key(&self.dice, &self.key_blob)?;
+ let private_key = decrypt_private_key(&self.secret_store.get_secret()?, &self.key_blob)?;
let sign_result = compos_native::sign(&private_key, data);
if sign_result.signature.is_empty() {
bail!("Failed to sign: {}", sign_result.error);
@@ -84,14 +98,69 @@
}
}
-fn encrypt_private_key(dice: &Dice, private_key: &[u8]) -> Result<Vec<u8>> {
- let cdi = dice.get_sealing_cdi()?;
- let aead_key = blob_encryption::derive_aead_key(&cdi)?;
+fn encrypt_private_key(vm_secret: &[u8], private_key: &[u8]) -> Result<Vec<u8>> {
+ let aead_key = blob_encryption::derive_aead_key(vm_secret)?;
blob_encryption::encrypt_bytes(aead_key, private_key)
}
-fn decrypt_private_key(dice: &Dice, blob: &[u8]) -> Result<Vec<u8>> {
- let cdi = dice.get_sealing_cdi()?;
- let aead_key = blob_encryption::derive_aead_key(&cdi)?;
+fn decrypt_private_key(vm_secret: &[u8], blob: &[u8]) -> Result<Vec<u8>> {
+ let aead_key = blob_encryption::derive_aead_key(vm_secret)?;
blob_encryption::decrypt_bytes(aead_key, blob)
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ const SECRET: &[u8] = b"This is not very secret";
+
+ #[derive(Clone)]
+ struct TestSecretStore;
+
+ impl SecretStore for TestSecretStore {
+ fn get_secret(&self) -> Result<Vec<u8>> {
+ Ok(SECRET.to_owned())
+ }
+ }
+
+ type TestSigningKey = SigningKey<TestSecretStore>;
+
+ fn signing_key_for_test() -> TestSigningKey {
+ TestSigningKey::new(TestSecretStore)
+ }
+
+ #[test]
+ fn test_generated_key_verifies() -> Result<()> {
+ let signing_key = signing_key_for_test();
+ let key_pair = signing_key.generate()?;
+
+ signing_key.verify(&key_pair.keyBlob, &key_pair.publicKey)
+ }
+
+ #[test]
+ fn test_bogus_key_pair_rejected() -> Result<()> {
+ let signing_key = signing_key_for_test();
+ let key_pair = signing_key.generate()?;
+
+ // Swap public key & key blob - clearly invalid
+ assert!(signing_key.verify(&key_pair.publicKey, &key_pair.keyBlob).is_err());
+
+ Ok(())
+ }
+
+ #[test]
+ fn test_mismatched_key_rejected() -> Result<()> {
+ let signing_key = signing_key_for_test();
+ let key_pair1 = signing_key.generate()?;
+ let key_pair2 = signing_key.generate()?;
+
+ // Both pairs should be valid
+ signing_key.verify(&key_pair1.keyBlob, &key_pair1.publicKey)?;
+ signing_key.verify(&key_pair2.keyBlob, &key_pair2.publicKey)?;
+
+ // But using the public key from one and the private key from the other should not,
+ // even though both are well-formed
+ assert!(signing_key.verify(&key_pair1.publicKey, &key_pair2.keyBlob).is_err());
+ Ok(())
+ }
+}