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

Add option to query for Landlock support #43

Open
cd-work opened this issue Aug 25, 2023 · 11 comments
Open

Add option to query for Landlock support #43

cd-work opened this issue Aug 25, 2023 · 11 comments
Labels
enhancement New feature or request

Comments

@cd-work
Copy link
Contributor

cd-work commented Aug 25, 2023

I don't think the current rust-landlock crate has a way to query for landlock support, however even after #12 this might still be desirable.

Some applications might want to give users the choice on how they would like to handle missing support for specific landlock versions, without having to create a full ruleset, so it would be good to query ahead of time if such a prompt can be skipped entirely because everything required is already supported.

@l0kod
Copy link
Member

l0kod commented Aug 31, 2023

Which kind of applications and use cases do you have in mind?

@l0kod
Copy link
Member

l0kod commented Aug 31, 2023

This might be related to #36

@cd-work
Copy link
Contributor Author

cd-work commented Aug 31, 2023

Which kind of applications and use cases do you have in mind?

I was just playing around with the idea to print warnings if sandboxing wasn't supported or if specific features weren't part of the current kernel. With a forced sandbox that does not support FS_REFER it would be difficult to handle the failure and tell the user why it failed after when sandboxing third-party applications. But checking for landlock support first and then warning them that some operations might not be supported on their kernel could be helpful.

That said, I'm probably going for an alternative solution with my specific problem anyway, so I don't think I will need this in the near future (which is why I didn't open a PR).

However this seems like a pretty simple thing to implement and it definitely feels like something that might be useful to people, even if it's just for debugging purposes (logging supported landlock version at startup for example).

#36 seems both very similar and slightly different. I can see #36 getting solved by implementing this, in which case you can feel free to close this. But solutions for #36 don't necessarily also solve this issue.

@cd-work
Copy link
Contributor Author

cd-work commented Sep 1, 2023

I just saw that ABI already has a new_current function that is used internally. If that was made public, that would be sufficient to implement this.

Alternatively if we wanted to avoid having to call this method multiple times, another option would be to retrieve ABI version from Ruleset::new. So people could create Ruleset::new just to make the landlock version call, then decide to create the sandbox or not.

@l0kod
Copy link
Member

l0kod commented Sep 11, 2023

I'd like to avoid exposing the current ABI because I'm convinced that would lead developers to do their own version check and miss things or not sandboxing as best as possible. Instead, I'd prefer this library to manage the complexity.

According to phylum-dev/birdcage#41, I think existing features should be enough, but you might indeed want to warn users to update their kernels to be better protected. This is why #36 looks like a good first step. We might want a way to get the minimal and maximum supported ABI too, but a string representation should be enough.

@cd-work
Copy link
Contributor Author

cd-work commented Sep 12, 2023

I just feel like it would be better to provide a predictable way to retrieve this in the landlock crate, rather than people inevitably trying to satisfy the requirement for it by calling straight to the C functions.

phylum-dev/birdcage#41 doesn't really require it in its current form, but previous iterations did rely on it for providing better user feedback.

@l0kod l0kod added the enhancement New feature or request label Feb 21, 2024
@praveen-pk
Copy link

praveen-pk commented Mar 7, 2024

I am implementing this as below:

fn get_supported_abi() -> i32 {
    for abi in ABI::iter().skip(1).rev() {
        let rs = Ruleset::default().handle_access(AccessFs::from_all(abi));
        if rs.is_err() {
            return -1;
        }
        let create_rs = rs.unwrap().create();
        match create_rs {
            Ok(_) => return abi as i32,
            Err(_) => continue,
        }
    }
    return -1;
}

This will return -1 if landlock is not supported on the host. Else, this method will return the ABI version itself.

Thoughts/comments on the approach?

I am working on adding a Unit test, with some mocking. If no objections on the approach, I will push a PR in the next few days.

@l0kod
Copy link
Member

l0kod commented Mar 8, 2024

We don't need any unwrap() call. Using match or if let instead would be better.
We should leverage typing by returning a proper error type instead of -1.
The private ABI::new_current() already returns the ABI version.

I don't really see the point of getting the ABI version though. A good library should handle the compatibility itself rather that pushing this work to the developer.

In the end, developers may want to know if an environment supports a set of tested restrictions. Getting the ABI version is just a means to an end right?

However, it might be useful to get the minimal version required to support a set of restrictions, which would be an improvement of #36.

@praveen-pk
Copy link

We don't need any unwrap() call. Using match or if let instead would be better. We should leverage typing by returning a proper error type instead of -1.

Sure will add proper Error case

The private ABI::new_current() already returns the ABI version.

I see, I can directly use that and return without iterating thorugh all the ABI versions.

I don't really see the point of getting the ABI version though. A good library should handle the compatibility itself rather that pushing this work to the developer.

In the end, developers may want to know if an environment supports a set of tested restrictions. Getting the ABI version is just a means to an end right?

Exactly. In case developers do have such a requirement, they have to make another call to get the ABI version. I wanted to address both the below queries in one shot:

  1. Is landlock supported
  2. What is the ABI version supported?

However, it might be useful to get the minimal version required to support a set of restrictions, which would be an improvement of #36.

+1

@praveen-pk
Copy link

@l0kod do we need a method to check if Landlock is supported in the host OR a method to check if a set of restrictions are supported in the? I am going for the former.

As the mapping from ABI version to restrictions is well defined, I am thinking it is better to allow users to query the ABI version alone. Any queries against what restrictions are supported could get a bit complicated to implement.

@l0kod
Copy link
Member

l0kod commented Apr 2, 2024

(I thought I had replied but it seems not)

As the mapping from ABI version to restrictions is well defined, I am thinking it is better to allow users to query the ABI version alone. Any queries against what restrictions are supported could get a bit complicated to implement.

The mapping is well defined but asking developers to implement this logic in their application instead of providing it would not make sense for a library. We then need to provide this mapping and make it easy to use and useful. The simplest way I can think of is to use (almost) the same build calls as to currently build a sandbox. For instance, we could replace the call to restrict_self() with a call to status() or supported():

// Gets the current status according to the running kernel (without enforcing anything).
RulesetCreated::status() -> RulesetStatus {}

// Gets all potential status, which may be useful for developers to know which minimal ABI is required.
RulesetCreated::supported() -> RulesetSupported {}

struct RulesetSupported {
    full: ABI,
    partial: ABI
}

impl ABI {
    pub kernel(&self) -> &str {}
}

See RulesetStatus returned by restrict_self().

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

No branches or pull requests

3 participants