Access control for virtualizationservice
The access to the virtualizationservice is now controlled via Android
permissions:
* android.permission.MANAGE_VIRTUAL_MACHINE
* android.permission.DEBUG_VIRTUAL_MACHINE
The two permissions are defined in a resource-only APK
android.system.virtualmachine.res. Virtualizationservice is modified to
do the permission check by using the permission controller service.
Bug: 168588769
Test: /apex/com.android.virt/bin/vm run-app --log /dev/null
/data/local/tmp/virt/MicrodroidDemoApp.apk
/data/local/tmp/virt/MicrodroidDemoApp.apk.idsig assets/vm_config.json
Change-Id: Id210d2a55bc57bf03200c3c8546e3c63aa2a4c52
diff --git a/virtualizationservice/Android.bp b/virtualizationservice/Android.bp
index 40aa139..a1dba43 100644
--- a/virtualizationservice/Android.bp
+++ b/virtualizationservice/Android.bp
@@ -21,6 +21,7 @@
prefer_rlib: true,
rustlibs: [
"android.system.virtualizationservice-rust",
+ "android.os.permissions_aidl-rust",
"libandroid_logger",
"libanyhow",
"libcommand_fds",
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 661abdc..1f56731 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -19,6 +19,7 @@
use crate::payload::make_payload_disk;
use crate::{Cid, FIRST_GUEST_CID};
+use android_os_permissions_aidl::aidl::android::os::IPermissionController;
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::IVirtualizationService::IVirtualizationService;
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::DiskImage::DiskImage;
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::IVirtualMachine::{
@@ -54,10 +55,6 @@
/// Directory in which to write disk image files used while running VMs.
const TEMPORARY_DIRECTORY: &str = "/data/misc/virtualizationservice";
-// TODO(qwandor): Use PermissionController once it is available to Rust.
-/// Only processes running with one of these UIDs are allowed to call debug methods.
-const DEBUG_ALLOWED_UIDS: [u32; 2] = [0, 2000];
-
/// The list of APEXes which microdroid requires.
/// TODO(b/192200378) move this to microdroid.json?
const MICRODROID_REQUIRED_APEXES: [&str; 3] =
@@ -87,6 +84,7 @@
config: &VirtualMachineConfig,
log_fd: Option<&ParcelFileDescriptor>,
) -> binder::Result<Strong<dyn IVirtualMachine>> {
+ check_manage_access()?;
let state = &mut *self.state.lock().unwrap();
let log_fd = log_fd.map(clone_file).transpose()?;
let requester_uid = ThreadState::get_calling_uid();
@@ -183,6 +181,7 @@
image_fd: &ParcelFileDescriptor,
size: i64,
) -> binder::Result<()> {
+ check_manage_access()?;
let size = size.try_into().map_err(|e| {
new_binder_exception(
ExceptionCode::ILLEGAL_ARGUMENT,
@@ -431,17 +430,36 @@
})
}
-/// Check whether the caller of the current Binder method is allowed to call debug methods.
-fn check_debug_access() -> binder::Result<()> {
- let uid = ThreadState::get_calling_uid();
- log::trace!("Debug method call from UID {}.", uid);
- if DEBUG_ALLOWED_UIDS.contains(&uid) {
+/// Checks whether the caller has a specific permission
+fn check_permission(perm: &str) -> binder::Result<()> {
+ let calling_pid = ThreadState::get_calling_pid();
+ let calling_uid = ThreadState::get_calling_uid();
+ // Root can do anything
+ if calling_uid == 0 {
+ return Ok(());
+ }
+ let perm_svc: Strong<dyn IPermissionController::IPermissionController> =
+ binder::get_interface("permission")?;
+ if perm_svc.checkPermission(perm, calling_pid, calling_uid as i32)? {
Ok(())
} else {
- Err(new_binder_exception(ExceptionCode::SECURITY, "Debug access denied"))
+ Err(new_binder_exception(
+ ExceptionCode::SECURITY,
+ format!("does not have the {} permission", perm),
+ ))
}
}
+/// Check whether the caller of the current Binder method is allowed to call debug methods.
+fn check_debug_access() -> binder::Result<()> {
+ check_permission("android.permission.DEBUG_VIRTUAL_MACHINE")
+}
+
+/// Check whether the caller of the current Binder method is allowed to manage VMs
+fn check_manage_access() -> binder::Result<()> {
+ check_permission("android.permission.MANAGE_VIRTUAL_MACHINE")
+}
+
/// Implementation of the AIDL `IVirtualMachine` interface. Used as a handle to a VM.
#[derive(Debug)]
struct VirtualMachine {
@@ -459,10 +477,14 @@
impl IVirtualMachine for VirtualMachine {
fn getCid(&self) -> binder::Result<i32> {
+ // Don't check permission. The owner of the VM might have passed this binder object to
+ // others.
Ok(self.instance.cid as i32)
}
fn isRunning(&self) -> binder::Result<bool> {
+ // Don't check permission. The owner of the VM might have passed this binder object to
+ // others.
Ok(self.instance.running())
}
@@ -470,6 +492,9 @@
&self,
callback: &Strong<dyn IVirtualMachineCallback>,
) -> binder::Result<()> {
+ // Don't check permission. The owner of the VM might have passed this binder object to
+ // others.
+ //
// TODO: Should this give an error if the VM is already dead?
self.instance.callbacks.add(callback.clone());
Ok(())