Skip to content

Commit

Permalink
Improve error handling, using thiserror, anyhow crates
Browse files Browse the repository at this point in the history
- Replace Box<dyn std::erro::Error> with custom error types – or
  simple static strings
- Use anyhow crate to add context where appropriate
  • Loading branch information
hillu committed Dec 13, 2023
1 parent 134c1b1 commit 957f00f
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 105 deletions.
8 changes: 8 additions & 0 deletions Cargo.lock

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

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ tinyvec = { version = "1", features = ["alloc"] }
log = "0.4"
simple_logger = ">= 1"
syslog = "6"
thiserror = "1"
anyhow = "1"

[target.'cfg(target_os = "linux")'.dependencies]
caps = "0.5"
Expand Down
66 changes: 31 additions & 35 deletions src/bin/laurel/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use getopts::Options;
use std::env;
use std::error::Error;
use std::error::Error as StdError;
use std::fs;
use std::io::{self, BufRead, BufReader, BufWriter, Read, Write};
use std::ops::AddAssign;
Expand All @@ -17,6 +17,8 @@ use std::sync::{
};
use std::time::{Duration, SystemTime};

use anyhow::{anyhow, Context};

use nix::unistd::{chown, execve, Uid, User};
#[cfg(target_os = "linux")]
use nix::unistd::{setresgid, setresuid};
Expand Down Expand Up @@ -58,24 +60,22 @@ impl AddAssign for Stats {
/// environment variables from arbitrary processes
/// (/proc/$PID/environ).
#[cfg(target_os = "linux")]
fn drop_privileges(runas_user: &User) -> Result<(), Box<dyn Error>> {
fn drop_privileges(runas_user: &User) -> anyhow::Result<()> {
set_keepcaps(true)?;
let uid = runas_user.uid;
let gid = runas_user.gid;
setresgid(gid, gid, gid).map_err(|e| format!("setresgid({}): {}", gid, e))?;
setresuid(uid, uid, uid).map_err(|e| format!("setresuid({}): {}", uid, e))?;
setresgid(gid, gid, gid).with_context(|| format!("setresgid({gid})"))?;
setresuid(uid, uid, uid).with_context(|| format!("setresuid({uid})"))?;

#[cfg(feature = "procfs")]
{
let mut capabilities = std::collections::HashSet::new();
capabilities.insert(Capability::CAP_SYS_PTRACE);
capabilities.insert(Capability::CAP_DAC_READ_SEARCH);
caps::set(None, CapSet::Permitted, &capabilities)
.map_err(|e| format!("set permitted capabilities: {}", e))?;
caps::set(None, CapSet::Effective, &capabilities)
.map_err(|e| format!("set effective capabilities: {}", e))?;
caps::set(None, CapSet::Permitted, &capabilities).context("set permitted capabilities")?;
caps::set(None, CapSet::Effective, &capabilities).context("set effective capabilities")?;
caps::set(None, CapSet::Inheritable, &capabilities)
.map_err(|e| format!("set inheritable capabilities: {}", e))?;
.context("set inheritable capabilities")?;
}

set_keepcaps(false)?;
Expand All @@ -97,26 +97,25 @@ impl Logger {
self.output.flush().unwrap();
}

fn new(def: &Logfile, dir: &Path) -> Result<Self, Box<dyn Error>> {
fn new(def: &Logfile, dir: &Path) -> anyhow::Result<Self> {
match &def.file {
p if p.as_os_str() == "-" => Ok(Logger {
prefix: def.line_prefix.clone(),
output: BufWriter::new(Box::new(io::stdout())),
}),
p if p.has_root() && p.parent().is_none() => Err(format!(
p if p.has_root() && p.parent().is_none() => Err(anyhow!(
"invalid file directory={} file={}",
dir.to_string_lossy(),
p.to_string_lossy()
)
.into()),
)),
p => {
let mut filename = dir.to_path_buf();
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(|| format!("user {} not found", &user))?
.ok_or_else(|| anyhow!("user {user} not found"))?
.uid,
);
}
Expand All @@ -135,7 +134,7 @@ impl Logger {
}
}

fn run_app() -> Result<(), Box<dyn Error>> {
fn run_app() -> Result<(), Box<dyn StdError>> {
let args: Vec<String> = env::args().collect();

let mut opts = Options::new();
Expand All @@ -158,20 +157,17 @@ fn run_app() -> Result<(), Box<dyn Error>> {
let config: Config = match matches.opt_str("c") {
Some(f_name) => {
if fs::metadata(&f_name)
.map_err(|e| format!("stat {}: {}", &f_name, &e))?
.with_context(|| format!("stat: {f_name}"))?
.permissions()
.mode()
& 0o002
!= 0
{
return Err(format!("Config file {} must not be world-writable", f_name).into());
}
let lines = fs::read(&f_name).map_err(|e| format!("read {}: {}", &f_name, &e))?;
toml::from_str(
&String::from_utf8(lines)
.map_err(|_| format!("parse: {}: contains invalid UTF-8 sequences", &f_name))?,
)
.map_err(|e| format!("parse {}: {}", f_name, e))?
let lines = fs::read(&f_name).with_context(|| format!("read(: {f_name}"))?;
toml::from_str(&String::from_utf8(lines).with_context(|| format!("parse: {f_name}"))?)
.with_context(|| format!("parse {f_name}"))?
}
None => Config::default(),
};
Expand All @@ -184,7 +180,7 @@ fn run_app() -> Result<(), Box<dyn Error>> {
Input::Stdin => Box::new(unsafe { std::fs::File::from_raw_fd(0) }),
Input::Unix(path) => Box::new(
UnixStream::connect(path)
.map_err(|e| format!("connect: {}: {}", path.to_string_lossy(), e))?,
.with_context(|| format!("connect: {}", path.to_string_lossy()))?,
),
};

Expand Down Expand Up @@ -217,7 +213,7 @@ fn run_app() -> Result<(), Box<dyn Error>> {
}
if dir
.metadata()
.map_err(|e| format!("stat {}: {}", dir.to_string_lossy(), &e))?
.with_context(|| format!("stat {}", dir.to_string_lossy()))?
.permissions()
.mode()
& 0o002
Expand All @@ -230,15 +226,15 @@ fn run_app() -> Result<(), Box<dyn Error>> {
}
} else {
fs::create_dir_all(&dir)
.map_err(|e| format!("create_dir: {}: {}", dir.to_string_lossy(), e))?;
.with_context(|| format!("create_dir: {}", dir.to_string_lossy()))?;
}
chown(&dir, Some(runas_user.uid), Some(runas_user.gid))
.map_err(|e| format!("chown: {}: {}", dir.to_string_lossy(), e))?;
.with_context(|| format!("chown: {}", dir.to_string_lossy()))?;
fs::set_permissions(&dir, PermissionsExt::from_mode(0o755))
.map_err(|e| format!("chmod: {}: {}", dir.to_string_lossy(), e))?;
.with_context(|| format!("chmod: {}", dir.to_string_lossy()))?;

let mut debug_logger = if let Some(l) = &config.debug.log {
Some(Logger::new(l, &dir).map_err(|e| format!("can't create debug logger: {}", e))?)
Some(Logger::new(l, &dir).context("can't create debug logger")?)
} else {
None
};
Expand Down Expand Up @@ -293,13 +289,12 @@ fn run_app() -> Result<(), Box<dyn Error>> {
let emit_fn_drop;
let emit_fn_log;

let mut logger = Logger::new(&config.auditlog, &dir)
.map_err(|e| format!("can't create audit logger: {}", e))?;
let mut logger = Logger::new(&config.auditlog, &dir).context("can't create audit logger")?;

if let laurel::config::FilterAction::Log = config.filter.filter_action {
log::info!("Logging filtered audit records");
let mut filter_logger = Logger::new(&config.filterlog, &dir)
.map_err(|e| format!("can't create filterlog logger: {}", e))?;
let mut filter_logger =
Logger::new(&config.filterlog, &dir).context("can't create filterlog logger")?;
emit_fn_log = move |e: &Event| {
if e.filter {
filter_logger.log(e)
Expand Down Expand Up @@ -344,7 +339,7 @@ fn run_app() -> Result<(), Box<dyn Error>> {
if let Some(ref mut l) = error_logger {
l.write_all(line)
.and_then(|_| l.flush())
.map_err(|e| format!("write log: {}", e))?;
.context("write log")?;
}
let line = String::from_utf8_lossy(line).replace('\n', "");
log::error!("Error {} processing msg: {}", e, &line);
Expand Down Expand Up @@ -373,7 +368,7 @@ fn run_app() -> Result<(), Box<dyn Error>> {
line.clear();
if input
.read_until(b'\n', &mut line)
.map_err(|e| format!("read from stdin: {}", e))?
.context("read from stdin")?
== 0
{
break;
Expand All @@ -387,7 +382,7 @@ fn run_app() -> Result<(), Box<dyn Error>> {
if let Some(ref mut l) = error_logger {
l.write_all(&line)
.and_then(|_| l.flush())
.map_err(|e| format!("write log: {}", e))?;
.context("write log")?;
}
let line = String::from_utf8_lossy(&line).replace('\n', "");
log::error!("Error {} processing msg: {}", e, &line);
Expand Down Expand Up @@ -419,6 +414,7 @@ fn run_app() -> Result<(), Box<dyn Error>> {
if dump_state_last_t.elapsed()? >= *p {
coalesce
.dump_state(&mut dl.output)
// .context("dump state")?;
.map_err(|e| format!("dump state: {}", e))?;
dump_state_last_t = SystemTime::now();
}
Expand Down
24 changes: 18 additions & 6 deletions src/coalesce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use serde_json::json;

use crate::constants::{msg_type::*, ARCH_NAMES, SYSCALL_NAMES};
use crate::label_matcher::LabelMatcher;
use crate::parser::parse;
use crate::parser::{parse, ParseError};
use crate::proc::{ContainerInfo, ProcTable, Process, ProcessKey};
#[cfg(all(feature = "procfs", target_os = "linux"))]
use crate::procfs;
Expand All @@ -17,6 +17,8 @@ use crate::sockaddr::SocketAddr;
use crate::types::*;
use crate::userdb::UserDB;

use thiserror::Error;

#[derive(Clone)]
pub struct Settings<'a> {
/// Generate ARGV and ARGV_STR from EXECVE
Expand Down Expand Up @@ -74,6 +76,16 @@ impl Default for Settings<'_> {
}
}

#[derive(Debug, Error)]
pub enum CoalesceError {
#[error("{0}")]
Parse(ParseError),
#[error("duplicate event id {0}")]
DuplicateEvent(EventID),
#[error("Event id {0} for EOE marker not found")]
SpuriousEOE(EventID),
}

/// Coalesce collects Audit Records from individual lines and assembles them to Events
pub struct Coalesce<'a> {
/// Events that are being collected/processed
Expand Down Expand Up @@ -1005,9 +1017,9 @@ impl<'a> Coalesce<'a> {
///
/// The line is consumed and serves as backing store for the
/// EventBody objects.
pub fn process_line(&mut self, line: Vec<u8>) -> Result<(), Box<dyn Error>> {
pub fn process_line(&mut self, line: Vec<u8>) -> Result<(), CoalesceError> {
let skip_enriched = self.settings.translate_universal && self.settings.translate_userdb;
let (node, typ, id, rv) = parse(line, skip_enriched)?;
let (node, typ, id, rv) = parse(line, skip_enriched).map_err(CoalesceError::Parse)?;
let nid = (node.clone(), id);

// clean out state every EXPIRE_PERIOD
Expand All @@ -1024,12 +1036,12 @@ impl<'a> Coalesce<'a> {

if typ == EOE {
if self.done.contains(&nid) {
return Err(format!("duplicate event id {}", id).into());
return Err(CoalesceError::DuplicateEvent(id));
}
let ev = self
.inflight
.remove(&nid)
.ok_or(format!("Event id {} for EOE marker not found", &id))?;
.ok_or(CoalesceError::SpuriousEOE(id))?;
self.emit_event(ev);
} else if typ.is_multipart() {
// kernel-level messages
Expand All @@ -1055,7 +1067,7 @@ impl<'a> Coalesce<'a> {
} else {
// user-space messages
if self.done.contains(&nid) {
return Err(format!("duplicate event id {}", id).into());
return Err(CoalesceError::DuplicateEvent(id));
}
let mut ev = Event::new(node, id);
ev.body.insert(typ, EventValues::Single(rv));
Expand Down
3 changes: 1 addition & 2 deletions src/label_matcher.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::error::Error;
use std::fmt;

use regex::bytes::RegexSet;
Expand All @@ -14,7 +13,7 @@ pub struct LabelMatcher {
}

impl LabelMatcher {
pub fn new(exprs: &[(&str, &str)]) -> Result<Self, Box<dyn Error>> {
pub fn new(exprs: &[(&str, &str)]) -> Result<Self, regex::Error> {
let mut regexes = Vec::with_capacity(exprs.len());
let mut tags = Vec::with_capacity(exprs.len());
for (r, t) in exprs {
Expand Down
Loading

0 comments on commit 957f00f

Please sign in to comment.