Replace enumflags2 with a macro we define ourself

This avoid a proc macro, makes the api more ergonomic by removing the need for the BitFlags wrapper type, makes it impossible to construct an Access* value with invalid bits, avoids exposing all() to the user and makes it possible for us to define the exact api we want to expose to the user. Signed-off-by: Björn Roy Baron <[email protected]>
landlock-lsm · Sep 14, 2024 · fc37a1b · fc37a1b
1 parent 94721d2
commit fc37a1b
Show file tree

Hide file tree

Showing 9 changed files with 272 additions and 224 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -13,7 +13,6 @@ exclude = [".gitignore"]
 readme = "README.md"
 
 [dependencies]
-enumflags2 = "0.7"
 libc = "0.2.133"
 thiserror = "1.0"
 

diff --git a/examples/sandboxer.rs b/examples/sandboxer.rs
@@ -3,7 +3,7 @@
 
 use anyhow::{anyhow, bail, Context};
 use landlock::{
-    Access, AccessFs, AccessNet, BitFlags, NetPort, PathBeneath, PathFd, Ruleset, RulesetAttr,
+    Access, AccessFs, AccessNet, NetPort, PathBeneath, PathFd, Ruleset, RulesetAttr,
     RulesetCreatedAttr, RulesetStatus, ABI,
 };
 use std::env;
@@ -19,7 +19,7 @@ const ENV_TCP_CONNECT_NAME: &str = "LL_TCP_CONNECT";
 
 struct PathEnv {
     paths: Vec<u8>,
-    access: BitFlags<AccessFs>,
+    access: AccessFs,
 }
 
 impl PathEnv {
@@ -31,7 +31,7 @@ impl PathEnv {
     ///   allowed. Paths are separated with ":", e.g. "/bin:/lib:/usr:/proc". In case an empty
     ///   string is provided, NO restrictions are applied.
     /// * `access`: Set of access-rights allowed for each of the parsed paths.
-    fn new<'a>(name: &'a str, access: BitFlags<AccessFs>) -> anyhow::Result<Self> {
+    fn new<'a>(name: &'a str, access: AccessFs) -> anyhow::Result<Self> {
         Ok(Self {
             paths: env::var_os(name)
                 .ok_or(anyhow!("missing environment variable {name}"))?

diff --git a/src/access.rs b/src/access.rs
@@ -1,21 +1,132 @@
+use std::ops::{BitAnd, BitOr, Not};
+
 use crate::{
-    AccessError, AddRuleError, AddRulesError, BitFlags, CompatError, CompatResult,
-    HandleAccessError, HandleAccessesError, Ruleset, TailoredCompatLevel, TryCompat, ABI,
+    AccessError, AddRuleError, AddRulesError, CompatError, CompatResult, HandleAccessError,
+    HandleAccessesError, Ruleset, TailoredCompatLevel, TryCompat, ABI,
 };
-use enumflags2::BitFlag;
 
 #[cfg(test)]
-use crate::{make_bitflags, AccessFs, CompatLevel, CompatState, Compatibility};
+use crate::{AccessFs, CompatLevel, CompatState, Compatibility};
+
+#[macro_export]
+macro_rules! make_bitflags {
+    ($bitflag_type:ident :: {$($flag:ident)|*}) => {
+        $bitflag_type::EMPTY $(.union($bitflag_type::$flag))*
+    };
+}
+
+macro_rules! bitflags_type {
+    (
+        $(#[$bitflags_attr:meta])*
+        $vis:vis struct $bitflags_name:ident: $bitflags_type:ty {
+            $(
+                $(#[$flag_attr:meta])*
+                const $flag_name:ident = $flag_val:expr;
+            )*
+        }
+    ) => {
+        $(#[$bitflags_attr])*
+        #[derive(Copy, Clone, Debug, PartialEq, Eq, Default)]
+        $vis struct $bitflags_name($bitflags_type);
+
+        impl $bitflags_name {
+            $(
+                #[allow(non_upper_case_globals)]
+                $(#[$flag_attr])*
+                $vis const $flag_name: Self = Self($flag_val);
+            )*
+
+            $vis const EMPTY: Self = Self(0);
+
+            $vis const fn is_empty(&self) -> bool {
+                self.0 == 0
+            }
+
+            $vis const fn union(self, rhs: Self) -> Self {
+                Self(self.0 | rhs.0)
+            }
+
+            $vis const fn contains(self, rhs: Self) -> bool {
+                self.0 & rhs.0 == rhs.0
+            }
+
+            pub(crate) const fn all() -> Self {
+                Self(0 $(| $flag_val)*)
+            }
+
+            pub(crate) const fn bits(self) -> $bitflags_type {
+                self.0
+            }
+        }
+
+        impl core::ops::BitAnd for $bitflags_name {
+            type Output = Self;
+
+            fn bitand(self, rhs: Self) -> Self {
+                Self(self.0 & rhs.0)
+            }
+        }
+
+        impl core::ops::BitAndAssign for $bitflags_name {
+            fn bitand_assign(&mut self, rhs: Self) {
+                self.0 &= rhs.0;
+            }
+        }
+
+        impl core::ops::BitOr for $bitflags_name {
+            type Output = Self;
+
+            fn bitor(self, rhs: Self) -> Self {
+                Self(self.0 | rhs.0)
+            }
+        }
+
+        impl core::ops::BitOrAssign for $bitflags_name {
+            fn bitor_assign(&mut self, rhs: Self) {
+                self.0 |= rhs.0;
+            }
+        }
+
+        impl core::ops::BitXor for $bitflags_name {
+            type Output = Self;
+
+            fn bitxor(self, rhs: Self) -> Self {
+                Self(self.0 ^ rhs.0)
+            }
+        }
 
-pub trait Access: PrivateAccess {
+        impl core::ops::BitXorAssign for $bitflags_name {
+            fn bitxor_assign(&mut self, rhs: Self) {
+                self.0 ^= rhs.0;
+            }
+        }
+
+        impl core::ops::Not for $bitflags_name {
+            type Output = Self;
+
+            fn not(self) -> Self {
+                Self(!self.0) & Self::all()
+            }
+        }
+    };
+}
+pub(crate) use bitflags_type;
+
+pub trait Access: PrivateAccess + TailoredCompatLevel {
     /// Gets the access rights defined by a specific [`ABI`].
-    fn from_all(abi: ABI) -> BitFlags<Self>;
+    fn from_all(abi: ABI) -> Self;
 }
 
-pub trait PrivateAccess: BitFlag {
+pub trait PrivateAccess:
+    core::fmt::Debug + Copy + BitOr<Output = Self> + BitAnd<Output = Self> + Not<Output = Self>
+{
+    fn is_empty(self) -> bool
+    where
+        Self: Access;
+
     fn ruleset_handle_access(
         ruleset: &mut Ruleset,
-        access: BitFlags<Self>,
+        access: Self,
     ) -> Result<(), HandleAccessesError>
     where
         Self: Access;
@@ -29,51 +140,34 @@ pub trait PrivateAccess: BitFlag {
         Self: Access;
 }
 
-// Creates an illegal/overflowed BitFlags<T> with all its bits toggled, including undefined ones.
-fn full_negation<T>(flags: BitFlags<T>) -> BitFlags<T>
-where
-    T: Access,
-{
-    unsafe { BitFlags::<T>::from_bits_unchecked(!flags.bits()) }
-}
-
 #[test]
 fn bit_flags_full_negation() {
-    let scoped_negation = !BitFlags::<AccessFs>::all();
-    assert_eq!(scoped_negation, BitFlags::<AccessFs>::empty());
-    // !BitFlags::<AccessFs>::all() could be equal to full_negation(BitFlags::<AccessFs>::all()))
-    // if all the 64-bits would be used, which is not currently the case.
-    assert_ne!(scoped_negation, full_negation(BitFlags::<AccessFs>::all()));
+    let scoped_negation = !AccessFs::all();
+    assert_eq!(scoped_negation, AccessFs::EMPTY);
+    // !AccessFs::all() could be equal to !AccessFs::all().bits() if
+    // all the 64-bits would be used, which is not currently the case.
+    assert_ne!(scoped_negation.bits(), !AccessFs::all().bits());
 }
 
-impl<A> TailoredCompatLevel for BitFlags<A> where A: Access {}
-
-impl<A> TryCompat<A> for BitFlags<A>
+impl<A> TryCompat<A> for A
 where
     A: Access,
 {
     fn try_compat_inner(&mut self, abi: ABI) -> Result<CompatResult<A>, CompatError<A>> {
         if self.is_empty() {
             // Empty access-rights would result to a runtime error.
             Err(AccessError::Empty.into())
-        } else if !Self::all().contains(*self) {
-            // Unknown access-rights (at build time) would result to a runtime error.
-            // This can only be reached by using the unsafe BitFlags::from_bits_unchecked().
-            Err(AccessError::Unknown {
-                access: *self,
-                unknown: *self & full_negation(Self::all()),
-            }
-            .into())
         } else {
             let compat = *self & A::from_all(abi);
+            let incompatible_flags = *self & !A::from_all(abi);
             let ret = if compat.is_empty() {
                 Ok(CompatResult::No(
                     AccessError::Incompatible { access: *self }.into(),
                 ))
-            } else if compat != *self {
+            } else if !incompatible_flags.is_empty() {
                 let error = AccessError::PartiallyCompatible {
                     access: *self,
-                    incompatible: *self & full_negation(compat),
+                    incompatible: incompatible_flags,
                 }
                 .into();
                 Ok(CompatResult::Partial(error))
@@ -103,29 +197,14 @@ fn compat_bit_flags() {
     );
     assert!(compat.state == CompatState::Full);
 
-    let empty_access = BitFlags::<AccessFs>::empty();
+    let empty_access = AccessFs::EMPTY;
     assert!(matches!(
         empty_access
             .try_compat(compat.abi(), compat.level, &mut compat.state)
             .unwrap_err(),
         CompatError::Access(AccessError::Empty)
     ));
 
-    let all_unknown_access = unsafe { BitFlags::<AccessFs>::from_bits_unchecked(1 << 63) };
-    assert!(matches!(
-        all_unknown_access.try_compat(compat.abi(), compat.level, &mut compat.state).unwrap_err(),
-        CompatError::Access(AccessError::Unknown { access, unknown }) if access == all_unknown_access && unknown == all_unknown_access
-    ));
-    // An error makes the state final.
-    assert!(compat.state == CompatState::Dummy);
-
-    let some_unknown_access = unsafe { BitFlags::<AccessFs>::from_bits_unchecked(1 << 63 | 1) };
-    assert!(matches!(
-        some_unknown_access.try_compat(compat.abi(), compat.level, &mut compat.state).unwrap_err(),
-        CompatError::Access(AccessError::Unknown { access, unknown }) if access == some_unknown_access && unknown == all_unknown_access
-    ));
-    assert!(compat.state == CompatState::Dummy);
-
     compat = ABI::Unsupported.into();
 
     // Tests that the ruleset is marked as unsupported.

diff --git a/src/compat.rs b/src/compat.rs
@@ -15,7 +15,6 @@ use strum_macros::{EnumCount as EnumCountMacro, EnumIter};
 /// gets all the file system access rights defined by the first version.
 ///
 /// Without `ABI`, it would be hazardous to rely on the the full set of access flags
-/// (e.g., `BitFlags::<AccessFs>::all()` or `BitFlags::ALL`),
 /// a moving target that would change the semantics of your Landlock rule
 /// when migrating to a newer version of this crate.
 /// Indeed, a simple `cargo update` or `cargo install` run by any developer

diff --git a/src/errors.rs b/src/errors.rs
@@ -1,4 +1,4 @@
-use crate::{Access, AccessFs, AccessNet, BitFlags};
+use crate::{Access, AccessFs, AccessNet};
 use std::io;
 use std::path::PathBuf;
 use thiserror::Error;
@@ -84,10 +84,7 @@ where
     AddRuleCall { source: io::Error },
     /// The rule's access-rights are not all handled by the (requested) ruleset access-rights.
     #[error("access-rights not handled by the ruleset: {incompatible:?}")]
-    UnhandledAccess {
-        access: BitFlags<T>,
-        incompatible: BitFlags<T>,
-    },
+    UnhandledAccess { access: T, incompatible: T },
     #[error(transparent)]
     Compat(#[from] CompatError<T>),
 }
@@ -142,8 +139,8 @@ pub enum PathBeneathError {
     /// whereas the file descriptor doesn't point to a directory.
     #[error("incompatible directory-only access-rights: {incompatible:?}")]
     DirectoryAccess {
-        access: BitFlags<AccessFs>,
-        incompatible: BitFlags<AccessFs>,
+        access: AccessFs,
+        incompatible: AccessFs,
     },
 }
 
@@ -157,24 +154,14 @@ where
     /// kernel.
     #[error("empty access-right")]
     Empty,
-    /// The access-rights set was forged with the unsafe `BitFlags::from_bits_unchecked()` and it
-    /// contains unknown bits.
-    #[error("unknown access-rights (at build time): {unknown:?}")]
-    Unknown {
-        access: BitFlags<T>,
-        unknown: BitFlags<T>,
-    },
     /// The best-effort approach was (deliberately) disabled and the requested access-rights are
     /// fully incompatible with the running kernel.
     #[error("fully incompatible access-rights: {access:?}")]
-    Incompatible { access: BitFlags<T> },
+    Incompatible { access: T },
     /// The best-effort approach was (deliberately) disabled and the requested access-rights are
     /// partially incompatible with the running kernel.
     #[error("partially incompatible access-rights: {incompatible:?}")]
-    PartiallyCompatible {
-        access: BitFlags<T>,
-        incompatible: BitFlags<T>,
-    },
+    PartiallyCompatible { access: T, incompatible: T },
 }
 
 #[derive(Debug, Error)]