Repository navigation
core: use landlock crate - #305
Conversation
| )) | ||
| })?; | ||
| let status = ruleset.restrict_self()?; | ||
| if status.ruleset != RulesetStatus::FullyEnforced { |
There was a problem hiding this comment.
This check is load-bearing, not a sanity check: any try_compat failure under HardRequirement makes the crate state Dummy permanently, after which add_rule and restrict_self return Ok without syscalls (NotEnforced). The ignored let _ = add_rule(...) calls rely on this guard to fail closed. Please add a comment saying so, ideally a test too.
There was a problem hiding this comment.
Good catch! But tbf, API-wise, it's a bit nasty to first return an error, then return Ok for the consequent calls. Let's still have this as a sanity-check but I'd say let's not ignore the errors on add_rule. Right now, we already check whether the path exists - and successfully opened - by checking the result of path_beneath_rule. After that, we should not ignore the result of add_rule since it'd be catched in the FullyEnforced check anyways.
There was a problem hiding this comment.
I left this as is impl-wise. Added a comment on the behaviour and a test that covers it.
| } else { | ||
| LANDLOCK_ACCESS_NET_BIND_TCP | LANDLOCK_ACCESS_NET_CONNECT_TCP | ||
| }; | ||
| let mut mask = AccessNet::from_all(abi); |
There was a problem hiding this comment.
from_all(abi) silently widens with crate bumps. If a future AccessNet gains e.g. UDP, it becomes handled with no rules, denying all UDP. Prefer explicit BindTcp | ConnectTcp, or add exact-mask tests at ABI::V9 (same for AccessFs::from_all in compute_fs_mask).
There was a problem hiding this comment.
Done. Both places are now explicit.
| /// Minimum Landlock ABI version the host kernel must support for | ||
| /// this protection to be available. | ||
| pub const fn min_abi(self) -> u32 { | ||
| pub const fn min_abi(self) -> landlock::ABI { |
There was a problem hiding this comment.
Exposing landlock::ABI (here, ProtectionStatus::resolve, ProtectionUnavailable fields, compute_*_mask return types) makes every landlock crate bump a sandlock-core breaking change, and the CLI/FFI/tests now cast back to u32. Suggest keeping u32 publicly and converting inside confine_inner/active_protections.
There was a problem hiding this comment.
Done, now only converting to ABI when we have to to use the landlock crate.
| SandlockError::Runtime(crate::error::SandboxRuntimeError::Confinement(e)) | ||
| })?; | ||
|
|
||
| ruleset = ruleset.add_rule( |
There was a problem hiding this comment.
Kernel-side add_rule failures used to report add path rule for {path} / add net rule for port {port}; now the bare RulesetError loses the path/port. Please re-wrap with that context (also the net rules below).
There was a problem hiding this comment.
Done by adding the add_net_rule and add_path_beneath_rule helpers.
| ConfinementError::Landlock(format!("restrict_self: {}", e)), | ||
| )) | ||
| })?; | ||
| let status = ruleset.restrict_self()?; |
There was a problem hiding this comment.
Nit: the crate sets PR_SET_NO_NEW_PRIVS by default here, but every caller already does. .no_new_privs(false) would keep that responsibility in one place.
There was a problem hiding this comment.
Done, now using no_new_privs(false) and explicitly mentioning it in the public function warning the callers.
| if supervised && shadowed_files().any(|f| f.starts_with(path)) { | ||
| add_read_rules_around_shadowed(&ruleset_fd, path).map_err(|e| { | ||
| ruleset = add_read_rules_around_shadowed(ruleset, path).map_err(|e| { | ||
| SandlockError::Runtime(crate::error::SandboxRuntimeError::Confinement(e)) |
There was a problem hiding this comment.
Three error-conversion paths now: From<RulesetError> for SandlockError, cast_confinement, and this inline closure. Pick one.
There was a problem hiding this comment.
Fixed by only using From<ConfinementError> for SandlockError. It helps with not manually wrapping SandlockError::Runtime all the time.
7fb1aad to
7194982
Compare
Instead of writing our own landlock wrapper, we now use `landlock` crate. This will ease up handling of the landlock ops on multiple kernel versions. Signed-off-by: kernelnomad <kernelnomad@proton.me>
7194982 to
643cb09
Compare
Instead of writing our own landlock wrapper, we now use the
landlockcrate. This will ease up handling of the landlock ops on multiple kernel versions.Fixes #286