linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] NFS User Namespaces
@ 2020-10-21 12:05 Sargun Dhillon
  2020-10-21 12:05 ` [PATCH v3 1/3] NFS: NFSv2/NFSv3: Use cred from fs_context during mount Sargun Dhillon
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Sargun Dhillon @ 2020-10-21 12:05 UTC (permalink / raw)
  To: J . Bruce Fields, Chuck Lever, Trond Myklebust, Anna Schumaker,
	David Howells, Scott Mayhew
  Cc: Sargun Dhillon, linux-fsdevel, linux-nfs, kylea

This patchset adds some functionality to allow NFS to be used from
containers. It piggybacks on the previous work Trond did to properly
encode, and decode UIDs / GIDs based on user namespaces, and the work
that Scott did in order to use the new fs_context API.

I removed the samples in this patchset, and I added safety in this re-roll.

We can likely "pull back" on this safety over time, in that we can
enable/disable id mapping per mount, and add some logic to make nfs4idmap
user namespace aware. Doing this for GSS is more complicated though.


Changes since v2:
  * Removed samples
  * Split out NFSv2/v3 patchset from NFSv4 patchset
  * Added restrictions around use
Changes since v1:
  * Added samples

Sargun Dhillon (3):
  NFS: NFSv2/NFSv3: Use cred from fs_context during mount
  NFSv4: Refactor: reference user namespace from nfs4idmap
  NFSv4: Refactor NFS to be use user namespaces

 fs/nfs/client.c     | 10 ++++++++--
 fs/nfs/nfs4client.c | 27 ++++++++++++++++++++++++++-
 fs/nfs/nfs4idmap.c  | 17 +++++++++--------
 fs/nfs/nfs4idmap.h  |  3 ++-
 4 files changed, 45 insertions(+), 12 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v3 1/3] NFS: NFSv2/NFSv3: Use cred from fs_context during mount
  2020-10-21 12:05 [PATCH v3 0/3] NFS User Namespaces Sargun Dhillon
@ 2020-10-21 12:05 ` Sargun Dhillon
  2020-10-21 12:05 ` [PATCH v3 2/3] NFSv4: Refactor: reference user namespace from nfs4idmap Sargun Dhillon
  2020-10-21 12:05 ` [PATCH v3 3/3] NFSv4: Refactor NFS to be use user namespaces Sargun Dhillon
  2 siblings, 0 replies; 4+ messages in thread
From: Sargun Dhillon @ 2020-10-21 12:05 UTC (permalink / raw)
  To: J . Bruce Fields, Chuck Lever, Trond Myklebust, Anna Schumaker,
	David Howells, Scott Mayhew
  Cc: Sargun Dhillon, linux-fsdevel, linux-nfs, kylea

There was refactoring done to use the fs_context for mounting done in:
62a55d088cd87: NFS: Additional refactoring for fs_context conversion

This made it so that the net_ns is fetched from the fs_context (the netns
that fsopen is called in). This change also makes it so that the credential
fetched during fsopen is used as well as the net_ns.

NFS has already had a number of changes to prepare it for user namespaces:
1a58e8a0e5c1: NFS: Store the credential of the mount process in the nfs_server
264d948ce7d0: NFS: Convert NFSv3 to use the container user namespace
c207db2f5da5: NFS: Convert NFSv2 to use the container user namespace

Previously, different credentials could be used for creation of the
fs_context versus creation of the nfs_server, as FSCONFIG_CMD_CREATE did
the actual credential check, and that's where current_creds() were fetched.
This meant that the user namespace which fsopen was called in could be a
non-init user namespace. This still requires that the user that calls
FSCONFIG_CMD_CREATE has CAP_SYS_ADMIN in the init user ns.

This roughly allows a privileged user to mount on behalf of an unprivileged
usernamespace, by forking off and calling fsopen in the unprivileged user
namespace. It can then pass back that fsfd to the privileged process which
can configure the NFS mount, and then it can call FSCONFIG_CMD_CREATE
before switching back into the mount namespace of the container, and finish
up the mounting process and call fsmount and move_mount.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
---
 fs/nfs/client.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 4b8cc93913f7..c3afe448a512 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -571,7 +571,7 @@ static int nfs_start_lockd(struct nfs_server *server)
 					1 : 0,
 		.net		= clp->cl_net,
 		.nlmclnt_ops 	= clp->cl_nfs_mod->rpc_ops->nlmclnt_ops,
-		.cred		= current_cred(),
+		.cred		= server->cred,
 	};
 
 	if (nlm_init.nfs_version > 3)
@@ -985,7 +985,13 @@ struct nfs_server *nfs_create_server(struct fs_context *fc)
 	if (!server)
 		return ERR_PTR(-ENOMEM);
 
-	server->cred = get_cred(current_cred());
+	if (fc->cred->user_ns != &init_user_ns)
+		dprintk("%s: Using creds from non-init userns\n", __func__);
+	else if (fc->cred != current_cred())
+		dprintk("%s: Using creds from fs_context which are different than current_creds\n",
+			__func__);
+
+	server->cred = get_cred(fc->cred);
 
 	error = -ENOMEM;
 	fattr = nfs_alloc_fattr();
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 2/3] NFSv4: Refactor: reference user namespace from nfs4idmap
  2020-10-21 12:05 [PATCH v3 0/3] NFS User Namespaces Sargun Dhillon
  2020-10-21 12:05 ` [PATCH v3 1/3] NFS: NFSv2/NFSv3: Use cred from fs_context during mount Sargun Dhillon
@ 2020-10-21 12:05 ` Sargun Dhillon
  2020-10-21 12:05 ` [PATCH v3 3/3] NFSv4: Refactor NFS to be use user namespaces Sargun Dhillon
  2 siblings, 0 replies; 4+ messages in thread
From: Sargun Dhillon @ 2020-10-21 12:05 UTC (permalink / raw)
  To: J . Bruce Fields, Chuck Lever, Trond Myklebust, Anna Schumaker,
	David Howells, Scott Mayhew
  Cc: Sargun Dhillon, linux-fsdevel, linux-nfs, kylea

The nfs4idmapper only needs access to the user namespace, and not the
entire cred struct. This replaces the struct cred* member with struct
user_namespace*. This is so we don't have to hold onto the cred object,
which has extraneous references to things like user_struct. This also makes
switching away from init_user_ns more straightforward in the future.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
---
 fs/nfs/nfs4idmap.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index 62e6eea5c516..8d8aba305ecc 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -46,6 +46,7 @@
 #include <keys/user-type.h>
 #include <keys/request_key_auth-type.h>
 #include <linux/module.h>
+#include <linux/user_namespace.h>
 
 #include "internal.h"
 #include "netns.h"
@@ -69,13 +70,13 @@ struct idmap {
 	struct rpc_pipe		*idmap_pipe;
 	struct idmap_legacy_upcalldata *idmap_upcall_data;
 	struct mutex		idmap_mutex;
-	const struct cred	*cred;
+	struct user_namespace	*user_ns;
 };
 
 static struct user_namespace *idmap_userns(const struct idmap *idmap)
 {
-	if (idmap && idmap->cred)
-		return idmap->cred->user_ns;
+	if (idmap && idmap->user_ns)
+		return idmap->user_ns;
 	return &init_user_ns;
 }
 
@@ -286,7 +287,7 @@ static struct key *nfs_idmap_request_key(const char *name, size_t namelen,
 	if (ret < 0)
 		return ERR_PTR(ret);
 
-	if (!idmap->cred || idmap->cred->user_ns == &init_user_ns)
+	if (!idmap->user_ns || idmap->user_ns == &init_user_ns)
 		rkey = request_key(&key_type_id_resolver, desc, "");
 	if (IS_ERR(rkey)) {
 		mutex_lock(&idmap->idmap_mutex);
@@ -462,7 +463,7 @@ nfs_idmap_new(struct nfs_client *clp)
 		return -ENOMEM;
 
 	mutex_init(&idmap->idmap_mutex);
-	idmap->cred = get_cred(clp->cl_rpcclient->cl_cred);
+	idmap->user_ns = get_user_ns(clp->cl_rpcclient->cl_cred->user_ns);
 
 	rpc_init_pipe_dir_object(&idmap->idmap_pdo,
 			&nfs_idmap_pipe_dir_object_ops,
@@ -486,7 +487,7 @@ nfs_idmap_new(struct nfs_client *clp)
 err_destroy_pipe:
 	rpc_destroy_pipe_data(idmap->idmap_pipe);
 err:
-	put_cred(idmap->cred);
+	get_user_ns(idmap->user_ns);
 	kfree(idmap);
 	return error;
 }
@@ -503,7 +504,7 @@ nfs_idmap_delete(struct nfs_client *clp)
 			&clp->cl_rpcclient->cl_pipedir_objects,
 			&idmap->idmap_pdo);
 	rpc_destroy_pipe_data(idmap->idmap_pipe);
-	put_cred(idmap->cred);
+	put_user_ns(idmap->user_ns);
 	kfree(idmap);
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v3 3/3] NFSv4: Refactor NFS to be use user namespaces
  2020-10-21 12:05 [PATCH v3 0/3] NFS User Namespaces Sargun Dhillon
  2020-10-21 12:05 ` [PATCH v3 1/3] NFS: NFSv2/NFSv3: Use cred from fs_context during mount Sargun Dhillon
  2020-10-21 12:05 ` [PATCH v3 2/3] NFSv4: Refactor: reference user namespace from nfs4idmap Sargun Dhillon
@ 2020-10-21 12:05 ` Sargun Dhillon
  2 siblings, 0 replies; 4+ messages in thread
From: Sargun Dhillon @ 2020-10-21 12:05 UTC (permalink / raw)
  To: J . Bruce Fields, Chuck Lever, Trond Myklebust, Anna Schumaker,
	David Howells, Scott Mayhew
  Cc: Sargun Dhillon, linux-fsdevel, linux-nfs, kylea

In several patches work has been done to enable NFSv4 to use user
namespaces: 58002399da65: NFSv4: Convert the NFS client idmapper to use the
container user namespace 3b7eb5e35d0f: NFS: When mounting, don't share
filesystems between different user namespaces

Unfortunately, the userspace APIs were only such that the userspace facing side of the
filesystem (superblock s_user_ns) could be set to a non init user namespace. This furthers
the fs_context related refactoring, and piggybacks on top of that logic, so the superblock
user namespace, and the NFS user namespace are the same.

This change only allows those users whom are not using ID mapping to use user namespaces
because the upcall mechanism still needs to be made fully namespace aware. Currently,
it is only network namespace aware (and this patch doesn't impede that behaviour).
Also, there is currently a limitation that enabling / disabling ID mapping can only
be done on a machine-wide basis.

Eventually, we will need to at least:
  * Separate out the keyring cache by namespace
  * Come up with an upcall mechanism that can be triggered inside of the container,
    or safely triggered outside, with the requisite context to do the right mapping.
  * Handle whatever refactoring needs to be done in net/sunrpc.

Signed-off-by: Sargun Dhillon <sargun@sargun.me>
---
 fs/nfs/nfs4client.c | 27 ++++++++++++++++++++++++++-
 fs/nfs/nfs4idmap.c  |  2 +-
 fs/nfs/nfs4idmap.h  |  3 ++-
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index daacc78a3d48..0811e9540bf5 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -1151,7 +1151,19 @@ struct nfs_server *nfs4_create_server(struct fs_context *fc)
 	if (!server)
 		return ERR_PTR(-ENOMEM);
 
-	server->cred = get_cred(current_cred());
+	/*
+	 * current_cred() must have CAP_SYS_ADMIN in init_user_ns. All non
+	 * init user namespaces cannot mount NFS, but the fs_context
+	 * can be created in any user namespace.
+	 */
+	if (fc->cred->user_ns != &init_user_ns) {
+		dprintk("%s: Using creds from non-init userns\n", __func__);
+	} else if (fc->cred != current_cred()) {
+		dprintk("%s: Using creds from fs_context which are different than current_creds\n",
+			__func__);
+	}
+
+	server->cred = get_cred(fc->cred);
 
 	auth_probe = ctx->auth_info.flavor_len < 1;
 
@@ -1164,6 +1176,19 @@ struct nfs_server *nfs4_create_server(struct fs_context *fc)
 	if (error < 0)
 		goto error;
 
+	/*
+	 * nfs4idmap is not fully isolated by user namespaces. It is currently
+	 * only network namespace aware. If upcalls never happen, we do not
+	 * need to worry as nfs_client instances aren't shared between
+	 * user namespaces.
+	 */
+	if (idmap_userns(server->nfs_client->cl_idmap) != &init_user_ns &&
+		!(server->caps & NFS_CAP_UIDGID_NOMAP)) {
+		error = -EINVAL;
+		errorf(fc, "Mount credentials are from non init user namespace and ID mapping is enabled. This is not allowed.");
+		goto error;
+	}
+
 	return server;
 
 error:
diff --git a/fs/nfs/nfs4idmap.c b/fs/nfs/nfs4idmap.c
index 8d8aba305ecc..33dc9b76dc17 100644
--- a/fs/nfs/nfs4idmap.c
+++ b/fs/nfs/nfs4idmap.c
@@ -73,7 +73,7 @@ struct idmap {
 	struct user_namespace	*user_ns;
 };
 
-static struct user_namespace *idmap_userns(const struct idmap *idmap)
+struct user_namespace *idmap_userns(const struct idmap *idmap)
 {
 	if (idmap && idmap->user_ns)
 		return idmap->user_ns;
diff --git a/fs/nfs/nfs4idmap.h b/fs/nfs/nfs4idmap.h
index de44d7330ab3..2f5296497887 100644
--- a/fs/nfs/nfs4idmap.h
+++ b/fs/nfs/nfs4idmap.h
@@ -38,7 +38,7 @@
 
 #include <linux/uidgid.h>
 #include <uapi/linux/nfs_idmap.h>
-
+#include <linux/user_namespace.h>
 
 /* Forward declaration to make this header independent of others */
 struct nfs_client;
@@ -50,6 +50,7 @@ int nfs_idmap_init(void);
 void nfs_idmap_quit(void);
 int nfs_idmap_new(struct nfs_client *);
 void nfs_idmap_delete(struct nfs_client *);
+struct user_namespace *idmap_userns(const struct idmap *idmap);
 
 void nfs_fattr_init_names(struct nfs_fattr *fattr,
 		struct nfs4_string *owner_name,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-10-21 12:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 12:05 [PATCH v3 0/3] NFS User Namespaces Sargun Dhillon
2020-10-21 12:05 ` [PATCH v3 1/3] NFS: NFSv2/NFSv3: Use cred from fs_context during mount Sargun Dhillon
2020-10-21 12:05 ` [PATCH v3 2/3] NFSv4: Refactor: reference user namespace from nfs4idmap Sargun Dhillon
2020-10-21 12:05 ` [PATCH v3 3/3] NFSv4: Refactor NFS to be use user namespaces Sargun Dhillon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).