From 029977d76036af837c33e1d672bf21a29300ac8e Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Wed, 24 Nov 2021 21:56:49 +0900 Subject: [PATCH] Don't allow disk images labeled as app_data_file At least for T, we don't want to have a VM that is running a disk image that is not protected by AVB (Android Verified Boot). This means that the disk images must be from file in the built-in partitions (like system) or their updates (because updates are guaranteed to be from the same signer). This rule is currently being enforced by selinux policies around crosvm. However, there is one exception. It has the following allow rule. allow crosvm app_data_file:file { read ...}; app_data_file is for any file that is owned and dynamically created by apps. They could be a file downloaded from Internet, and therefore is definitely not protected by AVB. The only reason we have the above allow rule is the instance image. The file is mutable because it has per-VM identity information that is written when a pVM is first created. The file is owned by the owning app, otherwise other apps would be able to start the same VM. In other words, we don't intend to allow any app_data_file to be loadable in a pVM. But such an intention can't be expressed in sepolicy. This CL augments the sepolicy by adding a runtime check in virtualizationservice. Specifically, it rejects to spawn a VM if any of the disk images is labeled as app_data_file, except for the disk image for the instance partition. Bug: 204852957 Test: adb shell chcon u:object_r:app_data_file:s0 /data/local/tmp/virt/MicrodroidDemoApp.apk adb shell /apex/com.android.virt/bin/vm run-app --debug full \ /data/local/tmp/virt/MicrodroidDemoApp.apk \ /data/local/tmp/virt/MicrodroidDemoApp.apk.idsig \ /data/local/tmp/virt/instance.img assets/vm_config.json \ gives the following error as expected. Status(-8, EX_SERVICE_SPECIFIC): '-1: Partition microdroid-apk shouldn't be labeld as u:object_r:app_data_file:s0' Change-Id: Ia7081b8ecb2db1ecc1f7d4941a305ccff6016f3e --- virtualizationservice/Android.bp | 4 +- virtualizationservice/src/aidl.rs | 33 +++++++++ virtualizationservice/src/main.rs | 1 + virtualizationservice/src/selinux.rs | 104 +++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 1 deletion(-) create mode 100644 virtualizationservice/src/selinux.rs diff --git a/virtualizationservice/Android.bp b/virtualizationservice/Android.bp index 18d8adee..37350ffa 100644 --- a/virtualizationservice/Android.bp +++ b/virtualizationservice/Android.bp @@ -36,9 +36,10 @@ rust_defaults { "libmicrodroid_payload_config", "libonce_cell", "librustutils", + "libselinux_bindgen", + "libserde", "libserde_json", "libserde_xml_rs", - "libserde", "libshared_child", "libvmconfig", "libzip", @@ -48,6 +49,7 @@ rust_defaults { ], shared_libs: [ "libbinder_rpc_unstable", + "libselinux", ], } diff --git a/virtualizationservice/src/aidl.rs b/virtualizationservice/src/aidl.rs index 1c881746..af420f69 100644 --- a/virtualizationservice/src/aidl.rs +++ b/virtualizationservice/src/aidl.rs @@ -18,6 +18,7 @@ use crate::composite::make_composite_image; 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 @@ use android_system_virtualizationservice::aidl::android::system::virtualizations IVirtualMachine::{BnVirtualMachine, IVirtualMachine}, IVirtualMachineCallback::IVirtualMachineCallback, IVirtualizationService::IVirtualizationService, + Partition::Partition, PartitionType::PartitionType, VirtualMachineAppConfig::DebugLevel::DebugLevel, VirtualMachineAppConfig::VirtualMachineAppConfig, @@ -169,6 +171,8 @@ impl IVirtualizationService for VirtualizationService { } } + 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 @@ impl IVirtualizationService for VirtualizationService { }; 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 @@ fn check_manage_access() -> binder::Result<()> { 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 0e1e9745..69ae0763 100644 --- a/virtualizationservice/src/main.rs +++ b/virtualizationservice/src/main.rs @@ -18,6 +18,7 @@ mod aidl; 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 00000000..e450dee2 --- /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 { + Ok(Self::CString( + CString::new(con) + .with_context(|| format!("Failed to create SeContext with \"{}\"", con))?, + )) + } +} + +pub fn getfilecon(file: &File) -> Result { + 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"), + } +}