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

Compatibility mode #58

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Compatibility mode #58

wants to merge 3 commits into from

Conversation

l0kod
Copy link
Member

@l0kod l0kod commented Feb 21, 2024

Update Ruleset::new() to take a CompatMode argument, and add the required dependencies to handle incompatibility consequences with if_unmet(), CompatAccess...

See rationale here: https://fosdem.org/2023/schedule/event/rust_backward_and_forward_compatibility_for_security_features/

Proposal:

  • add a new (alternative) constructor to Ruleset to be able to enforce a "hard requirement" for everything (set to "best effort" with the default() constructor: Use Ruleset::default() instead of Ruleset::new() #44);
  • add a method to access rights to be able to set the "soft requirement" property.

Example with current API:

Ruleset::new()
    .set_compatibility(CompatLevel::HardRequirement)
    .handle_access(AccessFs::from_all(ABI::V1))?
    .set_compatibility(CompatLevel::SoftRequirement)
    .handle_access(AccessFs::Refer)?
    .set_compatibility(CompatLevel::HardRequirement)

Same example with the proposed API:

Ruleset::new(CompatMode::ErrorIfUnmet) 
    .handle_access(AccessFs::from_all(ABI::V1))?
    .handle_access(AccessFs::Refer.if_unmet(Consequence::DisableRuleset))?

Example with the best effort behavior:

Ruleset::default() 
    .handle_access(AccessFs::from_all(ABI::V1))?
    .handle_access(AccessFs::Refer)?

This design is simpler and it removes the mutable builder property (multiple .set_compatibility() calls) that may be confusing and error prone.

I started working on that a while ago but the code is still WIP.

WIP: This implementation works by leveraging CompatLevel that may need
to be replaced.  We also need to remove the public set_compatibility().

The use of new() might change because of future access right groups
development.

Signed-off-by: Mickaël Salaün <mic@digikod.net>
WIP: Clean up the CompatLevel side and correctly handle PathBeneath's
compat state.

WIP: replacing disable_sandbox_if_unmet() with if_unmet(Consequence),
which handles ReturnError.  Use case appliances handling a minimal
kernel version that is known to support a specific set of Landlock
features, but that might use a newer kernel and then opportunistically
restrict processes furthermore.

This is equivalent to call disable_sandbox_if_unmet(true) is equivalent
to set_compatibility(CompatLevel::SoftRequirement) and
disable_sandbox_if_unmet(false) is equivalent to
set_compatibility(previous_compat_level).

Signed-off-by: Mickaël Salaün <mic@digikod.net>
TODO: Squash with previous commit that adds CompatArg

Signed-off-by: Mickaël Salaün <mic@digikod.net>
@l0kod l0kod added enhancement New feature or request help wanted Extra attention is needed labels Feb 21, 2024
@praveen-pk
Copy link

From usage stand point, it would be better to make ErroIfUnmet the default and BestEffort user selectable. If a use case needs maximum compatibility, developers can proactively choose BestEffort during development.

By making BestEffort default, users may get the false sense that the application is sandboxed, when it really isn't.

@l0kod
Copy link
Member Author

l0kod commented Feb 22, 2024

From usage stand point, it would be better to make ErroIfUnmet the default and BestEffort user selectable. If a use case needs maximum compatibility, developers can proactively choose BestEffort during development.

As explained in the documentation, the best-effort approach should be the default. The main point being that app developers don't know the kernel on which their app is running, and we want to protect users as much as possible.

By making BestEffort default, users may get the false sense that the application is sandboxed, when it really isn't.

I think most users trust (or hope) developers implement the best secure solution for their usage. Most of the time users would not get a false sense of security because they may not see any security message implying that.

This is different when developping and testing a sandboxed software where we can control the kernel version and make sure it works (and restrict) as expected. This is why it is still handy to be able to change this behavior at run (or start) time.

@praveen-pk
Copy link

(Sorry about the delay with my response, I missed your previous comment.)

From usage stand point, it would be better to make ErroIfUnmet the default and BestEffort user selectable. If a use case needs maximum compatibility, developers can proactively choose BestEffort during development.

As explained in the documentation, the best-effort approach should be the default. The main point being that app developers don't know the kernel on which their app is running, and we want to protect users as much as possible.

Generally, I agree that software should take best-effort security approach when deployed. My only concern is about what the default value configured for CompatMode.

As a concrete example, let's assume a developer built an application with ABI:V3 and set CompatMode to BestEffort. If the kernel on the user end only supports ABI:V1, Landlock doesn't limit LANDLOCK_ACCESS_FS_REFER and LANDLOCK_ACCESS_FS_TRUNCATE operations. In such a case, the end-user might get the false impression that his/her application is sufficiently sandboxed, when the application can still perform above operations.

If the default mode for CompatMode is set to ErroIfUnmet, the application will fail to run when started in the user end, as the underlying kernel doesn't support all the required features. Now the user can pass a --compat-mode=yes(like) option to the application to use whatever features are available. In this case user is fully aware that their application is running in CompatMode and that means the application isn't locked down with all the features it can be.

As I wrote this down, I started thinking it may be unreasonable to expect every application to add '--compat-mode' argument and pass appropriate flags to Landlock. This could be a reason to choose CompatMode as the default.

By making BestEffort default, users may get the false sense that the application is sandboxed, when it really isn't.

I think most users trust (or hope) developers implement the best secure solution for their usage. Most of the time users would not get a false sense of security because they may not see any security message implying that.

This is different when developping and testing a sandboxed software where we can control the kernel version and make sure it works (and restrict) as expected. This is why it is still handy to be able to change this behavior at run (or start) time.

Understood.

@l0kod
Copy link
Member Author

l0kod commented Mar 8, 2024

(Sorry about the delay with my response, I missed your previous comment.)

From usage stand point, it would be better to make ErroIfUnmet the default and BestEffort user selectable. If a use case needs maximum compatibility, developers can proactively choose BestEffort during development.

As explained in the documentation, the best-effort approach should be the default. The main point being that app developers don't know the kernel on which their app is running, and we want to protect users as much as possible.

Generally, I agree that software should take best-effort security approach when deployed. My only concern is about what the default value configured for CompatMode.

As a concrete example, let's assume a developer built an application with ABI:V3 and set CompatMode to BestEffort. If the kernel on the user end only supports ABI:V1, Landlock doesn't limit LANDLOCK_ACCESS_FS_REFER and LANDLOCK_ACCESS_FS_TRUNCATE operations. In such a case, the end-user might get the false impression that his/her application is sufficiently sandboxed, when the application can still perform above operations.

What is best? To protect users as much as possible (according to their running kernel), or to only protect users that are using the same kernel version as the developer?

Good sandboxing is transparent sandboxing, so users should not notice any difference. So getting a false impression of security would not make much sense is this cases. However, thanks to this crate's error codes, developers can still inform users that they should update their kernel to get a better protection.

If the default mode for CompatMode is set to ErroIfUnmet, the application will fail to run when started in the user end, as the underlying kernel doesn't support all the required features.

In this case, most users would just try to disable sandboxing to get a working application, and I understand them. For instance, we can look for the number of questions for how to disable SELinux because something is not working as expected.

Now the user can pass a --compat-mode=yes(like) option to the application to use whatever features are available. In this case user is fully aware that their application is running in CompatMode and that means the application isn't locked down with all the features it can be.

This is indeed one way to do it, but I prefer by default for things to just work, and optionally print a warning.

As I wrote this down, I started thinking it may be unreasonable to expect every application to add '--compat-mode' argument and pass appropriate flags to Landlock. This could be a reason to choose CompatMode as the default.

Indeed. We should make it simpler to protect most users by default.

By making BestEffort default, users may get the false sense that the application is sandboxed, when it really isn't.

I think most users trust (or hope) developers implement the best secure solution for their usage. Most of the time users would not get a false sense of security because they may not see any security message implying that.
This is different when developping and testing a sandboxed software where we can control the kernel version and make sure it works (and restrict) as expected. This is why it is still handy to be able to change this behavior at run (or start) time.

Understood.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants