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

[Jeff Layton <jlayton at redhat dot com>]

On Mon, 20 Dec 2010 00:01:28 -0500
Chuck Lever <chucklever@gmail.com> wrote:

> On Sun, Dec 19, 2010 at 12:16 PM, Jeff Layton <jlayton@redhat.com> wrote:
> > On Sat, 18 Dec 2010 20:26:28 -0500 Chuck Lever <chucklever@gmail.com> wrote:
> >> > 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.
> 
> I think we can also argue that of those that don't deal with long
> lists of supplemental groups, many of them are just like showmount and
> umount, where it doesn't matter that the server doesn't see all the
> supplemental groups anyway.
> 
> > 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.
> 
> I was ambivalent when we discussed this in February, but I'm beginning
> to disagree with allowing authunix_create_default() to fail in this
> case.
> 
> RPC library implementations have already addressed this question, and
> have decided that truncating is the conventional way to deal with it.
> If an application needs a long supplemental group list, the correct
> general solution is to convert it to use AUTH_GSS, but that's a lot of
> work.  In the meantime, the convention of truncating that list in the
> default case allows a myriad of legacy applications to continue to
> interoperate without change.
> 
> This is legacy behavior that is expected by applications that use the
> default AUTH_SYS authenticator.  Thus I believe more people want this
> API to simply work rather than to be rigged to identify further bugs.
> Failing to send all supplemental groups is less surprising behavior
> than failing outright, in this case.  authunix_create_default() is a
> high-level "do the best you can" kind of API, so it should try harder
> not to fail.
> 
> Steve's stated preference at the beginning of this thread was to shape
> libtirpc to ease the transition of applications from glibc's RPC API
> to libtirpc.  This behavior seems to be high up on the list of things
> that break applications during such a transition (as you know, it also
> bit nfs-utils).
> 
> A fix for our libtirpc is in:
> 
>   git://git.linux-nfs.org/projects/cel/libtirpc.git
> 
> I have a little more research to do on this to confirm it, but I think
> Andrew is correct: other implementations (including the Linux kernel
> RPC client) already handle AUTH_SYS with a long supplemental group
> list by truncating the list, so our libtirpc should do this as well in
> the default case.
> 

Fair enough. I don't feel strongly about it either way, TBH. I was
mostly playing Devil's advocate. :)

If the convention is to truncate the group list, then libtirpc should
probably follow it.

-- 
Jeff Layton <jlayton@redhat.com>