Run app payloads as non-root.
This is in preparation before moving to running multiple payload
processes in multiple UIDs.
Add a new payload user and group in the system-reserved range, only
within Microdroid, and assign them to the payload process. Fix up a
bunch of DAC permissions to make sure the payload still has access to
the things it should have.
Add a test to check we aren't running as root, and make some minor
test fixes.
This is a potentially breaking change, so for now I've disabled it via
Rust conditional compilation (and marked the new test as @Ignore). I
claim the changes that aren't protected by this are harmless.
I've run tests with and without the cfg option enabled.
Unrelated changes done in passing:
- Move a comment from reference to definition.
- Make sure encryptedstore logs any errors in full.
- Use with_context in a few more places.
Bug: 296393106
Test: atest MicrodroidTests
Change-Id: I6648580615a9fce906dd170f999e11f63e5874d9
diff --git a/microdroid_manager/Android.bp b/microdroid_manager/Android.bp
index 23f174e..fe0cf6a 100644
--- a/microdroid_manager/Android.bp
+++ b/microdroid_manager/Android.bp
@@ -32,6 +32,7 @@
"liblog_rust",
"libmicrodroid_metadata",
"libmicrodroid_payload_config",
+ "libmicrodroid_uids",
"libnix",
"libonce_cell",
"libopenssl",
diff --git a/microdroid_manager/microdroid_manager.rc b/microdroid_manager/microdroid_manager.rc
index e257547..da38564 100644
--- a/microdroid_manager/microdroid_manager.rc
+++ b/microdroid_manager/microdroid_manager.rc
@@ -8,8 +8,8 @@
# TODO(jooyung) remove this when microdroid_manager becomes a daemon
oneshot
# CAP_SYS_BOOT is required to exec kexecload from microdroid_manager
- # CAP_SETCAP is required to allow microdroid_manager to drop capabilities
+ # CAP_SETPCAP is required to allow microdroid_manager to drop capabilities
# before executing the payload
- capabilities AUDIT_CONTROL SYS_ADMIN SYS_BOOT SETPCAP
+ capabilities AUDIT_CONTROL SYS_ADMIN SYS_BOOT SETPCAP SETUID SETGID
user root
socket vm_payload_service stream 0666 system system
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index 319d2df..a48d540 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -528,8 +528,6 @@
}
impl Zipfuse {
- const MICRODROID_PAYLOAD_UID: u32 = 0; // TODO(b/264861173) should be non-root
- const MICRODROID_PAYLOAD_GID: u32 = 0; // TODO(b/264861173) should be non-root
fn mount(
&mut self,
noexec: MountForExec,
@@ -542,9 +540,13 @@
if let MountForExec::Disallowed = noexec {
cmd.arg("--noexec");
}
+ // Let root own the files in APK, so we can access them, but set the group to
+ // allow all payloads to have access too.
+ let (uid, gid) = (microdroid_uids::ROOT_UID, microdroid_uids::MICRODROID_PAYLOAD_GID);
+
cmd.args(["-p", &ready_prop, "-o", option]);
- cmd.args(["-u", &Self::MICRODROID_PAYLOAD_UID.to_string()]);
- cmd.args(["-g", &Self::MICRODROID_PAYLOAD_GID.to_string()]);
+ cmd.args(["-u", &uid.to_string()]);
+ cmd.args(["-g", &gid.to_string()]);
cmd.arg(zip_path).arg(mount_dir);
self.ready_properties.push(ready_prop);
cmd.spawn().with_context(|| format!("Failed to run zipfuse for {mount_dir:?}"))
@@ -850,10 +852,15 @@
fn exec_task(task: &Task, service: &Strong<dyn IVirtualMachineService>) -> Result<i32> {
info!("executing main task {:?}...", task);
let mut command = match task.type_ {
- TaskType::Executable => Command::new(&task.command),
+ TaskType::Executable => {
+ // TODO(b/296393106): Run system payloads as non-root.
+ Command::new(&task.command)
+ }
TaskType::MicrodroidLauncher => {
let mut command = Command::new("/system/bin/microdroid_launcher");
command.arg(find_library_path(&task.command)?);
+ command.uid(microdroid_uids::MICRODROID_PAYLOAD_UID);
+ command.gid(microdroid_uids::MICRODROID_PAYLOAD_GID);
command
}
};