Skip to content

newgidmap: enforce setgroups=deny if self-mapping a group#97

Merged
hallyn merged 2 commits intoshadow-maint:masterfrom
cyphar:newgidmap-secure-setgroups
Feb 16, 2018
Merged

newgidmap: enforce setgroups=deny if self-mapping a group#97
hallyn merged 2 commits intoshadow-maint:masterfrom
cyphar:newgidmap-secure-setgroups

Conversation

@cyphar
Copy link
Copy Markdown
Contributor

@cyphar cyphar commented Feb 15, 2018

This is necessary to match the kernel-side policy of "self-mapping in a
user namespace is fine, but you cannot drop groups" -- a policy that was
created in order to stop user namespaces from allowing trivial privilege
escalation by dropping supplementary groups that were "blacklisted" from
certain paths.

This is the simplest fix for the underlying issue, and effectively makes
it so that unless a user has a valid mapping set in /etc/subgid (which
only administrators can modify) -- and they are currently trying to use
that mapping -- then /proc/$pid/setgroups will be set to deny. This
workaround is only partial, because ideally it should be possible to set
an "allow_setgroups" or "deny_setgroups" flag in /etc/subgid to allow
administrators to further restrict newgidmap(1).

Ref: https://bugs.launchpad.net/ubuntu/+source/shadow/+bug/1729357
Fixes: CVE-2018-7169
Reported-by: Craig Furman craig.furman89@gmail.com
Signed-off-by: Aleksa Sarai asarai@suse.de

@cyphar cyphar force-pushed the newgidmap-secure-setgroups branch 3 times, most recently from f275a08 to 3bb0cde Compare February 15, 2018 14:38
@cyphar cyphar force-pushed the newgidmap-secure-setgroups branch from 3bb0cde to 8349efe Compare February 16, 2018 06:53
This is necessary to match the kernel-side policy of "self-mapping in a
user namespace is fine, but you cannot drop groups" -- a policy that was
created in order to stop user namespaces from allowing trivial privilege
escalation by dropping supplementary groups that were "blacklisted" from
certain paths.

This is the simplest fix for the underlying issue, and effectively makes
it so that unless a user has a valid mapping set in /etc/subgid (which
only administrators can modify) -- and they are currently trying to use
that mapping -- then /proc/$pid/setgroups will be set to deny. This
workaround is only partial, because ideally it should be possible to set
an "allow_setgroups" or "deny_setgroups" flag in /etc/subgid to allow
administrators to further restrict newgidmap(1).

We also don't write anything in the "allow" case because "allow" is the
default, and users may have already written "deny" even if they
technically are allowed to use setgroups. And we don't write anything if
the setgroups policy is already "deny".

Ref: https://bugs.launchpad.net/ubuntu/+source/shadow/+bug/1729357
Fixes: CVE-2018-7169
Reported-by: Craig Furman <craig.furman89@gmail.com>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
@cyphar cyphar force-pushed the newgidmap-secure-setgroups branch from 8349efe to 6d8be68 Compare February 16, 2018 06:56
@hallyn
Copy link
Copy Markdown
Member

hallyn commented Feb 16, 2018

Thanks, everyone! I like this.

@hallyn hallyn merged commit c53e4c1 into shadow-maint:master Feb 16, 2018
@hallyn
Copy link
Copy Markdown
Member

hallyn commented Feb 16, 2018

I suppose this means we should do a release.

@cyphar cyphar deleted the newgidmap-secure-setgroups branch February 16, 2018 15:44
@cyphar
Copy link
Copy Markdown
Contributor Author

cyphar commented Feb 16, 2018

I will work on the whole allow_setgroups patchset we discussed over the weekend (the complicated part is that subordinate_range is not accessible from src/newgidmap.c).

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

Successfully merging this pull request may close these issues.

5 participants