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

[<peter dot staubach at emc dot com>]

Hi.

I disagree that the least surprising thing is to simply truncate the
list.  How does the user debug something which fails with some sort of
access error, he looks to see if he is in the right group and is, but it
still fails?  Do we really expect the user to count down 16 groups in
the list and figure out whether the correct group was in the first 16 or
not?  I don't think so.

The correct action is the definitive one, to fail operations which can't
be correctly handled.  This matches the expectations of most RPC
applications which have been developed and expect to run in
heterogeneous environments.

		ps


-----Original Message-----
From: Jeff Layton [mailto:jlayton@redhat.com] 
Sent: Monday, December 20, 2010 7:56 AM
To: Chuck Lever
Cc: Andrew J. Schorr; Steve Dickson; libc-alpha@sourceware.org;
libtirpc; Staubach, Peter
Subject: Re: [Libtirpc-devel] Fwd: Re: proposed patch to rpcbind to
provide finer-grained security controls than offered by the -i option

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>