Switch compsvc to use authfs_service
With authfs_service, we no longer need compsvc_worker.rs and authfs.rs
for the task setup. Now, for each request, compsvc can just request
FDs from authfs_service then pass to the task.
Also, fixed the integer type of remote FD to match ParcelFileDescriptor.
Bug: 194717985
Test: atest ComposHostTestCases
Change-Id: I8c0be106243778ac20e7cd96a778db4e34aef051
diff --git a/compos/Android.bp b/compos/Android.bp
index ec3f67f..7f4f55c 100644
--- a/compos/Android.bp
+++ b/compos/Android.bp
@@ -30,6 +30,7 @@
name: "compsvc",
srcs: ["src/compsvc_main.rs"],
rustlibs: [
+ "authfs_aidl_interface-rust",
"compos_aidl_interface-rust",
"libandroid_logger",
"libanyhow",
@@ -49,27 +50,11 @@
}
rust_binary {
- name: "compsvc_worker",
- srcs: ["src/compsvc_worker.rs"],
- rustlibs: [
- "libandroid_logger",
- "libanyhow",
- "libclap",
- "liblog_rust",
- "libminijail_rust",
- "libnix",
- ],
- prefer_rlib: true,
- apex_available: [
- "com.android.compos",
- ],
-}
-
-rust_binary {
name: "compos_key_main",
srcs: ["src/compos_key_main.rs"],
edition: "2018",
rustlibs: [
+ "authfs_aidl_interface-rust",
"compos_aidl_interface-rust",
"android.system.keystore2-V1-rust",
"android.hardware.security.keymint-V1-rust",
diff --git a/compos/apex/Android.bp b/compos/apex/Android.bp
index 061c362..12d2f06 100644
--- a/compos/apex/Android.bp
+++ b/compos/apex/Android.bp
@@ -41,7 +41,6 @@
"compos_key_cmd",
"compos_key_main",
"compsvc",
- "compsvc_worker",
"pvm_exec",
],
diff --git a/compos/src/authfs.rs b/compos/src/authfs.rs
deleted file mode 100644
index ce9aaf8..0000000
--- a/compos/src/authfs.rs
+++ /dev/null
@@ -1,144 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-use anyhow::{bail, Context, Result};
-use log::warn;
-use minijail::Minijail;
-use nix::sys::statfs::{statfs, FsType};
-use std::fs::{File, OpenOptions};
-use std::path::Path;
-use std::thread::sleep;
-use std::time::{Duration, Instant};
-
-const AUTHFS_BIN: &str = "/system/bin/authfs";
-const AUTHFS_SETUP_POLL_INTERVAL_MS: Duration = Duration::from_millis(50);
-const AUTHFS_SETUP_TIMEOUT_SEC: Duration = Duration::from_secs(10);
-const FUSE_SUPER_MAGIC: FsType = FsType(0x65735546);
-
-/// The number that hints the future file descriptor. These are not really file descriptor, but
-/// represents the file descriptor number to pass to the task.
-pub type PseudoRawFd = i32;
-
-/// Annotation of input file descriptor.
-#[derive(Debug)]
-pub struct InFdAnnotation {
- /// A number/file descriptor that is supposed to represent a remote file.
- pub fd: PseudoRawFd,
-
- /// The file size of the remote file. Remote input files are supposed to be immutable and
- /// to be verified with fs-verity by authfs.
- pub file_size: u64,
-}
-
-/// Annotation of output file descriptor.
-#[derive(Debug)]
-pub struct OutFdAnnotation {
- /// A number/file descriptor that is supposed to represent a remote file.
- pub fd: PseudoRawFd,
-}
-
-/// An `AuthFs` instance is supposed to be backed by the `authfs` process. When the lifetime of the
-/// instance is over, the process is terminated and the FUSE is unmounted.
-pub struct AuthFs {
- mountpoint: String,
- jail: Minijail,
-}
-
-impl AuthFs {
- /// Mount an authfs at `mountpoint` with specified FD annotations.
- pub fn mount_and_wait(
- mountpoint: &str,
- in_fds: &[InFdAnnotation],
- out_fds: &[OutFdAnnotation],
- debuggable: bool,
- ) -> Result<AuthFs> {
- let jail = jail_authfs(mountpoint, in_fds, out_fds, debuggable)?;
- wait_until_authfs_ready(mountpoint)?;
- Ok(AuthFs { mountpoint: mountpoint.to_string(), jail })
- }
-
- /// Open a file at authfs' root directory.
- pub fn open_file(&self, basename: PseudoRawFd, writable: bool) -> Result<File> {
- OpenOptions::new()
- .read(true)
- .write(writable)
- .open(format!("{}/{}", self.mountpoint, basename))
- .with_context(|| format!("open authfs file {}", basename))
- }
-}
-
-impl Drop for AuthFs {
- fn drop(&mut self) {
- if let Err(e) = self.jail.kill() {
- if !matches!(e, minijail::Error::Killed(_)) {
- warn!("Failed to kill authfs: {}", e);
- }
- }
- }
-}
-
-fn jail_authfs(
- mountpoint: &str,
- in_fds: &[InFdAnnotation],
- out_fds: &[OutFdAnnotation],
- debuggable: bool,
-) -> Result<Minijail> {
- // TODO(b/185175567): Run in a more restricted sandbox.
- let jail = Minijail::new()?;
-
- let mut args = vec![
- AUTHFS_BIN.to_string(),
- mountpoint.to_string(),
- "--cid=2".to_string(), // Always use host unless we need to support other cases
- ];
- for conf in in_fds {
- // TODO(b/185178698): Many input files need to be signed and verified.
- // or can we use debug cert for now, which is better than nothing?
- args.push("--remote-ro-file-unverified".to_string());
- args.push(format!("{}:{}:{}", conf.fd, conf.fd, conf.file_size));
- }
- for conf in out_fds {
- args.push("--remote-new-rw-file".to_string());
- args.push(format!("{}:{}", conf.fd, conf.fd));
- }
-
- let preserve_fds = if debuggable {
- vec![1, 2] // inherit/redirect stdout/stderr for debugging
- } else {
- vec![]
- };
-
- let _pid = jail.run(Path::new(AUTHFS_BIN), &preserve_fds, &args)?;
- Ok(jail)
-}
-
-fn wait_until_authfs_ready(mountpoint: &str) -> Result<()> {
- let start_time = Instant::now();
- loop {
- if is_fuse(mountpoint)? {
- break;
- }
- if start_time.elapsed() > AUTHFS_SETUP_TIMEOUT_SEC {
- bail!("Time out mounting authfs");
- }
- sleep(AUTHFS_SETUP_POLL_INTERVAL_MS);
- }
- Ok(())
-}
-
-fn is_fuse(path: &str) -> Result<bool> {
- Ok(statfs(path)?.filesystem_type() == FUSE_SUPER_MAGIC)
-}
diff --git a/compos/src/compsvc.rs b/compos/src/compsvc.rs
index ae242de..14b520e 100644
--- a/compos/src/compsvc.rs
+++ b/compos/src/compsvc.rs
@@ -15,34 +15,35 @@
*/
//! compsvc is a service to run computational tasks in a PVM upon request. It is able to set up
-//! file descriptors backed by fd_server and pass the file descriptors to the actual tasks for
-//! read/write. The service also attempts to sandbox the execution so that one task cannot leak or
-//! impact future tasks.
-//!
-//! The current architecture / process hierarchy looks like:
-//! - compsvc (handle requests)
-//! - compsvc_worker (for environment setup)
-//! - authfs (fd translation)
-//! - actual task
+//! file descriptors backed by authfs (via authfs_service) and pass the file descriptors to the
+//! actual tasks.
use anyhow::Result;
use log::error;
use minijail::{self, Minijail};
-use std::path::PathBuf;
+use std::ffi::CString;
+use std::os::unix::io::AsRawFd;
+use std::path::{Path, PathBuf};
use crate::signer::Signer;
+use authfs_aidl_interface::aidl::com::android::virt::fs::{
+ AuthFsConfig::AuthFsConfig, IAuthFs::IAuthFs, IAuthFsService::IAuthFsService,
+ InputFdAnnotation::InputFdAnnotation, OutputFdAnnotation::OutputFdAnnotation,
+};
+use authfs_aidl_interface::binder::ParcelFileDescriptor;
use compos_aidl_interface::aidl::com::android::compos::ICompService::{
BnCompService, ICompService,
};
use compos_aidl_interface::aidl::com::android::compos::Metadata::Metadata;
use compos_aidl_interface::binder::{
- BinderFeatures, Interface, Result as BinderResult, Status, StatusCode, Strong,
+ BinderFeatures, ExceptionCode, Interface, Result as BinderResult, Status, StatusCode, Strong,
};
-const WORKER_BIN: &str = "/apex/com.android.compos/bin/compsvc_worker";
+const AUTHFS_SERVICE_NAME: &str = "authfs_service";
-// TODO: Replace with a valid directory setup in the VM.
-const AUTHFS_MOUNTPOINT: &str = "/data/local/tmp";
+/// The number that represents the file descriptor number expecting by the task. The number may be
+/// meaningless in the current process.
+pub type PseudoRawFd = i32;
/// Constructs a binder object that implements ICompService. task_bin is the path to the binary that will
/// be run when execute() is called. If debuggable is true then stdout/stderr from the binary will be
@@ -52,83 +53,128 @@
debuggable: bool,
signer: Option<Box<dyn Signer>>,
) -> Strong<dyn ICompService> {
- let service = CompService {
- worker_bin: PathBuf::from(WORKER_BIN.to_owned()),
- task_bin,
- debuggable,
- signer,
- };
+ let service = CompService { task_bin: PathBuf::from(task_bin), debuggable, signer };
BnCompService::new_binder(service, BinderFeatures::default())
}
struct CompService {
- task_bin: String,
- worker_bin: PathBuf,
+ task_bin: PathBuf,
debuggable: bool,
#[allow(dead_code)] // TODO: Make use of this
signer: Option<Box<dyn Signer>>,
}
-impl CompService {
- fn run_worker_in_jail_and_wait(&self, args: &[String]) -> Result<(), minijail::Error> {
- let mut jail = Minijail::new()?;
-
- // TODO(b/185175567): New user and uid namespace when supported. Run as nobody.
- // New mount namespace to isolate the FUSE mount.
- jail.namespace_vfs();
-
- let inheritable_fds = if self.debuggable {
- vec![1, 2] // inherit/redirect stdout/stderr for debugging
- } else {
- vec![]
- };
- let _pid = jail.run(&self.worker_bin, &inheritable_fds, args)?;
- jail.wait()
- }
-
- fn build_worker_args(&self, args: &[String], metadata: &Metadata) -> Vec<String> {
- let mut worker_args = vec![
- WORKER_BIN.to_string(),
- "--authfs-root".to_string(),
- AUTHFS_MOUNTPOINT.to_string(),
- ];
- for annotation in &metadata.input_fd_annotations {
- worker_args.push("--in-fd".to_string());
- worker_args.push(format!("{}:{}", annotation.fd, annotation.file_size));
- }
- for annotation in &metadata.output_fd_annotations {
- worker_args.push("--out-fd".to_string());
- worker_args.push(annotation.fd.to_string());
- }
- if self.debuggable {
- worker_args.push("--debug".to_string());
- }
- worker_args.push("--".to_string());
-
- // Do not accept arbitrary code execution. We want to execute some specific task of this
- // service. Use the associated executable.
- worker_args.push(self.task_bin.clone());
- worker_args.extend_from_slice(&args[1..]);
- worker_args
- }
-}
-
impl Interface for CompService {}
impl ICompService for CompService {
fn execute(&self, args: &[String], metadata: &Metadata) -> BinderResult<i8> {
- let worker_args = self.build_worker_args(args, metadata);
+ // Mount authfs (via authfs_service).
+ let authfs_config = build_authfs_config(metadata);
+ let authfs = get_authfs_service()?.mount(&authfs_config)?;
- match self.run_worker_in_jail_and_wait(&worker_args) {
+ // The task expects to receive FD numbers that match its flags (e.g. --zip-fd=42) prepared
+ // on the host side. Since the local FD opened from authfs (e.g. /authfs/42) may not match
+ // the task's expectation, prepare a FD mapping and let minijail prepare the correct FD
+ // setup.
+ let fd_mapping =
+ open_authfs_files_for_fd_mapping(&authfs, &authfs_config).map_err(|e| {
+ new_binder_exception(
+ ExceptionCode::SERVICE_SPECIFIC,
+ format!("Failed to create FDs on authfs: {:?}", e),
+ )
+ })?;
+
+ let jail =
+ spawn_jailed_task(&self.task_bin, args, fd_mapping, self.debuggable).map_err(|e| {
+ new_binder_exception(
+ ExceptionCode::SERVICE_SPECIFIC,
+ format!("Failed to spawn the task: {:?}", e),
+ )
+ })?;
+ let jail_result = jail.wait();
+
+ // Be explicit about the lifetime, which should last at least until the task is finished.
+ drop(authfs);
+
+ match jail_result {
Ok(_) => Ok(0), // TODO(b/161471326): Sign the output on succeed.
Err(minijail::Error::ReturnCode(exit_code)) => {
error!("Task failed with exit code {}", exit_code);
Err(Status::from(StatusCode::FAILED_TRANSACTION))
}
Err(e) => {
- error!("Unexpected error: {}", e);
+ error!("Unexpected minijail error: {}", e);
Err(Status::from(StatusCode::UNKNOWN_ERROR))
}
}
}
}
+
+fn get_authfs_service() -> BinderResult<Strong<dyn IAuthFsService>> {
+ Ok(authfs_aidl_interface::binder::get_interface(AUTHFS_SERVICE_NAME)?)
+}
+
+fn build_authfs_config(metadata: &Metadata) -> AuthFsConfig {
+ AuthFsConfig {
+ port: 3264, // TODO: support dynamic port
+ inputFdAnnotations: metadata
+ .input_fd_annotations
+ .iter()
+ .map(|x| InputFdAnnotation { fd: x.fd, fileSize: x.file_size })
+ .collect(),
+ outputFdAnnotations: metadata
+ .output_fd_annotations
+ .iter()
+ .map(|x| OutputFdAnnotation { fd: x.fd })
+ .collect(),
+ }
+}
+
+fn open_authfs_files_for_fd_mapping(
+ authfs: &Strong<dyn IAuthFs>,
+ config: &AuthFsConfig,
+) -> Result<Vec<(ParcelFileDescriptor, PseudoRawFd)>> {
+ let mut fd_mapping = Vec::new();
+
+ let results: Result<Vec<_>> = config
+ .inputFdAnnotations
+ .iter()
+ .map(|annotation| Ok((authfs.openFile(annotation.fd, false)?, annotation.fd)))
+ .collect();
+ fd_mapping.append(&mut results?);
+
+ let results: Result<Vec<_>> = config
+ .outputFdAnnotations
+ .iter()
+ .map(|annotation| Ok((authfs.openFile(annotation.fd, true)?, annotation.fd)))
+ .collect();
+ fd_mapping.append(&mut results?);
+
+ Ok(fd_mapping)
+}
+
+fn spawn_jailed_task(
+ executable: &Path,
+ args: &[String],
+ fd_mapping: Vec<(ParcelFileDescriptor, PseudoRawFd)>,
+ debuggable: bool,
+) -> Result<Minijail> {
+ // TODO(b/185175567): Run in a more restricted sandbox.
+ let jail = Minijail::new()?;
+
+ let mut preserve_fds = if debuggable {
+ // Inherit/redirect stdout/stderr for debugging, assuming no conflict
+ vec![(1, 1), (2, 2)]
+ } else {
+ vec![]
+ };
+
+ preserve_fds.extend(fd_mapping.iter().map(|(f, id)| (f.as_raw_fd(), *id)));
+
+ let _pid = jail.run_remap(executable, preserve_fds.as_slice(), args)?;
+ Ok(jail)
+}
+
+fn new_binder_exception<T: AsRef<str>>(exception: ExceptionCode, message: T) -> Status {
+ Status::new_exception(exception, CString::new(message.as_ref()).as_deref().ok())
+}
diff --git a/compos/src/compsvc_worker.rs b/compos/src/compsvc_worker.rs
deleted file mode 100644
index f33659e..0000000
--- a/compos/src/compsvc_worker.rs
+++ /dev/null
@@ -1,161 +0,0 @@
-/*
- * Copyright (C) 2021 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-//! This executable works as a child/worker for the main compsvc service. This worker is mainly
-//! responsible for setting up the execution environment, e.g. to create file descriptors for
-//! remote file access via an authfs mount.
-
-mod authfs;
-
-use anyhow::{bail, Result};
-use minijail::Minijail;
-use std::fs::File;
-use std::os::unix::io::AsRawFd;
-use std::path::Path;
-use std::process::exit;
-
-use crate::authfs::{AuthFs, InFdAnnotation, OutFdAnnotation, PseudoRawFd};
-
-fn open_authfs_files_for_mapping(
- authfs: &AuthFs,
- config: &Config,
-) -> Result<Vec<(File, PseudoRawFd)>> {
- let mut fd_mapping = Vec::with_capacity(config.in_fds.len() + config.out_fds.len());
-
- let results: Result<Vec<_>> =
- config.in_fds.iter().map(|conf| Ok((authfs.open_file(conf.fd, false)?, conf.fd))).collect();
- fd_mapping.append(&mut results?);
-
- let results: Result<Vec<_>> =
- config.out_fds.iter().map(|conf| Ok((authfs.open_file(conf.fd, true)?, conf.fd))).collect();
- fd_mapping.append(&mut results?);
-
- Ok(fd_mapping)
-}
-
-fn spawn_jailed_task(config: &Config, fd_mapping: Vec<(File, PseudoRawFd)>) -> Result<Minijail> {
- // TODO(b/185175567): Run in a more restricted sandbox.
- let jail = Minijail::new()?;
- let mut preserve_fds: Vec<_> = fd_mapping.iter().map(|(f, id)| (f.as_raw_fd(), *id)).collect();
- if config.debuggable {
- // inherit/redirect stdout/stderr for debugging
- preserve_fds.push((1, 1));
- preserve_fds.push((2, 2));
- }
- let _pid =
- jail.run_remap(&Path::new(&config.args[0]), preserve_fds.as_slice(), &config.args)?;
- Ok(jail)
-}
-
-struct Config {
- authfs_root: String,
- in_fds: Vec<InFdAnnotation>,
- out_fds: Vec<OutFdAnnotation>,
- args: Vec<String>,
- debuggable: bool,
-}
-
-fn parse_args() -> Result<Config> {
- #[rustfmt::skip]
- let matches = clap::App::new("compsvc_worker")
- .arg(clap::Arg::with_name("authfs-root")
- .long("authfs-root")
- .value_name("DIR")
- .required(true)
- .takes_value(true))
- .arg(clap::Arg::with_name("in-fd")
- .long("in-fd")
- .multiple(true)
- .takes_value(true)
- .requires("authfs-root"))
- .arg(clap::Arg::with_name("out-fd")
- .long("out-fd")
- .multiple(true)
- .takes_value(true)
- .requires("authfs-root"))
- .arg(clap::Arg::with_name("debug")
- .long("debug"))
- .arg(clap::Arg::with_name("args")
- .last(true)
- .required(true)
- .multiple(true))
- .get_matches();
-
- // Safe to unwrap since the arg is required by the clap rule
- let authfs_root = matches.value_of("authfs-root").unwrap().to_string();
-
- let results: Result<Vec<_>> = matches
- .values_of("in-fd")
- .unwrap_or_default()
- .into_iter()
- .map(|arg| {
- if let Some(index) = arg.find(':') {
- let (fd, size) = arg.split_at(index);
- Ok(InFdAnnotation { fd: fd.parse()?, file_size: size[1..].parse()? })
- } else {
- bail!("Invalid argument: {}", arg);
- }
- })
- .collect();
- let in_fds = results?;
-
- let results: Result<Vec<_>> = matches
- .values_of("out-fd")
- .unwrap_or_default()
- .into_iter()
- .map(|arg| Ok(OutFdAnnotation { fd: arg.parse()? }))
- .collect();
- let out_fds = results?;
-
- let args: Vec<_> = matches.values_of("args").unwrap().map(|s| s.to_string()).collect();
- let debuggable = matches.is_present("debug");
-
- Ok(Config { authfs_root, in_fds, out_fds, args, debuggable })
-}
-
-fn main() -> Result<()> {
- let log_level =
- if env!("TARGET_BUILD_VARIANT") == "eng" { log::Level::Trace } else { log::Level::Info };
- android_logger::init_once(
- android_logger::Config::default().with_tag("compsvc_worker").with_min_level(log_level),
- );
-
- let config = parse_args()?;
-
- let authfs = AuthFs::mount_and_wait(
- &config.authfs_root,
- &config.in_fds,
- &config.out_fds,
- config.debuggable,
- )?;
- let fd_mapping = open_authfs_files_for_mapping(&authfs, &config)?;
-
- let jail = spawn_jailed_task(&config, fd_mapping)?;
- let jail_result = jail.wait();
-
- // Be explicit about the lifetime, which should last at least until the task is finished.
- drop(authfs);
-
- match jail_result {
- Ok(_) => Ok(()),
- Err(minijail::Error::ReturnCode(exit_code)) => {
- exit(exit_code as i32);
- }
- Err(e) => {
- bail!("Unexpected minijail error: {}", e);
- }
- }
-}