Merge "Add MetricsProcessor to calculate metrics stats"
diff --git a/demo/java/com/android/microdroid/demo/MainActivity.java b/demo/java/com/android/microdroid/demo/MainActivity.java
index 747e98c..6266c18 100644
--- a/demo/java/com/android/microdroid/demo/MainActivity.java
+++ b/demo/java/com/android/microdroid/demo/MainActivity.java
@@ -38,7 +38,6 @@
import androidx.lifecycle.AndroidViewModel;
import androidx.lifecycle.LiveData;
import androidx.lifecycle.MutableLiveData;
-import androidx.lifecycle.Observer;
import androidx.lifecycle.ViewModelProvider;
import com.android.microdroid.testservice.ITestService;
@@ -64,44 +63,39 @@
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_main);
- Button runStopButton = (Button) findViewById(R.id.runStopButton);
- TextView consoleView = (TextView) findViewById(R.id.consoleOutput);
- TextView logView = (TextView) findViewById(R.id.logOutput);
- TextView payloadView = (TextView) findViewById(R.id.payloadOutput);
- ScrollView scrollConsoleView = (ScrollView) findViewById(R.id.scrollConsoleOutput);
- ScrollView scrollLogView = (ScrollView) findViewById(R.id.scrollLogOutput);
+ Button runStopButton = findViewById(R.id.runStopButton);
+ TextView consoleView = findViewById(R.id.consoleOutput);
+ TextView logView = findViewById(R.id.logOutput);
+ TextView payloadView = findViewById(R.id.payloadOutput);
+ ScrollView scrollConsoleView = findViewById(R.id.scrollConsoleOutput);
+ ScrollView scrollLogView = findViewById(R.id.scrollLogOutput);
VirtualMachineModel model = new ViewModelProvider(this).get(VirtualMachineModel.class);
// When the button is clicked, run or stop the VM
runStopButton.setOnClickListener(
- new View.OnClickListener() {
- public void onClick(View v) {
- if (model.getStatus().getValue() == VirtualMachine.Status.RUNNING) {
- model.stop();
- } else {
- CheckBox debugModeCheckBox = (CheckBox) findViewById(R.id.debugMode);
- final boolean debug = debugModeCheckBox.isChecked();
- model.run(debug);
- }
+ v -> {
+ if (model.getStatus().getValue() == VirtualMachine.Status.RUNNING) {
+ model.stop();
+ } else {
+ CheckBox debugModeCheckBox = (CheckBox) findViewById(R.id.debugMode);
+ final boolean debug = debugModeCheckBox.isChecked();
+ model.run(debug);
}
});
// When the VM status is updated, change the label of the button
model.getStatus()
.observeForever(
- new Observer<VirtualMachine.Status>() {
- @Override
- public void onChanged(VirtualMachine.Status status) {
- if (status == VirtualMachine.Status.RUNNING) {
- runStopButton.setText("Stop");
- // Clear the outputs from the previous run
- consoleView.setText("");
- logView.setText("");
- payloadView.setText("");
- } else {
- runStopButton.setText("Run");
- }
+ status -> {
+ if (status == VirtualMachine.Status.RUNNING) {
+ runStopButton.setText("Stop");
+ // Clear the outputs from the previous run
+ consoleView.setText("");
+ logView.setText("");
+ payloadView.setText("");
+ } else {
+ runStopButton.setText("Run");
}
});
@@ -109,30 +103,19 @@
// corresponding text view.
model.getConsoleOutput()
.observeForever(
- new Observer<String>() {
- @Override
- public void onChanged(String line) {
- consoleView.append(line + "\n");
- scrollConsoleView.fullScroll(View.FOCUS_DOWN);
- }
+ line -> {
+ consoleView.append(line + "\n");
+ scrollConsoleView.fullScroll(View.FOCUS_DOWN);
});
model.getLogOutput()
.observeForever(
- new Observer<String>() {
- @Override
- public void onChanged(String line) {
- logView.append(line + "\n");
- scrollLogView.fullScroll(View.FOCUS_DOWN);
- }
+ line -> {
+ logView.append(line + "\n");
+ scrollLogView.fullScroll(View.FOCUS_DOWN);
});
model.getPayloadOutput()
.observeForever(
- new Observer<String>() {
- @Override
- public void onChanged(String line) {
- payloadView.append(line + "\n");
- }
- });
+ line -> payloadView.append(line + "\n"));
}
/** Reads data from an input stream and posts it to the output data */
@@ -178,7 +161,6 @@
/** Runs a VM */
public void run(boolean debug) {
// Create a VM and run it.
- // TODO(jiyong): remove the call to idsigPath
mExecutorService = Executors.newFixedThreadPool(4);
VirtualMachineCallback callback =
@@ -274,7 +256,7 @@
}
@Override
- public void onDied(VirtualMachine vm, @DeathReason int reason) {
+ public void onDied(VirtualMachine vm, int reason) {
mService.shutdownNow();
mStatus.postValue(VirtualMachine.Status.STOPPED);
}
diff --git a/tests/benchmark_hostside/java/android/avf/test/AVFHostTestCase.java b/tests/benchmark_hostside/java/android/avf/test/AVFHostTestCase.java
index 143a35a..21f2d6d 100644
--- a/tests/benchmark_hostside/java/android/avf/test/AVFHostTestCase.java
+++ b/tests/benchmark_hostside/java/android/avf/test/AVFHostTestCase.java
@@ -126,8 +126,9 @@
// Boot time with compilation OS test.
reInstallApex(REINSTALL_APEX_TIMEOUT_SEC);
compileStagedApex(COMPILE_STAGED_APEX_TIMEOUT_SEC);
+ getDevice().nonBlockingReboot();
long start = System.nanoTime();
- rebootAndWaitBootCompleted();
+ waitForBootCompleted();
long elapsedWithCompOS = System.nanoTime() - start;
double elapsedSec = elapsedWithCompOS / NANOS_IN_SEC;
bootWithCompOsTime.add(elapsedSec);
@@ -135,8 +136,9 @@
// Boot time without compilation OS test.
reInstallApex(REINSTALL_APEX_TIMEOUT_SEC);
+ getDevice().nonBlockingReboot();
start = System.nanoTime();
- rebootAndWaitBootCompleted();
+ waitForBootCompleted();
long elapsedWithoutCompOS = System.nanoTime() - start;
elapsedSec = elapsedWithoutCompOS / NANOS_IN_SEC;
bootWithoutCompOsTime.add(elapsedSec);
@@ -202,8 +204,7 @@
getDevice().enableAdbRoot();
}
- private void rebootAndWaitBootCompleted() throws Exception {
- getDevice().nonBlockingReboot();
+ private void waitForBootCompleted() throws Exception {
getDevice().waitForDeviceOnline(BOOT_COMPLETE_TIMEOUT_MS);
getDevice().waitForBootComplete(BOOT_COMPLETE_TIMEOUT_MS);
getDevice().enableAdbRoot();
diff --git a/tests/helper/src/java/com/android/microdroid/test/MicrodroidDeviceTestBase.java b/tests/helper/src/java/com/android/microdroid/test/MicrodroidDeviceTestBase.java
index a80111f..3273635 100644
--- a/tests/helper/src/java/com/android/microdroid/test/MicrodroidDeviceTestBase.java
+++ b/tests/helper/src/java/com/android/microdroid/test/MicrodroidDeviceTestBase.java
@@ -22,7 +22,6 @@
import android.content.Context;
import android.os.ParcelFileDescriptor;
import android.os.SystemProperties;
-import android.sysprop.HypervisorProperties;
import android.system.virtualmachine.VirtualMachine;
import android.system.virtualmachine.VirtualMachineCallback;
import android.system.virtualmachine.VirtualMachineConfig;
@@ -106,18 +105,29 @@
return;
}
if (protectedVm) {
- assume().withMessage("Skip where protected VMs aren't support")
- .that(HypervisorProperties.hypervisor_protected_vm_supported().orElse(false))
+ assume().withMessage("Skip where protected VMs aren't supported")
+ .that(hypervisor_protected_vm_supported())
.isTrue();
} else {
- assume().withMessage("Skip where VMs aren't support")
- .that(HypervisorProperties.hypervisor_vm_supported().orElse(false))
+ assume().withMessage("Skip where VMs aren't supported")
+ .that(hypervisor_vm_supported())
.isTrue();
}
Context context = ApplicationProvider.getApplicationContext();
mInner = new Inner(context, protectedVm, VirtualMachineManager.getInstance(context));
}
+ // These are inlined from android.sysprop.HypervisorProperties which isn't @SystemApi.
+ // TODO(b/243642678): Move to using a proper Java API for this.
+
+ private boolean hypervisor_vm_supported() {
+ return SystemProperties.getBoolean("ro.boot.hypervisor.vm.supported", false);
+ }
+
+ private boolean hypervisor_protected_vm_supported() {
+ return SystemProperties.getBoolean("ro.boot.hypervisor.protected_vm.supported", false);
+ }
+
public abstract static class VmEventListener implements VirtualMachineCallback {
private ExecutorService mExecutorService = Executors.newSingleThreadExecutor();
private OptionalLong mVcpuStartedNanoTime = OptionalLong.empty();
diff --git a/tests/testapk/Android.bp b/tests/testapk/Android.bp
index 325ebdb..47116eb 100644
--- a/tests/testapk/Android.bp
+++ b/tests/testapk/Android.bp
@@ -28,7 +28,6 @@
min_sdk_version: "33",
}
-// TODO(jiyong): make this a binary, not a shared library
cc_library_shared {
name: "MicrodroidTestNativeLib",
srcs: ["src/native/testbinary.cpp"],
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 473cbd5..6a4cc93 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -146,7 +146,7 @@
let size = size.try_into().map_err(|e| {
Status::new_exception_str(
ExceptionCode::ILLEGAL_ARGUMENT,
- Some(format!("Invalid size {}: {}", size, e)),
+ Some(format!("Invalid size {}: {:?}", size, e)),
)
})?;
let image = clone_file(image_fd)?;
@@ -154,13 +154,13 @@
image.set_len(0).map_err(|e| {
Status::new_service_specific_error_str(
-1,
- Some(format!("Failed to reset a file: {}", e)),
+ Some(format!("Failed to reset a file: {:?}", e)),
)
})?;
let mut part = QcowFile::new(image, size).map_err(|e| {
Status::new_service_specific_error_str(
-1,
- Some(format!("Failed to create QCOW2 image: {}", e)),
+ Some(format!("Failed to create QCOW2 image: {:?}", e)),
)
})?;
@@ -175,7 +175,7 @@
.map_err(|e| {
Status::new_service_specific_error_str(
-1,
- Some(format!("Failed to initialize partition as {:?}: {}", partition_type, e)),
+ Some(format!("Failed to initialize partition as {:?}: {:?}", partition_type, e)),
)
})?;
@@ -251,7 +251,7 @@
for incoming_stream in listener.incoming() {
let mut incoming_stream = match incoming_stream {
Err(e) => {
- warn!("invalid incoming connection: {}", e);
+ warn!("invalid incoming connection: {:?}", e);
continue;
}
Ok(s) => s,
@@ -305,7 +305,7 @@
std::thread::spawn(|| {
if let Err(e) = handle_stream_connection_tombstoned() {
- warn!("Error receiving tombstone from guest or writing them. Error: {}", e);
+ warn!("Error receiving tombstone from guest or writing them. Error: {:?}", e);
}
});
@@ -366,7 +366,7 @@
Status::new_service_specific_error_str(
-1,
Some(format!(
- "Failed to create temporary directory {:?} for VM files: {}",
+ "Failed to create temporary directory {:?} for VM files: {:?}",
temporary_directory, e
)),
)
@@ -382,7 +382,7 @@
Status::new_service_specific_error_str(
-1,
Some(format!(
- "Failed to load app config from {}: {}",
+ "Failed to load app config from {}: {:?}",
&config.configPath, e
)),
)
@@ -395,29 +395,28 @@
// Check if partition images are labeled incorrectly. This is to prevent random images
// which are not protected by the Android Verified Boot (e.g. bits downloaded by apps) from
- // being loaded in a pVM. Specifically, for images in the raw config, nothing is allowed
- // to be labeled as app_data_file. For images in the app config, nothing but the instance
- // partition is allowed to be labeled as such.
+ // being loaded in a pVM. This applies to everything in the raw config, and everything but
+ // the non-executable, generated partitions in the app config.
config
.disks
.iter()
.flat_map(|disk| disk.partitions.iter())
.filter(|partition| {
if is_app_config {
- partition.label != "vm-instance"
+ !is_safe_app_partition(&partition.label)
} else {
true // all partitions are checked
}
})
.try_for_each(check_label_for_partition)
- .map_err(|e| Status::new_service_specific_error_str(-1, Some(e.to_string())))?;
+ .map_err(|e| Status::new_service_specific_error_str(-1, Some(format!("{:?}", e))))?;
let zero_filler_path = temporary_directory.join("zero.img");
write_zero_filler(&zero_filler_path).map_err(|e| {
error!("Failed to make composite image: {:?}", e);
Status::new_service_specific_error_str(
-1,
- Some(format!("Failed to make composite image: {}", e)),
+ Some(format!("Failed to make composite image: {:?}", e)),
)
})?;
@@ -442,10 +441,10 @@
// will be sent back to the client (i.e. the VM owner) for readout.
let ramdump_path = temporary_directory.join("ramdump");
let ramdump = prepare_ramdump_file(&ramdump_path).map_err(|e| {
- error!("Failed to prepare ramdump file: {}", e);
+ error!("Failed to prepare ramdump file: {:?}", e);
Status::new_service_specific_error_str(
-1,
- Some(format!("Failed to prepare ramdump file: {}", e)),
+ Some(format!("Failed to prepare ramdump file: {:?}", e)),
)
})?;
@@ -482,7 +481,7 @@
error!("Failed to create VM with config {:?}: {:?}", config, e);
Status::new_service_specific_error_str(
-1,
- Some(format!("Failed to create VM: {}", e)),
+ Some(format!("Failed to create VM: {:?}", e)),
)
})?,
);
@@ -499,7 +498,7 @@
for stream in listener.incoming() {
let stream = match stream {
Err(e) => {
- warn!("invalid incoming connection: {}", e);
+ warn!("invalid incoming connection: {:?}", e);
continue;
}
Ok(s) => s,
@@ -572,7 +571,7 @@
error!("Failed to make composite image with config {:?}: {:?}", disk, e);
Status::new_service_specific_error_str(
-1,
- Some(format!("Failed to make composite image: {}", e)),
+ Some(format!("Failed to make composite image: {:?}", e)),
)
})?;
@@ -730,10 +729,33 @@
/// Check if a partition has selinux labels that are not allowed
fn check_label_for_partition(partition: &Partition) -> Result<()> {
let ctx = getfilecon(partition.image.as_ref().unwrap().as_ref())?;
- if ctx == SeContext::new("u:object_r:app_data_file:s0").unwrap() {
- Err(anyhow!("Partition {} shouldn't be labeled as {}", &partition.label, ctx))
- } else {
- Ok(())
+ check_label_is_allowed(&ctx).with_context(|| format!("Partition {} invalid", &partition.label))
+}
+
+// Return whether a partition is exempt from selinux label checks, because we know that it does
+// not contain code and is likely to be generated in an app-writable directory.
+fn is_safe_app_partition(label: &str) -> bool {
+ // See make_payload_disk in payload.rs.
+ label == "vm-instance"
+ || label == "microdroid-apk-idsig"
+ || label == "payload-metadata"
+ || label.starts_with("extra-idsig-")
+}
+
+fn check_label_is_allowed(ctx: &SeContext) -> Result<()> {
+ // We only want to allow code in a VM payload to be sourced from places that apps, and the
+ // system, do not have write access to.
+ // (Note that sepolicy must also grant read access for these types to both virtualization
+ // service and crosvm.)
+ // App private data files are deliberately excluded, to avoid arbitrary payloads being run on
+ // user devices (W^X).
+ match ctx.selinux_type()? {
+ | "system_file" // immutable dm-verity protected partition
+ | "apk_data_file" // APKs of an installed app
+ | "staging_data_file" // updated/staged APEX imagess
+ | "shell_data_file" // test files created via adb shell
+ => Ok(()),
+ _ => bail!("Label {} is not allowed", ctx),
}
}
@@ -802,7 +824,7 @@
VsockStream::connect_with_cid_port(self.instance.cid, port as u32).map_err(|e| {
Status::new_service_specific_error_str(
-1,
- Some(format!("Failed to connect: {}", e)),
+ Some(format!("Failed to connect: {:?}", e)),
)
})?;
Ok(vsock_stream_to_pfd(stream))
@@ -881,7 +903,7 @@
let pfd = ParcelFileDescriptor::new(ramdump);
for callback in callbacks {
if let Err(e) = callback.onRamdump(cid as i32, &pfd) {
- error!("Error notifying ramdump of VM CID {}: {}", cid, e);
+ error!("Error notifying ramdump of VM CID {}: {:?}", cid, e);
}
}
}
@@ -983,7 +1005,7 @@
file.as_ref().try_clone().map_err(|e| {
Status::new_exception_str(
ExceptionCode::BAD_PARCELABLE,
- Some(format!("Failed to clone File from ParcelFileDescriptor: {}", e)),
+ Some(format!("Failed to clone File from ParcelFileDescriptor: {:?}", e)),
)
})
}
@@ -1005,7 +1027,7 @@
VersionReq::parse(s).map_err(|e| {
Status::new_exception_str(
ExceptionCode::BAD_PARCELABLE,
- Some(format!("Invalid platform version requirement {}: {}", s, e)),
+ Some(format!("Invalid platform version requirement {}: {:?}", s, e)),
)
})
}
@@ -1132,3 +1154,33 @@
)
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn test_is_allowed_label_for_partition() -> Result<()> {
+ let expected_results = vec![
+ ("u:object_r:system_file:s0", true),
+ ("u:object_r:apk_data_file:s0", true),
+ ("u:object_r:app_data_file:s0", false),
+ ("u:object_r:app_data_file:s0:c512,c768", false),
+ ("u:object_r:privapp_data_file:s0:c512,c768", false),
+ ("invalid", false),
+ ("user:role:apk_data_file:severity:categories", true),
+ ("user:role:apk_data_file:severity:categories:extraneous", false),
+ ];
+
+ for (label, expected_valid) in expected_results {
+ let context = SeContext::new(label)?;
+ let result = check_label_is_allowed(&context);
+ if expected_valid {
+ assert!(result.is_ok(), "Expected label {} to be allowed, got {:?}", label, result);
+ } else if result.is_ok() {
+ bail!("Expected label {} to be disallowed", label);
+ }
+ }
+ Ok(())
+ }
+}
diff --git a/virtualizationservice/src/main.rs b/virtualizationservice/src/main.rs
index 93a5966..cb10eff 100644
--- a/virtualizationservice/src/main.rs
+++ b/virtualizationservice/src/main.rs
@@ -72,12 +72,3 @@
}
Ok(())
}
-
-#[cfg(test)]
-mod tests {
- /// We need to have at least one test to avoid errors running the test suite, so this is a
- /// placeholder until we have real tests.
- #[test]
- #[ignore]
- fn placeholder() {}
-}
diff --git a/virtualizationservice/src/payload.rs b/virtualizationservice/src/payload.rs
index 42c51a1..7807cd6 100644
--- a/virtualizationservice/src/payload.rs
+++ b/virtualizationservice/src/payload.rs
@@ -201,7 +201,7 @@
}
/// Creates a DiskImage with partitions:
-/// metadata: metadata
+/// payload-metadata: metadata
/// microdroid-apex-0: apex 0
/// microdroid-apex-1: apex 1
/// ..
diff --git a/virtualizationservice/src/selinux.rs b/virtualizationservice/src/selinux.rs
index e450dee..0485943 100644
--- a/virtualizationservice/src/selinux.rs
+++ b/virtualizationservice/src/selinux.rs
@@ -14,7 +14,7 @@
//! Wrapper to libselinux
-use anyhow::{anyhow, Context, Result};
+use anyhow::{anyhow, bail, Context, Result};
use std::ffi::{CStr, CString};
use std::fmt;
use std::fs::File;
@@ -30,6 +30,7 @@
/// `freecon` to free the resources when dropped. In its second variant it stores
/// an `std::ffi::CString` that can be initialized from a Rust string slice.
#[derive(Debug)]
+#[allow(dead_code)] // CString variant is used in tests
pub enum SeContext {
/// Wraps a raw context c-string as returned by libselinux.
Raw(*mut ::std::os::raw::c_char),
@@ -78,12 +79,27 @@
impl SeContext {
/// Initializes the `SeContext::CString` variant from a Rust string slice.
+ #[allow(dead_code)] // Used in tests
pub fn new(con: &str) -> Result<Self> {
Ok(Self::CString(
CString::new(con)
.with_context(|| format!("Failed to create SeContext with \"{}\"", con))?,
))
}
+
+ pub fn selinux_type(&self) -> Result<&str> {
+ let context = self.deref().to_str().context("Label is not valid UTF8")?;
+
+ // The syntax is user:role:type:sensitivity[:category,...],
+ // ignoring security level ranges, which don't occur on Android. See
+ // https://github.com/SELinuxProject/selinux-notebook/blob/main/src/security_context.md
+ // We only want the type.
+ let fields: Vec<_> = context.split(':').collect();
+ if fields.len() < 4 || fields.len() > 5 {
+ bail!("Syntactically invalid label {}", self);
+ }
+ Ok(fields[2])
+ }
}
pub fn getfilecon(file: &File) -> Result<SeContext> {