Re: [PATCH] buildchroot: Align UID and GID of builder user with caller

[Henning Schild <henning.schild@siemens.com>]

Am Mon, 12 Nov 2018 13:27:34 +0100
schrieb Jan Kiszka <jan.kiszka@siemens.com>:

> On 12.11.18 13:11, Jan Kiszka wrote:
> > On 12.11.18 12:58, Henning Schild wrote:  
> >> Am Mon, 12 Nov 2018 11:09:27 +0100
> >> schrieb Jan Kiszka <jan.kiszka@siemens.com>:
> >>  
> >>> On 12.11.18 11:06, Henning Schild wrote:  
> >>>> Am Mon, 12 Nov 2018 10:52:22 +0100
> >>>> schrieb Jan Kiszka <jan.kiszka@siemens.com>:  
> >>>>> On 12.11.18 10:42, Henning Schild wrote:  
> >>>>>> Am Mon, 12 Nov 2018 10:19:54 +0100
> >>>>>> schrieb Jan Kiszka <jan.kiszka@siemens.com>:  
> >>>>>>> On 12.11.18 10:16, [ext] Henning Schild wrote:  
> >>>>>>>> I am afraid that this is not correct. The ids you are taking
> >>>>>>>> from the "host" might be taken inside the chroot. As a result
> >>>>>>>> creating the user/group would fail. Chances might be low ...
> >>>>>>>> This also assumes that  
> >>>>>>>
> >>>>>>> Really? I thought that these commands are run very early
> >>>>>>> during bootstrap where there are no other users - if not,
> >>>>>>> that would be a bug.  
> >>>>>>
> >>>>>> I think the only uid/gid you can really be sure about is 0. 1
> >>>>>> could already be a regular user on the host, and 1 is "daemon"
> >>>>>> on a current debian ... probably there right after
> >>>>>> debootstrap.  
> >>>>>
> >>>>> Let me check if we can move the ID assignment earlier, to reduce
> >>>>> that risk.  
> >>>>
> >>>> I will look into it. Knowing a problem and reducing the risk is
> >>>> not good enough.  
> >>>>>>
> >>>>>> 1000 being the first "user" is more a convention than something
> >>>>>> you can rely on for any host. (/etc/login.defs UID_MIN/MAX
> >>>>>> etc.)  
> >>>>>
> >>>>> We are talking about transferring the ID's from the host Debian
> >>>>> to the buildchroot Debian - is there really a realistic risk of
> >>>>> friction?  
> >>>>
> >>>> Now you are assuming that everyone is using your container ;).
> >>>> While  
> >>>
> >>> No, this is not about the container. We already solved the problem
> >>> for the container long ago (by aligning IDs). This breakage is
> >>> about the host (in the container or on your host) and the
> >>> buildchroot.  
> >>
> >> If the container has aligned IDs that was a hack as well. I guess
> >> for the same problem i just found ....  
> > 
> > No, this is working smoothly for quite a while now, also in many CI
> > setup, because we control the container content and the fact that
> > there are practically no ID overlaps. It is a mandatory feature
> > there, for the same reasons like here. 
> >> The problem is a "chown -R" in do_unpack that brings the hosts uid
> >> into the chroot.
> >> That should be fixed properly some day ... i just sent a workaround
> >> patch.  
> > 
> > I'm rather in favor of a proper fix to the ID mess than more
> > working around it. The benefit of going for the calling user ID
> > would be - if implemented properly 
> > - that we will have less files owned by root or some other users
> > unknown to the build host, and can therefore only be purged by
> > means of sudo.  
> 
> Actually, also 624b7c484bf5 could be reverted if we managed to align
> IDs...

I did not go all they way back to blame that chown. In fact reverting
624b7c484bf5 should solve the issue.
In a buildchroot we used to have just one user "root", so chowning to
anyone else is just wrong. After my patch you can now chown to
builder. From inside with "builder" or from outside with the id you
extract from etc/passwd. If "file server convenience" is the only reason
behind that patch, please revert that one and forget my update.

Henning

> Jan
>