Update error priority and no_new_privs behavior #67
Conversation
This file contains 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
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 <mic@digikod.net>
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 <mic@digikod.net>
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 <mic@digikod.net>
Merged
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.
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 (inrestrict_self()), even whenSoftRequirementwas 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 becausePR_SET_NO_NEW_PRIVSsupported by very old kernels. Anyway, this will be part of a release with a new major version.