Re: [gentoo-dev] [PATCH] user.eclass: die if hard coded UID or GID is already in use

2019-05-29 Thread Marek Szuba
On 2019-05-27 16:45, William Hubbs wrote:

> If a package hard codes the UID or GID when adding a user or group to
> the system and that UID/GID already exists, we should abort rather than
> changing the UID/GID.

+1. It is of course my personal opinion but years of having been working
with various scientific software pipelines have made me rabidly allergic
to tools which do a different thing than what they were asked to do
unless explicitly allowed to do it this way. And what is the point of
specifying a "preferred" xID anyway? We cannot rely on these values
anywhere so IMHO all they do is add noise. And no, in my book "it has
always been like this" is not by itself a good reason to maintain the
status quo.

-- 
MS



Re: [gentoo-dev] [PATCH] user.eclass: die if hard coded UID or GID is already in use

2019-05-28 Thread Alec Warner
On Tue, May 28, 2019 at 11:05 AM Mike Gilbert  wrote:

> On Tue, May 28, 2019 at 10:43 AM William Hubbs 
> wrote:
> >
> > On Tue, May 28, 2019 at 02:21:23AM -0400, Mike Gilbert wrote:
> > > On Tue, May 28, 2019 at 1:41 AM Robin H. Johnson 
> wrote:
> > > >
> > > > On Mon, May 27, 2019 at 08:44:09PM -0400, Mike Gilbert wrote:
> > > > > On Mon, May 27, 2019 at 11:45 AM William Hubbs <
> willi...@gentoo.org> wrote:
> > > > > >
> > > > > > If a package hard codes the UID or GID when adding a user or
> group to
> > > > > > the system and that UID/GID already exists, we should abort
> rather than
> > > > > > changing the UID/GID.
> > > > > These functions have behaved this way for a long time.
> > > > Yes, I recall this breakage being raised even prior to GLEP27.
> > > >
> > > > Some coverage of prior work firstly.
> > > > 2003/May: eid_database [1]
> > > > 2004/May: GLEP27 [2]
> > > > 2006/Summer: GSOC Project for GLEP27 implementation [3]
> > > > Later: Creandus [4][5]
> > > >
> > > > [1]
> https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-src/eid_database/
> > > > [2] https://www.gentoo.org/glep/glep-0027.html
> > > > [3]
> https://github.com/creandus/creandus.github.com/blob/master/glep27-proposal.txt
> > > > [4] https://github.com/creandus/
> > > > [5] http://creandus.github.io/
> > > >
> > > > > What problem are you trying to solve here?
> > > > Specifically:
> > > > 1. Package A is emerged, and the ebuild specifies enewuser/enewgroup
> > > > with -1 as the numeric input, and some user or group is created with
> > > > value X.
> > > > 2.  Package B is emerged, and the ebuild specifies enewuser/enewgroup
> > > > with a fixed UID/GID value Y && explicitly depends on having that
> > > > specific value.
> > >
> > > Could you provide some examples of packages that require a specific
> > > numeric UID/GID to function properly? That seems like a significant
> > > design flaw.
> >
> > The example I specifically found was kubernetes security contexts -- to
> > set one of these up, you need numeric uids/gids [1].
> >
> > [1]
> https://kubernetes.io/docs/tasks/configure-pod-container/security-context/
> >
> > I agree this could be possibly seen as a flaw, but I don't know enough
> > about kubernetes to know if symbolic uids/gids could be used.
> >
> > I am speculating here, but I'm guessing that if the pod spans
> > multiple hosts, the uids/gids might need to be the same, which would
> > require fixed UIDs/GIDs.
>
> Your kubernetes example is not a very compelling one. Firstly, none of
> the kubernetes-related ebuilds in the gentoo repo actually call
> enewuser or enewgroup with a static id. Secondly, I would expect the
> sysadmin of a kubernetes cluster to allocate a UID/GID range outside
> the "system range" defined in /etc/login.defs.
>
> > > As mgorny suggests, I suspect the static UID/GID that is sometimes
> > > passed to enewuser/enewgroup is more of a preference/suggestion than a
> > > requirement.
> > >
> > > I do not believe William's patch should be merged without some very
> > > good reason for the change in behavior.
> >
> > Consider this situation:
> >
> > in package-a.ebuild:
> > enewuser usera 300 ...
> > enewgroup groupa 300 ...
> >
> > in package-b.ebuild:
> > enewuser userb 300 ...
> > enewgroup groupb 300 ...
> >
> > Ok, cool, if I need a security context for package b, the uid/gid is 300
> > correct? Yes, unless package a gets installed on some host where package
> > b is without me knowing about it.
> >
> > I see no way around this other than to make uids/gids fixed when
> > packages request specific ones.
>
> If you are running a cluster system that requires static ids, you
> should really create these ids yourself as part of the cluster node
> setup process.
>

Pretty much +1 to this. I agree this is a problem as many services use
shared uid / gid to handle authorization. I'm not convinced solving this
problem at the package manager level is correct, so every time its proposed
I vote no.

If you need to share UID / GID, across hosts, there are many solutions
available.
 - lib_nssldap
 - lib_nsscache
 - lib_nssfiles with configuration management on top.

These are all strictly better than trying to do this in ebuilds.

-A


Re: [gentoo-dev] [PATCH] user.eclass: die if hard coded UID or GID is already in use

2019-05-28 Thread Mike Gilbert
On Tue, May 28, 2019 at 10:43 AM William Hubbs  wrote:
>
> On Tue, May 28, 2019 at 02:21:23AM -0400, Mike Gilbert wrote:
> > On Tue, May 28, 2019 at 1:41 AM Robin H. Johnson  wrote:
> > >
> > > On Mon, May 27, 2019 at 08:44:09PM -0400, Mike Gilbert wrote:
> > > > On Mon, May 27, 2019 at 11:45 AM William Hubbs  
> > > > wrote:
> > > > >
> > > > > If a package hard codes the UID or GID when adding a user or group to
> > > > > the system and that UID/GID already exists, we should abort rather 
> > > > > than
> > > > > changing the UID/GID.
> > > > These functions have behaved this way for a long time.
> > > Yes, I recall this breakage being raised even prior to GLEP27.
> > >
> > > Some coverage of prior work firstly.
> > > 2003/May: eid_database [1]
> > > 2004/May: GLEP27 [2]
> > > 2006/Summer: GSOC Project for GLEP27 implementation [3]
> > > Later: Creandus [4][5]
> > >
> > > [1] https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-src/eid_database/
> > > [2] https://www.gentoo.org/glep/glep-0027.html
> > > [3] 
> > > https://github.com/creandus/creandus.github.com/blob/master/glep27-proposal.txt
> > > [4] https://github.com/creandus/
> > > [5] http://creandus.github.io/
> > >
> > > > What problem are you trying to solve here?
> > > Specifically:
> > > 1. Package A is emerged, and the ebuild specifies enewuser/enewgroup
> > > with -1 as the numeric input, and some user or group is created with
> > > value X.
> > > 2.  Package B is emerged, and the ebuild specifies enewuser/enewgroup
> > > with a fixed UID/GID value Y && explicitly depends on having that
> > > specific value.
> >
> > Could you provide some examples of packages that require a specific
> > numeric UID/GID to function properly? That seems like a significant
> > design flaw.
>
> The example I specifically found was kubernetes security contexts -- to
> set one of these up, you need numeric uids/gids [1].
>
> [1] https://kubernetes.io/docs/tasks/configure-pod-container/security-context/
>
> I agree this could be possibly seen as a flaw, but I don't know enough
> about kubernetes to know if symbolic uids/gids could be used.
>
> I am speculating here, but I'm guessing that if the pod spans
> multiple hosts, the uids/gids might need to be the same, which would
> require fixed UIDs/GIDs.

Your kubernetes example is not a very compelling one. Firstly, none of
the kubernetes-related ebuilds in the gentoo repo actually call
enewuser or enewgroup with a static id. Secondly, I would expect the
sysadmin of a kubernetes cluster to allocate a UID/GID range outside
the "system range" defined in /etc/login.defs.

> > As mgorny suggests, I suspect the static UID/GID that is sometimes
> > passed to enewuser/enewgroup is more of a preference/suggestion than a
> > requirement.
> >
> > I do not believe William's patch should be merged without some very
> > good reason for the change in behavior.
>
> Consider this situation:
>
> in package-a.ebuild:
> enewuser usera 300 ...
> enewgroup groupa 300 ...
>
> in package-b.ebuild:
> enewuser userb 300 ...
> enewgroup groupb 300 ...
>
> Ok, cool, if I need a security context for package b, the uid/gid is 300
> correct? Yes, unless package a gets installed on some host where package
> b is without me knowing about it.
>
> I see no way around this other than to make uids/gids fixed when
> packages request specific ones.

If you are running a cluster system that requires static ids, you
should really create these ids yourself as part of the cluster node
setup process.



Re: [gentoo-dev] [PATCH] user.eclass: die if hard coded UID or GID is already in use

2019-05-28 Thread William Hubbs
On Tue, May 28, 2019 at 02:21:23AM -0400, Mike Gilbert wrote:
> On Tue, May 28, 2019 at 1:41 AM Robin H. Johnson  wrote:
> >
> > On Mon, May 27, 2019 at 08:44:09PM -0400, Mike Gilbert wrote:
> > > On Mon, May 27, 2019 at 11:45 AM William Hubbs  
> > > wrote:
> > > >
> > > > If a package hard codes the UID or GID when adding a user or group to
> > > > the system and that UID/GID already exists, we should abort rather than
> > > > changing the UID/GID.
> > > These functions have behaved this way for a long time.
> > Yes, I recall this breakage being raised even prior to GLEP27.
> >
> > Some coverage of prior work firstly.
> > 2003/May: eid_database [1]
> > 2004/May: GLEP27 [2]
> > 2006/Summer: GSOC Project for GLEP27 implementation [3]
> > Later: Creandus [4][5]
> >
> > [1] https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-src/eid_database/
> > [2] https://www.gentoo.org/glep/glep-0027.html
> > [3] 
> > https://github.com/creandus/creandus.github.com/blob/master/glep27-proposal.txt
> > [4] https://github.com/creandus/
> > [5] http://creandus.github.io/
> >
> > > What problem are you trying to solve here?
> > Specifically:
> > 1. Package A is emerged, and the ebuild specifies enewuser/enewgroup
> > with -1 as the numeric input, and some user or group is created with
> > value X.
> > 2.  Package B is emerged, and the ebuild specifies enewuser/enewgroup
> > with a fixed UID/GID value Y && explicitly depends on having that
> > specific value.
> 
> Could you provide some examples of packages that require a specific
> numeric UID/GID to function properly? That seems like a significant
> design flaw.

The example I specifically found was kubernetes security contexts -- to
set one of these up, you need numeric uids/gids [1].

[1] https://kubernetes.io/docs/tasks/configure-pod-container/security-context/

I agree this could be possibly seen as a flaw, but I don't know enough
about kubernetes to know if symbolic uids/gids could be used.

I am speculating here, but I'm guessing that if the pod spans
multiple hosts, the uids/gids might need to be the same, which would
require fixed UIDs/GIDs.

> 
> > I'd like to accept WilliamH's patch as-is, and then add an additional
> > patch that moves the range of dynamically allocated users such that it
> > does not conflict.
> 
> The "dynamic range" currently defaults to 101 to 999 as defined by
> SYS_{UID,GID}_{MIN,MAX} in /etc/login.defs. It is quite likely that
> live production systems have ids scattered throughout this range.
> Changing enewuser/enewgroup to fail if there are "conflicts" may make
> some ebuilds unusable on said systems.

Another part of this I want to do is audit all of the packages
requesting fixed UIDS/GIDS; they may not need to be doing so.

> 
> As mgorny suggests, I suspect the static UID/GID that is sometimes
> passed to enewuser/enewgroup is more of a preference/suggestion than a
> requirement.
> 
> I do not believe William's patch should be merged without some very
> good reason for the change in behavior.
 
Consider this situation:

in package-a.ebuild:
enewuser usera 300 ...
enewgroup groupa 300 ...

in package-b.ebuild:
enewuser userb 300 ...
enewgroup groupb 300 ...

Ok, cool, if I need a security context for package b, the uid/gid is 300
correct? Yes, unless package a gets installed on some host where package
b is without me knowing about it.

I see no way around this other than to make uids/gids fixed when
packages request specific ones.


William



signature.asc
Description: Digital signature


Re: [gentoo-dev] [PATCH] user.eclass: die if hard coded UID or GID is already in use

2019-05-28 Thread Mike Gilbert
On Tue, May 28, 2019 at 1:41 AM Robin H. Johnson  wrote:
>
> On Mon, May 27, 2019 at 08:44:09PM -0400, Mike Gilbert wrote:
> > On Mon, May 27, 2019 at 11:45 AM William Hubbs  wrote:
> > >
> > > If a package hard codes the UID or GID when adding a user or group to
> > > the system and that UID/GID already exists, we should abort rather than
> > > changing the UID/GID.
> > These functions have behaved this way for a long time.
> Yes, I recall this breakage being raised even prior to GLEP27.
>
> Some coverage of prior work firstly.
> 2003/May: eid_database [1]
> 2004/May: GLEP27 [2]
> 2006/Summer: GSOC Project for GLEP27 implementation [3]
> Later: Creandus [4][5]
>
> [1] https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-src/eid_database/
> [2] https://www.gentoo.org/glep/glep-0027.html
> [3] 
> https://github.com/creandus/creandus.github.com/blob/master/glep27-proposal.txt
> [4] https://github.com/creandus/
> [5] http://creandus.github.io/
>
> > What problem are you trying to solve here?
> Specifically:
> 1. Package A is emerged, and the ebuild specifies enewuser/enewgroup
> with -1 as the numeric input, and some user or group is created with
> value X.
> 2.  Package B is emerged, and the ebuild specifies enewuser/enewgroup
> with a fixed UID/GID value Y && explicitly depends on having that
> specific value.

Could you provide some examples of packages that require a specific
numeric UID/GID to function properly? That seems like a significant
design flaw.

> I'd like to accept WilliamH's patch as-is, and then add an additional
> patch that moves the range of dynamically allocated users such that it
> does not conflict.

The "dynamic range" currently defaults to 101 to 999 as defined by
SYS_{UID,GID}_{MIN,MAX} in /etc/login.defs. It is quite likely that
live production systems have ids scattered throughout this range.
Changing enewuser/enewgroup to fail if there are "conflicts" may make
some ebuilds unusable on said systems.

As mgorny suggests, I suspect the static UID/GID that is sometimes
passed to enewuser/enewgroup is more of a preference/suggestion than a
requirement.

I do not believe William's patch should be merged without some very
good reason for the change in behavior.



Re: [gentoo-dev] [PATCH] user.eclass: die if hard coded UID or GID is already in use

2019-05-27 Thread Robin H. Johnson
On Mon, May 27, 2019 at 08:44:09PM -0400, Mike Gilbert wrote:
> On Mon, May 27, 2019 at 11:45 AM William Hubbs  wrote:
> >
> > If a package hard codes the UID or GID when adding a user or group to
> > the system and that UID/GID already exists, we should abort rather than
> > changing the UID/GID.
> These functions have behaved this way for a long time.
Yes, I recall this breakage being raised even prior to GLEP27.

Some coverage of prior work firstly.
2003/May: eid_database [1]
2004/May: GLEP27 [2]
2006/Summer: GSOC Project for GLEP27 implementation [3]
Later: Creandus [4][5]

[1] https://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-src/eid_database/
[2] https://www.gentoo.org/glep/glep-0027.html
[3] 
https://github.com/creandus/creandus.github.com/blob/master/glep27-proposal.txt
[4] https://github.com/creandus/
[5] http://creandus.github.io/

> What problem are you trying to solve here?
Specifically:
1. Package A is emerged, and the ebuild specifies enewuser/enewgroup
with -1 as the numeric input, and some user or group is created with
value X.
2.  Package B is emerged, and the ebuild specifies enewuser/enewgroup
with a fixed UID/GID value Y && explicitly depends on having that
specific value.

If X == Y, then the actual outcome is that B gets dynamic allocation of
the UID/GID values, rather than the intended fixed values, and can
break. It also goes against the intent of passing a fixed value in the
first place!

Debian's solution [6][7] here is to have separate numeric ranges for
statically assigned values vs dynamically assigned values, so that they
will never conflict.

This does still run into trouble for some cases where multiple systems
want the same user/group to have the same numeric value, typically where
something is being shared (user databases, files).

I used to know RedHat's solution as well, but I can't remember any
details of it at the moment.

I'd like to accept WilliamH's patch as-is, and then add an additional
patch that moves the range of dynamically allocated users such that it
does not conflict.

[6] https://www.debian.org/doc/debian-policy/ch-opersys.html#introduction
[7] https://salsa.debian.org/debian/base-passwd

-- 
Robin Hugh Johnson
Gentoo Linux: Dev, Infra Lead, Foundation Treasurer
E-Mail   : robb...@gentoo.org
GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85
GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136


signature.asc
Description: PGP signature


Re: [gentoo-dev] [PATCH] user.eclass: die if hard coded UID or GID is already in use

2019-05-27 Thread Michał Górny
On Mon, 2019-05-27 at 10:45 -0500, William Hubbs wrote:
> If a package hard codes the UID or GID when adding a user or group to
> the system and that UID/GID already exists, we should abort rather than
> changing the UID/GID.

I think the major usage of this argument is not to enforce a specific
UID/GID but to specify a 'preferred' UID/GID, i.e. somewhat attempt
to build Gentoo systems with stable UID/GIDs.  That's why it's non-
fatal.

I don't have a strong opinion on changing it.  I don't know if we have
any actual use cases where UID/GID needs to be enforced.

> ---
>  eclass/user.eclass | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/eclass/user.eclass b/eclass/user.eclass
> index f6a10a6bee2..0d0f9d9eb89 100644
> --- a/eclass/user.eclass
> +++ b/eclass/user.eclass
> @@ -130,7 +130,8 @@ enewuser() {
>   if [[ -n ${euid} && ${euid} != -1 ]] ; then
>   if [[ ${euid} -gt 0 ]] ; then
>   if [[ -n $(egetent passwd ${euid}) ]] ; then
> - euid="next"
> + eerror "UID is already taken"
> + die "user ${euser} needs a new UID"
>   fi
>   else
>   eerror "Userid given but is not greater than 0 !"
> @@ -290,7 +291,8 @@ enewgroup() {
>   if [[ ! -z ${egid} ]] ; then
>   if [[ ${egid} -gt 0 ]] ; then
>   if [[ -n $(egetent group ${egid}) ]] ; then
> - egid="next available; requested gid taken"
> + eerror "GID is already taken"
> + die "group ${egroup} needs a new GID"
>   fi
>   else
>   eerror "Groupid given but is not greater than 0 !"

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part


Re: [gentoo-dev] [PATCH] user.eclass: die if hard coded UID or GID is already in use

2019-05-27 Thread Mike Gilbert
On Mon, May 27, 2019 at 11:45 AM William Hubbs  wrote:
>
> If a package hard codes the UID or GID when adding a user or group to
> the system and that UID/GID already exists, we should abort rather than
> changing the UID/GID.

These functions have behaved this way for a long time.

What problem are you trying to solve here?



[gentoo-dev] [PATCH] user.eclass: die if hard coded UID or GID is already in use

2019-05-27 Thread William Hubbs
If a package hard codes the UID or GID when adding a user or group to
the system and that UID/GID already exists, we should abort rather than
changing the UID/GID.
---
 eclass/user.eclass | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/eclass/user.eclass b/eclass/user.eclass
index f6a10a6bee2..0d0f9d9eb89 100644
--- a/eclass/user.eclass
+++ b/eclass/user.eclass
@@ -130,7 +130,8 @@ enewuser() {
if [[ -n ${euid} && ${euid} != -1 ]] ; then
if [[ ${euid} -gt 0 ]] ; then
if [[ -n $(egetent passwd ${euid}) ]] ; then
-   euid="next"
+   eerror "UID is already taken"
+   die "user ${euser} needs a new UID"
fi
else
eerror "Userid given but is not greater than 0 !"
@@ -290,7 +291,8 @@ enewgroup() {
if [[ ! -z ${egid} ]] ; then
if [[ ${egid} -gt 0 ]] ; then
if [[ -n $(egetent group ${egid}) ]] ; then
-   egid="next available; requested gid taken"
+   eerror "GID is already taken"
+   die "group ${egroup} needs a new GID"
fi
else
eerror "Groupid given but is not greater than 0 !"
-- 
2.21.0