Skip to content

Commit

Permalink
Warn if target user home directory is not accessible (#139)
Browse files Browse the repository at this point in the history
Adds additional log output when target user's home directory seems incorrectly configured. This would make issues like #131 less puzzling.

* Also added snapshot tests for log output using new `testing_logger` dependency.
* Also now using an unprivileged user to build in Docker, as tests assume `/root/` directory is not accessible.
  • Loading branch information
intgr authored Jun 25, 2023
1 parent b7c36a2 commit 0e247d1
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 4 deletions.
10 changes: 10 additions & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,4 @@ default = []
[dev-dependencies]
clap_complete = "~4.3.1"
snapbox = "0.4.11"
testing_logger = "0.1.1"
43 changes: 42 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,16 @@ extern crate simple_error;
use crate::cli::{parse_args, Method};
use crate::errors::{print_error, AnyErr, ErrorWithHint};
use crate::util::{exec_command, have_command, run_command, sd_booted};
use log::{debug, info, warn};
use log::{debug, info, log, warn, Level};
use nix::libc::uid_t;
use nix::unistd::{Uid, User};
use posix_acl::{PosixACL, Qualifier, ACL_EXECUTE, ACL_READ, ACL_RWX};
use simple_error::SimpleError;
use std::env::VarError;
use std::fs::DirBuilder;
use std::io::ErrorKind::PermissionDenied;
use std::os::unix::fs::DirBuilderExt;
use std::os::unix::fs::MetadataExt;
use std::os::unix::fs::PermissionsExt;
use std::path::{Path, PathBuf};
use std::process::exit;
Expand All @@ -24,11 +26,13 @@ mod logging;
mod tests;
mod util;

#[derive(Clone)]
struct EgoContext {
runtime_dir: PathBuf,
target_user: String,
target_uid: uid_t,
target_user_shell: PathBuf,
target_user_homedir: PathBuf,
}

fn main_inner() -> Result<(), AnyErr> {
Expand All @@ -43,6 +47,8 @@ fn main_inner() -> Result<(), AnyErr> {
ctx.target_user, ctx.target_uid
);

check_user_homedir(&ctx);

let ret = prepare_runtime_dir(&ctx);
if let Err(msg) = ret {
bail!("Error preparing runtime dir: {msg}");
Expand Down Expand Up @@ -144,6 +150,7 @@ fn create_context(username: String) -> Result<EgoContext, AnyErr> {
target_user: user.name,
target_uid: user.uid.as_raw(),
target_user_shell: user.shell,
target_user_homedir: user.dir,
})
}

Expand All @@ -154,6 +161,40 @@ fn add_file_acl(path: &Path, uid: u32, flags: u32) -> Result<(), AnyErr> {
Ok(())
}

/// Report warning if user home directory does not exist or has wrong ownership
fn check_user_homedir(ctx: &EgoContext) {
let home = &ctx.target_user_homedir;
match fs::metadata(home) {
Ok(meta) => {
if meta.uid() != ctx.target_uid {
warn!(
"User {} home directory {} has incorrect ownership (expected UID {}, found {})",
ctx.target_user,
home.display(),
ctx.target_uid,
meta.uid()
);
}
}
Err(err) => {
// Report PermissionDenied as `info` level, user home directory is probably in a parent
// directory we have no access to, avoid nagging.
let level = match err.kind() {
PermissionDenied => Level::Info,
_ => Level::Warn,
};

log!(
level,
"User {} home directory {} is not accessible: {}",
ctx.target_user,
home.display(),
err
);
}
}
}

/// Add execute perm to runtime dir, e.g. `/run/user/1000`
fn prepare_runtime_dir(ctx: &EgoContext) -> Result<(), AnyErr> {
let path = &ctx.runtime_dir;
Expand Down
7 changes: 7 additions & 0 deletions src/snapshots/check_user_homedir.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
INFO: TEST: Success (no output)
INFO: TEST: Home does not exist
WARN: User nope home directory /tmp/path-does-not-exist.example is not accessible: No such file or directory (os error 2)
INFO: TEST: Permission denied
INFO: User root home directory /root/path-is-not-accessible.example is not accessible: Permission denied (os error 13)
INFO: TEST: Wrong owner
WARN: User root home directory /root has incorrect ownership (expected UID 1234, found 0)
57 changes: 54 additions & 3 deletions src/tests.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,31 @@
use std::env;
use std::path::PathBuf;
use std::fmt::Write;
use std::path::{Path, PathBuf};

use clap_complete::shells::{Bash, Fish, Zsh};
use clap_complete::Generator;
use log::Level;
use log::{info, Level};

use crate::cli::{build_cli, parse_args, Method};
use crate::util::have_command;
use crate::{get_wayland_socket, EgoContext};
use crate::{check_user_homedir, get_wayland_socket, EgoContext};

/// `vec![]` constructor that converts arguments to String
macro_rules! string_vec {
($($x:expr),*) => (vec![$($x.to_string()),*] as Vec<String>);
}

/// Compare log output with snapshot file. Call `testing_logger::setup()` at beginning of test.
fn assert_log_snapshot(expected_path: impl AsRef<Path>) {
testing_logger::validate(|logs| {
let output = logs.iter().fold(String::new(), |mut a, b| {
write!(a, "{}: {}\n", b.level.as_str(), b.body).unwrap();
a
});
snapbox::assert_eq_path(&expected_path, output);
})
}

fn render_completion(generator: impl Generator) -> Vec<u8> {
let mut buf = Vec::<u8>::new();
let mut app = build_cli();
Expand Down Expand Up @@ -60,6 +72,7 @@ fn test_context() -> EgoContext {
target_user: "ego".into(),
target_uid: 155,
target_user_shell: "/bin/bash".into(),
target_user_homedir: "/home/ego".into(),
}
}

Expand Down Expand Up @@ -130,3 +143,41 @@ fn test_have_command() {
assert!(have_command("sh"));
assert!(!have_command("what-is-this-i-don't-even"));
}

#[test]
fn test_check_user_homedir() {
let ctx = EgoContext {
runtime_dir: Default::default(),
target_user: "root".to_string(),
target_uid: 0,
target_user_shell: Default::default(),
target_user_homedir: "/root".into(),
};

// Capture log output from called functions
testing_logger::setup();

info!("TEST: Success (no output)");
check_user_homedir(&ctx);

info!("TEST: Home does not exist");
check_user_homedir(&EgoContext {
target_user: "nope".into(),
target_user_homedir: "/tmp/path-does-not-exist.example".into(),
..ctx.clone()
});

info!("TEST: Permission denied");
check_user_homedir(&EgoContext {
target_user_homedir: "/root/path-is-not-accessible.example".into(),
..ctx.clone()
});

info!("TEST: Wrong owner");
check_user_homedir(&EgoContext {
target_uid: 1234,
..ctx.clone()
});

assert_log_snapshot("src/snapshots/check_user_homedir.txt");
}
7 changes: 7 additions & 0 deletions varia/Dockerfile.tests
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,14 @@ ENV RUSTFLAGS="-D warnings"
RUN apt-get update && \
apt-get install -y libacl1-dev && \
rm -rf /var/lib/apt/lists/*

# Build as unprivileged user
RUN useradd build --create-home
WORKDIR /home/build
USER build

RUN rustup component add rustfmt clippy

# Build Cargo dependencies for cache
COPY Cargo.toml Cargo.lock ./
RUN mkdir src/ && \
Expand Down

0 comments on commit 0e247d1

Please sign in to comment.