diff --git a/Cargo.toml b/Cargo.toml index a067137..f28b7d2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,16 +13,16 @@ edition = "2018" [badges] travis-ci = { repository = "mullvad/pfctl-rs" } - [dependencies] +derive_builder = "0.9" errno = "0.2" -error-chain = "0.12" ioctl-sys = "0.6.0" -libc = "0.2.29" -derive_builder = "0.9" ipnetwork = "0.16" +libc = "0.2.29" +thiserror = "1" [dev-dependencies] assert_matches = "1.1.0" -uuid = { version = "0.8", features = ["v4"] } +error-chain = "0.12" scopeguard = "1.0" +uuid = { version = "0.8", features = ["v4"] } diff --git a/examples/enable.rs b/examples/enable.rs index 9d76199..a1b0f68 100644 --- a/examples/enable.rs +++ b/examples/enable.rs @@ -21,7 +21,10 @@ fn run() -> Result<()> { // Try to enable the firewall. Equivalent to the CLI command "pfctl -e". match pf.enable() { Ok(_) => println!("Enabled PF"), - Err(pfctl::Error(pfctl::ErrorKind::StateAlreadyActive, _)) => (), + Err(pfctl::Error { + source: pfctl::ErrorSource::StateAlreadyActive(_), + .. + }) => (), err => err.chain_err(|| "Unable to enable PF")?, } Ok(()) diff --git a/src/lib.rs b/src/lib.rs index 3d50d16..59e1495 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -59,15 +59,14 @@ #![deny(rust_2018_idioms)] -#[macro_use] -pub extern crate error_chain; - use std::{ ffi::CStr, + fmt::Display, fs::File, mem, os::unix::io::{AsRawFd, RawFd}, }; +use thiserror::Error; pub use ipnetwork; @@ -92,40 +91,89 @@ pub use crate::ruleset::*; mod transaction; pub use crate::transaction::*; -mod errors { - error_chain! { - errors { - DeviceOpenError(s: &'static str) { - description("Unable to open PF device file") - display("Unable to open PF device file at '{}'", s) - } - InvalidArgument(s: &'static str) { - display("Invalid argument: {}", s) - } - StateAlreadyActive { - description("Target state is already active") - } - InvalidRuleCombination(s: String) { - description("Rule contains incompatible values") - display("Incompatible values in rule: {}", s) - } - AnchorDoesNotExist { - display("Anchor does not exist") - } +#[derive(Error, Debug)] +#[non_exhaustive] +pub enum ErrorSource { + #[error("Unable to open PF device file at {}", _0)] + DeviceOpen(&'static str, #[source] ::std::io::Error), + + #[error("Lower port is greater than upper port.")] + LowerIsGreaterPort, + + #[error("String does not fit destination")] + StrCopyNotFits, + + #[error("String has null byte")] + StrCopyNullByte, + + #[error("Target state is already active")] + StateAlreadyActive(#[source] ::std::io::Error), + + #[error("Incompatible values in rule: {}", _0)] + InvalidRuleCombination(String), + + #[error("Anchor does not exist")] + AnchorDoesNotExist, + + #[error("Cstr not null terminated")] + CstrNotTerminated, + + #[error("Ioctl Error")] + Ioctl(#[from] ::std::io::Error), +} + +#[derive(Error, Debug)] +pub struct Error { + pub info: Option, + #[source] + pub source: ErrorSource, +} + +impl Error { + fn new(info: ErrorInfo, err: Error) -> Self { + Self { + info: Some(info), + source: err.source, } - foreign_links { - IoctlError(::std::io::Error); + } +} + +impl Display for Error { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(info) = self.info.as_ref() { + info.fmt(f)?; } + Ok(()) } } -pub use crate::errors::*; -/// Returns the given input result, except if it is an `Err` matching the given `ErrorKind`, +impl From for Error { + fn from(source: ErrorSource) -> Self { + Self { info: None, source } + } +} + +#[derive(Error, Debug)] +#[non_exhaustive] +pub enum ErrorInfo { + #[error("Invalid anchor name")] + InvalidAnchorName, + + #[error("Incompatible interface name")] + IncompatibleInterfaceName, + + #[error("Invalid route target")] + InvalidRouteTarget, +} + +pub type Result = ::std::result::Result; + +/// Returns the given input result, except if it is an `Err` matching the given `Error`, /// then it returns `Ok(())` instead, so the error is ignored. macro_rules! ignore_error_kind { ($result:expr, $kind:pat) => { match $result { - Err($crate::Error($kind, _)) => Ok(()), + Err($crate::Error { source: $kind, .. }) => Ok(()), result => result, } }; @@ -148,7 +196,9 @@ use crate::conversion::*; /// Internal function to safely compare Rust string with raw C string slice fn compare_cstr_safe(s: &str, cchars: &[std::os::raw::c_char]) -> Result { - ensure!(cchars.iter().any(|&c| c == 0), "Not null terminated"); + if !(cchars.iter().any(|&c| c == 0)) { + return Err(ErrorSource::CstrNotTerminated.into()); + } let cs = unsafe { CStr::from_ptr(cchars.as_ptr()) }; Ok(s.as_bytes() == cs.to_bytes()) } @@ -174,7 +224,7 @@ impl PfCtl { /// Same as `enable`, but `StateAlreadyActive` errors are supressed and exchanged for /// `Ok(())`. pub fn try_enable(&mut self) -> Result<()> { - ignore_error_kind!(self.enable(), ErrorKind::StateAlreadyActive) + ignore_error_kind!(self.enable(), crate::ErrorSource::StateAlreadyActive(_)) } /// Tries to disable PF. If the firewall is already disabled it will return an @@ -186,7 +236,7 @@ impl PfCtl { /// Same as `disable`, but `StateAlreadyActive` errors are supressed and exchanged for /// `Ok(())`. pub fn try_disable(&mut self) -> Result<()> { - ignore_error_kind!(self.disable(), ErrorKind::StateAlreadyActive) + ignore_error_kind!(self.disable(), crate::ErrorSource::StateAlreadyActive(_)) } /// Tries to determine if PF is enabled or not. @@ -201,7 +251,7 @@ impl PfCtl { pfioc_rule.rule.action = kind.into(); name.try_copy_to(&mut pfioc_rule.anchor_call[..]) - .chain_err(|| ErrorKind::InvalidArgument("Invalid anchor name"))?; + .map_err(|e| Error::new(ErrorInfo::InvalidAnchorName, e))?; ioctl_guard!(ffi::pf_insert_rule(self.fd(), &mut pfioc_rule))?; Ok(()) @@ -210,7 +260,10 @@ impl PfCtl { /// Same as `add_anchor`, but `StateAlreadyActive` errors are supressed and exchanged for /// `Ok(())`. pub fn try_add_anchor(&mut self, name: &str, kind: AnchorKind) -> Result<()> { - ignore_error_kind!(self.add_anchor(name, kind), ErrorKind::StateAlreadyActive) + ignore_error_kind!( + self.add_anchor(name, kind), + crate::ErrorSource::StateAlreadyActive(_) + ) } pub fn remove_anchor(&mut self, name: &str, kind: AnchorKind) -> Result<()> { @@ -224,7 +277,7 @@ impl PfCtl { pub fn try_remove_anchor(&mut self, name: &str, kind: AnchorKind) -> Result<()> { ignore_error_kind!( self.remove_anchor(name, kind), - ErrorKind::AnchorDoesNotExist + crate::ErrorSource::AnchorDoesNotExist ) } @@ -236,7 +289,7 @@ impl PfCtl { pfioc_rule.ticket = utils::get_ticket(self.fd(), &anchor, AnchorKind::Filter)?; anchor .try_copy_to(&mut pfioc_rule.anchor[..]) - .chain_err(|| ErrorKind::InvalidArgument("Invalid anchor name"))?; + .map_err(|e| Error::new(ErrorInfo::InvalidAnchorName, e))?; rule.try_copy_to(&mut pfioc_rule.rule)?; pfioc_rule.action = ffi::pfvar::PF_CHANGE_ADD_TAIL as u32; @@ -287,7 +340,7 @@ impl PfCtl { /// Clear states created by rules in anchor. /// Returns total number of removed states upon success, otherwise - /// ErrorKind::AnchorDoesNotExist if anchor does not exist. + /// Error::AnchorDoesNotExist if anchor does not exist. pub fn clear_states(&mut self, anchor_name: &str, kind: AnchorKind) -> Result { let pfsync_states = self.get_states()?; if !pfsync_states.is_empty() { @@ -317,7 +370,7 @@ impl PfCtl { let mut pfioc_state_kill = unsafe { mem::zeroed::() }; interface .try_copy_to(&mut pfioc_state_kill.psk_ifname) - .chain_err(|| ErrorKind::InvalidArgument("Incompatible interface name"))?; + .map_err(|e| Error::new(ErrorInfo::IncompatibleInterfaceName, e))?; ioctl_guard!(ffi::pf_clear_states(self.fd(), &mut pfioc_state_kill))?; // psk_af holds the number of killed states Ok(pfioc_state_kill.psk_af as u32) @@ -342,7 +395,7 @@ impl PfCtl { /// The return value from closure is transparently passed to the caller. /// /// - Returns Result from call to closure on match. - /// - Returns `ErrorKind::AnchorDoesNotExist` on mismatch, the closure is not called in that + /// - Returns `Error::AnchorDoesNotExist` on mismatch, the closure is not called in that /// case. fn with_anchor_rule(&self, name: &str, kind: AnchorKind, f: F) -> Result where @@ -359,7 +412,7 @@ impl PfCtl { return f(pfioc_rule); } } - bail!(ErrorKind::AnchorDoesNotExist); + Err(ErrorSource::AnchorDoesNotExist.into()) } /// Returns global number of states created by all stateful rules (see keep_state) @@ -419,7 +472,10 @@ mod tests { let cchars: &[i8] = unsafe { mem::transmute(cstr.as_bytes()) }; assert_matches!( compare_cstr_safe("Hello", cchars), - Err(ref e) if e.description() == "Not null terminated" + Err(Error { + source: ErrorSource::CstrNotTerminated, + .. + }) ); } diff --git a/src/macros.rs b/src/macros.rs index 0832b65..4be5790 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -16,12 +16,12 @@ macro_rules! ioctl_guard { ($func:expr, $already_active:expr) => { if unsafe { $func } == $crate::macros::IOCTL_ERROR { let ::errno::Errno(error_code) = ::errno::errno(); - let io_error = ::std::io::Error::from_raw_os_error(error_code); - let mut err = Err($crate::ErrorKind::IoctlError(io_error).into()); + let err = ::std::io::Error::from_raw_os_error(error_code); if error_code == $already_active { - err = err.chain_err(|| $crate::ErrorKind::StateAlreadyActive); + Err($crate::ErrorSource::StateAlreadyActive(err).into()) + } else { + Err($crate::ErrorSource::from(err).into()) } - err } else { Ok(()) as $crate::Result<()> } diff --git a/src/pooladdr.rs b/src/pooladdr.rs index 1c3cfb9..a08ba1f 100644 --- a/src/pooladdr.rs +++ b/src/pooladdr.rs @@ -10,7 +10,7 @@ use crate::{ conversion::{CopyTo, TryCopyTo}, ffi, Interface, Ip, Result, }; -use std::{mem, ptr, vec::Vec}; +use std::{mem, ptr}; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct PoolAddr { diff --git a/src/rule/gid.rs b/src/rule/gid.rs index 5bd9f91..0c273a7 100644 --- a/src/rule/gid.rs +++ b/src/rule/gid.rs @@ -6,7 +6,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -pub use super::uid::{Id, IdUnaryModifier}; +pub use super::uid::Id; use crate::{ conversion::TryCopyTo, ffi::pfvar::{pf_rule_gid, PF_OP_NONE}, diff --git a/src/rule/mod.rs b/src/rule/mod.rs index bd541b5..eeb97c4 100644 --- a/src/rule/mod.rs +++ b/src/rule/mod.rs @@ -8,7 +8,7 @@ use crate::{ conversion::{CopyTo, TryCopyTo}, - ffi, ErrorKind, Result, ResultExt, + ffi, Error, ErrorInfo, ErrorSource, Result, }; use derive_builder::Builder; use ipnetwork::IpNetwork; @@ -99,7 +99,7 @@ pub struct FilterRule { impl FilterRuleBuilder { pub fn build(&self) -> Result { self.build_internal() - .map_err(|e| ErrorKind::InvalidRuleCombination(e).into()) + .map_err(|e| ErrorSource::InvalidRuleCombination(e).into()) } } @@ -128,7 +128,7 @@ impl FilterRule { "StatePolicy {:?} and protocol {:?} are incompatible", state_policy, proto ); - bail!(ErrorKind::InvalidRuleCombination(msg)); + Err(ErrorSource::InvalidRuleCombination(msg).into()) } } } @@ -148,7 +148,7 @@ impl TryCopyTo for FilterRule { self.interface .try_copy_to(&mut pf_rule.ifname) - .chain_err(|| ErrorKind::InvalidArgument("Incompatible interface name"))?; + .map_err(|e| Error::new(ErrorInfo::IncompatibleInterfaceName, e))?; pf_rule.proto = self.proto.into(); pf_rule.af = self.get_af()?.into(); @@ -198,7 +198,7 @@ pub struct RedirectRule { impl RedirectRuleBuilder { pub fn build(&self) -> Result { self.build_internal() - .map_err(|e| ErrorKind::InvalidRuleCombination(e).into()) + .map_err(|e| ErrorSource::InvalidRuleCombination(e).into()) } } @@ -225,7 +225,7 @@ impl TryCopyTo for RedirectRule { pf_rule.log = (&self.log).into(); self.interface .try_copy_to(&mut pf_rule.ifname) - .chain_err(|| ErrorKind::InvalidArgument("Incompatible interface name"))?; + .map_err(|e| Error::new(ErrorInfo::IncompatibleInterfaceName, e))?; pf_rule.proto = self.proto.into(); pf_rule.af = self.get_af()?.into(); @@ -246,7 +246,7 @@ fn compatible_af(af1: AddrFamily, af2: AddrFamily) -> Result { (AddrFamily::Any, af) => Ok(af), (af1, af2) => { let msg = format!("AddrFamily {} and {} are incompatible", af1, af2); - bail!(ErrorKind::InvalidRuleCombination(msg)); + Err(ErrorSource::InvalidRuleCombination(msg).into()) } } } @@ -485,14 +485,12 @@ impl> TryCopyTo<[i8]> for T { fn try_copy_to(&self, dst: &mut [i8]) -> Result<()> { let src_i8: &[i8] = unsafe { &*(self.as_ref().as_bytes() as *const _ as *const _) }; - ensure!( - src_i8.len() < dst.len(), - ErrorKind::InvalidArgument("String does not fit destination") - ); - ensure!( - !src_i8.contains(&0), - ErrorKind::InvalidArgument("String has null byte") - ); + if src_i8.len() >= dst.len() { + return Err(ErrorSource::StrCopyNotFits.into()); + } + if src_i8.contains(&0) { + return Err(ErrorSource::StrCopyNullByte.into()); + } dst[..src_i8.len()].copy_from_slice(src_i8); // Terminate ffi string with null byte diff --git a/src/rule/port.rs b/src/rule/port.rs index 388df93..3446c4e 100644 --- a/src/rule/port.rs +++ b/src/rule/port.rs @@ -6,7 +6,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use crate::{conversion::TryCopyTo, ffi, ErrorKind, Result}; +use crate::{conversion::TryCopyTo, ffi, ErrorSource, Result}; // Port range representation #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -43,10 +43,9 @@ impl TryCopyTo for Port { pf_port_range.port[1] = 0; } Port::Range(start_port, end_port, modifier) => { - ensure!( - start_port <= end_port, - ErrorKind::InvalidArgument("Lower port is greater than upper port.") - ); + if start_port > end_port { + return Err(ErrorSource::LowerIsGreaterPort.into()); + } pf_port_range.op = modifier.into(); // convert port range to network byte order pf_port_range.port[0] = start_port.to_be(); @@ -71,10 +70,9 @@ impl TryCopyTo for Port { pf_pool.proxy_port[1] = 0; } Port::Range(start_port, end_port, modifier) => { - ensure!( - start_port <= end_port, - ErrorKind::InvalidArgument("Lower port is greater than upper port.") - ); + if start_port > end_port { + return Err(ErrorSource::LowerIsGreaterPort.into()); + } pf_pool.port_op = modifier.into(); pf_pool.proxy_port[0] = start_port; pf_pool.proxy_port[1] = end_port; diff --git a/src/transaction.rs b/src/transaction.rs index b753c10..7afb511 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -7,8 +7,8 @@ // except according to those terms. use crate::{ - conversion::TryCopyTo, ffi, utils, ErrorKind, FilterRule, PoolAddrList, RedirectRule, Result, - ResultExt, RulesetKind, + conversion::TryCopyTo, ffi, utils, Error, ErrorInfo, FilterRule, PoolAddrList, RedirectRule, + Result, RulesetKind, }; use std::{ collections::HashMap, @@ -112,7 +112,7 @@ impl Transaction { pfioc_rule.action = ffi::pfvar::PF_CHANGE_NONE as u32; anchor .try_copy_to(&mut pfioc_rule.anchor[..]) - .chain_err(|| ErrorKind::InvalidArgument("Invalid anchor name"))?; + .map_err(|e| Error::new(ErrorInfo::InvalidAnchorName, e))?; rule.try_copy_to(&mut pfioc_rule.rule)?; // request new address pool @@ -124,7 +124,7 @@ impl Transaction { // register pool address with firewall utils::add_pool_address(fd, pool_addr.clone(), pool_ticket)?; let pool_addr_list = PoolAddrList::new(&[pool_addr.clone()]) - .chain_err(|| ErrorKind::InvalidArgument("Invalid route target"))?; + .map_err(|e| Error::new(ErrorInfo::InvalidRouteTarget, e))?; pfioc_rule.rule.rpool.list = unsafe { pool_addr_list.to_palist() }; Some(pool_addr_list) @@ -146,7 +146,7 @@ impl Transaction { let mut pfioc_rule = unsafe { mem::zeroed::() }; anchor .try_copy_to(&mut pfioc_rule.anchor[..]) - .chain_err(|| ErrorKind::InvalidArgument("Invalid anchor name"))?; + .map_err(|e| Error::new(ErrorInfo::InvalidAnchorName, e))?; rule.try_copy_to(&mut pfioc_rule.rule)?; // register redirect address in newly created address pool @@ -186,7 +186,7 @@ impl Transaction { pfioc_trans_e.rs_num = ruleset_kind.into(); anchor .try_copy_to(&mut pfioc_trans_e.anchor[..]) - .chain_err(|| ErrorKind::InvalidArgument("Invalid anchor name"))?; + .map_err(|e| Error::new(ErrorInfo::InvalidAnchorName, e))?; Ok(pfioc_trans_e) } } diff --git a/src/utils.rs b/src/utils.rs index 160bb31..3705b0f 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -6,7 +6,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use crate::{conversion::TryCopyTo, ffi, AnchorKind, ErrorKind, PoolAddr, Result, ResultExt}; +use crate::{ + conversion::TryCopyTo, ffi, AnchorKind, Error, ErrorInfo, ErrorSource, PoolAddr, Result, +}; use std::{ fs::{File, OpenOptions}, mem, @@ -22,7 +24,7 @@ pub fn open_pf() -> Result { .read(true) .write(true) .open(PF_DEV_PATH) - .chain_err(|| ErrorKind::DeviceOpenError(PF_DEV_PATH)) + .map_err(|e| ErrorSource::DeviceOpen(PF_DEV_PATH, e).into()) } /// Add pool address using the pool ticket previously obtained via `get_pool_ticket()` @@ -50,7 +52,7 @@ pub fn get_ticket(fd: RawFd, anchor: &str, kind: AnchorKind) -> Result { pfioc_rule.rule.action = kind.into(); anchor .try_copy_to(&mut pfioc_rule.anchor[..]) - .chain_err(|| ErrorKind::InvalidArgument("Invalid anchor name"))?; + .map_err(|e| Error::new(ErrorInfo::InvalidAnchorName, e))?; ioctl_guard!(ffi::pf_change_rule(fd, &mut pfioc_rule))?; Ok(pfioc_rule.ticket) } diff --git a/tests/anchors.rs b/tests/anchors.rs index b541ee2..7e1ced7 100644 --- a/tests/anchors.rs +++ b/tests/anchors.rs @@ -30,7 +30,7 @@ test!(add_filter_anchor { assert_matches!( pf.add_anchor(&anchor_name, pfctl::AnchorKind::Filter), - Err(pfctl::Error(pfctl::ErrorKind::StateAlreadyActive, _)) + Err(pfctl::Error { source: pfctl::ErrorSource::StateAlreadyActive(_), .. }) ); assert_matches!(pf.try_add_anchor(&anchor_name, pfctl::AnchorKind::Filter), Ok(())); }); @@ -47,7 +47,7 @@ test!(remove_filter_anchor { assert_matches!( pf.remove_anchor(&anchor_name, pfctl::AnchorKind::Filter), - Err(pfctl::Error(pfctl::ErrorKind::AnchorDoesNotExist, _)) + Err(pfctl::Error { source: pfctl::ErrorSource::AnchorDoesNotExist, .. }) ); assert_matches!(pf.try_remove_anchor(&anchor_name, pfctl::AnchorKind::Filter), Ok(())); }); diff --git a/tests/enable_disable.rs b/tests/enable_disable.rs index a944a97..cc7e9c2 100644 --- a/tests/enable_disable.rs +++ b/tests/enable_disable.rs @@ -17,7 +17,7 @@ test!(enable_pf { assert_matches!(pfcli::disable_firewall(), Ok(())); assert_matches!(pf.enable(), Ok(())); assert_matches!(pfcli::is_enabled(), Ok(true)); - assert_matches!(pf.enable(), Err(pfctl::Error(pfctl::ErrorKind::StateAlreadyActive, _))); + assert_matches!(pf.enable(), Err(pfctl::Error{ source: pfctl::ErrorSource::StateAlreadyActive(_), .. })); assert_matches!(pf.try_enable(), Ok(())); assert_matches!(pfcli::is_enabled(), Ok(true)); }); @@ -28,7 +28,7 @@ test!(disable_pf { assert_matches!(pfcli::enable_firewall(), Ok(())); assert_matches!(pf.disable(), Ok(())); assert_matches!(pfcli::is_enabled(), Ok(false)); - assert_matches!(pf.disable(), Err(pfctl::Error(pfctl::ErrorKind::StateAlreadyActive, _))); + assert_matches!(pf.disable(), Err(pfctl::Error { source: pfctl::ErrorSource::StateAlreadyActive(_), .. })); assert_matches!(pf.try_disable(), Ok(())); assert_matches!(pfcli::is_enabled(), Ok(false)); });