seccomp: support seccomp listener

[giuseppe]

Signed-off-by: Giuseppe Scrivano [email protected]

[rhatdan]

LGTM
But could you add some description about why this is important?

[giuseppe]

the goal is to use it with containers/conmon#190

It is just a PoC now, conmon doesn't look like the right place to maintain the handlers code.

[AkihiroSuda]

This is also important for running rootless containers without /etc/subuid: https://github.com/rootless-containers/subuidless

[giuseppe]

This is also important for running rootless containers without /etc/subuid: https://github.com/rootless-containers/subuidless

I wonder if we can come up with a common interface that could be used as some sort of plugins.

There will surely be more implementations for seccomp notifications.

The API could be quite simple, just pass down a FD or have an exported function like int handle_req(struct seccomp_notif *sreq, struct seccomp_notif_resp *sresp){...}

I've some WIP, I'll publish it as soon as it is in a working PoC state

[AkihiroSuda]

👍

It would be great if we can propagate SIGCHLD events to the handler plugins as well (so that the plugin can clean-up process-associated data structure)

[AkihiroSuda]

The API could be quite simple, just pass down a FD or have an exported function like int handle_req(struct seccomp_notif *sreq, struct seccomp_notif_resp *sresp){...}

Passing down a FD is unlikely to work with multiple plugins?

[giuseppe]

Passing down a FD is unlikely to work with multiple plugins?

yes I agree, I was thinking of a simple API like:

int seccomp_notify_version() {return 1;}
int seccomp_notify_start(const char *conf);
int seccomp_notify_handle_req(struct seccomp_notif *sreq, struct seccomp_notif_resp *sresp, bool *handled);
int seccomp_notify_stop();

what do you think?

[giuseppe]

int seccomp_notify_handle_req(struct seccomp_notif *sreq, struct seccomp_notif_resp *sresp, bool *handled);

if the plugin doesn't set handled then we try again with the next plugin that is registered. If the request was not handled by any plugin then the runtime should fail with ENOSYS

[AkihiroSuda]

SGTM, but let's s/^seccomp/^run_oci_seccomp/g or choose other prefix so as to avoid potential conflict with libseccomp.

int seccomp_notify_start(const char *conf);

What is conf?

int seccomp_notify_handle_req(struct seccomp_notif *sreq, struct seccomp_notif_resp *sresp, bool *handled);

This function should also take struct run_oci_seccomp_notify_opt * as an argument for future extension.

[giuseppe]

What is conf?

just a plugin specific string to pass down some configuration if necessary. Most plugins will probably just ignore it.

but let's s/^seccomp/^run_oci_seccomp/g or choose other prefix so as to avoid potential conflict with libseccomp.
This function should also take struct run_oci_seccomp_notify_opt * as an argument for future extension.

SGTM

[AkihiroSuda]

just a plugin specific string

Should be a struct?

[giuseppe]

Should be a struct?

it can be a struct, just how to make it generic so that multiple plugins can use it?

[AkihiroSuda]

We can just declare an empty struct and call it a day.
Eventually we may want to have char *json in the struct.

[giuseppe]

We can just declare an empty struct and call it a day.
Eventually we may want to have char *json in the struct.

what json would that be? The config.json?

[AkihiroSuda]

Probably it would look like this, but should be revisited later

{
  "runtimeRootPath": "/run/user/1001/crun",
  "name": "foo",
  "bundlePath": "/home/foo/oci-bundles/alpine"
}

[giuseppe]

{
  "runtimeRootPath": "/run/user/1001/crun",
  "name": "foo",
  "bundlePath": "/home/foo/oci-bundles/alpine"
}

this could be part of the struct no? It is not going to change.

If we pass down the config.json file as well and I think the path is enough, at least a plugin has some way to be configured via annotations.

[giuseppe]

@AkihiroSuda pushed some new commits to add the plugin mechanism we discussed.

This is a plugin example:

8168733

[lgtm-com]

This pull request introduces 1 alert when merging 8168733 into ec3df81 - view on LGTM.com

new alerts:

• 1 for Too few arguments to formatting function

[AkihiroSuda]

Thanks

[AkihiroSuda]

Can we have the list of allowed and disallowed licenses for plugins?

[giuseppe]

I am not a lawyer, but LGPL allows being used with proprietary software as long as changes to the program itself are distributed. In this case only changes to libcrun matter

[AkihiroSuda]

IANAL either, but as crun CLI is licensed under the terms of GPL2-or-later, I'm wondering that plugins might need to be GPL-compatible

[AkihiroSuda]

An alternative design is to implement the plugin infra on a standalone receiver binary with a more permissive license, rather than on crun itself.

[giuseppe]

the idea is to use the same plugins from conmon, since it is already a long running process.

Having them supported also in crun makes just easier to debug stuff.

[AkihiroSuda]

When is a program and its plug-ins considered a single combined program?
...
If the main program dynamically links plug-ins, and they make function calls to each other and share data structures, we believe they form a single combined program, which must be treated as an extension of both the main program and the plug-ins.

https://www.gnu.org/licenses/gpl-faq.en.html#GPLPlugins

If I write a plug-in to use with a GPL-covered program, what requirements does that impose on the licenses I can use for distributing my plug-in?
...
If the main program and the plugins are a single combined program then this means you must license the plug-in under the GPL or a GPL-compatible free software license and distribute it with source code in a GPL-compliant way. A main program that is separate from its plug-ins makes no requirements for the plug-ins.

https://www.gnu.org/licenses/gpl-faq.en.html#GPLAndPlugins

---

If the plugin infra is directly implemented in the main crun binary, it is highly likely that the crun and the plugins are "a single combined program" and hence the plugins need to be GPL-compatible.

If the plugins need to be GPL-compatible, it is likely that the plugins cannot be used by runc, which is licensed under the terms of Apache License 2.0 (incompatible with GPL2), but IANAL.

cc @cyphar

[cyphar]

MIT/BSD/et al or MPL-2.0 licensed plugins would be compatible with both Apache-2.0 and LGPL-2.1, and thus wouldn't cause any issues.

But I would further note that you linked the GPL FAQ -- the LGPL treats dynamically shared libraries differently to the GPL, and dynamic plugins (especially of the form used here) are functionally identical to shared libraries. For a more famous example, the Name Service Switch (which is supported by glibc -- an LGPL project) supports using dynamic plugins. Those plugins do not need to be LGPL-3.0 licensed. So I would argue that it is not the case that plugins would need to be LGPL licensed. Outlook has an MIT-licensed NSS plugin for Azure ActiveDirectory.

[lgtm-com]

This pull request introduces 1 alert when merging 0c90558 into ec3df81 - view on LGTM.com

new alerts:

• 1 for Too few arguments to formatting function

[AkihiroSuda]

Relevant discussion in OCI: https://hackmd.io/El8Dd2xrTlCaCG59ns5cwg?view#September-16-2020

[giuseppe]

Relevant discussion in OCI: https://hackmd.io/El8Dd2xrTlCaCG59ns5cwg?view#September-16-2020

thanks! I've left a comment there

[giuseppe]

ETA of the next release?

sorry for the delay. Released 0.15 now

[AkihiroSuda]

thanks!