Remove onPayloadStarted stream argument
microdroid_manager creates a vsock connection with the host and
redirects the payload's stdin/stdout/stderr streams over it. This may
not necessarily be a securiy issue if the app never writes any secrets
to its standard output, but it would be safer to not open up
a communication channel like that by default. If the payload wishes to
pass its logs to the host, it should open up the connection explicitly.
Remove the vsock connection, the virtualizationservice server and the
corresponding file descriptor argument of onPayloadStarted() callback.
Instead, provide onPayloadStdio() that the payload can optinally call
to set up the connection.
Bug: 245727626
Bug: 253221932
Test: atest -p packages/modules/Virtualization:avf-presubmit
Change-Id: I89fd3a52aae9272db7300224b88d87c6f4d8a8a7
diff --git a/microdroid_manager/src/main.rs b/microdroid_manager/src/main.rs
index c18dd26..762a149 100644
--- a/microdroid_manager/src/main.rs
+++ b/microdroid_manager/src/main.rs
@@ -26,7 +26,7 @@
use crate::vm_payload_service::register_vm_payload_service;
use android_system_virtualizationcommon::aidl::android::system::virtualizationcommon::ErrorCode::ErrorCode;
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::{
- IVirtualMachineService, VM_BINDER_SERVICE_PORT, VM_STREAM_SERVICE_PORT,
+ IVirtualMachineService, VM_BINDER_SERVICE_PORT,
};
use android_system_virtualization_payload::aidl::android::system::virtualization::payload::IVmPayloadService::VM_APK_CONTENTS_PATH;
use anyhow::{anyhow, bail, ensure, Context, Error, Result};
@@ -49,15 +49,13 @@
use std::borrow::Cow::{Borrowed, Owned};
use std::convert::TryInto;
use std::env;
-use std::fs::{self, create_dir, File, OpenOptions};
+use std::fs::{self, create_dir, OpenOptions};
use std::io::Write;
-use std::os::unix::io::{FromRawFd, IntoRawFd};
use std::os::unix::process::ExitStatusExt;
use std::path::Path;
use std::process::{Child, Command, Stdio};
use std::str;
use std::time::{Duration, SystemTime};
-use vsock::VsockStream;
const WAIT_TIMEOUT: Duration = Duration::from_secs(10);
const MAIN_APK_PATH: &str = "/dev/block/by-name/microdroid-apk";
@@ -732,7 +730,14 @@
/// virtualizationservice in the host side.
fn exec_task(task: &Task, service: &Strong<dyn IVirtualMachineService>) -> Result<i32> {
info!("executing main task {:?}...", task);
- let mut command = build_command(task)?;
+ let mut command = match task.type_ {
+ TaskType::Executable => Command::new(&task.command),
+ TaskType::MicrodroidLauncher => {
+ let mut command = Command::new("/system/bin/microdroid_launcher");
+ command.arg(find_library_path(&task.command)?);
+ command
+ }
+ };
info!("notifying payload started");
service.notifyPayloadStarted()?;
@@ -751,40 +756,6 @@
}
}
-fn build_command(task: &Task) -> Result<Command> {
- let mut command = match task.type_ {
- TaskType::Executable => Command::new(&task.command),
- TaskType::MicrodroidLauncher => {
- let mut command = Command::new("/system/bin/microdroid_launcher");
- command.arg(find_library_path(&task.command)?);
- command
- }
- };
-
- match VsockStream::connect_with_cid_port(VMADDR_CID_HOST, VM_STREAM_SERVICE_PORT as u32) {
- Ok(stream) => {
- // SAFETY: the ownership of the underlying file descriptor is transferred from stream
- // to the file object, and then into the Command object. When the command is finished,
- // the file descriptor is closed.
- let file = unsafe { File::from_raw_fd(stream.into_raw_fd()) };
- command
- .stdin(Stdio::from(file.try_clone()?))
- .stdout(Stdio::from(file.try_clone()?))
- .stderr(Stdio::from(file));
- }
- Err(e) => {
- error!("failed to connect to virtualization service: {}", e);
- // Don't fail hard here. Even if we failed to connect to the virtualizationservice,
- // we keep executing the task. This can happen if the owner of the VM doesn't register
- // callback to accept the stream. Use /dev/null as the stream so that the task can
- // make progress without waiting for someone to consume the output.
- command.stdin(Stdio::null()).stdout(Stdio::null()).stderr(Stdio::null());
- }
- }
-
- Ok(command)
-}
-
fn find_library_path(name: &str) -> Result<String> {
let mut watcher = PropertyWatcher::new("ro.product.cpu.abilist")?;
let value = watcher.read(|_name, value| Ok(value.trim().to_string()))?;
diff --git a/microdroid_manager/src/vm_payload_service.rs b/microdroid_manager/src/vm_payload_service.rs
index fcfc79d..249a2d8 100644
--- a/microdroid_manager/src/vm_payload_service.rs
+++ b/microdroid_manager/src/vm_payload_service.rs
@@ -18,15 +18,18 @@
use android_system_virtualization_payload::aidl::android::system::virtualization::payload::IVmPayloadService::{
BnVmPayloadService, IVmPayloadService, VM_PAYLOAD_SERVICE_SOCKET_NAME};
use android_system_virtualmachineservice::aidl::android::system::virtualmachineservice::IVirtualMachineService::IVirtualMachineService;
-use anyhow::{bail, Result};
-use binder::{Interface, BinderFeatures, ExceptionCode, Status, Strong};
+use anyhow::{bail, Context, Result};
+use binder::{Interface, BinderFeatures, ExceptionCode, ParcelFileDescriptor, Status, Strong};
use log::{error, info};
use openssl::hkdf::hkdf;
use openssl::md::Md;
use rpcbinder::run_init_unix_domain_rpc_server;
+use std::fs::File;
use std::sync::mpsc;
use std::thread;
use std::time::Duration;
+use std::os::unix::io::{FromRawFd, IntoRawFd};
+use vsock::VsockListener;
/// Implementation of `IVmPayloadService`.
struct VmPayloadService {
@@ -67,6 +70,16 @@
self.check_restricted_apis_allowed()?;
Ok(self.dice.cdi_attest.to_vec())
}
+
+ fn setupStdioProxy(&self) -> binder::Result<ParcelFileDescriptor> {
+ let f = self.setup_payload_stdio_proxy().map_err(|e| {
+ Status::new_service_specific_error_str(
+ -1,
+ Some(format!("Failed to create stdio proxy: {:?}", e)),
+ )
+ })?;
+ Ok(ParcelFileDescriptor::new(f))
+ }
}
impl Interface for VmPayloadService {}
@@ -89,6 +102,22 @@
Err(Status::new_exception_str(ExceptionCode::SECURITY, Some("Use of restricted APIs")))
}
}
+
+ fn setup_payload_stdio_proxy(&self) -> Result<File> {
+ // Instead of a predefined port in the host, we open up a port in the guest and have
+ // the host connect to it. This makes it possible to have per-app instances of VS.
+ const ANY_PORT: u32 = 0;
+ let listener = VsockListener::bind_with_cid_port(libc::VMADDR_CID_HOST, ANY_PORT)
+ .context("Failed to create vsock listener")?;
+ let addr = listener.local_addr().context("Failed to resolve listener port")?;
+ self.virtual_machine_service
+ .connectPayloadStdioProxy(addr.port() as i32)
+ .context("Failed to connect to the host")?;
+ let (stream, _) =
+ listener.accept().context("Failed to accept vsock connection from the host")?;
+ // SAFETY: ownership is transferred from stream to the new File
+ Ok(unsafe { File::from_raw_fd(stream.into_raw_fd()) })
+ }
}
/// Registers the `IVmPayloadService` service.