uutils/coreutils
0
0
Fork 0
mirror of https://github.com/uutils/coreutils.git synced 2026-06-10 15:48:22 -07:00
Code Issues Packages Projects Releases Wiki Activity

chcon: anchor recursive relabel resolution to traversal dirfd (#12566)

Browse Source 
---------

Co-authored-by: can1357 <me@can.ac>
This commit is contained in:
Sylvestre Ledru
2026-06-05 22:58:32 +02:00
committed by GitHub
parent 58040feb5c
commit 7cfc74e47f
6 changed files with 232 additions and 110 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/workflows/CICD.yml
+3 -3
View File
@@ -900,10 +900,10 @@ jobs:
persist-credentials: false
- uses: dtolnay/rust-toolchain@stable
- uses: Swatinem/rust-cache@v2
- name: Install strace
run: sudo apt-get update && sudo apt-get install -y strace
- name: Install strace and libselinux headers (needed to build chcon)
run: sudo apt-get update && sudo apt-get install -y strace libselinux1-dev
- name: Build utilities with safe traversal
run: cargo build --profile=release-small -p uu_rm -p uu_chmod -p uu_chown -p uu_chgrp -p uu_mv -p uu_du -p uu_split
run: cargo build --profile=release-small -p uu_rm -p uu_chmod -p uu_chown -p uu_chgrp -p uu_mv -p uu_du -p uu_split -p uu_chcon
- name: Run safe traversal verification
run: ./util/check-safe-traversal.sh
Cargo.lock
Generated
+1
View File
@@ -3273,6 +3273,7 @@ dependencies = [
"fluent",
"fts-sys",
"libc",
"rustix",
"selinux",
"thiserror 2.0.18",
"uucore",
src/uu/chcon/Cargo.toml
+1
View File
@@ -27,6 +27,7 @@ selinux = { workspace = true }
thiserror = { workspace = true }
libc = { workspace = true }
fts-sys = { workspace = true }
rustix = { workspace = true, features = ["fs"] }
fluent = { workspace = true }
[[bin]]
src/uu/chcon/src/chcon.rs
+168 -105
View File
@@ -3,7 +3,7 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
// spell-checker:ignore (vars) RFILE
// spell-checker:ignore (vars) RFILE RDONLY CLOEXEC fgetfilecon fsetfilecon
#![cfg(any(target_os = "linux", target_os = "android"))]
#![allow(clippy::upper_case_acronyms)]
@@ -14,10 +14,12 @@ use uucore::translate;
use uucore::{display::Quotable, format_usage, show_error, show_warning};
use clap::{Arg, ArgAction, ArgMatches, Command};
use rustix::fs::{Mode, OFlags, openat};
use selinux::{OpaqueSecurityContext, SecurityContext};
use std::borrow::Cow;
use std::ffi::{CStr, CString, OsStr, OsString};
use std::os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd};
use std::os::raw::c_int;
use std::path::{Path, PathBuf};
use std::{fs, io};
@@ -499,111 +501,117 @@ fn process_file(
fts: &mut fts::FTS,
root_dev_ino: Option<DeviceAndINode>,
) -> Result<()> {
let mut entry = fts.last_entry_ref().unwrap();
let (file_full_name, fts_access_path, mut result) = {
let mut entry = fts
.last_entry_ref()
.expect("last_entry_ref is present after successful read_next_entry");
let file_full_name = entry.path().map(PathBuf::from).ok_or_else(|| {
Error::from_io(
translate!("chcon-op-file-name-validation"),
io::ErrorKind::InvalidInput.into(),
)
})?;
let file_full_name = entry.path().map(PathBuf::from).ok_or_else(|| {
Error::from_io(
translate!("chcon-op-file-name-validation"),
io::ErrorKind::InvalidInput.into(),
)
})?;
let fts_access_path = entry.access_path().ok_or_else(|| {
let err = io::ErrorKind::InvalidInput.into();
Error::from_io1(
translate!("chcon-op-file-name-validation"),
&file_full_name,
err,
)
})?;
let fts_access_path = entry.access_path().map(PathBuf::from).ok_or_else(|| {
let err = io::ErrorKind::InvalidInput.into();
Error::from_io1(
translate!("chcon-op-file-name-validation"),
&file_full_name,
err,
)
})?;
let err = |s, k: io::ErrorKind| Error::from_io1(s, &file_full_name, k.into());
let err = |s, k: io::ErrorKind| Error::from_io1(s, &file_full_name, k.into());
let fts_err = |s| {
let r = io::Error::from_raw_os_error(entry.errno());
Err(Error::from_io1(s, &file_full_name, r))
};
let fts_err = |s| {
let r = io::Error::from_raw_os_error(entry.errno());
Err(Error::from_io1(s, &file_full_name, r))
};
// SAFETY: If `entry.fts_statp` is not null, then it is assumed to be valid.
let file_dev_ino: DeviceAndINode = if let Some(st) = entry.stat() {
st.try_into()?
} else {
return Err(err(
translate!("chcon-op-getting-meta-data"),
io::ErrorKind::InvalidInput,
));
};
// SAFETY: If `entry.fts_statp` is not null, then it is assumed to be valid.
let file_dev_ino: DeviceAndINode = if let Some(st) = entry.stat() {
st.try_into()?
} else {
return Err(err(
translate!("chcon-op-getting-meta-data"),
io::ErrorKind::InvalidInput,
));
};
let mut result = Ok(());
let mut result = Ok(());
match entry.flags() {
fts_sys::FTS_D if options.recursive_mode.is_recursive() => {
if root_dev_ino_check(root_dev_ino, file_dev_ino) {
// This happens e.g., with "chcon -R --preserve-root ... /"
// and with "chcon -RH --preserve-root ... symlink-to-root".
root_dev_ino_warn(&file_full_name);
match entry.flags() {
fts_sys::FTS_D if options.recursive_mode.is_recursive() => {
if root_dev_ino_check(root_dev_ino, file_dev_ino) {
// This happens e.g., with "chcon -R --preserve-root ... /"
// and with "chcon -RH --preserve-root ... symlink-to-root".
root_dev_ino_warn(&file_full_name);
// Tell fts not to traverse into this hierarchy.
let _ignored = fts.set(fts_sys::FTS_SKIP);
// Tell fts not to traverse into this hierarchy.
let _ignored = fts.set(fts_sys::FTS_SKIP);
// Ensure that we do not process "/" on the second visit.
let _ignored = fts.read_next_entry();
// Ensure that we do not process "/" on the second visit.
let _ignored = fts.read_next_entry();
return Err(err(
translate!("chcon-op-modifying-root-path"),
io::ErrorKind::PermissionDenied,
));
}
return Err(err(
translate!("chcon-op-modifying-root-path"),
io::ErrorKind::PermissionDenied,
));
}
return Ok(());
}
fts_sys::FTS_DP if !options.recursive_mode.is_recursive() => {
return Ok(());
}
fts_sys::FTS_NS => {
// For a top-level file or directory, this FTS_NS (stat failed) indicator is determined
// at the time of the initial fts_open call. With programs like chmod, chown, and chgrp,
// that modify permissions, it is possible that the file in question is accessible when
// control reaches this point. So, if this is the first time we've seen the FTS_NS for
// this file, tell fts_read to stat it "again".
if entry.level() == 0 && entry.number() == 0 {
entry.set_number(1);
let _ignored = fts.set(fts_sys::FTS_AGAIN);
return Ok(());
}
result = fts_err(translate!("chcon-op-accessing"));
fts_sys::FTS_DP if !options.recursive_mode.is_recursive() => {
return Ok(());
}
fts_sys::FTS_NS => {
// For a top-level file or directory, this FTS_NS (stat failed) indicator is determined
// at the time of the initial fts_open call. With programs like chmod, chown, and chgrp,
// that modify permissions, it is possible that the file in question is accessible when
// control reaches this point. So, if this is the first time we've seen the FTS_NS for
// this file, tell fts_read to stat it "again".
if entry.level() == 0 && entry.number() == 0 {
entry.set_number(1);
let _ignored = fts.set(fts_sys::FTS_AGAIN);
return Ok(());
}
result = fts_err(translate!("chcon-op-accessing"));
}
fts_sys::FTS_ERR => result = fts_err(translate!("chcon-op-accessing")),
fts_sys::FTS_DNR => result = fts_err(translate!("chcon-op-reading-directory")),
fts_sys::FTS_DC
if cycle_warning_required(options.recursive_mode.fts_open_options(), &entry) =>
{
emit_cycle_warning(&file_full_name);
return Err(err(
translate!("chcon-op-reading-cyclic-directory"),
io::ErrorKind::InvalidData,
));
}
_ => {}
}
fts_sys::FTS_ERR => result = fts_err(translate!("chcon-op-accessing")),
fts_sys::FTS_DNR => result = fts_err(translate!("chcon-op-reading-directory")),
fts_sys::FTS_DC
if cycle_warning_required(options.recursive_mode.fts_open_options(), &entry) =>
if entry.flags() == fts_sys::FTS_DP
&& result.is_ok()
&& root_dev_ino_check(root_dev_ino, file_dev_ino)
{
emit_cycle_warning(&file_full_name);
return Err(err(
translate!("chcon-op-reading-cyclic-directory"),
io::ErrorKind::InvalidData,
root_dev_ino_warn(&file_full_name);
result = Err(err(
translate!("chcon-op-modifying-root-path"),
io::ErrorKind::PermissionDenied,
));
}
_ => {}
}
if entry.flags() == fts_sys::FTS_DP
&& result.is_ok()
&& root_dev_ino_check(root_dev_ino, file_dev_ino)
{
root_dev_ino_warn(&file_full_name);
result = Err(err(
translate!("chcon-op-modifying-root-path"),
io::ErrorKind::PermissionDenied,
));
}
(file_full_name, fts_access_path, result)
};
if result.is_ok() {
if options.verbose {
@@ -613,7 +621,17 @@ fn process_file(
);
}
result = change_file_context(options, context, fts_access_path);
let traversal_dir_fd = fts
.current_dir_fd()
.map_err(|r| Error::from_io1(translate!("chcon-op-accessing"), &file_full_name, r))?;
result = change_file_context(
options,
context,
traversal_dir_fd.as_fd(),
&fts_access_path,
&file_full_name,
);
}
if !options.recursive_mode.is_recursive() {
@@ -622,13 +640,51 @@ fn process_file(
result
}
fn open_target_fd(
traversal_dir_fd: BorrowedFd<'_>,
target_path: &Path,
affect_symlink_referent: bool,
display_path: &Path,
) -> Result<OwnedFd> {
// Anchor the open to the traversal directory fd so a concurrent rename or
// symlink swap cannot redirect the relabel off-tree. O_PATH hands back the
// entry itself without opening it for I/O: it never blocks on a FIFO and
// does not require read permission, yet the SELinux get/set still work
// through /proc/self/fd (see change_file_context). O_NOFOLLOW keeps us on
// the symlink itself unless we were asked to act on its referent.
let mut flags = OFlags::PATH | OFlags::CLOEXEC;
if !affect_symlink_referent {
flags |= OFlags::NOFOLLOW;
}
openat(traversal_dir_fd, target_path, flags, Mode::empty())
.map_err(io::Error::from)
.map_err(|err| Error::from_io1(translate!("chcon-op-accessing"), display_path, err))
}
fn change_file_context(
options: &Options,
context: &SELinuxSecurityContext,
path: &Path,
traversal_dir_fd: BorrowedFd<'_>,
target_path: &Path,
display_path: &Path,
) -> Result<()> {
type SetValueProc = fn(&OpaqueSecurityContext, &CStr) -> selinux::errors::Result<()>;
let target_fd = open_target_fd(
traversal_dir_fd,
target_path,
options.affect_symlink_referent,
display_path,
)?;
// The fd is O_PATH, so fgetfilecon/fsetfilecon would fail with EBADF. Reach
// the same inode through its /proc/self/fd entry and always dereference it:
// the open above already encoded the follow/no-follow choice, so this magic
// symlink resolves to exactly the inode we anchored. `target_fd` must stay
// alive for as long as this path is used.
let target = PathBuf::from(format!("/proc/self/fd/{}", target_fd.as_raw_fd()));
match &options.mode {
CommandLineMode::Custom {
user,
@@ -641,21 +697,20 @@ fn change_file_context(
// components, there isn't really an obvious default. Thus, we just give up.
let op = translate!("chcon-op-applying-partial-context");
let err = io::ErrorKind::InvalidInput.into();
Err(Error::from_io1(op, path, err))
Err(Error::from_io1(op, display_path, err))
};
let file_context =
match SecurityContext::of_path(path, options.affect_symlink_referent, false) {
Ok(Some(context)) => context,
let file_context = match SecurityContext::of_path(&target, true, false) {
Ok(Some(context)) => context,
Ok(None) => return err0(),
Err(r) => {
return Err(Error::from_selinux(
translate!("chcon-op-getting-security-context"),
r,
));
}
};
Ok(None) => return err0(),
Err(r) => {
return Err(Error::from_selinux(
translate!("chcon-op-getting-security-context"),
r,
));
}
};
let c_file_context = match file_context.to_c_string() {
Ok(Some(context)) => context,
@@ -672,7 +727,11 @@ fn change_file_context(
let se_context =
OpaqueSecurityContext::from_c_str(c_file_context.as_ref()).map_err(|_r| {
let err = io::ErrorKind::InvalidInput.into();
Error::from_io1(translate!("chcon-op-creating-security-context"), path, err)
Error::from_io1(
translate!("chcon-op-creating-security-context"),
display_path,
err,
)
})?;
let list: &[(&Option<OsString>, SetValueProc)] = &[
@@ -686,7 +745,11 @@ fn change_file_context(
if let Some(new_value) = new_value {
let c_new_value = os_str_to_c_string(new_value).map_err(|_r| {
let err = io::ErrorKind::InvalidInput.into();
Error::from_io1(translate!("chcon-op-creating-security-context"), path, err)
Error::from_io1(
translate!("chcon-op-creating-security-context"),
display_path,
err,
)
})?;
set_value_proc(&se_context, &c_new_value).map_err(|r| {
@@ -703,7 +766,7 @@ fn change_file_context(
Ok(()) // Nothing to change.
} else {
SecurityContext::from_c_str(&context_string, false)
.set_for_path(path, options.affect_symlink_referent, false)
.set_for_path(&target, true, false)
.map_err(|r| {
Error::from_selinux(translate!("chcon-op-setting-security-context"), r)
})
@@ -713,7 +776,7 @@ fn change_file_context(
CommandLineMode::ReferenceBased { .. } | CommandLineMode::ContextBased { .. } => {
if let Some(c_context) = context.to_c_string()? {
SecurityContext::from_c_str(c_context.as_ref(), false)
.set_for_path(path, options.affect_symlink_referent, false)
.set_for_path(&target, true, false)
.map_err(|r| {
Error::from_selinux(translate!("chcon-op-setting-security-context"), r)
})
@@ -721,7 +784,7 @@ fn change_file_context(
let err = io::ErrorKind::InvalidInput.into();
Err(Error::from_io1(
translate!("chcon-op-setting-security-context"),
path,
display_path,
err,
))
}
src/uu/chcon/src/fts.rs
+14 -1
View File
@@ -3,14 +3,19 @@
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
// spell-checker:ignore (vars) RDONLY CLOEXEC
#![cfg(any(target_os = "linux", target_os = "android"))]
use std::ffi::{CStr, CString, OsStr};
use std::marker::PhantomData;
use std::os::fd::OwnedFd;
use std::os::raw::{c_int, c_long, c_short};
use std::path::Path;
use std::{io, iter, ptr, slice};
use rustix::fs::{Mode, OFlags, open};
use crate::errors::{Error, Result};
use crate::os_str_to_c_string;
@@ -98,8 +103,16 @@ impl FTS {
Ok(())
}
}
}
pub(crate) fn current_dir_fd(&self) -> io::Result<OwnedFd> {
open(
Path::new("."),
OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC,
Mode::empty(),
)
.map_err(io::Error::from)
}
}
impl Drop for FTS {
fn drop(&mut self) {
// SAFETY: We assume calling fts_close() is safe with a non-null `fts`
util/check-safe-traversal.sh
+45 -1
View File
@@ -191,7 +191,7 @@ if [ "$USE_MULTICALL" -eq 1 ]; then
AVAILABLE_UTILS=$($COREUTILS_BIN --list)
else
AVAILABLE_UTILS=""
for util in rm chmod chown chgrp du mv cp split; do
for util in rm chmod chown chgrp du mv cp chcon split; do
if [ -f "$PROJECT_ROOT/target/${PROFILE}/$util" ]; then
AVAILABLE_UTILS="$AVAILABLE_UTILS $util"
fi
@@ -241,6 +241,50 @@ if echo "$AVAILABLE_UTILS" | grep -q "chgrp"; then
assert_descent_nofollow "chgrp" strace_chgrp_recursive_chgrp.log
fi
# chcon recursive relabel must resolve each target relative to the traversal
# directory fd with O_NOFOLLOW and operate on the resulting fd (fd-based xattr,
# or /proc/self/fd), never re-resolving the entry by path. Otherwise a
# rename/symlink race could redirect a privileged recursive relabel off-tree
# (issue #11402). This holds even without SELinux: the fd-anchored open happens
# before the SELinux get/set, so the syscalls are observable regardless.
if echo "$AVAILABLE_UTILS" | grep -q "chcon"; then
if [ "$USE_MULTICALL" -eq 1 ]; then
chcon_cmd="$COREUTILS_BIN chcon"
else
chcon_cmd="$PROJECT_ROOT/target/${PROFILE}/chcon"
fi
mkdir -p chcon_tree/sub
echo a > chcon_tree/file
echo b > chcon_tree/sub/nested
strace -f -e trace=openat,getxattr,lgetxattr,fgetxattr,setxattr,lsetxattr,fsetxattr \
-o strace_chcon_recursive.log \
$chcon_cmd -R -t etc_t chcon_tree 2>/dev/null || true
# Each relabel target is opened relative to a numeric dirfd with O_NOFOLLOW.
if ! grep -qE 'openat\([0-9]+, "(file|sub|nested)", [^)]*O_NOFOLLOW' strace_chcon_recursive.log; then
cat strace_chcon_recursive.log
fail_immediately "chcon -R must open relabel targets relative to the traversal dirfd with O_NOFOLLOW (issue #11402)"
fi
# SELinux xattr ops must be fd-anchored: fgetxattr/fsetxattr on a numeric fd,
# or *xattr on /proc/self/fd. A path-based xattr on the traversal entry is the
# TOCTOU pattern the fix removes.
path_based_xattr=$(grep -E '\bl?(get|set)xattr\("' strace_chcon_recursive.log | grep -v '"/proc/self/fd/' || true)
if [ -n "$path_based_xattr" ]; then
echo "$path_based_xattr"
fail_immediately "chcon -R is using path-based SELinux xattr (TOCTOU; expected fd-anchored access, issue #11402)"
fi
# The relabel must actually reach SELinux through the anchored fd. Without
# this positive check, a chcon that aborts before the get/set (e.g. EBADF
# from f*filecon on an O_PATH fd) would still pass the assertions above.
if ! grep -qE '(get|set)xattr\("/proc/self/fd/|f(get|set)xattr\([0-9]+,' strace_chcon_recursive.log; then
cat strace_chcon_recursive.log
fail_immediately "chcon -R never reached an fd-anchored SELinux xattr op (relabel aborted before get/set?, issue #11402)"
fi
echo "✓ chcon -R anchors relabel to the traversal dirfd (O_NOFOLLOW, fd-based access)"
rm -rf chcon_tree
fi
# Test du - should use openat, newfstatat
if echo "$AVAILABLE_UTILS" | grep -q "du"; then
cp -r test_dir test_du