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> {