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.

Sign up for GitHub

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

Jump to bottom

newsubgid: add deny_setgroups option to /etc/subgid #99

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

newsubgid: add deny_setgroups option to /etc/subgid #99

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fdfb3b9
libmisc: list: update helpers
cyphar Feb 16, 2018
4a43149
lib: subordinateio: add options field to /etc/sub{uid,gid}
cyphar Feb 18, 2018
0d368da
src: new{uid,gid}map: add basic (noop) support for options
cyphar Feb 18, 2018
b1a1af3
newgidmap: add deny_setgroups option to /etc/subgid
cyphar Feb 18, 2018
626af91
man: add documentation for new setgroups(2) semantics
cyphar Feb 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
man: add documentation for new setgroups(2) semantics 
Add documentation for allow_setgroups, deny_setgroups, the new option
format of /etc/sub{uid,gid}, and fix some errors in the groupmod(8) man
page that stopped it from building properly on my machine.

Signed-off-by: Aleksa Sarai <[email protected]>
  • Loading branch information
@cyphar
cyphar committed Feb 19, 2018
commit 626af91ee917609a166b64f0748e33f0eef2b4e6
9 changes: 6 additions & 3 deletions man/groupmod.8.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
Copyright (c) 1991 , Julianne Frances Haugh
Copyright (c) 2007 - 2011, Nicolas François
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions
are met:
Expand All @@ -15,7 +15,7 @@
3. The name of the copyright holders or contributors may not be used to
endorse or promote products derived from this software without
specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
Expand Down Expand Up @@ -136,7 +136,7 @@
<option>-n</option>, <option>--new-name</option>&nbsp;<replaceable>NEW_GROUP</replaceable>
</term>
<listitem>
<para>
<para>
The name of the group will be changed from <replaceable>GROUP</replaceable>
to <replaceable>NEW_GROUP</replaceable> name.
</para>
Expand Down Expand Up @@ -278,16 +278,19 @@
<para>E_GRP_UPDATE: can't update group file</para>
</listitem>
</varlistentry>
<varlistentry>
<term><replaceable>11</replaceable></term>
<listitem>
<para>E_CLEANUP_SERVICE: can't setup cleanup service</para>
</listitem>
</varlistentry>
<varlistentry>
<term><replaceable>12</replaceable></term>
<listitem>
<para>E_PAM_USERNAME: can't determine your username for use with pam</para>
</listitem>
</varlistentry>
<varlistentry>
<term><replaceable>13</replaceable></term>
<listitem>
<para>E_PAM_ERROR: pam returned an error, see syslog facility id groupmod for the PAM error message</para>
Expand Down
21 changes: 19 additions & 2 deletions man/newgidmap.1.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<!--
Copyright (c) 2013 Eric W. Biederman
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions
are met:
Expand All @@ -14,7 +14,7 @@
3. The name of the copyright holders or contributors may not be used to
endorse or promote products derived from this software without
specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
Expand Down Expand Up @@ -122,12 +122,29 @@
of the above sets, each of the GIDs in the range [lowergid,
lowergid+count] is allowed to the caller according to
<filename>/etc/subgid</filename> before setting
<filename>/proc/[pid]/setgroups</filename> and
<filename>/proc/[pid]/gid_map</filename>.
</para>

<para>
Note that newgidmap may be used only once for a given process.
</para>

<para>
<command>newgidmap</command> also allows you to map a user's own
effective group ID without it being specified in
<filename>/etc/subgid</filename> (in order to match the "unprivileged
user namespaces" feature in Linux 3.8). If this is the only mapping
requested (in order to match the security protections from Linux 3.19),
<command>newgidmap</command> will ensure that
<filename>/proc/[pid]/setgroups</filename> is set to "deny" (either by
writing "deny" itself or seeing that it is already set to "deny"), and
will fail if writing "deny" failed. This restriction is also applied if
any of the mappings given to <command>newgidmap</command> has the
<option>deny_setgroups</option> option set in
<filename>/etc/subgid</filename>.
</para>

</refsect1>

<refsect1 id='options'>
Expand Down
45 changes: 42 additions & 3 deletions man/subgid.5.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<!--
Copyright (c) 2013 Eric W. Biederman
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions
are met:
Expand All @@ -14,7 +14,7 @@
3. The name of the copyright holders or contributors may not be used to
endorse or promote products derived from this software without
specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
Expand Down Expand Up @@ -51,7 +51,7 @@
a user name and a range of subordinate group ids that user
is allowed to use.

This is specified with three fields delimited by colons
This is specified with four fields delimited by colons
(<quote>:</quote>).
These fields are:
</para>
Expand All @@ -65,6 +65,9 @@
<listitem>
<para>numerical subordinate group ID count</para>
</listitem>
<listitem>
<para>comma-separated list of options (optional)</para>
</listitem>
</itemizedlist>

<para>
Expand All @@ -86,6 +89,42 @@

</refsect1>

<refsect1 id='options'>
<title>OPTIONS</title>

<para>
Options are comma-separated. Empty options are ignored, and if the
field is missing entirely it is treated as an empty string. Attempting
to use an unknown option will cause <command>newgidmap</command> to emit
an error. Setting options from the same option-set multiple times in a
single entry will result in the last option specified taking precedence.
</para>

<variablelist>
<varlistentry>
<term><option>allow_setgroups</option> (default),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, is this the right place to put this kind of thing though? Maybe it is, but:

The only case I know of for deny-setgroups is when an administrator has placed a user in a group with a negative acl, to prevent that user from accessing some resource.

In that case, does it make more sense to place a sort of "group_locked" option on the group itself in /etc/group? Then when newgidmap runs, it checks whether the target pid is in any locked group, and if so sets deny_setgroup.

Placing the deny/allow_setgroup variable in the gid mapping seems a step removed, and might cause an admin to not notice that "hey, the user being allowd to grab those gids should have been locked into group nos3critdocs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I hadn't though of this point.

You're quite right that most users of negative ACLs are done with groups (and I think it does make intuitive sense to be able to say "if you are in a bad group then you cannot use setgroups"), but I have a couple of questions about edge cases:

  • Should an administrator be able to override this decision for particular users or groups? Should that also be in /etc/groups (or /etc/passwd)?

  • What if it's a numeric group (so not one registered in /etc/groups)? Is the solution to just tell people "tough luck, you should register your group"?

  • This is a bit of an edge condition (and is an edge condition that actually isn't really easy to handle with the current implementation either), but how will this affect containers on the system that are also using group blacklisting? While /etc/sub* are global, if you have containers using allocated portions of the UID/GID space then you would expect that a user shouldn't be able to use newgidmap to drop the negative ACLs for a different group set than the one you are joining? Now, this is quite a weird case because it would require you to have a quite strange ACL setup -- so maybe this is a use case we don't care about?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should an administrator be able to override this decision for particular users or groups? Should that also be in /etc/groups (or /etc/passwd)?

I don't really think so, as you either want to deny the group certain access or you don't, but my gut says there are more subtle cases. So let's say yes.

What if it's a numeric group (so not one registered in /etc/groups)? Is the solution to just tell people "tough luck, you should register your group"?

I think requiring a name makes sense. Do you?

how will this affect containers on the system that are also using group blacklisting?

Hmm - that applies to your approach too right?

It really seems like this requires a new kernel functionality to make the most sense - a new per-process 'lockedgrp' from which groups can never be removed. Otherwise all we can really do in the parent container is say "never let the child container do a setgroup" (right? or have i confused myself on this topic once again?)

<option>deny_setgroups</option></term>
<listitem>
<para>
Specify whether
<citerefentry>
<refentrytitle>setgroups</refentrytitle><manvolnum>2</manvolnum>
</citerefentry>
will be disabled by <command>newgidmap</command>. If more than one
mapping is given to <command>newgidmap</command>, and they have
different <option>*_setgroups</option> options set,
<option>deny_setgroups</option> always takes precedence. See
<citerefentry>
<refentrytitle>newgidmap</refentrytitle><manvolnum>1</manvolnum>
</citerefentry>
for more details.
</para>
</listitem>
</varlistentry>
</variablelist>

</refsect1>

<refsect1 id='files'>
<title>FILES</title>
<variablelist>
Expand Down
27 changes: 24 additions & 3 deletions man/subuid.5.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<!--
Copyright (c) 2013 Eric W. Biederman
All rights reserved.

Redistribution and use in source and binary forms, with or without
modification, are permitted provided that the following conditions
are met:
Expand All @@ -14,7 +14,7 @@
3. The name of the copyright holders or contributors may not be used to
endorse or promote products derived from this software without
specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
Expand Down Expand Up @@ -51,7 +51,7 @@
a user name and a range of subordinate user ids that user
is allowed to use.

This is specified with three fields delimited by colons
This is specified with four fields delimited by colons
(<quote>:</quote>).
These fields are:
</para>
Expand All @@ -65,6 +65,9 @@
<listitem>
<para>numerical subordinate user ID count</para>
</listitem>
<listitem>
<para>comma-separated list of options (optional)</para>
</listitem>
</itemizedlist>

<para>
Expand All @@ -86,6 +89,24 @@

</refsect1>

<refsect1 id='options'>
<title>OPTIONS</title>

<para>
Options are comma-separated. Empty options are ignored, and if the
field is missing entirely it is treated as an empty string. Attempting
to use an unknown option will cause <command>newuidmap</command> to emit
an error. Setting options from the same option-set multiple times in a
single entry will result in the last option specified taking precedence.
</para>

<para>
At the moment, no options are defined for
<filename>/etc/subuid</filename>.
</para>

</refsect1>

<refsect1 id='files'>
<title>FILES</title>
<variablelist>
Expand Down