Merge "virtmgr: Pass hugepages to crosvm" into main
diff --git a/docs/custom_vm.md b/docs/custom_vm.md
index b218a5e..57a1b0d 100644
--- a/docs/custom_vm.md
+++ b/docs/custom_vm.md
@@ -39,7 +39,7 @@
First, check out source code from the ChromiumOS and Chromium projects.
* Checking out ChromiumOS: https://www.chromium.org/chromium-os/developer-library/guides/development/developer-guide/
-* Checking out Chromium: https://g3doc.corp.google.com/chrome/chromeos/system_services_team/dev_instructions/g3doc/setup_checkout.md?cl=headless
+* Checking out Chromium: https://g3doc.corp.google.com/chrome/chromeos/system_services_team/dev_instructions/g3doc/setup_checkout.md?cl=head
Important: When you are at the step “Set up gclient args” in the Chromium checkout instruction, configure .gclient as follows.
diff --git a/java/framework/src/android/system/virtualmachine/VirtualMachineConfig.java b/java/framework/src/android/system/virtualmachine/VirtualMachineConfig.java
index a8f318c..1b915cd 100644
--- a/java/framework/src/android/system/virtualmachine/VirtualMachineConfig.java
+++ b/java/framework/src/android/system/virtualmachine/VirtualMachineConfig.java
@@ -601,7 +601,7 @@
config.name = Optional.ofNullable(customImageConfig.getName()).orElse("");
config.instanceId = new byte[64];
config.kernel =
- Optional.of(customImageConfig.getKernelPath())
+ Optional.ofNullable(customImageConfig.getKernelPath())
.map(
(path) -> {
try {
diff --git a/java/framework/src/android/system/virtualmachine/VirtualMachineCustomImageConfig.java b/java/framework/src/android/system/virtualmachine/VirtualMachineCustomImageConfig.java
index 8d294fd..8ec9d2c 100644
--- a/java/framework/src/android/system/virtualmachine/VirtualMachineCustomImageConfig.java
+++ b/java/framework/src/android/system/virtualmachine/VirtualMachineCustomImageConfig.java
@@ -16,7 +16,6 @@
package android.system.virtualmachine;
-import android.annotation.NonNull;
import android.annotation.Nullable;
import android.os.PersistableBundle;
@@ -38,7 +37,7 @@
private static final String KEY_KEYBOARD = "keyboard";
@Nullable private final String name;
- @NonNull private final String kernelPath;
+ @Nullable private final String kernelPath;
@Nullable private final String initrdPath;
@Nullable private final String bootloaderPath;
@Nullable private final String[] params;
@@ -62,7 +61,7 @@
return initrdPath;
}
- @NonNull
+ @Nullable
public String getKernelPath() {
return kernelPath;
}
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/client_vm_csr/src/lib.rs b/service_vm/client_vm_csr/src/lib.rs
index 512ecaf..0babfff 100644
--- a/service_vm/client_vm_csr/src/lib.rs
+++ b/service_vm/client_vm_csr/src/lib.rs
@@ -100,7 +100,7 @@
sign(message, cdi_leaf_priv.as_array()).map(|v| v.to_vec())
})?
.try_add_created_signature(attestation_key_sig_headers, aad, |message| {
- ecdsa_sign(message, attestation_key)
+ ecdsa_sign_cose(message, attestation_key)
})?
.build();
Ok(signed_data)
@@ -113,12 +113,18 @@
CoseSignatureBuilder::new().protected(protected).build()
}
-fn ecdsa_sign(message: &[u8], key: &EcKeyRef<Private>) -> Result<Vec<u8>> {
+fn ecdsa_sign_cose(message: &[u8], key: &EcKeyRef<Private>) -> Result<Vec<u8>> {
let digest = sha256(message);
// Passes the digest to `ECDSA_do_sign` as recommended in the spec:
// https://commondatastorage.googleapis.com/chromium-boringssl-docs/ecdsa.h.html#ECDSA_do_sign
let sig = EcdsaSig::sign::<Private>(&digest, key)?;
- Ok(sig.to_der()?)
+ ecdsa_sig_to_cose(&sig)
+}
+
+fn ecdsa_sig_to_cose(signature: &EcdsaSig) -> Result<Vec<u8>> {
+ let mut result = signature.r().to_vec_padded(ATTESTATION_KEY_AFFINE_COORDINATE_SIZE)?;
+ result.extend_from_slice(&signature.s().to_vec_padded(ATTESTATION_KEY_AFFINE_COORDINATE_SIZE)?);
+ Ok(result)
}
fn get_affine_coordinates(key: &EcKeyRef<Private>) -> Result<(Vec<u8>, Vec<u8>)> {
@@ -175,29 +181,38 @@
let chain = dice::Chain::from_cbor(&session, &csr.dice_cert_chain)?;
let public_key = chain.leaf().subject_public_key();
cose_sign
- .verify_signature(0, aad, |signature, message| public_key.verify(signature, message))?;
+ .verify_signature(0, aad, |signature, message| public_key.verify(signature, message))
+ .context("Verifying CDI_Leaf_Priv signature")?;
// Checks the second signature is signed with attestation key.
let attestation_public_key = CoseKey::from_slice(&csr_payload.public_key).unwrap();
let ec_public_key = to_ec_public_key(&attestation_public_key)?;
- cose_sign.verify_signature(1, aad, |signature, message| {
- ecdsa_verify(signature, message, &ec_public_key)
- })?;
+ cose_sign
+ .verify_signature(1, aad, |signature, message| {
+ ecdsa_verify_cose(signature, message, &ec_public_key)
+ })
+ .context("Verifying attestation key signature")?;
// Verifies that private key and the public key form a valid key pair.
let message = b"test message";
- let signature = ecdsa_sign(message, &ec_private_key)?;
- ecdsa_verify(&signature, message, &ec_public_key)?;
+ let signature = ecdsa_sign_cose(message, &ec_private_key)?;
+ ecdsa_verify_cose(&signature, message, &ec_public_key)
+ .context("Verifying signature with attested key")?;
Ok(())
}
- fn ecdsa_verify(
+ fn ecdsa_verify_cose(
signature: &[u8],
message: &[u8],
ec_public_key: &EcKeyRef<Public>,
) -> Result<()> {
- let sig = EcdsaSig::from_der(signature)?;
+ let coord_bytes = signature.len() / 2;
+ assert_eq!(signature.len(), coord_bytes * 2);
+
+ let r = BigNum::from_slice(&signature[..coord_bytes])?;
+ let s = BigNum::from_slice(&signature[coord_bytes..])?;
+ let sig = EcdsaSig::from_private_components(r, s)?;
let digest = sha256(message);
if sig.verify(&digest, ec_public_key)? {
Ok(())
diff --git a/service_vm/requests/src/client_vm.rs b/service_vm/requests/src/client_vm.rs
index 00effd7..2aa7113 100644
--- a/service_vm/requests/src/client_vm.rs
+++ b/service_vm/requests/src/client_vm.rs
@@ -66,7 +66,7 @@
// Verifies the second signature with the public key in the CSR payload.
let ec_public_key = EcKey::from_cose_public_key_slice(&csr_payload.public_key)?;
cose_sign.verify_signature(ATTESTATION_KEY_SIGNATURE_INDEX, aad, |signature, message| {
- ecdsa_verify(&ec_public_key, signature, message)
+ ecdsa_verify_cose(&ec_public_key, signature, message)
})?;
let subject_public_key_info = PKey::try_from(ec_public_key)?.subject_public_key_info()?;
@@ -110,20 +110,20 @@
RequestProcessingError::FailedToDecryptKeyBlob
})?;
let ec_private_key = EcKey::from_ec_private_key(private_key.as_slice())?;
- let signature = ecdsa_sign(&ec_private_key, &tbs_cert.to_der()?)?;
+ let signature = ecdsa_sign_der(&ec_private_key, &tbs_cert.to_der()?)?;
let certificate = cert::build_certificate(tbs_cert, &signature)?;
Ok(certificate.to_der()?)
}
-fn ecdsa_verify(key: &EcKey, signature: &[u8], message: &[u8]) -> bssl_avf::Result<()> {
+fn ecdsa_verify_cose(key: &EcKey, signature: &[u8], message: &[u8]) -> bssl_avf::Result<()> {
// The message was signed with ECDSA with curve P-256 and SHA-256 at the signature generation.
let digest = sha256(message)?;
- key.ecdsa_verify_der(signature, &digest)
+ key.ecdsa_verify_cose(signature, &digest)
}
-fn ecdsa_sign(key: &EcKey, message: &[u8]) -> bssl_avf::Result<Vec<u8>> {
+fn ecdsa_sign_der(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..247c34e 100644
--- a/service_vm/requests/src/dice.rs
+++ b/service_vm/requests/src/dice.rs
@@ -214,7 +214,7 @@
if !key_ops.is_empty()
&& !key_ops.contains(&KeyOperation::Assigned(iana::KeyOperation::Verify))
{
- error!("Public key does not support verification");
+ error!("Public key does not support verification - key_ops: {key_ops:?}");
return Err(RequestProcessingError::InvalidDiceChain);
}
Ok(Self(key))
@@ -231,7 +231,7 @@
/// PubKeyEd25519 / PubKeyECDSA256 / PubKeyECDSA384
///
/// The signature should be in the format defined by COSE in RFC 9053 section 2 for the
- /// specifric algorithm.
+ /// specific algorithm.
pub(crate) fn verify(&self, signature: &[u8], message: &[u8]) -> Result<()> {
match &self.0.kty {
KeyType::Assigned(iana::KeyType::EC2) => {
@@ -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 =
diff --git a/service_vm/requests/src/rkp.rs b/service_vm/requests/src/rkp.rs
index cdbd60e..4f2262f 100644
--- a/service_vm/requests/src/rkp.rs
+++ b/service_vm/requests/src/rkp.rs
@@ -123,9 +123,12 @@
"model" => "avf",
"device" => "avf",
"product" => "avf",
+ "vb_state" => "avf",
"manufacturer" => "aosp-avf",
- "vbmeta_digest" => Value::Bytes(vec![0u8; 0]),
+ "vbmeta_digest" => Value::Bytes(vec![1u8; 0]),
+ "security_level" => "avf",
"boot_patch_level" => 20240202,
+ "bootloader_state" => "avf",
"system_patch_level" => 202402,
"vendor_patch_level" => 20240202,
})
diff --git a/service_vm/test_apk/Android.bp b/service_vm/test_apk/Android.bp
index 72e411e..1ba156f 100644
--- a/service_vm/test_apk/Android.bp
+++ b/service_vm/test_apk/Android.bp
@@ -6,6 +6,7 @@
name: "vm_attestation_testapp_defaults",
test_suites: [
"general-tests",
+ "pts",
],
static_libs: [
"MicrodroidDeviceTestHelper",
diff --git a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
index 364e769..b2a77a7 100644
--- a/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
+++ b/tests/helper/src/java/com/android/microdroid/test/device/MicrodroidDeviceTestBase.java
@@ -48,6 +48,7 @@
import java.io.BufferedReader;
import java.io.ByteArrayOutputStream;
+import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
@@ -206,6 +207,11 @@
assume().withMessage("Device doesn't support AVF")
.that(mCtx.getPackageManager().hasSystemFeature(FEATURE_VIRTUALIZATION_FRAMEWORK))
.isTrue();
+ int vendorApiLevel = SystemProperties.getInt("ro.vendor.api_level", 0);
+ boolean isGsi = new File("/system/system_ext/etc/init/init.gsi.rc").exists();
+ assume().withMessage("GSI with vendor API level < 202404 may not support AVF")
+ .that(isGsi && vendorApiLevel < 202404)
+ .isFalse();
}
protected void assumeSupportedDevice() {
diff --git a/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java b/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
index 203bcae..41ddd48 100644
--- a/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
+++ b/tests/hostside/helper/java/com/android/microdroid/test/host/MicrodroidHostTestCaseBase.java
@@ -20,6 +20,7 @@
import static com.google.common.truth.Truth.assertWithMessage;
+import static org.junit.Assume.assumeFalse;
import static org.junit.Assume.assumeTrue;
import com.android.compatibility.common.tradefed.build.CompatibilityBuildHelper;
@@ -136,6 +137,15 @@
"Requires VM support",
testDevice.hasFeature("android.software.virtualization_framework"));
assumeTrue("Requires VM support", testDevice.supportsMicrodroid());
+
+ CommandRunner android = new CommandRunner(androidDevice);
+ long vendorApiLevel = androidDevice.getIntProperty("ro.vendor.api_level", 0);
+ boolean isGsi =
+ android.runForResult("[ -e /system/system_ext/etc/init/init.gsi.rc ]").getStatus()
+ == CommandStatus.SUCCESS;
+ assumeFalse(
+ "GSI with vendor API level < 202404 may not support AVF",
+ isGsi && vendorApiLevel < 202404);
}
public static void archiveLogThenDelete(TestLogData logs, ITestDevice device, String remotePath,
diff --git a/virtualizationservice/src/main.rs b/virtualizationservice/src/main.rs
index bcea1bc..8acfdd3 100644
--- a/virtualizationservice/src/main.rs
+++ b/virtualizationservice/src/main.rs
@@ -20,20 +20,18 @@
mod remote_provisioning;
mod rkpvm;
-use crate::aidl::{remove_temporary_dir, TEMPORARY_DIRECTORY, VirtualizationServiceInternal};
+use crate::aidl::{remove_temporary_dir, VirtualizationServiceInternal, TEMPORARY_DIRECTORY};
use android_logger::{Config, FilterBuilder};
-use android_system_virtualizationservice_internal::aidl::android::system::{
- virtualizationservice_internal::IVirtualizationServiceInternal::BnVirtualizationServiceInternal
-};
-use android_system_virtualizationmaintenance::aidl::android::system::virtualizationmaintenance::{
- IVirtualizationMaintenance::BnVirtualizationMaintenance
-};
+use android_system_virtualizationmaintenance::aidl::android::system::virtualizationmaintenance;
+use android_system_virtualizationservice_internal::aidl::android::system::virtualizationservice_internal;
use anyhow::{bail, Context, Error, Result};
use binder::{register_lazy_service, BinderFeatures, ProcessState, ThreadState};
use log::{error, info, LevelFilter};
use std::fs::{create_dir, read_dir};
use std::os::unix::raw::{pid_t, uid_t};
use std::path::Path;
+use virtualizationmaintenance::IVirtualizationMaintenance::BnVirtualizationMaintenance;
+use virtualizationservice_internal::IVirtualizationServiceInternal::BnVirtualizationServiceInternal;
const LOG_TAG: &str = "VirtualizationService";
pub(crate) const REMOTELY_PROVISIONED_COMPONENT_SERVICE_NAME: &str =
diff --git a/virtualizationservice/src/maintenance.rs b/virtualizationservice/src/maintenance.rs
index 8efc58d..4732e1f 100644
--- a/virtualizationservice/src/maintenance.rs
+++ b/virtualizationservice/src/maintenance.rs
@@ -24,11 +24,6 @@
mod vmdb;
use vmdb::{VmId, VmIdDb};
-/// Indicate whether an app ID belongs to a system core component.
-fn core_app_id(app_id: i32) -> bool {
- app_id < 10000
-}
-
/// Interface name for the Secretkeeper HAL.
const SECRETKEEPER_SERVICE: &str = "android.hardware.security.secretkeeper.ISecretkeeper/default";
@@ -45,6 +40,11 @@
/// State related to VM secrets.
pub struct State {
+ /// The real state, lazily created when we first need it.
+ inner: Option<InnerState>,
+}
+
+struct InnerState {
sk: binder::Strong<dyn ISecretkeeper>,
/// Database of VM IDs,
vm_id_db: VmIdDb,
@@ -53,20 +53,69 @@
impl State {
pub fn new() -> Option<Self> {
- let sk = match Self::find_sk() {
- Some(sk) => sk,
- None => {
- warn!("failed to find a Secretkeeper instance; skipping secret management");
- return None;
- }
+ if is_sk_present() {
+ // Don't instantiate the inner state yet, that will happen when it is needed.
+ Some(Self { inner: None })
+ } else {
+ // If the Secretkeeper HAL doesn't exist, there's never any point in trying to
+ // handle maintenance for it.
+ info!("Failed to find a Secretkeeper instance; skipping secret management");
+ None
+ }
+ }
+
+ /// Return the existing inner state, or create one if there isn't one.
+ /// This is done on demand as in early boot (before we need Secretkeeper) it may not be
+ /// available to connect to. See b/331417880.
+ fn get_inner(&mut self) -> Result<&mut InnerState> {
+ if self.inner.is_none() {
+ self.inner = Some(InnerState::new()?);
+ }
+ Ok(self.inner.as_mut().unwrap())
+ }
+
+ /// Record a new VM ID. If there is an existing owner (user_id, app_id) for the VM ID,
+ /// it will be replaced.
+ pub fn add_id(&mut self, vm_id: &VmId, user_id: u32, app_id: u32) -> Result<()> {
+ self.get_inner()?.add_id(vm_id, user_id, app_id)
+ }
+
+ /// Delete the VM IDs associated with Android user ID `user_id`.
+ pub fn delete_ids_for_user(&mut self, user_id: i32) -> Result<()> {
+ self.get_inner()?.delete_ids_for_user(user_id)
+ }
+
+ /// Delete the VM IDs associated with `(user_id, app_id)`.
+ pub fn delete_ids_for_app(&mut self, user_id: i32, app_id: i32) -> Result<()> {
+ self.get_inner()?.delete_ids_for_app(user_id, app_id)
+ }
+
+ /// Delete the provided VM IDs from both Secretkeeper and the database.
+ pub fn delete_ids(&mut self, vm_ids: &[VmId]) {
+ let Ok(inner) = self.get_inner() else {
+ warn!("No Secretkeeper available, not deleting secrets");
+ return;
};
- let (vm_id_db, created) = match VmIdDb::new(PERSISTENT_DIRECTORY) {
- Ok(v) => v,
- Err(e) => {
- error!("skipping secret management, failed to connect to database: {e:?}");
- return None;
- }
- };
+
+ inner.delete_ids(vm_ids)
+ }
+
+ /// Perform reconciliation to allow for possibly missed notifications of user or app removal.
+ pub fn reconcile(
+ &mut self,
+ callback: &Strong<dyn IVirtualizationReconciliationCallback>,
+ ) -> Result<()> {
+ self.get_inner()?.reconcile(callback)
+ }
+}
+
+impl InnerState {
+ fn new() -> Result<Self> {
+ info!("Connecting to {SECRETKEEPER_SERVICE}");
+ let sk = binder::wait_for_interface::<dyn ISecretkeeper>(SECRETKEEPER_SERVICE)
+ .context("Connecting to {SECRETKEEPER_SERVICE}")?;
+ let (vm_id_db, created) = VmIdDb::new(PERSISTENT_DIRECTORY)
+ .context("Connecting to secret management database")?;
if created {
// If the database did not previously exist, then this appears to be the first run of
// `virtualizationservice` since device setup or factory reset. In case of the latter,
@@ -76,32 +125,15 @@
if let Err(e) = sk.deleteAll() {
error!("failed to delete previous secrets, dropping database: {e:?}");
vm_id_db.delete_db_file(PERSISTENT_DIRECTORY);
- return None;
+ return Err(e.into());
}
} else {
info!("re-using existing VM ID DB");
}
- Some(Self { sk, vm_id_db, batch_size: DELETE_MAX_BATCH_SIZE })
+ Ok(Self { sk, vm_id_db, batch_size: DELETE_MAX_BATCH_SIZE })
}
- fn find_sk() -> Option<binder::Strong<dyn ISecretkeeper>> {
- if let Ok(true) = binder::is_declared(SECRETKEEPER_SERVICE) {
- match binder::get_interface(SECRETKEEPER_SERVICE) {
- Ok(sk) => Some(sk),
- Err(e) => {
- error!("failed to connect to {SECRETKEEPER_SERVICE}: {e:?}");
- None
- }
- }
- } else {
- info!("instance {SECRETKEEPER_SERVICE} not declared");
- None
- }
- }
-
- /// Record a new VM ID. If there is an existing owner (user_id, app_id) for the VM ID,
- /// it will be replaced.
- pub fn add_id(&mut self, vm_id: &VmId, user_id: u32, app_id: u32) -> Result<()> {
+ fn add_id(&mut self, vm_id: &VmId, user_id: u32, app_id: u32) -> Result<()> {
let user_id: i32 = user_id.try_into().context(format!("user_id {user_id} out of range"))?;
let app_id: i32 = app_id.try_into().context(format!("app_id {app_id} out of range"))?;
@@ -125,8 +157,7 @@
self.vm_id_db.add_vm_id(vm_id, user_id, app_id)
}
- /// Delete the VM IDs associated with Android user ID `user_id`.
- pub fn delete_ids_for_user(&mut self, user_id: i32) -> Result<()> {
+ fn delete_ids_for_user(&mut self, user_id: i32) -> Result<()> {
let vm_ids = self.vm_id_db.vm_ids_for_user(user_id)?;
info!(
"delete_ids_for_user(user_id={user_id}) triggers deletion of {} secrets",
@@ -136,8 +167,7 @@
Ok(())
}
- /// Delete the VM IDs associated with `(user_id, app_id)`.
- pub fn delete_ids_for_app(&mut self, user_id: i32, app_id: i32) -> Result<()> {
+ fn delete_ids_for_app(&mut self, user_id: i32, app_id: i32) -> Result<()> {
let vm_ids = self.vm_id_db.vm_ids_for_app(user_id, app_id)?;
info!(
"delete_ids_for_app(user_id={user_id}, app_id={app_id}) removes {} secrets",
@@ -147,8 +177,7 @@
Ok(())
}
- /// Delete the provided VM IDs from both Secretkeeper and the database.
- pub fn delete_ids(&mut self, mut vm_ids: &[VmId]) {
+ fn delete_ids(&mut self, mut vm_ids: &[VmId]) {
while !vm_ids.is_empty() {
let len = std::cmp::min(vm_ids.len(), self.batch_size);
let batch = &vm_ids[..len];
@@ -171,8 +200,7 @@
}
}
- /// Perform reconciliation to allow for possibly missed notifications of user or app removal.
- pub fn reconcile(
+ fn reconcile(
&mut self,
callback: &Strong<dyn IVirtualizationReconciliationCallback>,
) -> Result<()> {
@@ -245,19 +273,24 @@
}
}
+/// Indicate whether an app ID belongs to a system core component.
+fn core_app_id(app_id: i32) -> bool {
+ app_id < 10000
+}
+
+fn is_sk_present() -> bool {
+ matches!(binder::is_declared(SECRETKEEPER_SERVICE), Ok(true))
+}
+
#[cfg(test)]
mod tests {
use super::*;
+ use android_hardware_security_authgraph::aidl::android::hardware::security::authgraph;
+ use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper;
+ use authgraph::IAuthGraphKeyExchange::IAuthGraphKeyExchange;
+ use secretkeeper::ISecretkeeper::BnSecretkeeper;
use std::sync::{Arc, Mutex};
- use android_hardware_security_authgraph::aidl::android::hardware::security::authgraph::{
- IAuthGraphKeyExchange::IAuthGraphKeyExchange,
- };
- use android_hardware_security_secretkeeper::aidl::android::hardware::security::secretkeeper::{
- ISecretkeeper::BnSecretkeeper
- };
- use virtualizationmaintenance::IVirtualizationReconciliationCallback::{
- BnVirtualizationReconciliationCallback
- };
+ use virtualizationmaintenance::IVirtualizationReconciliationCallback::BnVirtualizationReconciliationCallback;
/// Fake implementation of Secretkeeper that keeps a history of what operations were invoked.
#[derive(Default)]
@@ -298,7 +331,12 @@
let vm_id_db = vmdb::new_test_db();
let sk = FakeSk { history };
let sk = BnSecretkeeper::new_binder(sk, binder::BinderFeatures::default());
- State { sk, vm_id_db, batch_size }
+ let inner = InnerState { sk, vm_id_db, batch_size };
+ State { inner: Some(inner) }
+ }
+
+ fn get_db(state: &mut State) -> &mut VmIdDb {
+ &mut state.inner.as_mut().unwrap().vm_id_db
}
struct Reconciliation {
@@ -360,11 +398,11 @@
let history = Arc::new(Mutex::new(Vec::new()));
let mut sk_state = new_test_state(history.clone(), 2);
- sk_state.vm_id_db.add_vm_id(&VM_ID1, USER1, APP_A).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID2, USER1, APP_A).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID3, USER2, APP_B).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID4, USER3, APP_A).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID5, USER3, APP_C).unwrap(); // Overwrites APP_A
+ get_db(&mut sk_state).add_vm_id(&VM_ID1, USER1, APP_A).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID2, USER1, APP_A).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID3, USER2, APP_B).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID4, USER3, APP_A).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID5, USER3, APP_C).unwrap(); // Overwrites APP_A
assert_eq!((*history.lock().unwrap()).clone(), vec![]);
sk_state.delete_ids_for_app(USER2, APP_B).unwrap();
@@ -376,11 +414,14 @@
vec![SkOp::DeleteIds(vec![VM_ID3]), SkOp::DeleteIds(vec![VM_ID4, VM_ID5]),]
);
- assert_eq!(vec![VM_ID1, VM_ID2], sk_state.vm_id_db.vm_ids_for_user(USER1).unwrap());
- assert_eq!(vec![VM_ID1, VM_ID2], sk_state.vm_id_db.vm_ids_for_app(USER1, APP_A).unwrap());
+ assert_eq!(vec![VM_ID1, VM_ID2], get_db(&mut sk_state).vm_ids_for_user(USER1).unwrap());
+ assert_eq!(
+ vec![VM_ID1, VM_ID2],
+ get_db(&mut sk_state).vm_ids_for_app(USER1, APP_A).unwrap()
+ );
let empty: Vec<VmId> = Vec::new();
- assert_eq!(empty, sk_state.vm_id_db.vm_ids_for_app(USER2, APP_B).unwrap());
- assert_eq!(empty, sk_state.vm_id_db.vm_ids_for_user(USER3).unwrap());
+ assert_eq!(empty, get_db(&mut sk_state).vm_ids_for_app(USER2, APP_B).unwrap());
+ assert_eq!(empty, get_db(&mut sk_state).vm_ids_for_user(USER3).unwrap());
}
#[test]
@@ -388,16 +429,19 @@
let history = Arc::new(Mutex::new(Vec::new()));
let mut sk_state = new_test_state(history.clone(), 20);
- sk_state.vm_id_db.add_vm_id(&VM_ID1, USER1, APP_A).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID2, USER1, APP_A).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID3, USER2, APP_B).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID4, USER2, CORE_APP_A).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID5, USER3, APP_C).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID1, USER1, APP_A).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID2, USER1, APP_A).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID3, USER2, APP_B).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID4, USER2, CORE_APP_A).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID5, USER3, APP_C).unwrap();
- assert_eq!(vec![VM_ID1, VM_ID2], sk_state.vm_id_db.vm_ids_for_user(USER1).unwrap());
- assert_eq!(vec![VM_ID1, VM_ID2], sk_state.vm_id_db.vm_ids_for_app(USER1, APP_A).unwrap());
- assert_eq!(vec![VM_ID3], sk_state.vm_id_db.vm_ids_for_app(USER2, APP_B).unwrap());
- assert_eq!(vec![VM_ID5], sk_state.vm_id_db.vm_ids_for_user(USER3).unwrap());
+ assert_eq!(vec![VM_ID1, VM_ID2], get_db(&mut sk_state).vm_ids_for_user(USER1).unwrap());
+ assert_eq!(
+ vec![VM_ID1, VM_ID2],
+ get_db(&mut sk_state).vm_ids_for_app(USER1, APP_A).unwrap()
+ );
+ assert_eq!(vec![VM_ID3], get_db(&mut sk_state).vm_ids_for_app(USER2, APP_B).unwrap());
+ assert_eq!(vec![VM_ID5], get_db(&mut sk_state).vm_ids_for_user(USER3).unwrap());
// Perform a reconciliation and pretend that USER1 and [CORE_APP_A, APP_B] are gone.
let reconciliation =
@@ -409,12 +453,12 @@
sk_state.reconcile(&callback).unwrap();
let empty: Vec<VmId> = Vec::new();
- assert_eq!(empty, sk_state.vm_id_db.vm_ids_for_user(USER1).unwrap());
- assert_eq!(empty, sk_state.vm_id_db.vm_ids_for_app(USER1, APP_A).unwrap());
+ assert_eq!(empty, get_db(&mut sk_state).vm_ids_for_user(USER1).unwrap());
+ assert_eq!(empty, get_db(&mut sk_state).vm_ids_for_app(USER1, APP_A).unwrap());
// VM for core app stays even though it's reported as absent.
- assert_eq!(vec![VM_ID4], sk_state.vm_id_db.vm_ids_for_user(USER2).unwrap());
- assert_eq!(empty, sk_state.vm_id_db.vm_ids_for_app(USER2, APP_B).unwrap());
- assert_eq!(vec![VM_ID5], sk_state.vm_id_db.vm_ids_for_user(USER3).unwrap());
+ assert_eq!(vec![VM_ID4], get_db(&mut sk_state).vm_ids_for_user(USER2).unwrap());
+ assert_eq!(empty, get_db(&mut sk_state).vm_ids_for_app(USER2, APP_B).unwrap());
+ assert_eq!(vec![VM_ID5], get_db(&mut sk_state).vm_ids_for_user(USER3).unwrap());
}
#[test]
@@ -427,11 +471,11 @@
let mut vm_id = [0u8; 64];
vm_id[0..8].copy_from_slice(&(idx as u64).to_be_bytes());
sk_state.add_id(&vm_id, USER1 as u32, APP_A as u32).unwrap();
- assert_eq!(idx + 1, sk_state.vm_id_db.count_vm_ids_for_app(USER1, APP_A).unwrap());
+ assert_eq!(idx + 1, get_db(&mut sk_state).count_vm_ids_for_app(USER1, APP_A).unwrap());
}
assert_eq!(
MAX_VM_IDS_PER_APP,
- sk_state.vm_id_db.count_vm_ids_for_app(USER1, APP_A).unwrap()
+ get_db(&mut sk_state).count_vm_ids_for_app(USER1, APP_A).unwrap()
);
// Beyond the limit it's one in, one out.
@@ -441,12 +485,12 @@
sk_state.add_id(&vm_id, USER1 as u32, APP_A as u32).unwrap();
assert_eq!(
MAX_VM_IDS_PER_APP,
- sk_state.vm_id_db.count_vm_ids_for_app(USER1, APP_A).unwrap()
+ get_db(&mut sk_state).count_vm_ids_for_app(USER1, APP_A).unwrap()
);
}
assert_eq!(
MAX_VM_IDS_PER_APP,
- sk_state.vm_id_db.count_vm_ids_for_app(USER1, APP_A).unwrap()
+ get_db(&mut sk_state).count_vm_ids_for_app(USER1, APP_A).unwrap()
);
}
@@ -467,10 +511,10 @@
let history = Arc::new(Mutex::new(Vec::new()));
let mut sk_state = new_test_state(history.clone(), 20);
- sk_state.vm_id_db.add_vm_id(&VM_ID1, USER1, APP_A).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID2, USER1, APP_A).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID3, USER2, APP_B).unwrap();
- sk_state.vm_id_db.add_vm_id(&VM_ID5, USER3, APP_C).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID1, USER1, APP_A).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID2, USER1, APP_A).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID3, USER2, APP_B).unwrap();
+ get_db(&mut sk_state).add_vm_id(&VM_ID5, USER3, APP_C).unwrap();
sk_state.delete_ids_for_user(USER1).unwrap();
sk_state.delete_ids_for_user(USER2).unwrap();
sk_state.delete_ids_for_user(USER3).unwrap();
diff --git a/virtualizationservice/vfio_handler/src/aidl.rs b/virtualizationservice/vfio_handler/src/aidl.rs
index c0967af..b527260 100644
--- a/virtualizationservice/vfio_handler/src/aidl.rs
+++ b/virtualizationservice/vfio_handler/src/aidl.rs
@@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-//! Implementation of the AIDL interface of the VirtualizationService.
+//! Implementation of the AIDL interface of VfioHandler.
use anyhow::{anyhow, Context};
use android_system_virtualizationservice_internal::aidl::android::system::virtualizationservice_internal::IBoundDevice::{IBoundDevice, BnBoundDevice};
diff --git a/vmlauncher_app/java/com/android/virtualization/vmlauncher/MainActivity.java b/vmlauncher_app/java/com/android/virtualization/vmlauncher/MainActivity.java
index 4c42bb4..ec0f8e8 100644
--- a/vmlauncher_app/java/com/android/virtualization/vmlauncher/MainActivity.java
+++ b/vmlauncher_app/java/com/android/virtualization/vmlauncher/MainActivity.java
@@ -91,7 +91,7 @@
.forEach(customImageConfigBuilder::addParam);
}
if (json.has("bootloader")) {
- customImageConfigBuilder.setInitrdPath(json.getString("bootloader"));
+ customImageConfigBuilder.setBootloaderPath(json.getString("bootloader"));
}
if (json.has("disks")) {
JSONArray diskArr = json.getJSONArray("disks");