landlock-lsm / go-landlock

A Go library for the Linux Landlock sandboxing feature
MIT License
105 stars 7 forks source link

Ensure that PathAccess(...).accessFS ⊆ cfg.handledAccessFS #18

Closed gnoack closed 2 years ago

gnoack commented 2 years ago

Ensure that

PathAccess(...).accessFS ⊆ cfg.handledAccessFS

for all path options constructed with PathAccess().

This is a conservative choice, but it's easier to enforce that way.

RODirs() and friends are more "magic" - they let you give broad access permissions on entire directories, and that is supposed to work even when the user specifies a smaller AccessFSSet in their config. On the other hand, when users pass a custom AccessFSSet to PathAccess(), they should have a clear understanding of the configuration that they will use it in, and it can be expected that they ensure it is a subset.

Examples

This is a good example. On library upgrade, it should be enough to bump the version number, in most cases. Note that a Go-landlock library that supports V3 at some point still needs to do the exact same thing in the V2 case though:

landlock.V2.BestEffort().RestrictPaths(
  landlock.RODirs("/bin", "/usr", "/etc"),
  landlock.RWDirs("/tmp"),
)

This is a good example as well. Making and removing directories is forbidden everywhere except in /tmp.

landlock.MustConfig(landlock.AccessFSSet(ll.AccessFSMakeDir|ll.AccessFSRemoveDir)).BestEffort().RestrictPaths(
  landlock.RWDirs("/tmp"),
)

This is a good example as well. Making and removing directories is forbidden, except in /tmp and creating them in /home/x/tmp. It's verbose, but the author knows exactly what is being restricted and what is the scope of each exception:

landlock.MustConfig(landlock.AccessFSSet(ll.AccessFSMakeDir|ll.AccessFSRemoveDir)).BestEffort().RestrictPaths(
  landlock.PathAccess(landlock.AccessFSSet(ll.AccessFSMakeDir|ll.AccessFSRemoveDir), "/tmp"),
  landlock.PathAccess(landlock.AccessFSSet(ll.AccessFSMakeDir), "/home/x/tmp"),
)

The following example is on the fence:

landlock.V99.BestEffort().RestrictPaths(
  landlock.PathAccess(landlock.AccessFSSet(ll.AccessTransmogrifyFile), "/tmp"),
)

This last case is the one that the bug is about.

gnoack commented 2 years ago

I should note that this is a breaking change. It's unlikely that there are existing users making this mistake. But with runc-like tools delegating the configuration of AccessFSSets to users completely, it's likely that people will eventually make the mistake accidentally.

l0kod commented 2 years ago

Nice! Does it check the subset consistency even when Landlock is not enabled at runtime?

gnoack commented 2 years ago

Nice! Does it check the subset consistency even when Landlock is not enabled at runtime?

Yes, it does.