landlock: fix disable() of FS protections; reject disable(FsRefer)#90
Open
congwang-mk wants to merge 1 commit into
Open
landlock: fix disable() of FS protections; reject disable(FsRefer)#90congwang-mk wants to merge 1 commit into
congwang-mk wants to merge 1 commit into
Conversation
Signed-off-by: Cong Wang <cwang@multikernel.io>
Contributor
Author
|
@dzerik Please take a look |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the protection opt-out work merged in #71, addressing the blocking review comment about
disable()of FS protections (#71 (comment)).1. Blocking bug:
disable()of an FS protection fails with EINVAL when the sandbox has a writable pathcompute_fs_maskremoves the disabled bit (REFER / TRUNCATE / IOCTL_DEV) fromhandled_access_fs, but the per-path write mask was derived fromwrite_access(abi)alone. So withdisable(FsTruncate)(orFsIoctlDev, or the now-rejectedFsRefer) plus any writable path, the writable-path rule still requested the dropped bit, it was no longer a subset of the ruleset's handled accesses, andlandlock_add_rule(2)rejected it with EINVAL. That broke nearly every real sandbox (any with a writable path). The net path was already gated this way vianet_tcp_active; the FS path did not get the matching treatment.Fix (
landlock.rs): intersect the per-path write mask with the resolved handled set, so every installed rule is a subset by construction.This covers both the directory rule (
access) and the file rule (access & ACCESS_FILE).Regression test: a forked
confine_filesystemagainst the real host kernel withdisable(FsTruncate)/disable(FsIoctlDev)plusfs_write("/tmp"), asserting clean confinement. It fails (EINVAL) before the fix and passes after. The existingmask_contract_testscould not catch this class because they exercisecompute_fs_maskin isolation and never install a path rule.2.
disable(FsRefer)is rejected at buildPer
landlock(7), REFER is denied by default by every ruleset even when it is not handled. Controlled cross-directory rename within writable areas works precisely because REFER is handled and granted on writable paths. Disabling REFER un-handles it, which can only make the sandbox stricter, never looser, so it cannot do whatdisable()promises and is a footgun.build()andbuild_unchecked()now returnSandboxError::Invalidfordisable(Protection::FsRefer), surfaced uniformly through the Rust, C ABI, and Python bindings.allow_degraded(FsRefer)stays meaningful and is unchanged. Thedisable()rustdoc and the--disableCLI help carry the caveat.Validation
Full workspace green on a v7 host: 334 core lib + 242 core integration + 70 FFI/CLI, plus 12 Python protection tests. New tests:
disable_fs_protection_with_writable_path_does_not_einvalanddisable_fsrefer_is_rejected_at_build.