Seccomp filtering for setgroups

Hey folks

I’ve run into an issue with trying to confine nginx (and also squid) in a snap.

Nginx uses the glibc function initgroups(), which calls the syscall setgroups(), regardless of whether your processes gid is that group already.

So, even if I explicitly configure nginx to run as with uid and gid of 0, it ends up calling setgroups(1, [0]), which while it would be a no-op, as the process is already has a gid of 0, the process is still killed as it gets blocked by the seccomp blacklist.

Squid has a similar problem, but more complex, as it explicitly checks and disallows for running as root as well. The proxy user/group exist in the core snap, but we can’t switch to them as we hit seccomp blocking setgroups again.

I suspect other servers are likely to use glibc’s initgroups also.

Would it be possible to add seccomp filtering for setgroups? Maybe to allow it to use any gid that is defined in the core snap, like proxy, nogroup, etc?

Thanks


Simon

i guess that will be covered via …

It seems like use case 1 kind of fits.

As long set setgroups to root is always allowed, it should work. I don’t even need changing to another user to fix this issue with nginx.

We use nginx for the OpenStack snaps and we currently patch out the code that drops privileges, since it is a no-op from root->root. https://github.com/openstack/snap-keystone/blob/master/snapcraft.yaml#L94

1 Like

seccomp argument filtering can’t be used with setgroups in the way requested because its second argument is an array and not an integer. This is a limitation with the kernel implementation of seccomp argument filtering.

The document ogra pointed to will allow privilege dropping via setuid/setgid (and friends) to proxy and more importantly, snapd managed users. The best we’ll be able to do with setgroups however is to allow use of ‘setgroups(0, NULL)’ to drop all supplementary groups (non-portable, Linux-only). With this, snapcraft-preload could, in theory, catch initgroups and use the above instead. I suspect most applications will require a code change.

We could add ‘setgroups 0 -’ today-- @bloodearnest, if you patch your code to use ‘setgroups(0, NULL)’ instead of initgroups, and you add to /var/lib/snapd/seccomp/bpf/snap.your.command.src to have ‘setgroups 0 -’, then run:

sudo /usr/lib/snapd/snap-seccomp compile /var/lib/snapd/seccomp/bpf/sn/var/lib/snapd/seccomp/bpf/snap.your.command.srcap.your.command.bin

do you get farther along?

@jdstrand Is there any benefit to doing that vs. the patch that @coreycb and co. use?

Today, no. The information would be interesting for how my suggestion might work in practice.

Thanks for this, Corey.

We didn’t want to build/patch nginx if we didn’t have to, we wanted to consume the binary from universe, so I instead went with patching out initgroups with LD_PRELOAD

For reference, in case anyone else has this issue, you need to add:

env LD_PRELOAD;

to your nginx config, in order to pass that env var through to worker processes

And then the following c code:

#include <sys/types.h>
#include <stdio.h>

int initgroups(const char * user, gid_t group)
{
    fprintf(stderr, "snapstore: stubbing out call to initgroups()");
    return 0;
}

built with:

gcc -fPIC -shared -o stub.so stub.c -ldl

can then be used like so:

LD_PRELOAD=/path/to/stub.so /usr/sbin/nginx

Hope this helps others.


Simon

2 Likes

Thanks for the info @jdstrand

Fwiw, I don’t think allowing setgroups(0, NULL) will help in this case. As currently implemented, initgroups() will always call setgroups(1, [gid]) - it requires an explicit gid_t group parameter. If was going to patch anything, I would simply patch out the call to initgroups alltogether, as @coreycb’s solution did.

So, if I understand correctly, the proposed solution in the linked document will not work for software that calls initgroups(), as that implicitly calls setgroups(), which is not able to filtered safely, is this correct?

This is likely a problem. Lots of server software uses glibc’s initgroups().

Should I file a bug to track this issue? Or comment on the linked thread?

I’m not sure what a proper solution to this would be, but I do think it is important for their to be one in the long run.

Perhaps we can submit a patch to glib c that only calls setgroups iff you are not already that user? That might work in this case, but it might be a while before we could use it.

Thanks


Simon

1 Like

I was suggesting that if people didn’t want to patch, snapcraft-preload could help, intercepting the initgroups() call and simply calling ‘setgroups(0, NULL)’.

FYI, https://git.launchpad.net/~jdstrand/+git/test-setgroups/tree/lib.c has an example to adjust LD_PRELOAD for setgroups() and initgroups().

1 Like