Revert^2 "pVM to use Secretkeeper protected secrets"
9f145f29aa217d89543fd86d7cc8e51d7dd6e071
These secrets are to be stored in Secretkeeper which provides
tamper-evident storage for pVMs.
Regular binder proxy cannot be transferred over RPC binder, so we build
SecretkeeperProxy service that forwards the rpc binder request from
within pVM to Secretkeeper HAL which is a regular binderized HAL. This
proxy service is hosted by virtualizationmanager.
Note on supported device: (is_sk_supported() method): Non protected VM
trusts the claim, whilst for protected VM, we require authentication
data from pvmfw. Support for pVM is not fully done (this doesn't affect
security since pvmfw does code_hash check).
Issue with original patch - SecretkeeperProxy did not implement
deleteIds & deleteAll, which were added to aidl few commits back & hence
presubmits didn't catch them.
Bug: 291213394
Test: MicrodroidTests#encryptedStorageIsPersistent
Change-Id: Ib2cc3d21bd6bd4c8b4e173f32ea680d67fb1d9ac
diff --git a/microdroid_manager/Android.bp b/microdroid_manager/Android.bp
index 8481edf..cb3b2aa 100644
--- a/microdroid_manager/Android.bp
+++ b/microdroid_manager/Android.bp
@@ -5,7 +5,10 @@
rust_defaults {
name: "microdroid_manager_defaults",
crate_name: "microdroid_manager",
- defaults: ["avf_build_flags_rust"],
+ defaults: [
+ "avf_build_flags_rust",
+ "secretkeeper_use_latest_hal_aidl_rust",
+ ],
srcs: ["src/main.rs"],
edition: "2021",
prefer_rlib: true,
@@ -43,6 +46,8 @@
"libprotobuf",
"librpcbinder_rs",
"librustutils",
+ "libsecretkeeper_client",
+ "libsecretkeeper_comm_nostd",
"libscopeguard",
"libserde",
"libserde_cbor",
@@ -51,6 +56,7 @@
"libuuid",
"libvsock",
"librand",
+ "libzeroize",
],
init_rc: ["microdroid_manager.rc"],
multilib: {
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 9e167a4..c94a937 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -105,7 +105,6 @@
MicrodroidError::PayloadInvalidConfig(msg) => {
(ErrorCode::PAYLOAD_INVALID_CONFIG, msg.to_string())
}
-
// Connection failure won't be reported to VS; return the default value
MicrodroidError::FailedToConnectToVirtualizationService(msg) => {
(ErrorCode::UNKNOWN, msg.to_string())
@@ -282,7 +281,8 @@
// To minimize the exposure to untrusted data, derive dice profile as soon as possible.
info!("DICE derivation for payload");
let dice_artifacts = dice_derivation(dice, &instance_data, &payload_metadata)?;
- let vm_secret = VmSecret::new(dice_artifacts).context("Failed to create VM secrets")?;
+ let vm_secret =
+ VmSecret::new(dice_artifacts, service).context("Failed to create VM secrets")?;
if cfg!(dice_changes) {
// Now that the DICE derivation is done, it's ok to allow payload code to run.
diff --git a/microdroid_manager/src/vm_secret.rs b/microdroid_manager/src/vm_secret.rs
index d84c2e2..df5d318 100644
--- a/microdroid_manager/src/vm_secret.rs
+++ b/microdroid_manager/src/vm_secret.rs
@@ -14,18 +14,28 @@
//! Class for encapsulating & managing represent VM secrets.
-use anyhow::Result;
+use anyhow::{anyhow, ensure, Result};
+use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::IVirtualMachineService;
+use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::ISecretkeeper::ISecretkeeper;
+use secretkeeper_comm::data_types::request::Request;
+use binder::{Strong};
+use coset::CborSerializable;
use diced_open_dice::{DiceArtifacts, OwnedDiceArtifacts};
use keystore2_crypto::ZVec;
use openssl::hkdf::hkdf;
use openssl::md::Md;
use openssl::sha;
+use secretkeeper_client::SkSession;
+use secretkeeper_comm::data_types::{Id, ID_SIZE, Secret, SECRET_SIZE};
+use secretkeeper_comm::data_types::response::Response;
+use secretkeeper_comm::data_types::packet::{ResponsePacket, ResponseType};
+use secretkeeper_comm::data_types::request_response_impl::{
+ StoreSecretRequest, GetSecretResponse, GetSecretRequest};
+use secretkeeper_comm::data_types::error::SecretkeeperError;
+use zeroize::Zeroizing;
const ENCRYPTEDSTORE_KEY_IDENTIFIER: &str = "encryptedstore_key";
-// Size of the secret stored in Secretkeeper.
-const SK_SECRET_SIZE: usize = 64;
-
// Generated using hexdump -vn32 -e'14/1 "0x%02X, " 1 "\n"' /dev/urandom
const SALT_ENCRYPTED_STORE: &[u8] = &[
0xFC, 0x1D, 0x35, 0x7B, 0x96, 0xF3, 0xEF, 0x17, 0x78, 0x7D, 0x70, 0xED, 0xEA, 0xFE, 0x1D, 0x6F,
@@ -36,6 +46,24 @@
0x55, 0xF8, 0x08, 0x23, 0x81, 0x5F, 0xF5, 0x16, 0x20, 0x3E, 0xBE, 0xBA, 0xB7, 0xA8, 0x43, 0x92,
];
+// TODO(b/291213394): Remove this once policy is generated from dice_chain
+const HYPOTHETICAL_DICE_POLICY: [u8; 43] = [
+ 0x83, 0x01, 0x81, 0x83, 0x01, 0x80, 0xA1, 0x01, 0x00, 0x82, 0x83, 0x01, 0x81, 0x01, 0x73, 0x74,
+ 0x65, 0x73, 0x74, 0x69, 0x6E, 0x67, 0x5F, 0x64, 0x69, 0x63, 0x65, 0x5F, 0x70, 0x6F, 0x6C, 0x69,
+ 0x63, 0x79, 0x83, 0x02, 0x82, 0x03, 0x18, 0x64, 0x19, 0xE9, 0x75,
+];
+// TODO(b/291213394): Differentiate the Id of nPVM based on 'salt'
+const ID_NP_VM: [u8; ID_SIZE] = [
+ 0xF1, 0xB2, 0xED, 0x3B, 0xD1, 0xBD, 0xF0, 0x7D, 0xE1, 0xF0, 0x01, 0xFC, 0x61, 0x71, 0xD3, 0x42,
+ 0xE5, 0x8A, 0xAF, 0x33, 0x6C, 0x11, 0xDC, 0xC8, 0x6F, 0xAE, 0x12, 0x5C, 0x26, 0x44, 0x6B, 0x86,
+ 0xCC, 0x24, 0xFD, 0xBF, 0x91, 0x4A, 0x54, 0x84, 0xF9, 0x01, 0x59, 0x25, 0x70, 0x89, 0x38, 0x8D,
+ 0x5E, 0xE6, 0x91, 0xDF, 0x68, 0x60, 0x69, 0x26, 0xBE, 0xFE, 0x79, 0x58, 0xF7, 0xEA, 0x81, 0x7D,
+];
+const SKP_SECRET_NP_VM: [u8; SECRET_SIZE] = [
+ 0xA9, 0x89, 0x97, 0xFE, 0xAE, 0x97, 0x55, 0x4B, 0x32, 0x35, 0xF0, 0xE8, 0x93, 0xDA, 0xEA, 0x24,
+ 0x06, 0xAC, 0x36, 0x8B, 0x3C, 0x95, 0x50, 0x16, 0x67, 0x71, 0x65, 0x26, 0xEB, 0xD0, 0xC3, 0x98,
+];
+
pub enum VmSecret {
// V2 secrets are derived from 2 independently secured secrets:
// 1. Secretkeeper protected secrets (skp secret).
@@ -54,15 +82,47 @@
V1 { dice: OwnedDiceArtifacts },
}
+fn get_id() -> [u8; ID_SIZE] {
+ if super::is_strict_boot() {
+ todo!("Id for protected VM is not implemented");
+ } else {
+ ID_NP_VM
+ }
+}
+
impl VmSecret {
- pub fn new(dice_artifacts: OwnedDiceArtifacts) -> Result<VmSecret> {
- if is_sk_supported() {
- // TODO(b/291213394): Change this to real Sk protected secret.
- let fake_skp_secret = ZVec::new(SK_SECRET_SIZE)?;
- return Ok(Self::V2 { dice: dice_artifacts, skp_secret: fake_skp_secret });
+ pub fn new(
+ dice_artifacts: OwnedDiceArtifacts,
+ vm_service: &Strong<dyn IVirtualMachineService>,
+ ) -> Result<VmSecret> {
+ ensure!(dice_artifacts.bcc().is_some(), "Dice chain missing");
+
+ if let Some(sk_service) = is_sk_supported(vm_service)? {
+ let id = get_id();
+ let mut skp_secret = Zeroizing::new([0u8; SECRET_SIZE]);
+ if super::is_strict_boot() {
+ if super::is_new_instance() {
+ *skp_secret = rand::random();
+ store_secret(sk_service.clone(), id, skp_secret.clone(), &dice_artifacts)?;
+ } else {
+ // Subsequent run of the pVM -> get the secret stored in Secretkeeper.
+ *skp_secret = get_secret(sk_service.clone(), id, &dice_artifacts)?;
+ }
+ } else {
+ // TODO(b/291213394): Non protected VM don't need to use Secretkeeper, remove this
+ // once we have sufficient testing on protected VM.
+ store_secret(sk_service.clone(), id, SKP_SECRET_NP_VM.into(), &dice_artifacts)?;
+ *skp_secret = get_secret(sk_service.clone(), id, &dice_artifacts)?;
+ }
+ return Ok(Self::V2 {
+ dice: dice_artifacts,
+ skp_secret: ZVec::try_from(skp_secret.to_vec())?,
+ });
}
+ // Use V1 secrets if Secretkeeper is not supported.
Ok(Self::V1 { dice: dice_artifacts })
}
+
pub fn dice(&self) -> &OwnedDiceArtifacts {
match self {
Self::V2 { dice, .. } => dice,
@@ -94,13 +154,87 @@
}
}
-// Does the hardware support Secretkeeper.
-fn is_sk_supported() -> bool {
- if cfg!(llpvm_changes) {
- return false;
+fn store_secret(
+ secretkeeper: binder::Strong<dyn ISecretkeeper>,
+ id: [u8; ID_SIZE],
+ secret: Zeroizing<[u8; SECRET_SIZE]>,
+ _dice_chain: &OwnedDiceArtifacts,
+) -> Result<()> {
+ // Start a new secretkeeper session!
+ let session = SkSession::new(secretkeeper).map_err(anyhow_err)?;
+ let store_request = StoreSecretRequest {
+ id: Id(id),
+ secret: Secret(*secret),
+ // TODO(b/291233371): Construct policy out of dice_chain.
+ sealing_policy: HYPOTHETICAL_DICE_POLICY.to_vec(),
};
- // TODO(b/292209416): This value should be extracted from device tree.
- // Note: this does not affect the security of pVM. pvmfw & microdroid_manager continue to block
- // upgraded images. Setting this true is equivalent to including constant salt in vm secrets.
- true
+ log::info!("Secretkeeper operation: {:?}", store_request);
+
+ let store_request = store_request.serialize_to_packet().to_vec().map_err(anyhow_err)?;
+ let store_response = session.secret_management_request(&store_request).map_err(anyhow_err)?;
+ let store_response = ResponsePacket::from_slice(&store_response).map_err(anyhow_err)?;
+ let response_type = store_response.response_type().map_err(anyhow_err)?;
+ ensure!(
+ response_type == ResponseType::Success,
+ "Secretkeeper store failed with error: {:?}",
+ *SecretkeeperError::deserialize_from_packet(store_response).map_err(anyhow_err)?
+ );
+ Ok(())
+}
+
+fn get_secret(
+ secretkeeper: binder::Strong<dyn ISecretkeeper>,
+ id: [u8; ID_SIZE],
+ _dice_chain: &OwnedDiceArtifacts,
+) -> Result<[u8; SECRET_SIZE]> {
+ // Start a new secretkeeper session!
+ let session = SkSession::new(secretkeeper).map_err(anyhow_err)?;
+ let get_request = GetSecretRequest {
+ id: Id(id),
+ // TODO(b/291233371): Construct policy out of dice_chain.
+ updated_sealing_policy: None,
+ };
+ log::info!("Secretkeeper operation: {:?}", get_request);
+
+ let get_request = get_request.serialize_to_packet().to_vec().map_err(anyhow_err)?;
+ let get_response = session.secret_management_request(&get_request).map_err(anyhow_err)?;
+ let get_response = ResponsePacket::from_slice(&get_response).map_err(anyhow_err)?;
+ let response_type = get_response.response_type().map_err(anyhow_err)?;
+ ensure!(
+ response_type == ResponseType::Success,
+ "Secretkeeper get failed with error: {:?}",
+ *SecretkeeperError::deserialize_from_packet(get_response).map_err(anyhow_err)?
+ );
+ let get_response =
+ *GetSecretResponse::deserialize_from_packet(get_response).map_err(anyhow_err)?;
+ Ok(get_response.secret.0)
+}
+
+#[inline]
+fn anyhow_err<E: core::fmt::Debug>(err: E) -> anyhow::Error {
+ anyhow!("{:?}", err)
+}
+
+// Get the secretkeeper connection if supported. Host can be consulted whether the device supports
+// secretkeeper but that should be used with caution for protected VM.
+fn is_sk_supported(
+ host: &Strong<dyn IVirtualMachineService>,
+) -> Result<Option<Strong<dyn ISecretkeeper>>> {
+ let sk = if cfg!(llpvm_changes) {
+ if super::is_strict_boot() {
+ // TODO: For protected VM check for Secretkeeper authentication data in device tree.
+ None
+ } else {
+ // For non-protected VM, believe what host claims.
+ host.getSecretkeeper()
+ // TODO rename this error!
+ .map_err(|e| {
+ super::MicrodroidError::FailedToConnectToVirtualizationService(e.to_string())
+ })?
+ }
+ } else {
+ // LLPVM flag is disabled
+ None
+ };
+ Ok(sk)
}
diff --git a/virtualizationmanager/Android.bp b/virtualizationmanager/Android.bp
index 33897b2..88e9c70 100644
--- a/virtualizationmanager/Android.bp
+++ b/virtualizationmanager/Android.bp
@@ -5,7 +5,11 @@
rust_defaults {
name: "virtualizationmanager_defaults",
crate_name: "virtualizationmanager",
- defaults: ["avf_build_flags_rust"],
+ defaults: [
+ "avf_build_flags_rust",
+ "secretkeeper_use_latest_hal_aidl_rust",
+ "authgraph_use_latest_hal_aidl_rust",
+ ],
edition: "2021",
// Only build on targets which crosvm builds on.
enabled: false,
diff --git a/virtualizationmanager/src/aidl.rs b/virtualizationmanager/src/aidl.rs
index 7f98fe8..7b30f48 100644
--- a/virtualizationmanager/src/aidl.rs
+++ b/virtualizationmanager/src/aidl.rs
@@ -52,6 +52,14 @@
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::{
BnVirtualMachineService, IVirtualMachineService,
};
+use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::ISecretkeeper::{BnSecretkeeper, ISecretkeeper};
+use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::SecretId::SecretId;
+use android_hardware_security_authgraph::aidl::android::hardware::security::authgraph::{
+ Arc::Arc as AuthgraphArc, IAuthGraphKeyExchange::IAuthGraphKeyExchange,
+ IAuthGraphKeyExchange::BnAuthGraphKeyExchange, Identity::Identity, KeInitResult::KeInitResult,
+ Key::Key, PubKey::PubKey, SessionIdSignature::SessionIdSignature, SessionInfo::SessionInfo,
+ SessionInitiationInfo::SessionInitiationInfo,
+};
use anyhow::{anyhow, bail, Context, Result};
use apkverify::{HashAlgorithm, V4Signature};
use avflog::LogResult;
@@ -102,6 +110,10 @@
const MICRODROID_OS_NAME: &str = "microdroid";
+// TODO(b/291213394): Use 'default' instance for secretkeeper instead of 'nonsecure'
+const SECRETKEEPER_IDENTIFIER: &str =
+ "android.hardware.security.secretkeeper.ISecretkeeper/nonsecure";
+
const UNFORMATTED_STORAGE_MAGIC: &str = "UNFORMATTED-STORAGE";
/// Roughly estimated sufficient size for storing vendor public key into DTBO.
@@ -1370,6 +1382,20 @@
}
}
+ fn getSecretkeeper(&self) -> binder::Result<Option<Strong<dyn ISecretkeeper>>> {
+ let sk = match binder::get_interface(SECRETKEEPER_IDENTIFIER) {
+ Ok(sk) => {
+ Some(BnSecretkeeper::new_binder(SecretkeeperProxy(sk), BinderFeatures::default()))
+ }
+ Err(StatusCode::NAME_NOT_FOUND) => None,
+ Err(e) => {
+ error!("unexpected error while fetching connection to Secretkeeper {:?}", e);
+ return Err(e.into());
+ }
+ };
+ Ok(sk)
+ }
+
fn requestAttestation(&self, csr: &[u8]) -> binder::Result<Vec<Certificate>> {
GLOBAL_SERVICE.requestAttestation(csr, get_calling_uid() as i32)
}
@@ -1554,3 +1580,67 @@
Ok(())
}
}
+
+struct SecretkeeperProxy(Strong<dyn ISecretkeeper>);
+
+impl Interface for SecretkeeperProxy {}
+
+impl ISecretkeeper for SecretkeeperProxy {
+ fn processSecretManagementRequest(&self, req: &[u8]) -> binder::Result<Vec<u8>> {
+ // Pass the request to the channel, and read the response.
+ self.0.processSecretManagementRequest(req)
+ }
+
+ fn getAuthGraphKe(&self) -> binder::Result<Strong<dyn IAuthGraphKeyExchange>> {
+ let ag = AuthGraphKeyExchangeProxy(self.0.getAuthGraphKe()?);
+ Ok(BnAuthGraphKeyExchange::new_binder(ag, BinderFeatures::default()))
+ }
+
+ fn deleteIds(&self, ids: &[SecretId]) -> binder::Result<()> {
+ self.0.deleteIds(ids)
+ }
+
+ fn deleteAll(&self) -> binder::Result<()> {
+ self.0.deleteAll()
+ }
+}
+
+struct AuthGraphKeyExchangeProxy(Strong<dyn IAuthGraphKeyExchange>);
+
+impl Interface for AuthGraphKeyExchangeProxy {}
+
+impl IAuthGraphKeyExchange for AuthGraphKeyExchangeProxy {
+ fn create(&self) -> binder::Result<SessionInitiationInfo> {
+ self.0.create()
+ }
+
+ fn init(
+ &self,
+ peer_pub_key: &PubKey,
+ peer_id: &Identity,
+ peer_nonce: &[u8],
+ peer_version: i32,
+ ) -> binder::Result<KeInitResult> {
+ self.0.init(peer_pub_key, peer_id, peer_nonce, peer_version)
+ }
+
+ fn finish(
+ &self,
+ peer_pub_key: &PubKey,
+ peer_id: &Identity,
+ peer_signature: &SessionIdSignature,
+ peer_nonce: &[u8],
+ peer_version: i32,
+ own_key: &Key,
+ ) -> binder::Result<SessionInfo> {
+ self.0.finish(peer_pub_key, peer_id, peer_signature, peer_nonce, peer_version, own_key)
+ }
+
+ fn authenticationComplete(
+ &self,
+ peer_signature: &SessionIdSignature,
+ shared_keys: &[AuthgraphArc; 2],
+ ) -> binder::Result<[AuthgraphArc; 2]> {
+ self.0.authenticationComplete(peer_signature, shared_keys)
+ }
+}
diff --git a/virtualizationservice/aidl/Android.bp b/virtualizationservice/aidl/Android.bp
index c69fe8f..8ca375a 100644
--- a/virtualizationservice/aidl/Android.bp
+++ b/virtualizationservice/aidl/Android.bp
@@ -57,7 +57,10 @@
aidl_interface {
name: "android.system.virtualmachineservice",
srcs: ["android/system/virtualmachineservice/**/*.aidl"],
- imports: ["android.system.virtualizationcommon"],
+ imports: [
+ "android.hardware.security.secretkeeper-V1",
+ "android.system.virtualizationcommon",
+ ],
unstable: true,
backend: {
java: {
diff --git a/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl b/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
index 3c60478..cf91302 100644
--- a/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
+++ b/virtualizationservice/aidl/android/system/virtualmachineservice/IVirtualMachineService.aidl
@@ -15,6 +15,7 @@
*/
package android.system.virtualmachineservice;
+import android.hardware.security.secretkeeper.ISecretkeeper;
import android.system.virtualizationcommon.Certificate;
import android.system.virtualizationcommon.ErrorCode;
@@ -54,4 +55,11 @@
* key's certificate chain. The attestation key is provided in the CSR.
*/
Certificate[] requestAttestation(in byte[] csr);
+
+ /**
+ * Request connection to Secretkeeper. This is used by pVM to store Anti-Rollback protected
+ * secrets. Note that the return value is nullable to reflect that Secretkeeper HAL may not be
+ * present.
+ */
+ @nullable ISecretkeeper getSecretkeeper();
}