RESTRICTED_STATD

RESTRICTED_STATD

[Chuck Lever]

Hi guys-

I was wondering if anyone ever builds nfs-utils with RESTRICTED_STATD  
undefined these days.  It seems totally insecure to do.  Is it still  
necessary to keep this?

It would be easier to understand, update, and test the logic in utils/ 
statd/monitor.c (IPv6-wise) if we could remove the unused parts of  
this code.

I propose removing RESTRICTED_STATD, leaving in the secure version of  
the code permanently and removing the insecure parts that are left out  
when RESTRICTED_STATD is undefined.

Thoughts?

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

Re: RESTRICTED_STATD

[NeilBrown]

On Wed, August 27, 2008 6:41 am, Chuck Lever wrote:
> Hi guys-
>
> I was wondering if anyone ever builds nfs-utils with RESTRICTED_STATD
> undefined these days.  It seems totally insecure to do.  Is it still
> necessary to keep this?
>
> It would be easier to understand, update, and test the logic in utils/
> statd/monitor.c (IPv6-wise) if we could remove the unused parts of
> this code.
>
> I propose removing RESTRICTED_STATD, leaving in the secure version of
> the code permanently and removing the insecure parts that are left out
> when RESTRICTED_STATD is undefined.
>
> Thoughts?

I fully agree and support the idea!

NeilBrown

Re: RESTRICTED_STATD

[Steve Dickson]

Chuck Lever wrote:
> Hi guys-
> 
> I was wondering if anyone ever builds nfs-utils with RESTRICTED_STATD
> undefined these days.  It seems totally insecure to do.  Is it still
> necessary to keep this?
> 
> It would be easier to understand, update, and test the logic in
> utils/statd/monitor.c (IPv6-wise) if we could remove the unused parts of
> this code.
> 
> I propose removing RESTRICTED_STATD, leaving in the secure version of
> the code permanently and removing the insecure parts that are left out
> when RESTRICTED_STATD is undefined.
> 
> Thoughts?
I seem to remember enabling RESTRICTED_STATD cause problems with
portmapper and kernel interactions which causes me to turn it off...
So if we do permanently turn on the code, let make sure lock recover
and such still work...

steved.
 

Re: RESTRICTED_STATD

[Chuck Lever]

On Wed, Aug 27, 2008 at 6:57 AM, Steve Dickson <SteveD@redhat.com> wrote:
>
>
> Chuck Lever wrote:
>> Hi guys-
>>
>> I was wondering if anyone ever builds nfs-utils with RESTRICTED_STATD
>> undefined these days.  It seems totally insecure to do.  Is it still
>> necessary to keep this?
>>
>> It would be easier to understand, update, and test the logic in
>> utils/statd/monitor.c (IPv6-wise) if we could remove the unused parts of
>> this code.
>>
>> I propose removing RESTRICTED_STATD, leaving in the secure version of
>> the code permanently and removing the insecure parts that are left out
>> when RESTRICTED_STATD is undefined.
>>
>> Thoughts?
> I seem to remember enabling RESTRICTED_STATD cause problems with
> portmapper and kernel interactions which causes me to turn it off...
> So if we do permanently turn on the code, let make sure lock recover
> and such still work...

Enabling RESTRICTED_STATD is the current default.

Disabling RESTRICTED_STATD allows remote hosts to register
notification requests with rpc.statd.  It's called out in a 1999 CERT
advisory.  I don't think any distribution would ever want to allow
this.

However, there may be some folks who build rpc.statd themselves for
specialized applications may miss it if we pull it out.

-- 
"If you simplify your English, you are freed from the worst follies of
orthodoxy."
 -- George Orwell

Re: RESTRICTED_STATD

[Chuck Lever]

On Aug 27, 2008, at Aug 27, 2008, 10:14 AM, Chuck Lever wrote:
> On Wed, Aug 27, 2008 at 6:57 AM, Steve Dickson <SteveD@redhat.com>  
> wrote:
>>
>>
>> Chuck Lever wrote:
>>> Hi guys-
>>>
>>> I was wondering if anyone ever builds nfs-utils with  
>>> RESTRICTED_STATD
>>> undefined these days.  It seems totally insecure to do.  Is it still
>>> necessary to keep this?
>>>
>>> It would be easier to understand, update, and test the logic in
>>> utils/statd/monitor.c (IPv6-wise) if we could remove the unused  
>>> parts of
>>> this code.
>>>
>>> I propose removing RESTRICTED_STATD, leaving in the secure version  
>>> of
>>> the code permanently and removing the insecure parts that are left  
>>> out
>>> when RESTRICTED_STATD is undefined.
>>>
>>> Thoughts?
>> I seem to remember enabling RESTRICTED_STATD cause problems with
>> portmapper and kernel interactions which causes me to turn it off...
>> So if we do permanently turn on the code, let make sure lock recover
>> and such still work...
>
> Enabling RESTRICTED_STATD is the current default.
>
> Disabling RESTRICTED_STATD allows remote hosts to register
> notification requests with rpc.statd.  It's called out in a 1999 CERT
> advisory.  I don't think any distribution would ever want to allow
> this.
>
> However, there may be some folks who build rpc.statd themselves for
> specialized applications may miss it if we pull it out.


A little follow-up here.

Steve and I looked at the nfs-utils-1.1.3 RPM for Fedora today.  I did  
an "rpmbuild -bc" and looked at it's config.h, and RESTRICTED_STATD is  
defined as 1.  So it uses the default.

Looking at the code, it appears that when RESTRICTED_STATD is set,  
NL_ADDR() is always going to be the loopback address.  Neil, is that  
your understanding of this code?

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

Re: RESTRICTED_STATD

[NeilBrown]

On Sat, August 30, 2008 6:56 am, Chuck Lever wrote:
> A little follow-up here.
>
> Steve and I looked at the nfs-utils-1.1.3 RPM for Fedora today.  I did
> an "rpmbuild -bc" and looked at it's config.h, and RESTRICTED_STATD is
> defined as 1.  So it uses the default.
>
> Looking at the code, it appears that when RESTRICTED_STATD is set,
> NL_ADDR() is always going to be the loopback address.  Neil, is that
> your understanding of this code?
>
Nearly.
If RESTRICTED_STATD is defined (to anything), MON, UNMON, UNMON_ALL
and SIMU_CRASH are only honour if they come from 127.0.0.1, so the
callback address (NL_ADDR) for any service that statd is monitoring
will always be local.
Only NOTIFY can come from other hosts (to tell us they rebooted).

Also, only the lockd callback service is recognised.  If any service
other than lockd registers a callback it will be ignored, even if it
is from localhost.

This last point is the only bit that could conceivably cause a problem.

However we don't really want any user to be able to request a callback
to any random service....
I wonder if anyone uses for statd for anything but lockd, and how
could we know?

NeilBrown

Re: RESTRICTED_STATD

[Chuck Lever]

On Aug 29, 2008, at Aug 29, 2008, 5:31 PM, NeilBrown wrote:
> On Sat, August 30, 2008 6:56 am, Chuck Lever wrote:
>> A little follow-up here.
>>
>> Steve and I looked at the nfs-utils-1.1.3 RPM for Fedora today.  I  
>> did
>> an "rpmbuild -bc" and looked at it's config.h, and RESTRICTED_STATD  
>> is
>> defined as 1.  So it uses the default.
>>
>> Looking at the code, it appears that when RESTRICTED_STATD is set,
>> NL_ADDR() is always going to be the loopback address.  Neil, is that
>> your understanding of this code?
>>
> Nearly.
> If RESTRICTED_STATD is defined (to anything), MON, UNMON, UNMON_ALL
> and SIMU_CRASH are only honour if they come from 127.0.0.1, so the
> callback address (NL_ADDR) for any service that statd is monitoring
> will always be local.

The value of NL_ADDR is determined only by sm_mon_1_svc() as far as I  
can tell.  If RESTRICTED_STATD is defined, NL_ADDR is always set to  
127.0.0.1.

For IPv6, we would need to check the incoming address for both  
127.0.0.1 or ::1.  Outgoing we can always use 127.0.0.1 for the time  
being.  I was hoping to eliminate NL_ADDR, and just use a constant  
127.0.0.1 where needed.

> Only NOTIFY can come from other hosts (to tell us they rebooted).

Right.  sm_notify_1_svc() grabs the callers IP address with

    svc_getcaller(rqstp->rq_xprt)->sin_addr

It converts this to a string and checks this against lp->dns_name, in  
addition to checking the mon_name that was originally registered to be  
monitored.  Shouldn't statd check only mon_name against dns_name?  Why  
does it check both?

This would need IPv6 support.  It would be easier just to eliminate it  
if possible, and just check mon_name.  It then wouldn't need any IPv6- 
specific logic.

> Also, only the lockd callback service is recognised.  If any service
> other than lockd registers a callback it will be ignored, even if it
> is from localhost.
>
> This last point is the only bit that could conceivably cause a  
> problem.
>
> However we don't really want any user to be able to request a callback
> to any random service....
> I wonder if anyone uses for statd for anything but lockd, and how
> could we know?

I think the real question is whether we should continue to support  
this "off-label" use.  It adds complexity and security problems, and  
the code paths that support this aren't ever tested these days, I'm  
willing to bet.

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

Re: RESTRICTED_STATD

[Neil Brown]

On Tuesday September 2, chuck.lever@oracle.com wrote:
> > Only NOTIFY can come from other hosts (to tell us they rebooted).
> 
> Right.  sm_notify_1_svc() grabs the callers IP address with
> 
>     svc_getcaller(rqstp->rq_xprt)->sin_addr
> 
> It converts this to a string and checks this against lp->dns_name, in  
> addition to checking the mon_name that was originally registered to be  
> monitored.  Shouldn't statd check only mon_name against dns_name?  Why  
> does it check both?

If it was to only check one, it would probably to check ip_addr
against dns_name.

The IP address of that the SM_NOTIFY came from is the most reliable
thing we have to identify which host just rebooted.  We use that to
find a 'dns_name' when we first MONitor a host, and use that name for
the file stored in /var/lib/nfs/sm.  We then match the source of
SM_NOTIFY against those file names.

So I think this part of the code really does need to be IPv6-aware.
Certainly matchhostname does.

> > However we don't really want any user to be able to request a callback
> > to any random service....
> > I wonder if anyone uses for statd for anything but lockd, and how
> > could we know?
> 
> I think the real question is whether we should continue to support  
> this "off-label" use.  It adds complexity and security problems, and  
> the code paths that support this aren't ever tested these days, I'm  
> willing to bet.

How about we subtly break it, and then we nobody complains for 12
months, remove it as it was broken anyway :-)

I'm think I'm happy with removing any support for non-lockd uses for
statd.

NeilBrown

Re: RESTRICTED_STATD

[Olaf Kirch]

On Thursday 04 September 2008 08:03:16 Neil Brown wrote:
> If it was to only check one, it would probably to check ip_addr
> against dns_name.

Right. Comparing the mon_name and the the result of the DNS reverse
lookup is additional paranoia, and should be configurable. In some
environments, not all machines will have reverse DNS entries, or
if they do, the name will not necessarily match what they've set
as their hostname. You could argue that this is a broken configuration,
but it is certainly not that uncommon.

> The IP address of that the SM_NOTIFY came from is the most reliable
> thing we have to identify which host just rebooted.  We use that to
> find a 'dns_name' when we first MONitor a host, and use that name for
> the file stored in /var/lib/nfs/sm.  We then match the source of
> SM_NOTIFY against those file names.
> 
> So I think this part of the code really does need to be IPv6-aware.
> Certainly matchhostname does.

Yes.

> > > However we don't really want any user to be able to request a callback
> > > to any random service....
> > > I wonder if anyone uses for statd for anything but lockd, and how
> > > could we know?
> > 
> > I think the real question is whether we should continue to support  
> > this "off-label" use.  It adds complexity and security problems, and  
> > the code paths that support this aren't ever tested these days, I'm  
> > willing to bet.
> 
> How about we subtly break it, and then we nobody complains for 12
> months, remove it as it was broken anyway :-)
> 
> I'm think I'm happy with removing any support for non-lockd uses for
> statd.

Me too. The whole NSM thing was totally over-engineered from day one.

Olaf
-- 
 And mention in the Fitz incident that DCOP is no ego shooter!
                                --micha istinie, 2001

Re: RESTRICTED_STATD

[Chuck Lever]

Hi Neil-

On Sep 4, 2008, at Sep 4, 2008, 2:03 AM, Neil Brown wrote:
> On Tuesday September 2, chuck.lever@oracle.com wrote:
>>> Only NOTIFY can come from other hosts (to tell us they rebooted).
>>
>> Right.  sm_notify_1_svc() grabs the callers IP address with
>>
>>    svc_getcaller(rqstp->rq_xprt)->sin_addr
>>
>> It converts this to a string and checks this against lp->dns_name, in
>> addition to checking the mon_name that was originally registered to  
>> be
>> monitored.  Shouldn't statd check only mon_name against dns_name?   
>> Why
>> does it check both?
>
> If it was to only check one, it would probably to check ip_addr
> against dns_name.
>
> The IP address of that the SM_NOTIFY came from is the most reliable
> thing we have to identify which host just rebooted.  We use that to
> find a 'dns_name' when we first MONitor a host, and use that name for
> the file stored in /var/lib/nfs/sm.  We then match the source of
> SM_NOTIFY against those file names.

Honestly, I would have thought exactly the opposite is true.  How  
would that work with multi-homed peers?  Or with peers that might get  
their IP addresses via DHCP?

AFAICT the reason why using only mon_name is sometimes broken is  
because the Linux implementation (and it is probably not alone in this  
regard) chooses a mon_name that is often not unique.

Sometimes it's simply the unqualified hostname.  If you have a  
"joe.server.example.com" and a "joe.desktop.example.com" it would be  
pretty difficult for statd to distinguish them by their unqualified  
hostnames alone.

Really lockd/sm-notify should select a mon_name with a better  
guarantee of uniqueness.  A base64 hash of one of the NICs on the  
peer, for example, would be much better.  Hash in the wall clock time  
and date, and store it in a file in /var/lib/nfs so it doesn't change  
if the NIC hardware is moved.

Is there any guidance in the Open Group standard on how to match  
SM_NOTIFY requests to SM_MON?  I had thought it was "use the mon_name,  
Luke".

> So I think this part of the code really does need to be IPv6-aware.
> Certainly matchhostname does.
>
>>> However we don't really want any user to be able to request a  
>>> callback
>>> to any random service....
>>> I wonder if anyone uses for statd for anything but lockd, and how
>>> could we know?
>>
>> I think the real question is whether we should continue to support
>> this "off-label" use.  It adds complexity and security problems, and
>> the code paths that support this aren't ever tested these days, I'm
>> willing to bet.
>
> How about we subtly break it, and then we nobody complains for 12
> months, remove it as it was broken anyway :-)

And normally I would take that approach.  In this case, folks want  
real IPv6 support yesterday...

> I'm think I'm happy with removing any support for non-lockd uses for
> statd.

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

Re: RESTRICTED_STATD

[Neil Brown]

Hi Chuck,

On Thursday September 4, chuck.lever@oracle.com wrote:
> >
> > The IP address of that the SM_NOTIFY came from is the most reliable
> > thing we have to identify which host just rebooted.  We use that to
> > find a 'dns_name' when we first MONitor a host, and use that name for
> > the file stored in /var/lib/nfs/sm.  We then match the source of
> > SM_NOTIFY against those file names.
> 
> Honestly, I would have thought exactly the opposite is true.  How  
> would that work with multi-homed peers?  Or with peers that might get  
> their IP addresses via DHCP?

The DNS is used specifically to deal with multi-homed peers.  We used
to just store IP addesses and match on IP addresses.  But that doesn't
work with multi-homed hosts.
The fully qualified DNS name is the closest thing we have to a
reliable unique host identifier, so that is what we use.


Clients that use DHCP might be a problem if it isn't hooked in to the
DNS properly.  I don't think there is any really good general solution
to that problem.  I certainly would trust the monname reported by such
a client.

The problem here is that STATMON is over-engineered and
under-specified.  You cannot implement it "correctly".  The best we
can do is to do our best.  And I think that means using DNS names as
the primary key.  It is at least a well understood primary key.

> 
> Is there any guidance in the Open Group standard on how to match  
> SM_NOTIFY requests to SM_MON?  I had thought it was "use the mon_name,  
> Luke".

I don't know what the Open Group standards say.  My vague memory is
"not very much" but I could be wrong.  However I think that "always
use the mon_name" doesn't actually work in practice, so it doesn't
really matter if it is a standard or not.

> 
> And normally I would take that approach.  In this case, folks want  
> real IPv6 support yesterday...

The cynic in me wonders if this is just so they can tick the box, or
if there is a real use case that demands it.  Hopefully it is the
latter.  :-)

NeilBrown

Re: RESTRICTED_STATD

[Chuck Lever]

On Thu, Sep 4, 2008 at 9:26 PM, Neil Brown <neilb@suse.de> wrote:
> Hi Chuck,
>
> On Thursday September 4, chuck.lever@oracle.com wrote:
>> >
>> > The IP address of that the SM_NOTIFY came from is the most reliable
>> > thing we have to identify which host just rebooted.  We use that to
>> > find a 'dns_name' when we first MONitor a host, and use that name for
>> > the file stored in /var/lib/nfs/sm.  We then match the source of
>> > SM_NOTIFY against those file names.
>>
>> Honestly, I would have thought exactly the opposite is true.  How
>> would that work with multi-homed peers?  Or with peers that might get
>> their IP addresses via DHCP?
>
> The DNS is used specifically to deal with multi-homed peers.  We used
> to just store IP addesses and match on IP addresses.  But that doesn't
> work with multi-homed hosts.
> The fully qualified DNS name is the closest thing we have to a
> reliable unique host identifier, so that is what we use.
>
>
> Clients that use DHCP might be a problem if it isn't hooked in to the
> DNS properly.  I don't think there is any really good general solution
> to that problem.  I certainly would trust the monname reported by such
> a client.
>
> The problem here is that STATMON is over-engineered and
> under-specified.  You cannot implement it "correctly".  The best we
> can do is to do our best.

Much like NLM.

> And I think that means using DNS names as
> the primary key.  It is at least a well understood primary key.

Yes, I now see that sm_mon_1_svc() converts the IP address to a DNS
name before it adds the host to its notification list.  That will have
to use getaddrinfo(3) to support IPv6 lookups.

I have to wonder how DNS resolution works for an unqualified mon_name.
 It likely tacks the local domain name on before doing the lookup,
which will cause incorrect results.

If the DNS configuration changes while a host is being monitored, we
are also hosed.

Would it ever be a problem for our statd implementation if the DNS
wasn't responding quickly or at all?

>> Is there any guidance in the Open Group standard on how to match
>> SM_NOTIFY requests to SM_MON?  I had thought it was "use the mon_name,
>> Luke".
>
> I don't know what the Open Group standards say.  My vague memory is
> "not very much" but I could be wrong.  However I think that "always
> use the mon_name" doesn't actually work in practice, so it doesn't
> really matter if it is a standard or not.

I appreciate the explanation.

>> And normally I would take that approach.  In this case, folks want
>> real IPv6 support yesterday...
>
> The cynic in me wonders if this is just so they can tick the box, or
> if there is a real use case that demands it.  Hopefully it is the
> latter.  :-)

As far as I know, it is some of both.  But frankly we can't expect
folks to develop and test real software on IPv6 until we have stable
infrastructure (NFS and everything else).

So we are blocking others until we have NFS support.  Plus the lead
time for getting these features into enterprise distributions is
nearly a year.

--
Chuck Lever

Re: RESTRICTED_STATD

[Olaf Kirch]

On Friday 05 September 2008 03:26:55 Neil Brown wrote:
> I don't know what the Open Group standards say.  My vague memory is

In essence, "use the mon_name, Luke":

  When an NSM receives an SM_NOTIFY call from remote NSM, it must
  search the notify list for the host specified in the SM_NOTIFY call,
  if it is found the RPC specified in "mon_id.my_id" is made.

> "not very much" but I could be wrong.  However I think that "always
> use the mon_name" doesn't actually work in practice, so it doesn't
> really matter if it is a standard or not.

It works as long as the client uses the same name in its lockd
calls (as nlm_host) and in its statd calls. And actually the IP
address is more volatile than the host name.

The are exceptions such as clustered environments, where services move
around along with their IPs. In these cases the IP address will be
constant, but the hostname may change. But that's a relatively rare
configuration, and I think I even added a switch to sm_notify at
some point to help people who use this.

The problem that always existed was lack of security. Anyone can fake
a SM_NOTIFY call, which in essence will drop all locks held by the
spoofed client. That's what I meant when I wrote in my previous email
"Comparing the mon_name and the the result of the DNS reverse
lookup is additional paranoia, and should be configurable."

The primary match when handling SM_NOTIFY should be based on the mon_id.
Comparison of DNS names are an additional paranoia check.

> The cynic in me wonders if this is just so they can tick the box, or
> if there is a real use case that demands it.  Hopefully it is the
> latter.  :-)

I still think we will have IPv6 one day. It's kind of inevitable -
but as long as we don't support it fully, people won't start
using it seriously. And the whole RPC area is one of the major
road blocks in ipv6 adoption in the Linux world.

Olaf
-- 
 And mention in the Fitz incident that DCOP is no ego shooter!
                                --micha istinie, 2001