compat: Switch from set_best_effort() to set_compatibility()

The set_compatibility() build method enables to set the compatibility
level of an object builder (e.g. Ruleset).

- set_compatibility(CompatLevel::BestEffort) is like
  set_best_effort(true).

- set_compatibility(CompatLevel::HardRequirement) is like
  set_best_effort(false).

- set_compatibility(CompatLevel::SoftRequirement) makes the ruleset moot
  if one of the following build requests (part of a build method) are
  not supported, but don't return a compatibility error.

CompatLevel::SoftRequirement is particularly useful for access rights
such as AccessFs::Refer.  Indeed, it can be used to ignore the whole
sandboxing if this could break the normal workflow of an application
(which legitimately requires to move files to arbitrary directories).

Signed-off-by: Mickaël Salaün <mic@digikod.net>

src/access.rs

@@ -1,6 +1,6 @@
 use crate::{
-    AccessError, AddRuleError, AddRulesError, BitFlags, CompatError, CompatState, Compatibility,
-    HandleAccessError, HandleAccessesError, Ruleset, TryCompat, ABI,
+    AccessError, AddRuleError, AddRulesError, BitFlags, CompatError, CompatLevel, CompatState,
+    Compatibility, HandleAccessError, HandleAccessesError, Ruleset, TryCompat, ABI,
 };
 use enumflags2::BitFlag;
 
@@ -65,7 +65,7 @@ impl<T> TryCompat<T> for BitFlags<T>
 where
     T: Access,
 {
-    fn try_compat(self, compat: &mut Compatibility) -> Result<Self, CompatError<T>> {
+    fn try_compat(self, compat: &mut Compatibility) -> Result<Option<Self>, CompatError<T>> {
         let (state, new_access) = if self.is_empty() {
             // Empty access-rights would result to a runtime error.
             return Err(AccessError::Empty.into());
@@ -80,29 +80,32 @@ where
         } else {
             let compat_bits = self & T::from_all(compat.abi);
             if compat_bits.is_empty() {
-                if compat.is_best_effort {
-                    // TODO: This creates an empty access-right and could be an issue with
-                    // future ABIs.  This method should return Result<Option<Self>,
-                    // CompatError> instead, and in this case Ok(None).
-                    (CompatState::No, compat_bits)
-                } else {
-                    return Err(AccessError::Incompatible { access: self }.into());
+                match compat.level {
+                    // Empty access-rights are ignored to avoid an error when passing them to
+                    // landlock_add_rule().
+                    CompatLevel::BestEffort => (CompatState::No, None),
+                    CompatLevel::SoftRequirement => (CompatState::Final, None),
+                    CompatLevel::HardRequirement => {
+                        return Err(AccessError::Incompatible { access: self }.into());
+                    }
                 }
             } else if compat_bits != self {
-                if compat.is_best_effort {
-                    (CompatState::Partial, compat_bits)
-                } else {
-                    return Err(AccessError::PartiallyCompatible {
-                        access: self,
-                        incompatible: self & full_negation(compat_bits),
+                match compat.level {
+                    CompatLevel::BestEffort => (CompatState::Partial, Some(compat_bits)),
+                    CompatLevel::SoftRequirement => (CompatState::Final, None),
+                    CompatLevel::HardRequirement => {
+                        return Err(AccessError::PartiallyCompatible {
+                            access: self,
+                            incompatible: self & full_negation(compat_bits),
+                        }
+                        .into());
                     }
-                    .into());
                 }
             } else {
-                (CompatState::Full, compat_bits)
+                (CompatState::Full, Some(compat_bits))
             }
         };
-        compat.state.update(state);
+        compat.update(state);
         Ok(new_access)
     }
 }
@@ -111,10 +114,14 @@ where
 fn compat_bit_flags() {
     use crate::ABI;
 
-    let mut compat = ABI::V1.into();
+    let mut compat: Compatibility = ABI::V1.into();
+    assert!(!compat.is_mooted());
 
     let ro_access = make_bitflags!(AccessFs::{Execute | ReadFile | ReadDir});
-    assert_eq!(ro_access, ro_access.try_compat(&mut compat).unwrap());
+    assert_eq!(
+        ro_access,
+        ro_access.try_compat(&mut compat).unwrap().unwrap()
+    );
 
     let empty_access = BitFlags::<AccessFs>::empty();
     assert!(matches!(
@@ -134,12 +141,19 @@ fn compat_bit_flags() {
         CompatError::Access(AccessError::Unknown { access, unknown }) if access == some_unknown_access && unknown == all_unknown_access
     ));
 
-    // Access-rights are valid (but ignored) when they are not required for the current ABI.
+    assert!(!compat.is_mooted());
+
     compat.abi = ABI::Unsupported;
-    assert_eq!(empty_access, ro_access.try_compat(&mut compat).unwrap());
+    assert!(!compat.is_mooted());
+
+    // Access-rights are valid (but ignored) when they are not required for the current ABI.
+    assert_eq!(None, ro_access.try_compat(&mut compat).unwrap());
+
+    // Tests that a ruleset in an unsupported state doesn't get spuriously mooted.
+    assert!(!compat.is_mooted());
 
     // Access-rights are not valid when they are required for the current ABI.
-    compat.is_best_effort = false;
+    compat.level = CompatLevel::HardRequirement;
     assert!(matches!(
         ro_access.try_compat(&mut compat).unwrap_err(),
         CompatError::Access(AccessError::Incompatible { access }) if access == ro_access

src/compat.rs

@@ -145,8 +145,8 @@ fn current_kernel_abi() {
 }
 
 /// Returned by ruleset builder.
-#[cfg_attr(test, derive(Debug, PartialEq))]
-#[derive(Copy, Clone)]
+#[cfg_attr(test, derive(Debug))]
+#[derive(Copy, Clone, PartialEq)]
 pub(crate) enum CompatState {
     /// All requested restrictions are enforced.
     Full,
@@ -159,7 +159,7 @@ pub(crate) enum CompatState {
 }
 
 impl CompatState {
-    pub(crate) fn update(&mut self, other: Self) {
+    fn update(&mut self, other: Self) {
         *self = match (*self, other) {
             (CompatState::Final, _) => CompatState::Final,
             (_, CompatState::Final) => CompatState::Final,
@@ -215,30 +215,47 @@ fn compat_state_update_2() {
 // Compatibility is not public outside this crate.
 pub struct Compatibility {
     pub(crate) abi: ABI,
-    pub(crate) is_best_effort: bool,
+    pub(crate) level: CompatLevel,
     pub(crate) state: CompatState,
+    // is_mooted is required to differenciate a kernel not supporting Landlock from an error that
+    // occured with CompatLevel::SoftRequirement.  is_mooted is only changed with update() and only
+    // used to not set no_new_privs in RulesetCreated::restrict_self().
+    is_mooted: bool,
 }
 
 impl From<ABI> for Compatibility {
     fn from(abi: ABI) -> Self {
         Compatibility {
             abi,
-            is_best_effort: true,
+            level: CompatLevel::default(),
             state: match abi {
                 // Forces the state as unsupported because all possible types will be useless.
                 ABI::Unsupported => CompatState::Final,
                 _ => CompatState::Full,
             },
+            is_mooted: false,
         }
     }
 }
 
 impl Compatibility {
     // Compatibility is an opaque struct.
     #[allow(clippy::new_without_default)]
-    pub fn new() -> Self {
+    pub(crate) fn new() -> Self {
         ABI::new_current().into()
     }
+
+    pub(crate) fn update(&mut self, state: CompatState) {
+        self.state.update(state);
+        if state == CompatState::Final {
+            self.abi = ABI::Unsupported;
+            self.is_mooted = true;
+        }
+    }
+
+    pub(crate) fn is_mooted(&self) -> bool {
+        self.is_mooted
+    }
 }
 
 /// Properly handles runtime unsupported features.
@@ -264,50 +281,101 @@ pub trait Compatible {
     /// However, on some rare circumstances,
     /// developers may want to have some guarantees that their applications
     /// will not run if a certain level of sandboxing is not possible.
-    /// If you really want to error out when not all your requested requirements are met,
-    /// then you can configure it with `set_best_effort(false)`.
+    /// If we really want to error out when not all our requested requirements are met,
+    /// then we can configure it with `set_compatibility()`.
+    ///
+    /// The `Compatible` trait is implemented for all object builders
+    /// (e.g. [`Ruleset`](crate::Ruleset)).
+    /// Such builders have a set of methods to incrementally build an object.
+    /// These build methods rely on kernel features that may not be available at runtime.
+    /// The `set_compatibility()` method enables to control the effect of
+    /// the following build method calls starting from this call.
+    /// Such effect can be:
+    /// * to silently ignore unsupported features
+    ///   and continue building ([`CompatLevel::BestEffort`]);
+    /// * to silently ignore unsupported features
+    ///   and ignore the whole build ([`CompatLevel::SoftRequirement`]);
+    /// * to return an error for any unsupported feature ([`CompatLevel::HardRequirement`]).
+    ///
+    /// Taking [`Ruleset`](crate::Ruleset) as an example,
+    /// the [`handle_access()`](crate::RulesetAttr::handle_access()) build method
+    /// returns a [`Result`] that can be [`Err(RulesetError)`](crate::RulesetError)
+    /// with a nested [`CompatError`].
+    /// Such error can only occur with a running Linux kernel not supporting the requested
+    /// Landlock accesses *and* if the current compatibility level is
+    /// [`CompatLevel::HardRequirement`].
+    /// However, such error is not possible with [`CompatLevel::BestEffort`]
+    /// nor [`CompatLevel::SoftRequirement`].
     ///
     /// The order of this call is important because
-    /// it defines the behavior of the following method calls that return a [`Result`].
-    /// If `set_best_effort(false)` is called on an object,
+    /// it defines the behavior of the following build method calls that return a [`Result`].
+    /// If `set_compatibility(CompatLevel::HardRequirement)` is called on an object,
     /// then a [`CompatError`] may be returned for the next method calls,
-    /// until the next call to `set_best_effort(true)`.
-    /// This enables to change the behavior of a set of build calls,
+    /// until the next call to `set_compatibility()`.
+    /// This enables to change the behavior of a set of build method calls,
     /// for instance to be sure that the sandbox will at least restrict some access rights.
     ///
+    /// New objects inherit the compatibility configuration of their parents, if any.
+    /// For instance, [`Ruleset::create()`](crate::Ruleset::create()) returns
+    /// a [`RulesetCreated`](crate::RulesetCreated) object that inherits the
+    /// `Ruleset`'s compatibility configuration.
+    ///
     /// # Example
     ///
-    /// Create a ruleset which will at least support execution constraints.
+    /// Create a ruleset which will at least support all restrictions provided by
+    /// the [first version of Landlock](ABI::V1), and may also support the
+    /// [`AccessFs::Refer`](crate::AccessFs::Refer) restriction according to the running kernel.
     ///
     /// ```
     /// use landlock::{
-    ///     Access, AccessFs, Compatible, PathBeneath, Ruleset, RulesetAttr, RulesetCreated, RulesetError,
-    ///     ABI,
+    ///     ABI, Access, AccessFs, CompatLevel, Compatible, PathBeneath, Ruleset, RulesetAttr,
+    ///     RulesetCreated, RulesetError,
     /// };
     ///
     /// fn ruleset_fragile() -> Result<RulesetCreated, RulesetError> {
     ///     Ok(Ruleset::new()
-    ///         // This ruleset must handle at least the execute access.
-    ///         .set_best_effort(false)
-    ///         // This handle_access() call will return
-    ///         // a wrapped AccessError<AccessFs>::Incompatible error
-    ///         // if the running kernel can't handle AccessFs::Execute.
-    ///         .handle_access(AccessFs::Execute)?
-    ///         // This ruleset may also handle other access rights
-    ///         // if they are supported by the running kernel.
-    ///         // Because handle_access() replaces the previously set value,
-    ///         // the new value must be a superset of AccessFs::Execute.
-    ///         .set_best_effort(true)
+    ///         // This ruleset must handle at least all accesses defined by
+    ///         // the first Landlock version (e.g. AccessFs::WriteFile).
+    ///         .set_compatibility(CompatLevel::HardRequirement)
+    ///         // This handle_access() call may now return a wrapped
+    ///         // AccessError<AccessFs>::Incompatible error if Landlock
+    ///         // is not supported by the running kernel.
     ///         .handle_access(AccessFs::from_all(ABI::V1))?
+    ///         // This ruleset may also handle the AccessFs::Refer right (defined by
+    ///         // the second version of Landlock) if it is supported by the running kernel.
+    ///         .set_compatibility(CompatLevel::BestEffort)
+    ///         // The following handle_access() calls will now never return an error.
+    ///         .handle_access(AccessFs::Refer)?
     ///         .create()?)
     /// }
     /// ```
-    fn set_best_effort(self, best_effort: bool) -> Self;
+    fn set_compatibility(self, level: CompatLevel) -> Self;
+}
+
+/// See the [`Compatible`] documentation.
+#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
+pub enum CompatLevel {
+    /// Takes into account the build requests if they are supported by the running system,
+    /// or silently ignores them otherwise.
+    /// Never returns a compatibility error.
+    #[default]
+    BestEffort,
+    /// Takes into account the build requests if they are supported by the running system,
+    /// or silently ignores the whole build object otherwise.
+    /// Never returns a compatibility error.
+    /// If not supported,
+    /// the call to [`RulesetCreated::restrict_self()`](crate::RulesetCreated::restrict_self())
+    /// will return a
+    /// [`RestrictionStatus { ruleset: RulesetStatus::NotEnforced, no_new_privs: false, }`](crate::RestrictionStatus).
+    SoftRequirement,
+    /// Takes into account the build requests if they are supported by the running system,
+    /// or returns a compatibility error otherwise ([`CompatError`]).
+    HardRequirement,
 }
 
 // TryCompat is not public outside this crate.
 pub trait TryCompat<T> {
-    fn try_compat(self, compat: &mut Compatibility) -> Result<Self, CompatError<T>>
+    fn try_compat(self, compat: &mut Compatibility) -> Result<Option<Self>, CompatError<T>>
     where
         Self: Sized,
         T: Access;