Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mkdir: added acl permissions inheritance for subdirectories #6676

Merged
merged 10 commits into from
Sep 11, 2024
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -519,6 +519,10 @@ rlimit = "0.10.1"
rand_pcg = "0.3.1"
xattr = { workspace = true }

# Used for acl entry testing test_mkdir
[target.'cfg(not(windows))'.dev-dependencies]
exacl = "0.12.0"

# Specifically used in test_uptime::test_uptime_with_file_containing_valid_boot_time_utmpx_record
# to deserialize a utmpx struct into a binary file
[target.'cfg(all(target_family= "unix",not(target_os = "macos")))'.dev-dependencies]
Expand Down
2 changes: 2 additions & 0 deletions src/uu/mkdir/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ path = "src/mkdir.rs"
[dependencies]
clap = { workspace = true }
uucore = { workspace = true, features = ["fs", "mode"] }
exacl = "0.12.0"
AnirbanHalder654322 marked this conversation as resolved.
Show resolved Hide resolved


[[bin]]
name = "mkdir"
Expand Down
48 changes: 37 additions & 11 deletions src/uu/mkdir/src/mkdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
use clap::builder::ValueParser;
use clap::parser::ValuesRef;
use clap::{crate_version, Arg, ArgAction, ArgMatches, Command};
#[cfg(not(windows))]
use exacl::{getfacl, AclOption};
use std::ffi::OsString;
use std::path::{Path, PathBuf};
#[cfg(not(windows))]
Expand Down Expand Up @@ -165,17 +167,14 @@ pub fn mkdir(path: &Path, recursive: bool, mode: u32, verbose: bool) -> UResult<
let path_buf = dir_strip_dot_for_creation(path);
let path = path_buf.as_path();

create_dir(path, recursive, verbose, false)?;
chmod(path, mode)
create_dir(path, recursive, verbose, false, mode)
}

#[cfg(any(unix, target_os = "redox"))]
fn chmod(path: &Path, mode: u32) -> UResult<()> {
use std::fs::{set_permissions, Permissions};
use std::os::unix::fs::PermissionsExt;

let mode = Permissions::from_mode(mode);

set_permissions(path, mode)
.map_err_context(|| format!("cannot set permissions {}", path.quote()))
}
Expand All @@ -188,8 +187,15 @@ fn chmod(_path: &Path, _mode: u32) -> UResult<()> {

// `is_parent` argument is not used on windows
#[allow(unused_variables)]
fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> UResult<()> {
if path.exists() && !recursive {
fn create_dir(
path: &Path,
recursive: bool,
verbose: bool,
is_parent: bool,
mode: u32,
) -> UResult<()> {
let path_exists = path.exists();
if path_exists && !recursive {
return Err(USimpleError::new(
1,
format!("{}: File exists", path.display()),
Expand All @@ -201,12 +207,13 @@ fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> U

if recursive {
match path.parent() {
Some(p) => create_dir(p, recursive, verbose, true)?,
Some(p) => create_dir(p, recursive, verbose, true, mode)?,
None => {
USimpleError::new(1, "failed to create whole tree");
}
}
}

match std::fs::create_dir(path) {
Ok(()) => {
if verbose {
Expand All @@ -217,14 +224,33 @@ fn create_dir(path: &Path, recursive: bool, verbose: bool, is_parent: bool) -> U
);
}
#[cfg(not(windows))]
if is_parent {
// directories created by -p have permission bits set to '=rwx,u+wx',
// which is umask modified by 'u+wx'
chmod(path, (!mode::get_umask() & 0o0777) | 0o0300)?;
if !path_exists {
let acl_perm_bits = get_acl_perm_bits(path);
if is_parent {
chmod(path, (!mode::get_umask() & 0o777) | (0o300 | acl_perm_bits))?;
} else {
chmod(path, mode | acl_perm_bits)?;
}
}
Ok(())
}

Err(_) if path.is_dir() => Ok(()),
Err(e) => Err(e.into()),
}
}

/// Only default acl entries get inherited by objects under the path i.e. if child directories
/// will have their permissions modified.
#[cfg(not(windows))]
fn get_acl_perm_bits(path: &Path) -> u32 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have this function:

pub fn retrieve_xattrs<P: AsRef<Path>>(source: P) -> std::io::Result<HashMap<OsString, Vec<u8>>> {

maybe use it ?

Copy link
Contributor Author

@AnirbanHalder654322 AnirbanHalder654322 Sep 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it but i couldn't figure out how to correctly and reliably interpret the Vec<u8> value , the key is system.posix_acl_default for a file when i do setfacl -d -m group::rmx test_file so that's fine. I just need to reliably parse the byte sequences to extract the permission bits.

I will try to look through the fsxattr source code in https://github.com/torvalds/linux/blob/master/fs/xattr.c to check how they interpret things. I would definitely appreciate some help or some other resources you can point me to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, makes sense
could you please move the function into fsxattr ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, makes sense could you please move the function into fsxattr ?

Okay, will do that.

if let Ok(entries) = getfacl(path, AclOption::DEFAULT_ACL) {
let mut perm: u32 = 0;
for entry in entries {
perm = (perm << 3) | entry.perms.bits();
}
perm
} else {
0
}
}
40 changes: 40 additions & 0 deletions tests/by-util/test_mkdir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,46 @@ fn test_recursive_reporting() {
.stdout_contains("created directory 'test_dir/../test_dir_a/../test_dir_b'");
}

#[test]
// Windows don't have acl entries
#[cfg(not(windows))]
fn test_mkdir_acl() {
use std::str::FromStr;

use exacl::{getfacl, setfacl, AclEntry, Flag, Perm};

let (at, mut ucmd) = at_and_ucmd!();

at.mkdir("a");
let mut acl_entries = getfacl(at.plus("a"), None).unwrap();

// Doing a setfacl on the command adds all required default flagged acl entries but
// the exacl library doesn't so we are manually pushing
acl_entries.push(AclEntry::allow_group(
"",
Perm::EXECUTE | Perm::READ | Perm::WRITE,
Flag::DEFAULT,
));
acl_entries.push(AclEntry::allow_user(
"",
Perm::EXECUTE | Perm::READ | Perm::WRITE,
Flag::DEFAULT,
));
acl_entries.push(AclEntry::allow_other(
Perm::from_str("rx").unwrap(),
Flag::DEFAULT,
));

println!("{:?}", acl_entries);
setfacl(&[at.plus_as_string("a")], &acl_entries, None).unwrap();
ucmd.arg("-p").arg("a/b").umask(0x077).succeeds();

let perms = at.metadata("a/b").permissions().mode();

// 0x770 would be user:rwx,group:rwx permissions
assert_eq!(perms, 16893);
}

#[test]
fn test_mkdir_trailing_dot() {
new_ucmd!().arg("-p").arg("-v").arg("test_dir").succeeds();
Expand Down
Loading