Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Mailing-List: linux-nfs@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: "NeilBrown" To: Mike Snitzer , Jeff Layton , Chuck Lever cc: Anna Schumaker , Trond Myklebust , Christoph Hellwig , linux-nfs@vger.kernel.org Subject: Security issue in NFS localio Date: Thu, 04 Jul 2024 08:24:44 +1000 Message-id: <172004548435.16071.5145237815071160040@noble.neil.brown.name> Xref: photonic.trudheim.com org.kernel.vger.linux-nfs:86554 Newsgroups: org.kernel.vger.linux-nfs Path: photonic.trudheim.com!nntp.lore.kernel.org!not-for-mail 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. 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. 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. Have I missed anything? Any other thoughts? Thanks, NeilBrown . X-CID-CACHE: Type:Local,Time:202407040836+08,HitQuantity:1 X-ns-mid: postfix-6685EF39-6795893 From: liuh To: trondmy@kernel.org, anna@kernel.org Cc: linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, liuh Subject: [PATCH] NFS: AIO doesn't require revert iterator Date: Thu, 4 Jul 2024 08:40:02 +0800 Message-ID: <20240704004002.5787-1-liuhuan01@kylinos.cn> X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Xref: photonic.trudheim.com org.kernel.vger.linux-kernel:1267319 org.kernel.vger.linux-nfs:86557 Newsgroups: org.kernel.vger.linux-kernel,org.kernel.vger.linux-nfs Path: photonic.trudheim.com!nntp.lore.kernel.org!not-for-mail For AIO, nfs_direct_wait return -EIOCBQUEUED would be expected. Revert iter is redundant. Signed-off-by: liuh --- fs/nfs/direct.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c index 90079ca13..1483f1965 100644 --- a/fs/nfs/direct.c +++ b/fs/nfs/direct.c @@ -469,7 +469,8 @@ ssize_t nfs_file_direct_read(struct kiocb *iocb, stru= ct iov_iter *iter, requested -=3D result; iocb->ki_pos +=3D result; } - iov_iter_revert(iter, requested); + if (is_sync_kiocb(iocb)) + iov_iter_revert(iter, requested); } else { result =3D requested; } @@ -1028,7 +1029,8 @@ ssize_t nfs_file_direct_write(struct kiocb *iocb, s= truct iov_iter *iter, /* XXX: should check the generic_write_sync retval */ generic_write_sync(iocb, result); } - iov_iter_revert(iter, requested); + if (is_sync_kiocb(iocb)) + iov_iter_revert(iter, requested); } else { result =3D requested; } --=20 2.27.0 .