Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:

- name: Run all E2E tests
working-directory: test-framework
run: cargo test -p e2e-tests
run: cargo test -p e2e-tests --features apparmor

- name: prevent the cache from growing too large
run: |
Expand Down
70 changes: 70 additions & 0 deletions src/apparmor.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
use std::ffi::{c_char, c_int, CStr, CString};
use std::{fs, io, mem};

use crate::cutils::cerr;

/// Set the profile for the next exec call if AppArmor is enabled
pub fn set_profile_for_next_exec(profile_name: &str) -> io::Result<()> {
if apparmor_is_enabled()? {
apparmor_prepare_exec(profile_name)
} else {
// if the sysadmin doesn't have apparmor enabled, fail softly
Ok(())
}
}

fn apparmor_is_enabled() -> io::Result<bool> {
match fs::read_to_string("/sys/module/apparmor/parameters/enabled") {
Ok(enabled) => Ok(enabled.starts_with("Y")),
Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(false),
Err(e) => Err(e),
}
}

/// Switch the apparmor profile to the given profile on the next exec call
fn apparmor_prepare_exec(new_profile: &str) -> io::Result<()> {
// SAFETY: Always safe to call
unsafe { libc::dlerror() }; // Clear any existing error

// SAFETY: Loading a known safe dylib. LD_LIBRARY_PATH is ignored because we are setuid.
let handle = unsafe { libc::dlopen(c"libapparmor.so.1".as_ptr(), libc::RTLD_NOW) };
if handle.is_null() {
// SAFETY: In case of an error, dlerror returns a valid C string.
return Err(io::Error::new(io::ErrorKind::NotFound, unsafe {
CStr::from_ptr(libc::dlerror())
.to_string_lossy()
.into_owned()
}));
}

// SAFETY: dlsym will either return a function pointer of the right signature or NULL.
let aa_change_onexec = unsafe { libc::dlsym(handle, cstr!("aa_change_onexec").as_ptr()) };

if aa_change_onexec.is_null() {
// SAFETY: Always safe to call
let err = unsafe { libc::dlerror() };
return Err(if err.is_null() {
// There was no error in dlsym, but the symbol itself was defined as NULL pointer.
// This is still an error for us, but dlerror will not return any error.
io::Error::new(
io::ErrorKind::Other,
"aa_change_onexec symbol is a NULL pointer",
)
} else {
// SAFETY: In case of an error, dlerror returns a valid C string.
io::Error::new(io::ErrorKind::NotFound, unsafe {
CStr::from_ptr(err).to_string_lossy().into_owned()
})
});
}

//SAFETY: aa_change_onexec is non-NULL, so we can cast it into a function pointer
let aa_change_onexec: unsafe extern "C" fn(*const c_char) -> c_int =
unsafe { mem::transmute(aa_change_onexec) };

let new_profile_cstr = CString::new(new_profile)?;
// SAFETY: new_profile_cstr provided by CString ensures a valid ptr
cerr(unsafe { aa_change_onexec(new_profile_cstr.as_ptr()) })?;

Ok(())
}
32 changes: 0 additions & 32 deletions src/apparmor/mod.rs

This file was deleted.

4 changes: 0 additions & 4 deletions src/apparmor/sys.rs

This file was deleted.

3 changes: 3 additions & 0 deletions test-framework/e2e-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,6 @@ version = "0.0.0"

[dependencies]
sudo-test.path = "../sudo-test"

[features]
apparmor = ["sudo-test/apparmor"]
45 changes: 45 additions & 0 deletions test-framework/e2e-tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,48 @@ fn sanity_check() {
"you must set `SUDO_UNDER_TEST=ours` when running this test suite"
);
}

#[test]
#[cfg(feature = "apparmor")]
fn dlopen_apparmor_ignores_ld_library_path() -> Result<(), Box<dyn std::error::Error>> {
use sudo_test::{Command, Env};

let env = Env("foo ALL=(ALL:ALL) APPARMOR_PROFILE=docker-default NOPASSWD: ALL")
.file(
"/tmp/crash_me.c",
"#include <stdlib.h>

void __attribute__((constructor)) do_not_load() {
abort();
}
",
)
.user("foo")
.apparmor("unconfined")
.build();

Command::new("gcc")
.args(["/tmp/crash_me.c", "-shared", "-o", "/tmp/libapparmor.so.1"])
.output(&env)
.assert_success();

let output = Command::new("sh")
.args([
"-c",
"LD_LIBRARY_PATH=/tmp sudo -s cat /proc/\\$\\$/attr/current",
])
.as_user("foo")
.output(&env);

output.assert_success();
assert_eq!(output.stdout(), "docker-default (enforce)");

let output = Command::new("sh")
.args(["-c", "LD_PRELOAD=/tmp/libapparmor.so.1 ls"])
.output(&env);

output.assert_exit_code(134); // SIGABRT
assert_eq!(output.stderr(), "Aborted (core dumped)");

Ok(())
}
5 changes: 0 additions & 5 deletions test-framework/sudo-compliance-tests/src/sudo/apparmor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ fn can_switch_the_apparmor_profile() -> Result<()> {
let output = Command::new("sudo")
.args(["-s", "cat", "/proc/$$/attr/current"])
.output(&env);
dbg!(&output);

output.assert_success();
assert_eq!(output.stdout(), "docker-default (enforce)");
Expand All @@ -25,8 +24,6 @@ fn cannot_switch_to_nonexisting_profile() -> Result<()> {

let output = Command::new("sudo").arg("true").output(&env);

dbg!(&output);

output.assert_exit_code(1);
assert_contains!(output.stderr(), "unable to change AppArmor profile");

Expand All @@ -44,7 +41,6 @@ Defaults apparmor_profile=docker-default
let output = Command::new("sudo")
.args(["-s", "cat", "/proc/$$/attr/current"])
.output(&env);
dbg!(&output);

output.assert_success();
assert_eq!(output.stdout(), "docker-default (enforce)");
Expand All @@ -63,7 +59,6 @@ Defaults apparmor_profile=docker-default
let output = Command::new("sudo")
.args(["-s", "cat", "/proc/$$/attr/current"])
.output(&env);
dbg!(&output);

output.assert_success();
assert_eq!(output.stdout(), "unconfined");
Expand Down
2 changes: 1 addition & 1 deletion test-framework/sudo-test/src/ours.linux.Dockerfile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
FROM rust:1-slim-bookworm
RUN apt-get update && \
apt-get install -y --no-install-recommends apparmor libpam0g-dev libapparmor-dev procps sshpass rsyslog ca-certificates tzdata
apt-get install -y --no-install-recommends apparmor libpam0g-dev libapparmor1 procps sshpass rsyslog ca-certificates tzdata
# cache the crates.io index in the image for faster local testing
RUN cargo search sudo
WORKDIR /usr/src/sudo
Expand Down
Loading