mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2025-02-05 18:20:34 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Don't rely on being able to look up users/groups

Browse source 
Some devices need to have their current uid mapped in their sandbox
for bind mounts to work as expected. Currently crosvm looks up the
uid/gid for "crosvm" and maps that.

This logic is dubious anyway, since crosvm should be using whatever
user/group it was started under rather then trying to switch (which is
a priviliged operation), but putting concierge in a user namespace
breaks it entierly because the crosvm user gets remapped to a
different numeric value.

Replace the current approach with mapping the current euid/egid,
whatever it may be.

BUG=chromium:1240116
TEST=Manually tested

Change-Id: I0e9b95ed04834da1adedb72bee52ac4359f06041
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3105907
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Allen Webb <allenwebb@google.com>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
This commit is contained in:
Fergus Dall 2021-08-19 12:54:26 +10:00 committed by Commit Bot
parent ddcf7bd2ab
commit 51200519a2
3 changed files with 13 additions and 156 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
src/linux.rs
View file

@ -7,7 +7,6 @@ use std::collections::BTreeMap;
use std::convert::TryFrom;
#[cfg(feature = "gpu")]
use std::env;
use std::ffi::CStr;
use std::fs::{File, OpenOptions};
use std::io::{self, stdin};
use std::iter;
@ -353,7 +352,7 @@ fn create_tpm_device(cfg: &Config) -> DeviceResult {
"size=20480",
)?;
let crosvm_ids = add_crosvm_user_to_jail(jail, "tpm")?;
let crosvm_ids = add_current_user_to_jail(jail)?;
let pid = process::id();
let tpm_pid_dir = format!("/run/vm/tpm.{}", pid);
@ -765,7 +764,7 @@ fn create_gpu_device(
jail.mount_bind(dir, dir, true)?;
}
add_crosvm_user_to_jail(&mut jail, "gpu")?;
add_current_user_to_jail(&mut jail)?;
// pvr driver requires read access to /proc/self/task/*/comm.
let proc_path = Path::new("/proc");
@ -834,7 +833,7 @@ fn create_wayland_device(
for dir in &wayland_socket_dirs {
jail.mount_bind(dir, dir, true)?;
}
add_crosvm_user_to_jail(&mut jail, "Wayland")?;
add_current_user_to_jail(&mut jail)?;
Some(jail)
}
@ -856,12 +855,8 @@ fn create_video_device(
let jail = match simple_jail(cfg, "video_device")? {
Some(mut jail) => {
match typ {
devices::virtio::VideoDeviceType::Decoder => {
add_crosvm_user_to_jail(&mut jail, "video-decoder")?
}
devices::virtio::VideoDeviceType::Encoder => {
add_crosvm_user_to_jail(&mut jail, "video-encoder")?
}
devices::virtio::VideoDeviceType::Decoder => add_current_user_to_jail(&mut jail)?,
devices::virtio::VideoDeviceType::Encoder => add_current_user_to_jail(&mut jail)?,
};
// Create a tmpfs in the device's root directory so that we can bind mount files.
@ -1139,7 +1134,7 @@ fn create_console_device(cfg: &Config, param: &SerialParameters) -> DeviceResult
(libc::MS_NODEV | libc::MS_NOEXEC | libc::MS_NOSUID) as usize,
"size=67108864",
)?;
add_crosvm_user_to_jail(&mut jail, "serial")?;
add_current_user_to_jail(&mut jail)?;
let res = param.add_bind_mounts(&mut jail);
if res.is_err() {
error!("failed to add bind mounts for console device");
@ -1629,32 +1624,18 @@ struct Ids {
// Set the uid/gid for the jailed process and give a basic id map. This is
// required for bind mounts to work.
fn add_crosvm_user_to_jail(jail: &mut Minijail, feature: &str) -> Result<Ids> {
let crosvm_user_group = CStr::from_bytes_with_nul(b"crosvm\0").unwrap();
fn add_current_user_to_jail(jail: &mut Minijail) -> Result<Ids> {
let crosvm_uid = geteuid();
let crosvm_gid = getegid();
let crosvm_uid = match get_user_id(crosvm_user_group) {
Ok(u) => u,
Err(e) => {
warn!("falling back to current user id for {}: {}", feature, e);
geteuid()
}
};
let crosvm_gid = match get_group_id(crosvm_user_group) {
Ok(u) => u,
Err(e) => {
warn!("falling back to current group id for {}: {}", feature, e);
getegid()
}
};
jail.change_uid(crosvm_uid);
jail.change_gid(crosvm_gid);
jail.uidmap(&format!("{0} {0} 1", crosvm_uid))
.map_err(Error::SettingUidMap)?;
jail.gidmap(&format!("{0} {0} 1", crosvm_gid))
.map_err(Error::SettingGidMap)?;
jail.change_uid(crosvm_uid);
jail.change_gid(crosvm_gid);
Ok(Ids {
uid: crosvm_uid,
gid: crosvm_gid,
@ -2378,7 +2359,7 @@ where
// Setup a bind mount to the system D-Bus socket if the powerd monitor is used.
#[cfg(feature = "power-monitor-powerd")]
{
add_crosvm_user_to_jail(&mut jail, "battery")?;
add_current_user_to_jail(&mut jail)?;
// Create a tmpfs in the device's root directory so that we can bind mount files.
jail.mount_with_data(

2
sys_util/src/lib.rs
View file

@ -31,7 +31,6 @@ pub mod file_traits;
mod fork;
mod mmap;
pub mod net;
mod passwd;
mod poll;
mod priority;
pub mod rand;
@ -62,7 +61,6 @@ pub use crate::file_flags::*;
pub use crate::fork::*;
pub use crate::ioctl::*;
pub use crate::mmap::*;
pub use crate::passwd::*;
pub use crate::poll::*;
pub use crate::priority::*;
pub use crate::raw_fd::*;

122
sys_util/src/passwd.rs
View file

@ -1,122 +0,0 @@
// Copyright 2017 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//! Wrappers for passwd and group file access.
use std::ffi::CStr;
use std::mem;
use std::ptr;
use libc::{self, getgrnam_r, getpwnam_r, gid_t, uid_t};
use crate::{errno_result, Result};
/// Safe wrapper for getting a uid from a user name with `getpwnam_r(3)`.
#[inline(always)]
pub fn get_user_id(user_name: &CStr) -> Result<uid_t> {
// libc::passwd is a C struct and can be safely initialized with zeroed memory.
let mut passwd: libc::passwd = unsafe { mem::zeroed() };
let mut passwd_result: *mut libc::passwd = ptr::null_mut();
let mut buf = [0; 256];
// For thread-safety, use the reentrant version of this function. This allows us to give it a
// buffer on the stack (instead of a global buffer). Unlike most libc functions, the return
// value of this doesn't really need to be checked, since the extra result pointer that is
// passed in indicates whether or not the function succeeded.
//
// This call is safe as long as it behaves as described in the man page. We pass in valid
// pointers to stack-allocated buffers, and the length check for the scratch buffer is correct.
unsafe {
handle_eintr_rc!(getpwnam_r(
user_name.as_ptr(),
&mut passwd,
buf.as_mut_ptr(),
buf.len(),
&mut passwd_result
))
};
if passwd_result.is_null() {
errno_result()
} else {
Ok(passwd.pw_uid)
}
}
/// Safe wrapper for getting a gid from a group name with `getgrnam_r(3)`.
#[inline(always)]
pub fn get_group_id(group_name: &CStr) -> Result<gid_t> {
// libc::group is a C struct and can be safely initialized with zeroed memory.
let mut group: libc::group = unsafe { mem::zeroed() };
let mut group_result: *mut libc::group = ptr::null_mut();
let mut buf = [0; 256];
// For thread-safety, use the reentrant version of this function. This allows us to give it a
// buffer on the stack (instead of a global buffer). Unlike most libc functions, the return
// value of this doesn't really need to be checked, since the extra result pointer that is
// passed in indicates whether or not the function succeeded.
//
// This call is safe as long as it behaves as described in the man page. We pass in valid
// pointers to stack-allocated buffers, and the length check for the scratch buffer is correct.
unsafe {
handle_eintr_rc!(getgrnam_r(
group_name.as_ptr(),
&mut group,
buf.as_mut_ptr(),
buf.len(),
&mut group_result
))
};
if group_result.is_null() {
errno_result()
} else {
Ok(group.gr_gid)
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn get_good_uid() {
let root_name = CStr::from_bytes_with_nul(b"root\0").unwrap();
// root's uid should always exist, and should be 0.
let root_uid = get_user_id(root_name).unwrap();
assert_eq!(root_uid, 0);
}
#[test]
fn get_bad_uid() {
let bad_name = CStr::from_bytes_with_nul(b"this better not be a user\0").unwrap();
// This user should give us an error. As a cruel joke, the getpwnam(3) man page allows
// ENOENT, ESRCH, EBADF, EPERM, or even 0 to be set in errno if a user isn't found. So
// instead of checking which error we got, just see that we did get one.
let bad_uid_result = get_user_id(bad_name);
assert!(bad_uid_result.is_err());
}
#[test]
fn get_good_gid() {
let root_name = CStr::from_bytes_with_nul(b"root\0").unwrap();
// root's gid should always exist, and should be 0.
let root_gid = get_group_id(root_name).unwrap();
assert_eq!(root_gid, 0);
}
#[test]
fn get_bad_gid() {
let bad_name = CStr::from_bytes_with_nul(b"this better not be a group\0").unwrap();
// This group should give us an error. As a cruel joke, the getgrnam(3) man page allows
// ENOENT, ESRCH, EBADF, EPERM, or even 0 to be set in errno if a group isn't found. So
// instead of checking which error we got, just see that we did get one.
let bad_gid_result = get_group_id(bad_name);
assert!(bad_gid_result.is_err());
}
}