Re: Security issue in NFS localio

Re: Security issue in NFS localio

[Chuck Lever III]

> On Jul 3, 2024, at 6:24 PM, NeilBrown <neilb@suse.de> wrote:
> 
> 
> I've been pondering security questions with localio - particularly
> wondering what questions I need to ask.  I've found three focal points
> which overlap but help me organise my thoughts:
> 1- the LOCALIO RPC protocol
> 2- the 'auth_domain' that nfsd uses to authorise access
> 3- the credential that is used to access the file
> 
> 1/ It occurs to me that I could find out the UUID reported by a given
> local server (just ask it over the RPC connection), find out the
> filehandle for some file that I don't have write access to (not too
> hard), and create a private NFS server (hacking nfs-ganasha?) which
> reports the same uuid and reports that I have access to a file with
> that filehandle.  If I then mount from that server inside a private
> container on the same host that is running the local server, I would get
> localio access to the target file.
> 
> I might not be able to write to it because of credential checking, but I
> think that is getting a lot closer to unauthorised access than I would
> like.
> 
> I would much prefer it if there was no credible way to subvert the
> LOCALIO protocol.
> 
> My current idea goes like this:
> - NFS client tells nfs_common it is going to probe for localio
>   and gets back a nonce.  nfs_common records that this probe is happening
> - NFS client sends the nonce to the server over LOCALIO.
> - server tells nfs_common "I just got this nonce - does it mean
>   anything?".  If it does, the server gets connected with the client
>   through nfs_common.  The server reports success over LOCALIO.
>   If it doesn't the server reports failure of LOCALIO.
> - NFS client gets the reply and tells nfs_common that it has a reply
>   so the nonce is invalidated.  If the reply was success and nfs_local
>   confirms there is a connection, then the two stay connected.
> 
> I think that having a nonce (single-use uuid) is better than using the
> same uuid for the life of the server, and I think that sending it
> proactively by client rather than reactively by the server is also
> safer.

This echoes typical security approaches, and I think it
would be very easy to convert the current LOCALIO protocol
to operate this way.


> 2/ The localio access should use exactly the same auth_domain as the
>   network access uses.  This ensure the credentials implied by
>   rootsquash and allsquash are used correctly.  I think the current
>   code has the client guessing what IP address the server will see and
>   finding an auth_domain based on that.  I'm not comfortable with that.
> 
>   In the new LOCALIO protocol I suggest above, the server registers
>   with nfs_common at the moment it receives an RPC request.  At that
>   moment it knows the characteristics of the connection - remote IP?
>   krb5?  tls?  - and can determine an auth_domain and give it to
>   nfs_common and so make it available to the client.

I wasn't sure about the "copy the IP address" logic. It doesn't
seem like it would provide adequate security across network
namespaces on the same physical host, but I haven't studied it
closely.

My sense is an administrator would want authorization for localIO
to behave just like it would if this were a normal NFS access.
So the export's IP address settings, secure/insecure, GSS, TLS,
and squashing options should all be effective and work exactly
the same without regard to the I/O method.

auth_domains are an area I still don't know much about, but that
seems like it would get us closer to having access authorization
behave the same in both cases. It looked like your __fh_verify()
clean-up was headed in that direction.


>   Jeff wondered about an export option to explicitly enable LOCALIO.  I
>   had wondered about that too.  But I think that if we firmly tie the
>   localio auth_domain to the connection as above, that shouldn't be needed.
> 
> 3/ The current code uses the 'struct cred' of the application to look up
>   the file in the server code.  When a request goes over the wire the
>   credential is translated to uid/gid (or krb identity) and this is
>   mapped back to a credential on the server which might be in a
>   different uid name space (might it?  Does that even work for nfsd?)
> 
>   I think that if rootsquash or allsquash is in effect the correct
>   server-side credential is used but otherwise the client-side
>   credential is used.  That is likely correct in many cases but I'd
>   like to be convinced that it is correct in all case.  Maybe it is
>   time to get a deeper understanding of uid name spaces.

I've wondered about the idmapping issues, actually. Thanks
for bringing that up. I think Christian and linux-fsdevel
need to be involved in this conversation; added.

--
Chuck Lever

Re: Security issue in NFS localio

[Dave Chinner]

On Thu, Jul 04, 2024 at 07:00:23PM +0000, Chuck Lever III wrote:
> 
> 
> > On Jul 3, 2024, at 6:24 PM, NeilBrown <neilb@suse.de> wrote:
> > 
> > 
> > I've been pondering security questions with localio - particularly
> > wondering what questions I need to ask.  I've found three focal points
> > which overlap but help me organise my thoughts:
> > 1- the LOCALIO RPC protocol
> > 2- the 'auth_domain' that nfsd uses to authorise access
> > 3- the credential that is used to access the file
> > 
> > 1/ It occurs to me that I could find out the UUID reported by a given
> > local server (just ask it over the RPC connection), find out the
> > filehandle for some file that I don't have write access to (not too
> > hard), and create a private NFS server (hacking nfs-ganasha?) which
> > reports the same uuid and reports that I have access to a file with
> > that filehandle.  If I then mount from that server inside a private
> > container on the same host that is running the local server, I would get
> > localio access to the target file.

This seems amazingly complex for something that is actually really
simple.  Keep in mind that I am speaking from having direct
experience with developing and maintaining NFS client IO bypass
infrastructure from when I worked at SGI as an NFS engineer.

So, let's look at the Irix NFS client/server and the "Bulk Data
Service" protocol extensions that SGI wrote for NFSv3 back in the
mid 1990s.  Here's an overview from the 1996 product documentation
"Getting Started with BDSpro":

https://irix7.com/techpubs/007-3274-001.pdf

At least read chapter 1 so you grok the fundamentals of how the IO
bypass worked. It should look familiar, because it isn't very
different to how NFS over RDMA or client side IO for pNFS works.

Essentially, The NFS client transparently sent all the data IO (read
and write) over a separate communications channel for any IO that
met the size and alignment constraints. This was effectively a
"remote-IO" bypass that streamed data rather than packetised it
(NFS_READ/NFS_WRITE is packetised data with RTT latency issues).
By getting rid of the round trip latency penalty, data could be
sent/recieved at full network throughput rates.

[ As an aside, the BDS side channel was also the mechanism that used
by SGI for NFS over RDMA with custom full stack network offload
hardware back in the mid 1990s. NFS w/ BDS ran at about 800MB/s on
those networks on machines with 200MHz CPUs (think MIPS r10k). ]

The client side userspace has no idea this low level protocol
hijacking occurs, and it doesn't need to because all it changes
is the read/write IO speed. The NFS protocol is still used for all
authorisation, access checks, metadata operations, etc, and all that
changes is how NFS_READ and NFS_WRITE operations are performed.

The local-io stuff is no different - we're just using a different
client side IO path in kernel. We don't need a new protocol, nor do
we need userspace to be involved *at all*.  The kernel NFS client
can easily discover that it is on the same host as the server. The
server already does this "client is on the same host", so both will
then know they can *transparently* enable the localio bypass without
involving userspace at all.

The NFS protocol still provides all the auth, creds, etc to allow
the NFS client read and write access to the file. The NFS server
provides the client with a filehandle build by the underlying
filesystem for the file the NFS client has been permission to
access.

The local filesystem will accept that filehandle from any kernel
side context via the export ops for that filesystem. This provides
a mechanism for the NFS client to convert that to a dentry
and so open the file directly from the file handle. This is what the
server already does, so it should be able to share the filehandle
decode and open code from the server, maybe even just reach into the
server export table directly....

IOWs, we don't need to care about whether the mount is visible to
the NFS client - the filesystem *export* is visible to the *kernel*
and the export ops allow unfettered filehandle decoding. Containers
are irrelevant - the server has granted access to the file, and so
the NFS client has effective permissions to resolve the filehandle
directly..

Fundamentally, this is the same permission and access model that
pNFS is built on. Hence I don't understand why this local-io bypass
needs something completely new and seemingly very complex...

-Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: Security issue in NFS localio

[Mike Snitzer]

On Fri, Jul 05, 2024 at 09:25:56AM +1000, Dave Chinner wrote:
> On Thu, Jul 04, 2024 at 07:00:23PM +0000, Chuck Lever III wrote:
> > 
> > 
> > > On Jul 3, 2024, at 6:24 PM, NeilBrown <neilb@suse.de> wrote:
> > > 
> > > 
> > > I've been pondering security questions with localio - particularly
> > > wondering what questions I need to ask.  I've found three focal points
> > > which overlap but help me organise my thoughts:
> > > 1- the LOCALIO RPC protocol
> > > 2- the 'auth_domain' that nfsd uses to authorise access
> > > 3- the credential that is used to access the file
> > > 
> > > 1/ It occurs to me that I could find out the UUID reported by a given
> > > local server (just ask it over the RPC connection), find out the
> > > filehandle for some file that I don't have write access to (not too
> > > hard), and create a private NFS server (hacking nfs-ganasha?) which
> > > reports the same uuid and reports that I have access to a file with
> > > that filehandle.  If I then mount from that server inside a private
> > > container on the same host that is running the local server, I would get
> > > localio access to the target file.
> 
> This seems amazingly complex for something that is actually really
> simple.

Could be completely wrong, but I'm inferring you've read more
linux-nfs email (particularly about alternative directions for
implementation) than looked at the localio code.  But more below.

> Keep in mind that I am speaking from having direct
> experience with developing and maintaining NFS client IO bypass
> infrastructure from when I worked at SGI as an NFS engineer.

Thanks for sharing all this about IRIX, really helpful.

> So, let's look at the Irix NFS client/server and the "Bulk Data
> Service" protocol extensions that SGI wrote for NFSv3 back in the
> mid 1990s.  Here's an overview from the 1996 product documentation
> "Getting Started with BDSpro":
> 
> https://irix7.com/techpubs/007-3274-001.pdf
> 
> At least read chapter 1 so you grok the fundamentals of how the IO
> bypass worked. It should look familiar, because it isn't very
> different to how NFS over RDMA or client side IO for pNFS works.
> 
> Essentially, The NFS client transparently sent all the data IO (read
> and write) over a separate communications channel for any IO that
> met the size and alignment constraints. This was effectively a
> "remote-IO" bypass that streamed data rather than packetised it
> (NFS_READ/NFS_WRITE is packetised data with RTT latency issues).
> By getting rid of the round trip latency penalty, data could be
> sent/recieved at full network throughput rates.
> 
> [ As an aside, the BDS side channel was also the mechanism that used
> by SGI for NFS over RDMA with custom full stack network offload
> hardware back in the mid 1990s. NFS w/ BDS ran at about 800MB/s on
> those networks on machines with 200MHz CPUs (think MIPS r10k). ]
> 
> The client side userspace has no idea this low level protocol
> hijacking occurs, and it doesn't need to because all it changes
> is the read/write IO speed. The NFS protocol is still used for all
> authorisation, access checks, metadata operations, etc, and all that
> changes is how NFS_READ and NFS_WRITE operations are performed.
> 
> The local-io stuff is no different - we're just using a different
> client side IO path in kernel. We don't need a new protocol, nor do
> we need userspace to be involved *at all*.  The kernel NFS client
> can easily discover that it is on the same host as the server. The
> server already does this "client is on the same host", so both will
> then know they can *transparently* enable the localio bypass without
> involving userspace at all.
> 
> The NFS protocol still provides all the auth, creds, etc to allow
> the NFS client read and write access to the file. The NFS server
> provides the client with a filehandle build by the underlying
> filesystem for the file the NFS client has been permission to
> access.
> 
> The local filesystem will accept that filehandle from any kernel
> side context via the export ops for that filesystem. This provides
> a mechanism for the NFS client to convert that to a dentry
> and so open the file directly from the file handle. This is what the
> server already does, so it should be able to share the filehandle
> decode and open code from the server, maybe even just reach into the
> server export table directly....
> 
> IOWs, we don't need to care about whether the mount is visible to
> the NFS client - the filesystem *export* is visible to the *kernel*
> and the export ops allow unfettered filehandle decoding. Containers
> are irrelevant - the server has granted access to the file, and so
> the NFS client has effective permissions to resolve the filehandle
> directly..

IRIX sounds well engineered.  The Linux NFS code's interfaces aren't
so clean/precise.  The data structures are pretty tightly coupled (one
big struct nfs_client, nfs_server, nfsd_net, etc.  sunrpc's svc_rqst
in particular carries auth info that is used as part of the the wire
protocol business end -- so NFS auth is in the layer localio wants to
bypass).  And as such interfaces tend to do a lot of different tasks
on behalf of structures that carry the kitchen sink.

So retrofitting the Linux NFS and RPC code to allow a subset of the
NFS client and server code to be used isn't so clean.

But what you described IRIX did is pretty much what my localio series
provides, see:
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=nfs-localio-for-next

(Always room for improvement, like I said to Christoph, especially on
the IO submission and handling side.. as you've seen it is doing
buffered IO and is synchronous.. really leaving performance lackluster
but lots of upside to be had making it async and support DIO).

> Fundamentally, this is the same permission and access model that
> pNFS is built on. Hence I don't understand why this local-io bypass
> needs something completely new and seemingly very complex...

pNFS doesn't need to have a direct role in any of this localio code,
but pNFS can use localio if it is enabled, e.g.:
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/commit/?h=nfs-localio-for-next&id=5e7bf77fbbecdbea0e6ae7174c97a69b11e3098a

Localio not being tightly coupled to pNFS enables localio to easily
support NFSv3.  NFSv3 support is a requirement and there is no reason
not to support it.

Anyway, I really think Neil's ideas for localio improvement are solid.
Especially factoring out the auth_domain to ensure bog standard NFS
authentication and security mechanisms used.  Though IMO the proposed
localio protocol changes aren't _really_ needed, but I also won't
fight to stop localio nfsd UUID sharing being more ephemeral and
risk-averse...

The only reason there is a sideband/auxilliary "localio protocol" is
the NFS protocol is very focused on enabling NFS spec implementation.
I actually framed it in terms of NFS encode and decode on the server
side and Chuck wanted me to make sure to decouple localio so that it
stood on its own (I agree with him, I just didn't think to do it that
way).  I just needed a a means to generate and get a UUID from the
server to anchor the mechanism for nfs_common to allow the client and
server to rendezvous, see:

nfs_common:
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/commit/?h=nfs-localio-for-next&id=cb542e791eda114adcc9291feb6c66a5ea338f7c
nfs server:
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/commit/?h=nfs-localio-for-next&id=877a8212c3af37b5ba32959275f4c49bfe805f24
nfs client:
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/commit/?h=nfs-localio-for-next&id=572b36de2bb1dde06d6da4488686c9fbbc79d7e1

Really quite simple.

And the pgio hooks used to branch to localio handling of READ, WRITE and
COMMIT are the interface point for then generating a kiocb and issuing
the IO accordingly (last 2 commits below):
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/commit/?h=nfs-localio-for-next&id=46569e6d92a074188bb1f0090d36c327729ab418

But even the buffered and direct IO in nfsd are really tightly coupled
to the wire protocol interface.  So localio hooks pgio and calls down
to the underlying filesystem with its own side channel (that uses
.read_iter and .write_iter), see fs/nfs/localio.c
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/commit/?h=nfs-localio-for-next&id=877a8212c3af37b5ba32959275f4c49bfe805f24
https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/commit/?h=nfs-localio-for-next&id=4222309dac70e485f089738d0ffe9113b9a5a1e1

Re: Security issue in NFS localio

[Christoph Hellwig]

On Thu, Jul 04, 2024 at 07:00:23PM +0000, Chuck Lever III wrote:
> > 3/ The current code uses the 'struct cred' of the application to look up
> >   the file in the server code.  When a request goes over the wire the
> >   credential is translated to uid/gid (or krb identity) and this is
> >   mapped back to a credential on the server which might be in a
> >   different uid name space (might it?  Does that even work for nfsd?)
> > 
> >   I think that if rootsquash or allsquash is in effect the correct
> >   server-side credential is used but otherwise the client-side
> >   credential is used.  That is likely correct in many cases but I'd
> >   like to be convinced that it is correct in all case.  Maybe it is
> >   time to get a deeper understanding of uid name spaces.
> 
> I've wondered about the idmapping issues, actually. Thanks
> for bringing that up. I think Christian and linux-fsdevel
> need to be involved in this conversation; added.

There is a lot more issues than just idmapping.  That's why I don't
think the current approach where the open is executed in the client
can work.  The right way is to ensure the open always happens in and
nfsd thread context which just pases the open file to client for doing
I/O.

Re: Security issue in NFS localio

[Chuck Lever III]

> On Jul 5, 2024, at 9:45 AM, Christoph Hellwig <hch@infradead.org> wrote:
> 
> On Thu, Jul 04, 2024 at 07:00:23PM +0000, Chuck Lever III wrote:
>>> 3/ The current code uses the 'struct cred' of the application to look up
>>>  the file in the server code.  When a request goes over the wire the
>>>  credential is translated to uid/gid (or krb identity) and this is
>>>  mapped back to a credential on the server which might be in a
>>>  different uid name space (might it?  Does that even work for nfsd?)
>>> 
>>>  I think that if rootsquash or allsquash is in effect the correct
>>>  server-side credential is used but otherwise the client-side
>>>  credential is used.  That is likely correct in many cases but I'd
>>>  like to be convinced that it is correct in all case.  Maybe it is
>>>  time to get a deeper understanding of uid name spaces.
>> 
>> I've wondered about the idmapping issues, actually. Thanks
>> for bringing that up. I think Christian and linux-fsdevel
>> need to be involved in this conversation; added.
> 
> There is a lot more issues than just idmapping.  That's why I don't
> think the current approach where the open is executed in the client
> can work.  The right way is to ensure the open always happens in and
> nfsd thread context which just pases the open file to client for doing
> I/O.

I have considered that approach, but I don't yet have a clear
enough understanding of the idmapping issues to say whether
it is going to be necessary.

Just in terms of primitives, the server has svc_wake_up() which
can enqueue non-transport work on an nfsd thread. Question then
is what is the form of the response -- how does it get back
to the "client" side.


--
Chuck Lever

Re: Security issue in NFS localio

[NeilBrown]

On Fri, 05 Jul 2024, Dave Chinner wrote:
> On Thu, Jul 04, 2024 at 07:00:23PM +0000, Chuck Lever III wrote:
> > 
> > 
> > > On Jul 3, 2024, at 6:24 PM, NeilBrown <neilb@suse.de> wrote:
> > > 
> > > 
> > > I've been pondering security questions with localio - particularly
> > > wondering what questions I need to ask.  I've found three focal points
> > > which overlap but help me organise my thoughts:
> > > 1- the LOCALIO RPC protocol
> > > 2- the 'auth_domain' that nfsd uses to authorise access
> > > 3- the credential that is used to access the file
> > > 
> > > 1/ It occurs to me that I could find out the UUID reported by a given
> > > local server (just ask it over the RPC connection), find out the
> > > filehandle for some file that I don't have write access to (not too
> > > hard), and create a private NFS server (hacking nfs-ganasha?) which
> > > reports the same uuid and reports that I have access to a file with
> > > that filehandle.  If I then mount from that server inside a private
> > > container on the same host that is running the local server, I would get
> > > localio access to the target file.
> 
> This seems amazingly complex for something that is actually really
> simple.  Keep in mind that I am speaking from having direct
> experience with developing and maintaining NFS client IO bypass
> infrastructure from when I worked at SGI as an NFS engineer.
> 
> So, let's look at the Irix NFS client/server and the "Bulk Data
> Service" protocol extensions that SGI wrote for NFSv3 back in the
> mid 1990s.  Here's an overview from the 1996 product documentation
> "Getting Started with BDSpro":
> 
> https://irix7.com/techpubs/007-3274-001.pdf

Interesting work.  Thanks for the pointer.

It appear to me that BDS uses a separate network protocol - possibly
over a separate TCP connection or even a separate fabric - to connect
client to server, and this protocol is tuned for high throughput data
transfer and nothing else.  Makes perfect sense.

It would seem to still use the IP address (or similar NFS-style
mechanism) to authenticate each party to the other and to establish a
path for the data to flow over.  This is the question facing localio in
the text of mine that you quote above.  We don't want a network data
flow.  We want to hand over a file descriptor (or 'struct file').  There
is no standard way to achieve this over an IP-connected channel.  So we
are creating one.

The proposed protocol is to send a unique number over the IP-connected
channel, and use that to achieve rendezvous between the in-kernel client
and the in-kernel server.  The interesting questions are around how
unique this number should be, which direction it should travel, and
whether anything else other than the file descriptor should be passed
through when the kernel sides rendezvous.

I don't think the documentation on BDS sheds any particular light on
this question.

Thanks,
NeilBrown

Re: Security issue in NFS localio

[NeilBrown]

On Fri, 05 Jul 2024, Chuck Lever III wrote:
> 
> 
> > On Jul 5, 2024, at 9:45 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > 
> > On Thu, Jul 04, 2024 at 07:00:23PM +0000, Chuck Lever III wrote:
> >>> 3/ The current code uses the 'struct cred' of the application to look up
> >>>  the file in the server code.  When a request goes over the wire the
> >>>  credential is translated to uid/gid (or krb identity) and this is
> >>>  mapped back to a credential on the server which might be in a
> >>>  different uid name space (might it?  Does that even work for nfsd?)
> >>> 
> >>>  I think that if rootsquash or allsquash is in effect the correct
> >>>  server-side credential is used but otherwise the client-side
> >>>  credential is used.  That is likely correct in many cases but I'd
> >>>  like to be convinced that it is correct in all case.  Maybe it is
> >>>  time to get a deeper understanding of uid name spaces.
> >> 
> >> I've wondered about the idmapping issues, actually. Thanks
> >> for bringing that up. I think Christian and linux-fsdevel
> >> need to be involved in this conversation; added.
> > 
> > There is a lot more issues than just idmapping.  That's why I don't
> > think the current approach where the open is executed in the client
> > can work.  The right way is to ensure the open always happens in and
> > nfsd thread context which just pases the open file to client for doing
> > I/O.
> 
> I have considered that approach, but I don't yet have a clear
> enough understanding of the idmapping issues to say whether
> it is going to be necessary.

I can't see that nfsd pays any attention to uid namespaces.  I think it
always uses the init_ns.  So we would need to ensure the client
credential was interpreted against the init namespace.  I still have
made time to understand what this means in practice.

> 
> Just in terms of primitives, the server has svc_wake_up() which
> can enqueue non-transport work on an nfsd thread. Question then
> is what is the form of the response -- how does it get back
> to the "client" side.

I think it would be quite easy to establish a new work-queue (lwq?) that
clients can attach a request structure too before calling svc_wake_up(),
and which the nfsd thread would examine after svc_recv() returns.

This request structure would include the filehandle, credential,
possible other context, place to store the result, and a completion.
The server thread would complete the completion and the client, after
queuing the request, would wait for the completion.

So: easy enough to do if it turns out to be necessary.

NeilBrown


> 
> 
> --
> Chuck Lever
> 
> 
>