Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update error priority and no_new_privs behavior #67

Merged
merged 3 commits into from
Jun 3, 2024

Conversation

l0kod
Copy link
Member

@l0kod l0kod commented Jun 3, 2024

This set of changes fix and improve error management. This is also a requirement to properly check port overflow in #55.

This also changes the behavior of prctl(PR_SET_NO_NEW_PRIVS, 1), which is now always called (in restrict_self()), even when SoftRequirement was explicitly set and the ruleset was made moot (because of the running kernel not supporting a feature). This should not be an issue in practice because PR_SET_NO_NEW_PRIVS supported by very old kernels. Anyway, this will be part of a release with a new major version.

l0kod added 3 commits June 3, 2024 14:43
When adding a rule to a ruleset created with ABI::Unsupported, its
compatibility state could change from CompatLevel::No to
CompatLevel::Partial, which is wrong and could lead to an unexpected
error when trying to restrict the caller with -1 as a ruleset file
descriptor.  This is especially visible with a following change to
prioritize error over incompatibility.

Fix that by setting the mapping of ABI::Unsupported to
CompatLevel::Dummy instead of CompatLevel::No.

As a side effect, prctl(PR_SET_NO_NEW_PRIVS, 1) is now always called
when no_new_privs is set to true.  This was not the case before when the
ruleset's compatibility level was explicitly set to SoftRequirement.
This means that no RestrictSelfError::SetNoNewPrivsCall can now be
returned in this specific case.

The rationale is that no_new_privs was introduced with Linux 3.5 whereas
the oldest supported kernel is now Linux 4.19 .  It is not worth it to
handle compatibility complexity for such widely available feature.
Moreover, this new behavior lead to a more deterministic and secure
execution, and no error should be returned by the kernel.

Add tests to check theses changes, and change some existing tests
to follow the new logic.

Signed-off-by: Mickaël Salaün <[email protected]>
Add path_beneath_try_compat_children() tests to check error ordering
with both children and inner errors.  This enables us to identify the
behavior change introduced with the upcoming error priority change.

Signed-off-by: Mickaël Salaün <[email protected]>
Reorder TryCompat::try_compat() checks to first call try_compat_inner()
and then also always call try_compat_children().  This ensures that
inner and children potential errors are always returned even if the
current object is incompatible.

For instance, this makes NetPort (see a following commit) able to check
port overflow even when the running kernel does not support ABI::V4.

Update the path_beneath_try_compat_children() test to reflect the error
ordering change:
- previously with try_compat_children() as first check and then
  try_compat_inner(),
- now with try_compat_inner() as first check and then
  try_compat_children().

This is now more consistent because the inner error is always returned
whatever there is a compatibility error or not.

Signed-off-by: Mickaël Salaün <[email protected]>
@l0kod l0kod merged commit ccb5a58 into landlock-lsm:main Jun 3, 2024
11 checks passed
@l0kod l0kod deleted the error-priority branch June 3, 2024 12:52
@l0kod l0kod mentioned this pull request Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant