svn commit: r277652 - in head/usr.sbin/pw: . tests

[Baptiste Daroussin]

On Tue, Jan 27, 2015 at 04:08:28PM -0800, Devin Teske wrote:
> 
> > On Jan 27, 2015, at 3:44 PM, Baptiste Daroussin <bapt at FreeBSD.org> wrote:
> > 
> > On Mon, Jan 26, 2015 at 01:20:28PM -0800, Devin Teske wrote:
> >> 
> >>> On Jan 25, 2015, at 7:31 AM, Bruce Evans <brde at optusnet.com.au> wrote:
> >>> 
> >>> On Sun, 25 Jan 2015, Slawa Olhovchenkov wrote:
> >>> 
> >>>> On Sun, Jan 25, 2015 at 04:56:24PM +1100, Bruce Evans wrote:
> >>>> 
> >>>>> Negative ids have historical abuses in places like mountd.
> >> 
> >> Which paves the way for the “accepted practice” argument
> >> and backed up by “in-the-field usage” statement(s).
> >> 
> >> 
> >> 
> >>>>> mountd still
> >>>>> hard-codes -2 and -2 for the default uid and gid of an unprivileged user.
> >>>>> It at least casts these values to uid_t and gid_t before using them.
> >>>>> This gives the ids the non-random values of UINT32_MAX-1 if uid_t and
> >>>>> gid_t are uint32_t.  (If uid_t and gid_t were signed, then it would
> >>>>> leave the values as negative, so invalid.)  These magic values may work
> >>>>> better than when ids were 16 bits, since there is less risk of them
> >>>>> conflicting with a normal id.  However, the non-conflict is probably
> >>>>> a bug.  FreeBSD uses the magic ids of 65534 for user nobody: group
> >>>>> nobody.  These would have been (id_t)-2 with 16-bit ids.  They no
> >>>>> longer match, so ls displays (id_t)-2 numerically.  FreeBSD also has
> >>>>> a group nogroup = 65553 that doesn't match the nfs usage.  However2,
> >>>>> in FreeBSD-1 wher ids were 16-bits, nobody was 32767 and nogroup was
> >>>>> 32766. so they didn't match nfs for other reasons.  The 2 non-groups
> >>>>> now seem to be just a bug -- FreeBSD-1 didn't have group nobody.
> >>>>> 4.4BSD-Lite2 has the same values as FreeBSD-1.
> >>>> 
> >>>> This is not full true for ZFS case.
> >>>> On ZFS nobody is 2^32-2.
> >>> 
> >>> File systems don't get to decide this.
> >> 
> >> +1 (and thanks for the historical account, bruce — sincerely)
> >> 
> >> However, I still want to make the argument that:
> >> 
> >> a. Because we’ve supported mapping negative inputs to unsigned values in pw *for over a decade*, that…
> >> 
> >> b. We should either revert or make a relnotes submission to note that we’re changing the long-standing accepted practice.
> >> 
> >> Changing the accepted practice broke code internally, it would have likely broken some external code as well — and people deserve to know about said change else we should continue to support accepted practice that is decade(s) old.
> > 
> > It has never been accepted by pw(8) it was just not checked
> 
> Those are indistinguishable from each other by an end-user (someone not
> reading the code; and even still there is the “spirit” of the code and the “letter
> of the code").
> 
> 
> > as a result it was
> > accepting *anything* and passed it unchecked directly to atoi(3) resulting in
> > for example pw groupdel -u plop removing wheel... or pw userdel -u something
> > trying to delete root. (was this an accepted behaviour for a decade as well?)
> > 
> 
> The erroneous deletion was inherently wrong. I do not want to see that bug
> resurface (and I thank you very much for fixing it).
> 
> However, I’m sure you know that said erroneous deletion was a side-effect of
> lacking error-checks.
> 
> Again, thank you fixing the PR itemizing the erroneous deletion of users/groups.
> Please note that Bruce sent me a private e-mail with some historical background,
> to which I replied and we’re working out some logic on the side. There shouldn’t
> be much more to this thread — I envision that we’ll be approaching a solid agree-
> meant that should lead to some commits (can’t say yet what direction it will go as
> I’m looking forward to Bruce’s touch-points).

Well you may have not noticed but I revert my fix and will let someone else
handle this (feel free to revert my revert if you think it has to be done, but
still negative uid looked wrong to me hence my revert).

That said I have spent too much time in pw(8) this thing should be entirely
rewritten this is a terrible can of worm.

Bapt
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/svn-src-head/attachments/20150128/c8e68db9/attachment.sig>