Merge "Revert "Don't install files for the virt APEX onto the system partition""
diff --git a/TEST_MAPPING b/TEST_MAPPING
index b07dc3b..87d8e39 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -11,6 +11,9 @@
},
{
"name": "MicrodroidTestApp"
+ },
+ {
+ "name": "art_standalone_dexpreopt_tests"
}
],
"imports": [
diff --git a/apex/product_packages.mk b/apex/product_packages.mk
index 02d7989..1a431d5 100644
--- a/apex/product_packages.mk
+++ b/apex/product_packages.mk
@@ -29,6 +29,6 @@
system/framework/oat/%@service-compos.jar@classes.odex \
system/framework/oat/%@service-compos.jar@classes.vdex \
-# PRODUCT_APEX_SYSTEM_SERVER_JARS := com.android.compos:service-compos
+PRODUCT_APEX_SYSTEM_SERVER_JARS := com.android.compos:service-compos
-# PRODUCT_SYSTEM_EXT_PROPERTIES := ro.config.isolated_compilation_enabled=true
+PRODUCT_SYSTEM_EXT_PROPERTIES := ro.config.isolated_compilation_enabled=true
diff --git a/compos/verify_key/verify_key.rs b/compos/verify_key/verify_key.rs
index 0a9d36b..945acb4 100644
--- a/compos/verify_key/verify_key.rs
+++ b/compos/verify_key/verify_key.rs
@@ -22,7 +22,7 @@
use compos_common::compos_client::{VmInstance, VmParameters};
use compos_common::{
COMPOS_DATA_ROOT, CURRENT_INSTANCE_DIR, INSTANCE_IMAGE_FILE, PENDING_INSTANCE_DIR,
- PRIVATE_KEY_BLOB_FILE, PUBLIC_KEY_FILE,
+ PRIVATE_KEY_BLOB_FILE, PUBLIC_KEY_FILE, TEST_INSTANCE_DIR,
};
use std::fs::{self, File};
use std::io::Read;
@@ -30,41 +30,54 @@
const MAX_FILE_SIZE_BYTES: u64 = 8 * 1024;
-fn main() -> Result<()> {
+fn main() {
android_logger::init_once(
android_logger::Config::default()
.with_tag("compos_verify_key")
.with_min_level(log::Level::Info),
);
+ if let Err(e) = try_main() {
+ log::error!("{:?}", e);
+ std::process::exit(-1)
+ }
+}
+
+fn try_main() -> Result<()> {
let matches = clap::App::new("compos_verify_key")
.arg(
clap::Arg::with_name("instance")
.long("instance")
.takes_value(true)
.required(true)
- .possible_values(&["pending", "current"]),
+ .possible_values(&["pending", "current", "test"]),
)
+ .arg(clap::Arg::with_name("debug").long("debug"))
.get_matches();
- let do_pending = matches.value_of("instance").unwrap() == "pending";
- let instance_dir: PathBuf =
- [COMPOS_DATA_ROOT, if do_pending { PENDING_INSTANCE_DIR } else { CURRENT_INSTANCE_DIR }]
- .iter()
- .collect();
+ let debug_mode = matches.is_present("debug");
+ let (promote_if_valid, instance_dir) = match matches.value_of("instance").unwrap() {
+ "pending" => (true, PENDING_INSTANCE_DIR),
+ "current" => (false, CURRENT_INSTANCE_DIR),
+ "test" => (false, TEST_INSTANCE_DIR),
+ _ => unreachable!("Unexpected instance name"),
+ };
+
+ let instance_dir: PathBuf = [COMPOS_DATA_ROOT, instance_dir].iter().collect();
if !instance_dir.is_dir() {
- bail!("{} is not a directory", instance_dir.display());
+ bail!("{:?} is not a directory", instance_dir);
}
// We need to start the thread pool to be able to receive Binder callbacks
ProcessState::start_thread_pool();
- let result = verify(&instance_dir).and_then(|_| {
- if do_pending {
- // If the pending instance is ok, then it must actually match the current system state,
+ let result = verify(debug_mode, &instance_dir).and_then(|_| {
+ log::info!("Verified {:?}", instance_dir);
+ if promote_if_valid {
+ // If the instance is ok, then it must actually match the current system state,
// so we promote it to current.
- log::info!("Promoting pending to current");
+ log::info!("Promoting to current");
promote_to_current(&instance_dir)
} else {
Ok(())
@@ -73,7 +86,7 @@
if result.is_err() {
// This is best efforts, and we still want to report the original error as our result
- log::info!("Removing {}", instance_dir.display());
+ log::info!("Removing {:?}", instance_dir);
if let Err(e) = fs::remove_dir_all(&instance_dir) {
log::warn!("Failed to remove directory: {}", e);
}
@@ -82,7 +95,7 @@
result
}
-fn verify(instance_dir: &Path) -> Result<()> {
+fn verify(debug_mode: bool, instance_dir: &Path) -> Result<()> {
let blob = instance_dir.join(PRIVATE_KEY_BLOB_FILE);
let public_key = instance_dir.join(PUBLIC_KEY_FILE);
let instance_image = instance_dir.join(INSTANCE_IMAGE_FILE);
@@ -93,7 +106,7 @@
let virtualization_service = VmInstance::connect_to_virtualization_service()?;
let vm_instance =
- VmInstance::start(&*virtualization_service, instance_image, &VmParameters::default())?;
+ VmInstance::start(&*virtualization_service, instance_image, &VmParameters { debug_mode })?;
let service = vm_instance.get_service()?;
let result = service.verifySigningKey(&blob, &public_key).context("Verifying signing key")?;
@@ -105,6 +118,17 @@
Ok(())
}
+fn promote_to_current(instance_dir: &Path) -> Result<()> {
+ let current_dir: PathBuf = [COMPOS_DATA_ROOT, CURRENT_INSTANCE_DIR].iter().collect();
+
+ // This may fail if the directory doesn't exist - which is fine, we only care about the rename
+ // succeeding.
+ let _ = fs::remove_dir_all(¤t_dir);
+
+ fs::rename(&instance_dir, ¤t_dir).context("Unable to promote instance to current")?;
+ Ok(())
+}
+
fn read_small_file(file: PathBuf) -> Result<Vec<u8>> {
let mut file = File::open(file)?;
if file.metadata()?.len() > MAX_FILE_SIZE_BYTES {
@@ -114,15 +138,3 @@
file.read_to_end(&mut data)?;
Ok(data)
}
-
-fn promote_to_current(instance_dir: &Path) -> Result<()> {
- let current_dir: PathBuf = [COMPOS_DATA_ROOT, CURRENT_INSTANCE_DIR].iter().collect();
-
- // This may fail if the directory doesn't exist - which is fine, we only care about the rename
- // succeeding.
- let _ = fs::remove_dir_all(¤t_dir);
-
- fs::rename(&instance_dir, ¤t_dir)
- .context("Unable to promote pending instance to current")?;
- Ok(())
-}
diff --git a/virtualizationservice/Android.bp b/virtualizationservice/Android.bp
index 18d8ade..37350ff 100644
--- a/virtualizationservice/Android.bp
+++ b/virtualizationservice/Android.bp
@@ -36,9 +36,10 @@
"libmicrodroid_payload_config",
"libonce_cell",
"librustutils",
+ "libselinux_bindgen",
+ "libserde",
"libserde_json",
"libserde_xml_rs",
- "libserde",
"libshared_child",
"libvmconfig",
"libzip",
@@ -48,6 +49,7 @@
],
shared_libs: [
"libbinder_rpc_unstable",
+ "libselinux",
],
}
diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs
index 1c88174..af420f6 100644
--- a/virtualizationservice/src/aidl.rs
+++ b/virtualizationservice/src/aidl.rs
@@ -18,6 +18,7 @@
use crate::crosvm::{CrosvmConfig, DiskFile, PayloadState, VmInstance, VmState};
use crate::payload::add_microdroid_images;
use crate::{Cid, FIRST_GUEST_CID, SYSPROP_LAST_CID};
+use crate::selinux::{SeContext, getfilecon};
use ::binder::unstable_api::AsNative;
use android_os_permissions_aidl::aidl::android::os::IPermissionController;
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::{
@@ -25,6 +26,7 @@
IVirtualMachine::{BnVirtualMachine, IVirtualMachine},
IVirtualMachineCallback::IVirtualMachineCallback,
IVirtualizationService::IVirtualizationService,
+ Partition::Partition,
PartitionType::PartitionType,
VirtualMachineAppConfig::DebugLevel::DebugLevel,
VirtualMachineAppConfig::VirtualMachineAppConfig,
@@ -169,6 +171,8 @@
}
}
+ let is_app_config = matches!(config, VirtualMachineConfig::AppConfig(_));
+
let config = match config {
VirtualMachineConfig::AppConfig(config) => BorrowedOrOwned::Owned(
load_app_config(config, &temporary_directory).map_err(|e| {
@@ -183,6 +187,25 @@
};
let config = config.as_ref();
+ // Check if partition images are labeled incorrectly. This is to prevent random images
+ // which are not protected by the Android Verified Boot (e.g. bits downloaded by apps) from
+ // being loaded in a pVM. Specifically, for images in the raw config, nothing is allowed
+ // to be labeled as app_data_file. For images in the app config, nothing but the instance
+ // partition is allowed to be labeled as such.
+ config
+ .disks
+ .iter()
+ .flat_map(|disk| disk.partitions.iter())
+ .filter(|partition| {
+ if is_app_config {
+ partition.label != "vm-instance"
+ } else {
+ true // all partitions are checked
+ }
+ })
+ .try_for_each(check_label_for_partition)
+ .map_err(|e| new_binder_exception(ExceptionCode::SERVICE_SPECIFIC, e.to_string()))?;
+
let zero_filler_path = temporary_directory.join("zero.img");
write_zero_filler(&zero_filler_path).map_err(|e| {
error!("Failed to make composite image: {}", e);
@@ -606,6 +629,16 @@
check_permission("android.permission.MANAGE_VIRTUAL_MACHINE")
}
+/// Check if a partition has selinux labels that are not allowed
+fn check_label_for_partition(partition: &Partition) -> Result<()> {
+ let ctx = getfilecon(partition.image.as_ref().unwrap().as_ref())?;
+ if ctx == SeContext::new("u:object_r:app_data_file:s0").unwrap() {
+ Err(anyhow!("Partition {} shouldn't be labeled as {}", &partition.label, ctx))
+ } else {
+ Ok(())
+ }
+}
+
/// Implementation of the AIDL `IVirtualMachine` interface. Used as a handle to a VM.
#[derive(Debug)]
struct VirtualMachine {
diff --git a/virtualizationservice/src/main.rs b/virtualizationservice/src/main.rs
index 0e1e974..69ae076 100644
--- a/virtualizationservice/src/main.rs
+++ b/virtualizationservice/src/main.rs
@@ -18,6 +18,7 @@
mod composite;
mod crosvm;
mod payload;
+mod selinux;
use crate::aidl::{VirtualizationService, BINDER_SERVICE_IDENTIFIER, TEMPORARY_DIRECTORY};
use android_system_virtualizationservice::aidl::android::system::virtualizationservice::IVirtualizationService::BnVirtualizationService;
diff --git a/virtualizationservice/src/selinux.rs b/virtualizationservice/src/selinux.rs
new file mode 100644
index 0000000..e450dee
--- /dev/null
+++ b/virtualizationservice/src/selinux.rs
@@ -0,0 +1,104 @@
+// Copyright 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.
+
+//! Wrapper to libselinux
+
+use anyhow::{anyhow, Context, Result};
+use std::ffi::{CStr, CString};
+use std::fmt;
+use std::fs::File;
+use std::io;
+use std::ops::Deref;
+use std::os::raw::c_char;
+use std::os::unix::io::AsRawFd;
+use std::ptr;
+
+// Partially copied from system/security/keystore2/selinux/src/lib.rs
+/// SeContext represents an SELinux context string. It can take ownership of a raw
+/// s-string as allocated by `getcon` or `selabel_lookup`. In this case it uses
+/// `freecon` to free the resources when dropped. In its second variant it stores
+/// an `std::ffi::CString` that can be initialized from a Rust string slice.
+#[derive(Debug)]
+pub enum SeContext {
+ /// Wraps a raw context c-string as returned by libselinux.
+ Raw(*mut ::std::os::raw::c_char),
+ /// Stores a context string as `std::ffi::CString`.
+ CString(CString),
+}
+
+impl PartialEq for SeContext {
+ fn eq(&self, other: &Self) -> bool {
+ // We dereference both and thereby delegate the comparison
+ // to `CStr`'s implementation of `PartialEq`.
+ **self == **other
+ }
+}
+
+impl Eq for SeContext {}
+
+impl fmt::Display for SeContext {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ write!(f, "{}", self.to_str().unwrap_or("Invalid context"))
+ }
+}
+
+impl Drop for SeContext {
+ fn drop(&mut self) {
+ if let Self::Raw(p) = self {
+ // SAFETY: SeContext::Raw is created only with a pointer that is set by libselinux and
+ // has to be freed with freecon.
+ unsafe { selinux_bindgen::freecon(*p) };
+ }
+ }
+}
+
+impl Deref for SeContext {
+ type Target = CStr;
+
+ fn deref(&self) -> &Self::Target {
+ match self {
+ // SAFETY: the non-owned C string pointed by `p` is guaranteed to be valid (non-null
+ // and shorter than i32::MAX). It is freed when SeContext is dropped.
+ Self::Raw(p) => unsafe { CStr::from_ptr(*p) },
+ Self::CString(cstr) => cstr,
+ }
+ }
+}
+
+impl SeContext {
+ /// Initializes the `SeContext::CString` variant from a Rust string slice.
+ pub fn new(con: &str) -> Result<Self> {
+ Ok(Self::CString(
+ CString::new(con)
+ .with_context(|| format!("Failed to create SeContext with \"{}\"", con))?,
+ ))
+ }
+}
+
+pub fn getfilecon(file: &File) -> Result<SeContext> {
+ let fd = file.as_raw_fd();
+ let mut con: *mut c_char = ptr::null_mut();
+ // SAFETY: the returned pointer `con` is wrapped in SeContext::Raw which is freed with
+ // `freecon` when it is dropped.
+ match unsafe { selinux_bindgen::fgetfilecon(fd, &mut con) } {
+ 1.. => {
+ if !con.is_null() {
+ Ok(SeContext::Raw(con))
+ } else {
+ Err(anyhow!("fgetfilecon returned a NULL context"))
+ }
+ }
+ _ => Err(anyhow!(io::Error::last_os_error())).context("fgetfilecon failed"),
+ }
+}