switch_user_group(): rpmlint reports call to setgroups before setuid

[jfkw]

switch_user_group(): rpmlint reports call to setgroups before setuid:

Building coreos-metadata-3.0.1, Crate users is pulled in via dependency crate update-ssh-keys-0.3.0. rpmlint reports the warning:

RPMLINT report:
===============
coreos-metadata.x86_64:
W: missing-call-to-setgroups-before-setuid /usr/bin/coreos-metadata
This executable is calling setuid and setgid without setgroups or initgroups.
There is a high probability this means it didn't relinquish all groups, and
this would be a potential security issue to be fixed. Seek POS36-C on the web
for details about the problem.

The warning may not indicate an actual problem, but it would be helpful to eliminate the warning. Upstream https://github.com/coreos/update-ssh-keys and https://github.com/coreos/coreos-metadata have been notified via https://github.com/coreos/coreos-metadata/issues/118.

Per @lucab discussion in that issue:

The warning is related to switch_user_group in the users crate (all versions): https://github.com/ogham/rust-users/blob/15af1576e40fd3e5592b918de353df6a976c42c4/src/switch.rs#L134-L143

We are calling that in update-ssh-keys: https://github.com/coreos/update-ssh-keys/blob/v0.3.0/src/lib.rs#L108

In our specific case I think this is not a security bug. We are calling that method in order to align user/group on file creation only, not to drop privileges for the process (those are reset when the guard value is dropped at the end of the function).

[ogham]

Interesting! I think you’re right in saying this doesn’t affect your case, but rust-users is a general library and someone else is bound to run into the same problem eventually. Also it looks like doing the Rust equivalent of setgroups(0, NULL); is enough to make the warning go away.

[lucab]

@ogham if you are going that route, it looks like the guard should record getgroups state before resetting groups and recover them back on drop.

[ogham]

This doesn’t go all the way to solving the problem, but I’ve changed the code to setgid before setuid in https://github.com/ogham/rust-users/commit/f1b9902212be41ea24633a4575a110da9a76c7be.