2009-03-26 08:58:27

by Greg Banks

[permalink] [raw]
Subject: Re: [PATCH 15/23] SUNRPC: rpcbind actually interprets r_owner string

Chuck Lever wrote:
>
> Our port of rpcbind (from Sun) assumes this string contains a numeric
> UID value, not alphabetical or symbolic characters, but checks this
> value only for AF_LOCAL RPCB_SET or RPCB_UNSET requests. In all other
> cases, rpcbind ignores the contents of the r_owner string.
>

Not that this makes the slightest difference to the usefulness of the
patch, but it sounds like pretty strange behaviour for an rpcbind server
to be using an incoming r_owner value off the wire under any circumstances.

If I read the Irix and Solaris rpcbind source correctly, they will
(sensibly, IMO) ignore incoming r_owner values in all cases. When an
owner string is needed (during SET for storing in the internal data
structure, and during UNSET for checking against the internal data
structure) it is calculated from SVCXPRT itself and not from anything
the client sends. The calculation returns one of:

a) if the transport is over TLI loopback, a numerical representation of
the peer process UID as reported by the special magical TLI kernel hack
which reliably returns that, or

b) if the transport is over IP and the peer port is privileged, the
string "superuser", or

c) otherwise, the string "unknown".

AFAICS the only sensible use for the r_owner field is reporting the
internally-calculated value back to the client, e.g. for the "rpcinfo
-s" display.

> The reference user space implementation of rpcb_set(3) uses a numeric
> UID for all SET/UNSET requests (even via the network) and an empty
> string for all other requests. We emulate that behavior here to
> maintain bug-for-bug compatibility.
>

Fair enough.

Reviewed-by: Greg Banks <[email protected]>

--
Greg Banks, P.Engineer, SGI Australian Software Group.
the brightly coloured sporks of revolution.
I don't speak for SGI.



2009-03-27 15:36:31

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 15/23] SUNRPC: rpcbind actually interprets r_owner string

On Mar 26, 2009, at 7:18 PM, Greg Banks wrote:
> Chuck Lever wrote:
>> On Mar 26, 2009, at 4:58 AM, Greg Banks wrote:
>>> Chuck Lever wrote:
>>>>
>>>> Our port of rpcbind (from Sun) assumes this string contains a
>>>> numeric
>>>> UID value, not alphabetical or symbolic characters, but checks this
>>>> value only for AF_LOCAL RPCB_SET or RPCB_UNSET requests. In all
>>>> other
>>>> cases, rpcbind ignores the contents of the r_owner string.
>>>
>>> Not that this makes the slightest difference to the usefulness of
>>> the
>>> patch, but it sounds like pretty strange behaviour for an rpcbind
>>> server
>>> to be using an incoming r_owner value off the wire under any
>>> circumstances.
>>
>> It's ignored for wire requests. r_owner is used only for AF_LOCAL
>> requests (ie a local file socket) where the kernel can guarantee the
>> owner.
>>
>>
> Sorry, I'm confused now. Consider the r_owner field in the rpcb
> structure whose XDR representation flows through the AF_LOCAL
> socket; is
> that used by rpcbind at all? I don't understand how the kernel would
> guarantee that?

The library uses geteuid(2) to generate the r_owner string during
RPCB_SET, as I mentioned before. This is likely legacy behavior.

rpcbind then throws this away and uses getsockopt(SO_PEERCRED). On
network sockets I think that's always going to return a uid of -1, but
for AF_LOCAL it will have a meaningful value.

I was loose about the use of the term "r_owner" : the r_owner
parameter of RPCB_SET is always ignored, but _owner_ _information_ is
used for requests coming in via AF_LOCAL. The rpcb_clnt patch for the
kernel is nothing more than a documentation change.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

2009-03-27 22:02:33

by Greg Banks

[permalink] [raw]
Subject: Re: [PATCH 15/23] SUNRPC: rpcbind actually interprets r_owner string

Chuck Lever wrote:
> On Mar 26, 2009, at 7:18 PM, Greg Banks wrote:
>
> rpcbind then throws this away and uses getsockopt(SO_PEERCRED). On
> network sockets I think that's always going to return a uid of -1, but
> for AF_LOCAL it will have a meaningful value.
Aha. Unconfused now, thanks.

--
Greg Banks, P.Engineer, SGI Australian Software Group.
the brightly coloured sporks of revolution.
I don't speak for SGI.


2009-03-26 15:44:34

by Chuck Lever III

[permalink] [raw]
Subject: Re: [PATCH 15/23] SUNRPC: rpcbind actually interprets r_owner string

On Mar 26, 2009, at 4:58 AM, Greg Banks wrote:
> Chuck Lever wrote:
>>
>> Our port of rpcbind (from Sun) assumes this string contains a numeric
>> UID value, not alphabetical or symbolic characters, but checks this
>> value only for AF_LOCAL RPCB_SET or RPCB_UNSET requests. In all
>> other
>> cases, rpcbind ignores the contents of the r_owner string.
>
> Not that this makes the slightest difference to the usefulness of the
> patch, but it sounds like pretty strange behaviour for an rpcbind
> server
> to be using an incoming r_owner value off the wire under any
> circumstances.

It's ignored for wire requests. r_owner is used only for AF_LOCAL
requests (ie a local file socket) where the kernel can guarantee the
owner.

>> The reference user space implementation of rpcb_set(3) uses a numeric
>> UID for all SET/UNSET requests (even via the network) and an empty
>> string for all other requests. We emulate that behavior here to
>> maintain bug-for-bug compatibility.
>
> Fair enough.
>
> Reviewed-by: Greg Banks <[email protected]>

Thanks.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

2009-03-26 23:19:31

by Greg Banks

[permalink] [raw]
Subject: Re: [PATCH 15/23] SUNRPC: rpcbind actually interprets r_owner string

Chuck Lever wrote:
> On Mar 26, 2009, at 4:58 AM, Greg Banks wrote:
>> Chuck Lever wrote:
>>>
>>> Our port of rpcbind (from Sun) assumes this string contains a numeric
>>> UID value, not alphabetical or symbolic characters, but checks this
>>> value only for AF_LOCAL RPCB_SET or RPCB_UNSET requests. In all other
>>> cases, rpcbind ignores the contents of the r_owner string.
>>
>> Not that this makes the slightest difference to the usefulness of the
>> patch, but it sounds like pretty strange behaviour for an rpcbind server
>> to be using an incoming r_owner value off the wire under any
>> circumstances.
>
> It's ignored for wire requests. r_owner is used only for AF_LOCAL
> requests (ie a local file socket) where the kernel can guarantee the
> owner.
>
>
Sorry, I'm confused now. Consider the r_owner field in the rpcb
structure whose XDR representation flows through the AF_LOCAL socket; is
that used by rpcbind at all? I don't understand how the kernel would
guarantee that?

--
Greg Banks, P.Engineer, SGI Australian Software Group.
the brightly coloured sporks of revolution.
I don't speak for SGI.