compat: Improve TryCompat trait type safety and fix consistency issue

It is possible to implement try_compat() in an inconsistent way compared
to other implementations.  Instead, implement the whole compatibility
logic in the default try_compat() implementation, and create tailored
method to be implemented by objects: try_compat_inner() and
try_compat_children().

The new CompatResult enum contains all the required informations to
implement a consistent try_compat() for all trait implementations: the
full, partial or not-supported state, the associated compatible value,
and the potential associated error.  This way, try_compat() has
everything to take the decision to either return a Some(Self) or a
CompatError according to the CompatLevel.

The try_compat() default implementation now always update the self's
CompatState, and then return a result according to its CompatLevel
thanks to clean and exhaustive CompatResult matches.

We now only use the Compatibility struct for Ruleset and RulesetCreated,
but we pass separately ABI, CompatLevel and &mut CompatState to
try_compat().  This enables to differentiate mutability and store
CompatState in all Compatible types.  ABI and CompatLevel should only
be inherited from Ruleset and RulesetCreated.

try_compat_inner() needs to be implemented by all TryCompat types.
This method only gets a non-mutable ABI, and returns a new CompatResult
or a CompatError.

Delegating nested TryCompat objects management (e.g. allowed_access by
PathBeneath) can lead to inconsistencies between different objects.  To
avoid this issue as much as possible, create a new try_compat_children()
method to specifically call try_compat() on all the nested objects, if
any.  For instance, PathBeneath objects contain an AccessFs object and
they both implement Compatible, which lets them have an independent
CompatLevel thanks to set_compatible().

It is not possible to call try_compat() from a try_compat_inner()
implementation because they don't return the same result type.  Instead,
the new try_compat_children() returns the same type as try_compat() and
can then call it for all the children.  try_compat_children() is only
called by try_compat(), which then makes all the compatibility process
consistent across different object implementations.

As a side effect, the PathBeneath implementation now first checks its
allowed_access generic compatibility (with try_compat_children), and
then the compatibility with the file type (with try_compat_inner).

To better fit with the Compatible contract, replace CompatLevel with
Option<CompatLevel>.  None is set by default until users change it.
This enables to inherit the parent compatibility level if the current
one is not set.  Add the TailoredCompatLevel trait with the
tailored_compat_level() method to get the current compatibility level
accordingly.  Automatically implements TailoredCompatLevel for each
Compatible implementation (only PathBeneath for now), which helps avoid
missing an implementation for a new Compatible types.

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

src/access.rs

@@ -1,11 +1,11 @@
 use crate::{
-    AccessError, AddRuleError, AddRulesError, BitFlags, CompatError, CompatLevel, CompatState,
-    Compatibility, HandleAccessError, HandleAccessesError, Ruleset, TryCompat, ABI,
+    AccessError, AddRuleError, AddRulesError, BitFlags, CompatError, CompatResult,
+    HandleAccessError, HandleAccessesError, Ruleset, TailoredCompatLevel, TryCompat, ABI,
 };
 use enumflags2::BitFlag;
 
 #[cfg(test)]
-use crate::{make_bitflags, AccessFs};
+use crate::{make_bitflags, AccessFs, CompatLevel, CompatState, Compatibility};
 
 pub trait Access: PrivateAccess {
     /// Gets the access rights defined by a specific [`ABI`].
@@ -61,57 +61,41 @@ fn bit_flags_full_negation() {
     assert_ne!(scoped_negation, full_negation(BitFlags::<AccessFs>::all()));
 }
 
-impl<T> TryCompat<T> for BitFlags<T>
+impl<A> TailoredCompatLevel for BitFlags<A> where A: Access {}
+
+impl<A> TryCompat<A> for BitFlags<A>
 where
-    T: Access,
+    A: Access,
 {
-    fn try_compat(self, compat: &mut Compatibility) -> Result<Option<Self>, CompatError<T>> {
-        let (state, ret) = if self.is_empty() {
+    fn try_compat_inner(self, abi: ABI) -> Result<CompatResult<Self, A>, CompatError<A>> {
+        if self.is_empty() {
             // Empty access-rights would result to a runtime error.
-            (CompatState::Dummy, Err(AccessError::Empty.into()))
+            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().
-            (
-                CompatState::Dummy,
-                Err(AccessError::Unknown {
-                    access: self,
-                    unknown: self & full_negation(Self::all()),
-                }
-                .into()),
-            )
+            Err(AccessError::Unknown {
+                access: self,
+                unknown: self & full_negation(Self::all()),
+            }
+            .into())
         } else {
-            let compat_bits = self & T::from_all(compat.abi());
-            if compat_bits.is_empty() {
-                match compat.level {
-                    // Empty access-rights are ignored to avoid an error when passing them to
-                    // landlock_add_rule().
-                    CompatLevel::BestEffort => (CompatState::No, Ok(None)),
-                    CompatLevel::SoftRequirement => (CompatState::Dummy, Ok(None)),
-                    CompatLevel::HardRequirement => (
-                        CompatState::Dummy,
-                        Err(AccessError::Incompatible { access: self }.into()),
-                    ),
-                }
-            } else if compat_bits != self {
-                match compat.level {
-                    CompatLevel::BestEffort => (CompatState::Partial, Ok(Some(compat_bits))),
-                    CompatLevel::SoftRequirement => (CompatState::Dummy, Ok(None)),
-                    CompatLevel::HardRequirement => (
-                        CompatState::Dummy,
-                        Err(AccessError::PartiallyCompatible {
-                            access: self,
-                            incompatible: self & full_negation(compat_bits),
-                        }
-                        .into()),
-                    ),
+            let compat = self & A::from_all(abi);
+            if compat.is_empty() {
+                Ok(CompatResult::No(
+                    AccessError::Incompatible { access: self }.into(),
+                ))
+            } else if compat != self {
+                let error = AccessError::PartiallyCompatible {
+                    access: self,
+                    incompatible: self & full_negation(compat),
                 }
+                .into();
+                Ok(CompatResult::Partial(compat, error))
             } else {
-                (CompatState::Full, Ok(Some(compat_bits)))
+                Ok(CompatResult::Full(self))
             }
-        };
-        compat.update(state);
-        ret
+        }
     }
 }
 
@@ -125,27 +109,32 @@ fn compat_bit_flags() {
     let ro_access = make_bitflags!(AccessFs::{Execute | ReadFile | ReadDir});
     assert_eq!(
         ro_access,
-        ro_access.try_compat(&mut compat).unwrap().unwrap()
+        ro_access
+            .try_compat(compat.abi(), compat.level, &mut compat.state)
+            .unwrap()
+            .unwrap()
     );
     assert!(compat.state == CompatState::Full);
 
     let empty_access = BitFlags::<AccessFs>::empty();
     assert!(matches!(
-        empty_access.try_compat(&mut compat).unwrap_err(),
+        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(&mut compat).unwrap_err(),
+        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(&mut compat).unwrap_err(),
+        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);
@@ -156,16 +145,49 @@ fn compat_bit_flags() {
     assert!(compat.state == CompatState::No);
 
     // 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());
+    assert_eq!(
+        None,
+        ro_access
+            .try_compat(compat.abi(), compat.level, &mut compat.state)
+            .unwrap()
+    );
 
     // Tests that the ruleset is in an unsupported state, which is important to be able to still
     // enforce no_new_privs.
     assert!(compat.state == CompatState::No);
 
     // Access-rights are not valid when they are required for the current ABI.
-    compat.level = CompatLevel::HardRequirement;
+    compat.level = Some(CompatLevel::HardRequirement);
     assert!(matches!(
-        ro_access.try_compat(&mut compat).unwrap_err(),
+        ro_access.try_compat(compat.abi(), compat.level, &mut compat.state).unwrap_err(),
         CompatError::Access(AccessError::Incompatible { access }) if access == ro_access
     ));
+
+    compat = ABI::V1.into();
+
+    // Tests that the ruleset is marked as the unknown compatibility state.
+    assert!(compat.state == CompatState::Init);
+
+    // Access-rights are valid (but ignored) when they are not required for the current ABI.
+    assert_eq!(
+        ro_access,
+        ro_access
+            .try_compat(compat.abi(), compat.level, &mut compat.state)
+            .unwrap()
+            .unwrap()
+    );
+
+    // Tests that the ruleset is in an unsupported state, which is important to be able to still
+    // enforce no_new_privs.
+    assert!(compat.state == CompatState::Full);
+
+    let v2_access = ro_access | AccessFs::Refer;
+
+    // Access-rights are not valid when they are required for the current ABI.
+    compat.level = Some(CompatLevel::HardRequirement);
+    assert!(matches!(
+        v2_access.try_compat(compat.abi(), compat.level, &mut compat.state).unwrap_err(),
+        CompatError::Access(AccessError::PartiallyCompatible { access, incompatible })
+            if access == v2_access && incompatible == AccessFs::Refer
+    ));
 }