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/tests/aidl/com/android/microdroid/testservice/ITestService.aidl b/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
index 8d467cd..e81f6d7 100644
--- a/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
+++ b/tests/aidl/com/android/microdroid/testservice/ITestService.aidl
@@ -55,6 +55,9 @@
/** Returns a mask of effective capabilities that the process running the payload binary has. */
String[] getEffectiveCapabilities();
+ /* Return the uid of the process running the binary. */
+ int getUid();
+
/* write the content into the specified file. */
void writeToFile(String content, String path);
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 e6d90ea..9f03ab7 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
@@ -451,6 +451,7 @@
public String mApkContentsPath;
public String mEncryptedStoragePath;
public String[] mEffectiveCapabilities;
+ public int mUid;
public String mFileContent;
public byte[] mBcc;
public long[] mTimings;
diff --git a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
index f6dc1b8..a928dcf 100644
--- a/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
+++ b/tests/testapk/src/java/com/android/microdroid/test/MicrodroidTests.java
@@ -72,6 +72,7 @@
import org.junit.After;
import org.junit.Before;
+import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
import org.junit.function.ThrowingRunnable;
@@ -1523,6 +1524,30 @@
}
@Test
+ @Ignore // Figure out how to run this conditionally
+ @CddTest(requirements = {"9.17/C-1-1"})
+ public void payloadIsNotRoot() throws Exception {
+ assumeSupportedDevice();
+
+ VirtualMachineConfig config =
+ newVmConfigBuilder()
+ .setPayloadBinaryName("MicrodroidTestNativeLib.so")
+ .setMemoryBytes(minMemoryRequired())
+ .setDebugLevel(DEBUG_LEVEL_FULL)
+ .build();
+ VirtualMachine vm = forceCreateNewVirtualMachine("test_vm", config);
+ TestResults testResults =
+ runVmTestService(
+ TAG,
+ vm,
+ (ts, tr) -> {
+ tr.mUid = ts.getUid();
+ });
+ testResults.assertNoException();
+ assertThat(testResults.mUid).isNotEqualTo(0);
+ }
+
+ @Test
@CddTest(requirements = {"9.17/C-1-1"})
public void encryptedStorageIsPersistent() throws Exception {
assumeSupportedDevice();
@@ -1971,8 +1996,12 @@
| OsConstants.S_IROTH
| OsConstants.S_IWOTH
| OsConstants.S_IXOTH;
- assertThat(testResults.mFileMode & allPermissionsMask)
- .isEqualTo(OsConstants.S_IRUSR | OsConstants.S_IXUSR);
+ int expectedPermissions =
+ OsConstants.S_IRUSR
+ | OsConstants.S_IXUSR
+ | OsConstants.S_IRGRP
+ | OsConstants.S_IXGRP;
+ assertThat(testResults.mFileMode & allPermissionsMask).isEqualTo(expectedPermissions);
}
// Taken from bionic/libc/kernel/uapi/linux/mount.h
diff --git a/tests/testapk/src/native/testbinary.cpp b/tests/testapk/src/native/testbinary.cpp
index 297b505..c9b5e3a 100644
--- a/tests/testapk/src/native/testbinary.cpp
+++ b/tests/testapk/src/native/testbinary.cpp
@@ -248,6 +248,11 @@
return ScopedAStatus::ok();
}
+ ScopedAStatus getUid(int* out) override {
+ *out = getuid();
+ return ScopedAStatus::ok();
+ }
+
ScopedAStatus runEchoReverseServer() override {
auto result = start_echo_reverse_server();
if (result.ok()) {
diff --git a/tests/vmshareapp/src/java/com/android/microdroid/test/sharevm/VmShareServiceImpl.java b/tests/vmshareapp/src/java/com/android/microdroid/test/sharevm/VmShareServiceImpl.java
index 0ddf70b..dc8908b 100644
--- a/tests/vmshareapp/src/java/com/android/microdroid/test/sharevm/VmShareServiceImpl.java
+++ b/tests/vmshareapp/src/java/com/android/microdroid/test/sharevm/VmShareServiceImpl.java
@@ -220,6 +220,11 @@
}
@Override
+ public int getUid() throws RemoteException {
+ throw new UnsupportedOperationException("Not supported");
+ }
+
+ @Override
public void writeToFile(String content, String path) throws RemoteException {
throw new UnsupportedOperationException("Not supported");
}