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

[Chuck Lever <chucklever at gmail dot com>]

Hi Andrew-

On Sat, Dec 18, 2010 at 5:44 PM, Andrew J. Schorr
<ajschorr@alumni.princeton.edu> wrote:
> On Sat, Dec 18, 2010 at 05:33:54PM -0500, Chuck Lever wrote:
>> If there is a common solution to this issue among RPC implementations,
>> then we should adopt that.
>>
>> The discussion we had 9 months ago was among a former Sun NFS
>> engineer, a CIFS developer, and myself (10 years of experience with
>> the Linux NFS and RPC implementations in user space and in the
>> kernel). ?Either we were not aware that truncation is the prevalent
>> behavior, or we decided at the time that truncation was just as
>> incorrect as failing outright.
>>
>> But we can look into it again, as I said.
>
> I certainly don't claim to be an expert. ?My understanding (which could
> be flawed) is that NFS has traditionally handled this issue by
> truncating to 16 groups. ?One can google for NFS 16 groups, and there
> are many hits discussing this behavior. ?And truncation is obviously how
> glibc handles it. ?If the goal is to replace the glibc implementation
> with tirpc, then it seems rather dangerous to me to break various applications
> that used to work under glibc...

Yes, backwards compatibility is a concern, as is compatibility with
the existing RPC API specification (the Sun doc I referred to earlier
in this thread).

I've checked, and a modern version of libtirpc on Solaris (a
descendent of our libtirpc implementation, which originally came from
Sun in the 1990's) behaves just as you describe: it truncates at NGRPS
(16).  Existing glibc implementation aside, I take the modern Solaris
libtirpc as a reference implementation for the RPC API.  It's likely
this problem was fixed after our version of libtirpc forked from the
Sun version.

I also found Jeff's patch that removes the abort() calls, dated March
5, 2010, in my clone of the upstream libtirpc repo.  I wonder why your
version doesn't have this patch.

> As I mentioned, if there's doubt, let's just add both functions to the API and
> let programmers choose. ?My guess is that most would choose the version that
> truncates to 16 groups instead of crashing.

I don't think it's practical to introduce new APIs in this library,
for the reasons cited above.  But perhaps we don't have to, in light
of the fact that modern Solaris behavior agrees with what is
implemented in glibc today.

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.

-- 
"What is a pancake, if not a big, fluffy Eucharist?"
?-- Stephen Colbert