Bug 714977

Summary: mixing wildcard and host/subnet exports can allow unintended hosts to mount
Product: [Fedora] Fedora Reporter: Jeff Layton <jlayton>
Component: nfs-utilsAssignee: Jeff Layton <jlayton>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: bfields, chuck.lever, jlayton, steved
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 715391 (view as bug list) Environment:
Last Closed: 2011-07-05 07:43:36 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 715391    

Description Jeff Layton 2011-06-21 09:50:41 EDT
Opening this as a nfs-utils bug, but it may be a kernel problem. It's also a bit of a "Dr, it hurts when I do this..." problem, but may be indicative of something more fundamentally wrong.

If I put this in /etc/exports, then mounting via IPv6 will (correctly) fail:

    /export 192.168.1.0/24(rw)

Mounting then works if I add in an IPv6 subnet that matches, and correctly fails if the subnet doesn't match. For instance:

    /export 192.168.1.0/24(rw) 2001:470:8:d64::/64(rw)

If I instead alter that to export to something inside square brackets:

    /export 192.168.1.0/24(rw) [whiskeytangofoxtrot](rw)

...then mounting via IPv6 works. It doesn't seem to matter what's inside the brackets. Here's the contents of nfsd.export cache after mounting. Looks like it exported to IPv4 address:

# cat /proc/net/rpc/nfsd.export/content 
#path domain(flags)
/export	192.168.1.0/24(rw,root_squash,sync,wdelay,no_subtree_check,uuid=be37e6a4:6b5d41fe:9395e824:743e05ef)

...here's the output from mountd with debugging turned up.

# rpc.mountd -F -d all
rpc.mountd: v4root_create: path '/'
rpc.mountd: v4root_create: path '/'
rpc.mountd: Version 1.2.3 starting
rpc.mountd: auth_unix_ip: inbuf 'nfsd 2001:0470:0008:0d63:02e0:4dff:febf:06ba'
rpc.mountd: auth_unix_ip: client 0x7f6ea59aaae0 '192.168.1.0/24'
rpc.mountd: nfsd_fh: inbuf '192.168.1.0/24 1 \x00000000'
rpc.mountd: nfsd_fh: found 0x7f6ea59be1d0 path /
rpc.mountd: nfsd_export: inbuf '192.168.1.0/24 /export'
rpc.mountd: nfsd_export: found 0x7f6ea59bcf50 path /export
rpc.mountd: nfsd_fh: inbuf '192.168.1.0/24 7 \x6151080000000000be37e6a46b5d41fe9395e824743e05ef'
rpc.mountd: nfsd_fh: found 0x7f6ea59bcf60 path /export
Comment 1 Jeff Layton 2011-06-21 09:54:31 EDT
More importantly, here's the contents of auth_unix_ip:

# cat /proc/net/rpc/auth.unix.ip/content 
#class IP domain
nfsd 2001:0470:0008:0d63:02e0:4dff:febf:06ba 192.168.1.0/24

...that seems to be the problem. That address should not be matching an IPv4 address.
Comment 2 Jeff Layton 2011-06-21 10:05:18 EDT
client_gettype has this:

                if (*sp == '*' || *sp == '?' || *sp == '[')
                        return MCL_WILDCARD;

and it appears that the square brackets match a character class. I'm not sure that's actually working correctly however.

The DoMatch function doesn't look like it does the right thing (and is really difficult to follow to boot). It doesn't seem like my example should allow that to match and yet it does...

This is also not documented in the manpage. Perhaps we should remove this functionality. Thoughts anyone?
Comment 3 J. Bruce Fields 2011-06-21 11:49:29 EDT
Removing it sounds sensible to me.

In the highly unlikely event that someone does actually use this, hopefully removal would prompt them to complain, and we could find out what they need and make this work right.
Comment 4 Jeff Layton 2011-06-21 15:53:21 EDT
Ok. I'll see about spinning up a patch to do that...

In the meantime, I've figured out why this behavior occurs.

The key is client_resolve:

        if (clientlist[MCL_WILDCARD] || clientlist[MCL_NETGROUP])
                ai = host_reliable_addrinfo(sap);
        if (ai == NULL)
                ai = host_numeric_addrinfo(sap);

...so the presence of a wildcard entry in the exports list makes it do a "reliable" addrinfo which does a reverse and forward lookup to resolve the address.

In my setup, the client is dual-stack with both IPv4 and IPv6 addrs. Those addresses reverse-resolve to the same hostname, so my IPv4 address was ending up in the list of addresses, and that was matching the IPv4 subnet that I was exporting to.

It seems a little odd that we'd change how we resolve the address based on the presence of certain export types, but I don't see a better way to do it without some serious overhaul of this code.

In practice, while odd behavior, it's not an especially dangerous bug, as I don't see a way to exploit this to get at data that you wouldn't ordinarily be able to get to.
Comment 5 Jeff Layton 2011-06-22 06:51:32 EDT
Actually, I'm wrong and this is exploitable, as pointed out by Neil Brown:

---------------[snip]----------------
Suppose you export a filesystem to some subnet or FQDN and also to a wildcard
or netgroup, and I know the details of this (maybe showmount -e tells me)
Suppose further that I can get IP packets to your server..

Then I create a reverse mapping for my ipaddress to a domain that I own, say
"black.hat.org", and a forward mapping from that domain to my IP address, and
one of your IP addresses.

Then I try to mount your filesystem.  The IP address gets correctly mapped to
"black.hat.org" and then mapped to both my IP address and your IP address.

Then you search through all of your exports and find that one of the
addresses: yours - is allowed to access the filesystem.

So you create an export based on the addrinfo you have which allows my IP
address the same access as your IP address.


Looking at the code there are really 2 problems.

1/ host_reliable_addrinfo not checking the original address against the
   returned address list and rejecting if there is no match.

2/ The various check_ function in support_export_client.c treat 'ai' as
   having a list of addresses.  This is wrong - it should only have one
   address - the address that the request came from.  It can also have a
   hostname if wildcard or netgroup matching is used, but it should never
   have more than one adress in it, and we should only be testing the first.
---------------[snip]----------------
Comment 6 Jeff Layton 2011-07-05 07:43:36 EDT
This should now be fixed in rawhide...