Skip to content

Commit

Permalink
Don't fail setting log file ACLs with broken NSS backends
Browse files Browse the repository at this point in the history
Previously we would set POSIX ACL entries on log files by performing
an early UID lookup and passing a stringified version of the UID to
`exacl::AclEntry::allow_user`.

The exacl documentation states that names and decimal strings are
accepted. Apparently, exacl would try to look up the decimal string
using `getpwnam()` before considering that it should be parsed into a
numeric value. Unfortunately, this could cause a fatal error and
terminate the Laurel process:

    fatal error 'Error writing to filter log: Custom { kind: Other, error: Custom { kind: Other, error: "Invalid ACL: entry 3: Connection refused (os errno 111)" } }' at src/bin/laurel/main.rs:300,38

We still do an early uid lookup but pass the original string to exacl
instead of the decimal string.

Steps to reproduce: System is configured with sssd for AD integration;
/etc/nsswitch.conf contains the following lines:

    passwd:         files systemd sss
    group:          files systemd sss
    shadow:         files sss
    gshadow:        files

If the machine es rebooted and sssd happens to come up after auditd,
laurel fails to start and the error message above (or a similar one)
is written to the auditd service's journal

This can also be reproduced without rebooting by stopping sssd and
restarting auditd (so laurel is restarted).
  • Loading branch information
hillu committed Aug 1, 2024
1 parent 89fff20 commit cb89a4f
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 20 deletions.
29 changes: 16 additions & 13 deletions src/bin/laurel/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,8 @@ impl Logger {
filename.push(p);
let mut rot = FileRotate::new(filename);
for user in &def.clone().users.unwrap_or_default() {
rot = rot.with_uid(
User::from_name(user)?
.ok_or_else(|| anyhow!("user {user} not found"))?
.uid,
);
_ = User::from_name(user)?.ok_or_else(|| anyhow!("user {user} not found"))?;
rot = rot.with_user(user);
}
if let Some(generations) = &def.generations {
rot = rot.with_generations(*generations);
Expand Down Expand Up @@ -243,11 +240,8 @@ fn run_app() -> Result<(), anyhow::Error> {
filename.push(&def.file);
let mut rot = FileRotate::new(filename);
for user in &def.clone().users.unwrap_or_default() {
rot = rot.with_uid(
User::from_name(user)?
.ok_or_else(|| anyhow!("user {user} not found"))?
.uid,
);
_ = User::from_name(user)?.ok_or_else(|| anyhow!("user {user} not found"))?;
rot = rot.with_user(user);
}
if let Some(generations) = &def.generations {
rot = rot.with_generations(*generations);
Expand Down Expand Up @@ -297,17 +291,26 @@ fn run_app() -> Result<(), anyhow::Error> {
Logger::new(&config.filterlog, &dir).context("can't create filterlog logger")?;
emit_fn_log = move |e: &Event| {
if e.filter {
filter_logger.log(e).expect("Error writing to filter log");
filter_logger
.log(e)
.map_err(|e| anyhow!("Error writing to filter log: {e}"))
.unwrap();
} else {
logger.log(e).expect("Error writing to audit log");
logger
.log(e)
.map_err(|e| anyhow!("Error writing to audit log: {e}"))
.unwrap();
}
};
coalesce = Coalesce::new(emit_fn_log);
} else {
log::info!("Dropping filtered audit records");
emit_fn_drop = move |e: &Event| {
if !e.filter {
logger.log(e).expect("Error writing to audit log");
logger
.log(e)
.map_err(|e| anyhow!("Error writing to audit log: {e}"))
.unwrap();
}
};
coalesce = Coalesce::new(emit_fn_drop);
Expand Down
13 changes: 6 additions & 7 deletions src/rotate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::os::unix::io::AsRawFd;
use exacl::{setfacl, AclEntry, Perm};

use nix::sys::stat::{fchmod, Mode};
use nix::unistd::Uid;

/// A rotating (log) file writer
///
Expand All @@ -22,7 +21,7 @@ pub struct FileRotate {
/// size, a [`FileRotate::rotate`] operation is triggered.
pub filesize: u64,
pub generations: u64,
pub uids: Vec<Uid>,
pub users: Vec<String>,
file: Option<File>,
offset: u64,
}
Expand All @@ -36,7 +35,7 @@ impl FileRotate {
basename: OsString::from(path.as_ref()),
filesize: 0,
generations: 0,
uids: vec![],
users: vec![],
file: None,
offset: 0,
}
Expand All @@ -50,8 +49,8 @@ impl FileRotate {
self.generations = p;
self
}
pub fn with_uid(mut self, uid: Uid) -> Self {
self.uids.push(uid);
pub fn with_user(mut self, user: &str) -> Self {
self.users.push(user.into());
self
}

Expand Down Expand Up @@ -86,8 +85,8 @@ impl FileRotate {
#[cfg(any(target_os = "linux", target_os = "freebsd"))]
AclEntry::allow_other(Perm::empty(), None),
];
for uid in &self.uids {
acl.push(AclEntry::allow_user(&format!("{uid}"), Perm::READ, None));
for user in &self.users {
acl.push(AclEntry::allow_user(user, Perm::READ, None));
}

if let Ok(mut f) = OpenOptions::new().append(true).open(&self.basename) {
Expand Down

0 comments on commit cb89a4f

Please sign in to comment.