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

Rules deny listing #28

Open
nbouchinet-anssi opened this issue Mar 22, 2024 · 7 comments
Open

Rules deny listing #28

nbouchinet-anssi opened this issue Mar 22, 2024 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@nbouchinet-anssi
Copy link

The construction of a Landlock ruleset is based on an authorization list model. Thus, in order to sandbox an application, all necessary resources and their associated permissions must be first established (directories being recursively included).

It would be great to be able to deny itself access to specific resources (i.e. Give a RO access to / but deny access to /home).

Another benefit of this approach would be ruleset refining, in other word, dropping an access rule after ruleset enforcement.
To achieve this goal with the current API, the original ruleset contents and restrictions have to be maintained aside (i.e. an array of structure containing paths and their restrictions).
Once the task want to refines it restrictions, it can evict it from the maintained array, build back a ruleset and enforce it.
Maintaining such a data structure is cumbersome and would be made much easier by rule deny listing. (see #25)

For a first implementation, I think every Landlocks filesystem actions could be denied to the evicted resource.
Extra care should be given in order to avoid sandbox escapes, i.e. renaming a file in order to access it.
A deny rule should be applied only on resources already authorized in a Landlock ruleset.

@l0kod do you think of anything to add ?

@l0kod
Copy link
Member

l0kod commented Mar 22, 2024

The ability to exclude some access rights on file hierarchies in a ruleset would be a very welcome feature! Even if the current deny-by-default approach is good, being able to create deny lists mixed with allowed lists would definitely be useful in practice for users (e.g. deny access to ~/.ssh).

For a first implementation, I think every Landlocks filesystem actions could be denied to the evicted resource.

It should not be much more complex to implement this approach per access right. The interface could be an extension of struct landlock_path_beneath_attr adding a new denied_access field.

Extra care should be given in order to avoid sandbox escapes, i.e. renaming a file in order to access it.

Indeed, it should always be denied to rename/move a file hierarchy containing any inode with a denied_access set. This means that we probably need to (automatically) create "taint" rules for all parents of such inode/dentry (in the same mount point). See past discussion with @thejh.

A deny rule should be applied only on resources already authorized in a Landlock ruleset.

I guess you mean on handled accesses.

@l0kod l0kod added the enhancement New feature or request label Mar 22, 2024
@l0kod l0kod added this to Landlock Mar 22, 2024
@l0kod l0kod moved this to Ready in Landlock Mar 22, 2024
@nbouchinet-anssi
Copy link
Author

For a first implementation, I think every Landlocks filesystem actions could be denied to the evicted resource.

It should not be much more complex to implement this approach per access right. The interface could be an extension of struct landlock_path_beneath_attr adding a new denied_access field.

ACK.

Extra care should be given in order to avoid sandbox escapes, i.e. renaming a file in order to access it.

Indeed, it should always be denied to rename/move a file hierarchy containing any inode with a denied_access set. This means that we probably need to (automatically) create "taint" rules for all parents of such inode/dentry (in the same mount point). See past discussion with @thejh.

Thanks, will take a look at it.

A deny rule should be applied only on resources already authorized in a Landlock ruleset.

I guess you mean on handled accesses.

What I meant is that denying specific access right to a path should only be possible if the path already has been authorized in a allowed rule (at least on of the upper directories).

i.e. If one want to deny access to its /home/<username>/.ssh one of the upper directories should already have been allowed and enforced, either /, /home/ or /home/<username>/ for example.

Or do you see it more in a way where everything is allowed by default except denied resources for the first ruleset enforcement ?

@l0kod
Copy link
Member

l0kod commented Mar 27, 2024

What I meant is that denying specific access right to a path should only be possible if the path already has been authorized in a allowed rule (at least on of the upper directories).

i.e. If one want to deny access to its /home/<username>/.ssh one of the upper directories should already have been allowed and enforced, either /, /home/ or /home/<username>/ for example.

OK, I get it, and that could indeed make sense for users. However, one thing to keep in mind is that this kind of property may be difficult to check because rules are not directly tied together but only to inodes (not paths nor dentries): file hierarchies are only checked when an access is requested. Moreover, a file (or a directory with bind mounts) can be accessible from different paths. For consistency, we should also get the same result whatever the rules order is, so that would mean to check this rule hierarchy when calling landlock_restrict_self(), which would be too complex and would make related errors difficult to understand.

I don't see this as an issue because adding a deny (rule) to a deny (ruleset's handled access right) would always result to a deny.

For the file rename restriction, I think we should still be able to automatically "taint" parent directories from the same mount point because the landlock_add_rule() syscall gets a struct path from which we can walk through the mount root (see collect_domain_accesses()).

Or do you see it more in a way where everything is allowed by default except denied resources for the first ruleset enforcement ?

We should stick to the current behavior: a domain should always deny by default its handled access rights.

@nbouchinet-anssi
Copy link
Author

Noted, I'm starting to work on the issue.

@l0kod
Copy link
Member

l0kod commented Apr 6, 2024

See related discussion.

@bboozzoo
Copy link

I planned to reply to the mailing list discussion, since it linked to this ticket I'll leave a note here. Just wanted to add that I'm looking a bit into making use of Landlock in snapd. Unfortunately snapd relies heavily on AppArmor to complete the sandbox, which means the sandbox is degraded on systems where AppArmor is not enabled. Hence we are looking for an alternative, and since Landlock support is quite ubiquitous these days, I'm investigating how much of the sandbox we can set up with it.

In our specific case, when the home interface is connected, snap sandbox gives access to $HOME, but like @nbouchinet-anssi mentioned in the first comment we do not give access to $HOME/.ssh. This can only be obtained when snap is allowed to connect a separate interface (personal-files). So definitely, being able to selectively express a deny rule for a specific filesystem object would go a long way in being able to provide a meaningful alternative.

@l0kod
Copy link
Member

l0kod commented Sep 11, 2024

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
Status: Ready
Development

No branches or pull requests

3 participants