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

Converting allowedAccess strings into AccessFSSet uint64 #14

Open
BoardzMaster opened this issue Aug 27, 2021 · 4 comments
Open

Converting allowedAccess strings into AccessFSSet uint64 #14

BoardzMaster opened this issue Aug 27, 2021 · 4 comments

Comments

@BoardzMaster
Copy link

Hi Günther!
Could you please add a function which converts allowedAccess strings like { "execute, "writefile....} into AccessFSSet uint64.
There is already func (a AccessFSSet) String() string {....}. It would be nice to have the one that does the opposite work.

According to the last commit in opencontainers/runtime-spec#1111 (review)
Landlock access rules in spec file go in string format that is needed to be converted in AccessFSSet one:
image

@gnoack
Copy link
Collaborator

gnoack commented Aug 28, 2021

Thank you for your request.

This feature request would make the go-landlock library the place where the valid configuration values for your project's config files are defined. This set of flags might grow in future go-landlock versions, and you would immediately start supporting these by just updating your library dependencies then. You might not notice it when you do that upgrade and when the permitted values in your config language change.

Is that what you want?

If you want to do that string-to-flag lookup on your side, you could do it like this:

import (
  "github.com/landlock-lsm/go-landlock/landlock"
  ll "github.com/landlock-lsm/go-landlock/landlock/syscall"
)

var accessFSFlagLookup = map[string]AccessFSSet{
  "execute": landlock.AccessFSSet(ll.AccessFSExecute),
  // ...
}

but then you would be spelling out the supported values anyway, to avoid the "surprise feature upgrade" scenario above.

If your goal is to get notified when you need to upgrade that list, a technique that works in some project setups is to have a unit test to check whether the enums in one project and the enums in the dependency match. Then that unit test would fail when you update your dependencies and remind you to update your map of flags. (It's not exposed in a nice way right now, but as a hack you can probe which flags are supported by calling landlock.NewConfig() and checking for errors.)

What do you think?

@BoardzMaster
Copy link
Author

Thanks for the reply. And sorry for the delay.

  1. I got your point about future library updating. That makes sense.
  2. About update notifications, I'm not sure that about using unit tests. I suppose it would be better to get notifications on the run as you suggested by calling landlock.NewConfig or some other particular function. It would be better to save all previous versions like V1, V2... so the user just can choose the version he likes and gets update notifications if there is a mismatch between the version he uses and a map of flags.

@gnoack
Copy link
Collaborator

gnoack commented Sep 3, 2021

@BoardzMaster if you're letting the user specify the HandledAccessFS set on their own, there is no point in specifying a Landlock version. landlock.V1 is really only a shortcut to say landlock.NewConfig(...) with the full set of AccessFS operations supported by that Landlock version. The Go landlock library will on its own figure out whether the specified set of AccessFS operations can be enforced with the Landlock version that the currently running kernel supports.

In terms of compatibility, go-landlock supports two modes:

  • In "regular" mode, RestrictPaths() fails if the kernel does not support the given AccessFS flags
  • In "best effort" mode, RestrictPaths() uses a subset of the given AccessFS flags if needed.

Passing in flags that are not known to the used version of go-landlock is always an error. You'll need to update to using a newer version of go-landlock to use features of Landlock that aren't available yet.

When defining a Landlock ruleset, it's a good idea to develop it in "regular" mode, and then set it to "best effort" if that helps your users.

@gnoack
Copy link
Collaborator

gnoack commented Sep 5, 2021

Whoops, undid the git push for now after reading opencontainers/runc#3194 (comment) - let's digest this a bit more to come to an agreement. In the meantime, or if we end up thinking this is not a good idea, it's still possible to hardcode the same list as in the linked review.

@gnoack gnoack reopened this Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants