This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [Libtirpc-devel] Fwd: Re: proposed patch to rpcbind to providefiner-grained security controls than offered by the -i option


On Sat, 18 Dec 2010 20:26:28 -0500
Chuck Lever <chucklever@gmail.com> wrote:

> On Sat, Dec 18, 2010 at 6:44 PM, Jeff Layton <jlayton@redhat.com> wrote:
> > On Sat, 18 Dec 2010 18:01:42 -0500
> > Chuck Lever <chucklever@gmail.com> wrote:
> >>
> >> Jeff will probably remember exactly why we did this, but I seem to
> >> recall that the application that was crashing didn't like the
> >> truncation solution either. ?I could be misremembering. ?We'll have to
> >> check in with that bug again to ensure we don't introduce a
> >> regression.
> >>
> >> I'll post a more-or-less official patch for this later, and see if I
> >> can hook up with Jeff to find out more.
> >>
> >
> > Actually, I originally proposed a fix similar to what Andrew is
> > suggesting. Peter objected and said that returning NULL was more
> > correct.
> >
> > The original problem was that umount.nfs was crashing (due to the
> > abort() calls), so we ended up fixing it to use setgroups to drop
> > supplimentary groups since it was not needed for umounts anyway. I also
> > removed those abort calls from the library.
> 
> It sounds like it shouldn't cause any problems for umount.nfs to
> truncate the supplemental groups list in the libtirpc library, just as
> glibc does.
> 

Right. umount.nfs pre-truncates the group list now so it doesn't really
matter what libtirpc does in this case.

> > I can see arguments for either way. On the one hand, people porting to
> > libtirpc are likely to be fixing code anyway -- fixing this ought to be
> > doable at the same time. It's really not hard to call setgroups to fix
> > up the groups list before you call this function.
> 
> The application should deal with this before the call anyway, IMO.
> That way it can detect the issue, intervene if needed, and warn and
> quit if it can't handle the truncation.
> 

Yes, really the application needs to be aware of the limitations of
AUTH_SYS and deal with long lists of groups appropriately. Of course, a
lot of them don't.

So really this comes down to whether you want to try and paper over
those bugs by just arbitrarily truncating the list of groups, or return
NULL. The former will mean that more programs will "just work" when
built against libtirpc (until they don't). The latter means more
hard failures when the list of groups is too long.

There's an argument to be made that the latter behavior is better since
it'll mean more programs get fixed the right way.

> > OTOH, adopting glibc's behavioe might ease the transition, even if it
> > is a hack that might lead to unreliable application behavior.
> 
> It seems to me that it has been adequately demonstrated that group
> list truncation is appropriate behavior for this interface.
> 

Yes, but there are downsides and we need to be aware of them. The
truncation is arbitrary and the problems it causes are may be
really elusive.

Since we're tossing around ideas, we could make this behavior switchable
by an environment variable. Leave the default to be a NULL return in
this situation, and allow someone to set a particular env var to make
the lib truncate the list of groups instead.

That gives a mechanism to work around this difference in behavior while
still encouraging those porting applications to deal with it more
appropriately.

-- 
Jeff Layton <jlayton@redhat.com>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]