Skip to content

Commit

Permalink
compat: Fix SoftRequirement consistency
Browse files Browse the repository at this point in the history 
Complete CompatState with the Init state to fully rely on CompatState
instead of ABI or is_mooted().  This enables to not change a ruleset ABI
once set.

Always update CompatState for any BitFlags<Access>::try_compat() call.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
  • Loading branch information
@l0kod
l0kod committed Jan 2, 2023
1 parent a4e3154 commit 250e84e
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 94 deletions.
62 changes: 36 additions & 26 deletions src/access.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,47 +66,52 @@ where
T: Access,
{
fn try_compat(self, compat: &mut Compatibility) -> Result<Option<Self>, CompatError<T>> {
let (state, new_access) = if self.is_empty() {
let (state, ret) = if self.is_empty() {
// Empty access-rights would result to a runtime error.
return Err(AccessError::Empty.into());
(CompatState::Dummy, 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().
return Err(AccessError::Unknown {
access: self,
unknown: self & full_negation(Self::all()),
}
.into());
(
CompatState::Dummy,
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, None),
CompatLevel::SoftRequirement => (CompatState::Dummy, None),
CompatLevel::HardRequirement => {
return Err(AccessError::Incompatible { access: self }.into());
}
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, Some(compat_bits)),
CompatLevel::SoftRequirement => (CompatState::Dummy, None),
CompatLevel::HardRequirement => {
return Err(AccessError::PartiallyCompatible {
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());
}
.into()),
),
}
} else {
(CompatState::Full, Some(compat_bits))
(CompatState::Full, Ok(Some(compat_bits)))
}
};
compat.update(state);
Ok(new_access)
ret
}
}

Expand All @@ -115,13 +120,14 @@ fn compat_bit_flags() {
use crate::ABI;

let mut compat: Compatibility = ABI::V1.into();
assert!(!compat.is_mooted());
assert!(compat.state() == CompatState::Init);

let ro_access = make_bitflags!(AccessFs::{Execute | ReadFile | ReadDir});
assert_eq!(
ro_access,
ro_access.try_compat(&mut compat).unwrap().unwrap()
);
assert!(compat.state() == CompatState::Full);

let empty_access = BitFlags::<AccessFs>::empty();
assert!(matches!(
Expand All @@ -134,23 +140,27 @@ fn compat_bit_flags() {
all_unknown_access.try_compat(&mut compat).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(),
CompatError::Access(AccessError::Unknown { access, unknown }) if access == some_unknown_access && unknown == all_unknown_access
));

assert!(!compat.is_mooted());
assert!(compat.state() == CompatState::Dummy);

compat = ABI::Unsupported.into();
assert!(!compat.is_mooted());

// Tests that the ruleset is marked as unsupported.
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());

// Tests that a ruleset in an unsupported state doesn't get spuriously mooted.
assert!(!compat.is_mooted());
// 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;
Expand Down
22 changes: 6 additions & 16 deletions src/compat.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ fn current_kernel_abi() {
#[cfg_attr(test, derive(Debug))]
#[derive(Copy, Clone, PartialEq)]
pub(crate) enum CompatState {
/// Initial undefined state.
Init,
/// All requested restrictions are enforced.
Full,
/// Some requested restrictions are enforced, following a best-effort approach.
Expand All @@ -161,6 +163,7 @@ pub(crate) enum CompatState {
impl CompatState {
fn update(&mut self, other: Self) {
*self = match (*self, other) {
(CompatState::Init, other) => other,
(CompatState::Dummy, _) => CompatState::Dummy,
(_, CompatState::Dummy) => CompatState::Dummy,
(CompatState::No, CompatState::No) => CompatState::No,
Expand Down Expand Up @@ -217,10 +220,6 @@ pub struct Compatibility {
abi: ABI,
pub(crate) level: CompatLevel,
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 {
Expand All @@ -229,11 +228,10 @@ impl From<ABI> for Compatibility {
abi,
level: CompatLevel::default(),
state: match abi {
// Forces the state as unsupported because all possible types will be useless.
ABI::Unsupported => CompatState::Dummy,
_ => CompatState::Full,
// Don't forces the state as Dummy because no_new_privs may still be legitimate.
ABI::Unsupported => CompatState::No,
_ => CompatState::Init,
},
is_mooted: false,
}
}
}
Expand All @@ -247,10 +245,6 @@ impl Compatibility {

pub(crate) fn update(&mut self, state: CompatState) {
self.state.update(state);
if state == CompatState::Dummy {
self.abi = ABI::Unsupported;
self.is_mooted = true;
}
}

pub(crate) fn abi(&self) -> ABI {
Expand All @@ -260,10 +254,6 @@ impl Compatibility {
pub(crate) fn state(&self) -> CompatState {
self.state
}

pub(crate) fn is_mooted(&self) -> bool {
self.is_mooted
}
}

/// Properly handles runtime unsupported features.
Expand Down
18 changes: 17 additions & 1 deletion src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ mod tests {
}

#[test]
fn abi_v2_refer() {
fn abi_v2_exec_refer() {
check_ruleset_support(
ABI::V1,
ABI::V2,
Expand All @@ -246,4 +246,20 @@ mod tests {
false,
);
}

#[test]
fn abi_v2_refer_only() {
// When no access is handled, do not try to create a ruleset without access.
check_ruleset_support(
ABI::V2,
ABI::V2,
|ruleset: Ruleset| -> _ {
Ok(ruleset
.handle_access(AccessFs::Refer)?
.create()?
.restrict_self()?)
},
false,
);
}
}
Loading

0 comments on commit 250e84e

Please sign in to comment.