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
This commit is contained in:
Jiyong Park 2021-11-24 21:56:49 +09:00
parent f67031972d
commit 029977d760
4 changed files with 141 additions and 1 deletions

View File

@ -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",
],
}

View File

@ -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 {

View File

@ -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;

View File

@ -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"),
}
}