[PATCH 0/4] keys: work correctly with user namespaces

[PATCH 0/4] keys: work correctly with user namespaces

[Serge E. Hallyn]

Hi James,

the following 4 patches apply on top of current security-testing-2.6 next
branch.  They implement strict separation of keys in different user
namespaces.  Do you mind sticking these in the security-testing tree?

With these patches I can run the keyutils-tests simultaneously in two
namespaces with no problems or failures.

thanks,
-serge

[PATCH 1/4] keys: distinguish per-uid keys in different namespaces

[Serge E. Hallyn]

per-uid keys were looked by uid only.  Use the user namespace
to distinguish the same uid in different namespaces.

This does not address key_permission.  So a task can for instance
try to join a keyring owned by the same uid in another namespace.
That will be handled by a separate patch.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
---
 kernel/user.c                |    2 +-
 security/keys/internal.h     |    4 +++-
 security/keys/key.c          |   11 +++++++++--
 security/keys/keyctl.c       |    2 +-
 security/keys/process_keys.c |    2 ++
 security/keys/request_key.c  |    2 +-
 6 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/kernel/user.c b/kernel/user.c
index 477b666..d8b332c 100644
--- a/kernel/user.c
+++ b/kernel/user.c
@@ -20,7 +20,7 @@
 
 struct user_namespace init_user_ns = {
 	.kref = {
-		.refcount	= ATOMIC_INIT(1),
+		.refcount	= ATOMIC_INIT(2),
 	},
 	.creator = &root_user,
 };
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 81932ab..9fb679c 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -53,6 +53,7 @@ struct key_user {
 	atomic_t		nkeys;		/* number of keys */
 	atomic_t		nikeys;		/* number of instantiated keys */
 	uid_t			uid;
+	struct user_namespace	*user_ns;
 	int			qnkeys;		/* number of keys allocated to this user */
 	int			qnbytes;	/* number of bytes allocated to this user */
 };
@@ -61,7 +62,8 @@ extern struct rb_root	key_user_tree;
 extern spinlock_t	key_user_lock;
 extern struct key_user	root_key_user;
 
-extern struct key_user *key_user_lookup(uid_t uid);
+extern struct key_user *key_user_lookup(uid_t uid,
+					struct user_namespace *user_ns);
 extern void key_user_put(struct key_user *user);
 
 /*
diff --git a/security/keys/key.c b/security/keys/key.c
index f76c8a5..4a1297d 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -18,6 +18,7 @@
 #include <linux/workqueue.h>
 #include <linux/random.h>
 #include <linux/err.h>
+#include <linux/user_namespace.h>
 #include "internal.h"
 
 static struct kmem_cache	*key_jar;
@@ -60,7 +61,7 @@ void __key_check(const struct key *key)
  * get the key quota record for a user, allocating a new record if one doesn't
  * already exist
  */
-struct key_user *key_user_lookup(uid_t uid)
+struct key_user *key_user_lookup(uid_t uid, struct user_namespace *user_ns)
 {
 	struct key_user *candidate = NULL, *user;
 	struct rb_node *parent = NULL;
@@ -79,6 +80,10 @@ struct key_user *key_user_lookup(uid_t uid)
 			p = &(*p)->rb_left;
 		else if (uid > user->uid)
 			p = &(*p)->rb_right;
+		else if (user_ns < user->user_ns)
+			p = &(*p)->rb_left;
+		else if (user_ns > user->user_ns)
+			p = &(*p)->rb_right;
 		else
 			goto found;
 	}
@@ -106,6 +111,7 @@ struct key_user *key_user_lookup(uid_t uid)
 	atomic_set(&candidate->nkeys, 0);
 	atomic_set(&candidate->nikeys, 0);
 	candidate->uid = uid;
+	candidate->user_ns = get_user_ns(user_ns);
 	candidate->qnkeys = 0;
 	candidate->qnbytes = 0;
 	spin_lock_init(&candidate->lock);
@@ -136,6 +142,7 @@ void key_user_put(struct key_user *user)
 	if (atomic_dec_and_lock(&user->usage, &key_user_lock)) {
 		rb_erase(&user->node, &key_user_tree);
 		spin_unlock(&key_user_lock);
+		put_user_ns(user->user_ns);
 
 		kfree(user);
 	}
@@ -234,7 +241,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
 	quotalen = desclen + type->def_datalen;
 
 	/* get hold of the key tracking for this user */
-	user = key_user_lookup(uid);
+	user = key_user_lookup(uid, cred->user->user_ns);
 	if (!user)
 		goto no_memory_1;
 
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 0979679..9a98346 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -725,7 +725,7 @@ long keyctl_chown_key(key_serial_t id, uid_t uid, gid_t gid)
 	/* change the UID */
 	if (uid != (uid_t) -1 && uid != key->uid) {
 		ret = -ENOMEM;
-		newowner = key_user_lookup(uid);
+		newowner = key_user_lookup(uid, current_user_ns());
 		if (!newowner)
 			goto error_put;
 
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index 2f5d89e..276d278 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -17,6 +17,7 @@
 #include <linux/fs.h>
 #include <linux/err.h>
 #include <linux/mutex.h>
+#include <linux/user_namespace.h>
 #include <asm/uaccess.h>
 #include "internal.h"
 
@@ -34,6 +35,7 @@ struct key_user root_key_user = {
 	.nkeys		= ATOMIC_INIT(2),
 	.nikeys		= ATOMIC_INIT(2),
 	.uid		= 0,
+	.user_ns	= &init_user_ns,
 };
 
 /*****************************************************************************/
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 0e04f72..22a3158 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -365,7 +365,7 @@ static struct key *construct_key_and_link(struct key_type *type,
 
 	kenter("");
 
-	user = key_user_lookup(current_fsuid());
+	user = key_user_lookup(current_fsuid(), current_user_ns());
 	if (!user)
 		return ERR_PTR(-ENOMEM);
 
-- 
1.5.4.3

[PATCH 2/4] keys: consider user namespace in key_permission

[Serge E. Hallyn]

If a key is owned by another user namespace, then treat the
key as though it is owned by both another uid and gid.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
---
 security/keys/permission.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/security/keys/permission.c b/security/keys/permission.c
index 5d9fc7b..0ed802c 100644
--- a/security/keys/permission.c
+++ b/security/keys/permission.c
@@ -35,6 +35,9 @@ int key_task_permission(const key_ref_t key_ref, const struct cred *cred,
 
 	key = key_ref_to_ptr(key_ref);
 
+	if (key->user->user_ns != cred->user->user_ns)
+		goto use_other_perms;
+
 	/* use the second 8-bits of permissions for keys the caller owns */
 	if (key->uid == cred->fsuid) {
 		kperm = key->perm >> 16;
@@ -56,6 +59,8 @@ int key_task_permission(const key_ref_t key_ref, const struct cred *cred,
 		}
 	}
 
+use_other_perms:
+
 	/* otherwise use the least-significant 8-bits */
 	kperm = key->perm;
 
-- 
1.5.4.3

[PATCH 3/4] keys: skip keys from another user namespace

[Serge E. Hallyn]

When listing keys, do not return keys belonging to the
same uid in another user namespace.  Otherwise uid 500
in another user namespace will return keyrings called
uid.500 for another user namespace.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
---
 security/keys/keyring.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index ed85157..3dba81c 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -539,6 +539,9 @@ struct key *find_keyring_by_name(const char *name, bool skip_perm_check)
 				    &keyring_name_hash[bucket],
 				    type_data.link
 				    ) {
+			if (keyring->user->user_ns != current_user_ns())
+				continue;
+
 			if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
 				continue;
 
-- 
1.5.4.3

[PATCH 4/4] keys: make procfiles per-user-namespace

[Serge E. Hallyn]

Restrict the /proc/keys and /proc/key-users output to keys
belonging to the same user namespace as the reading task.

We may want to make this more complicated - so that any
keys in a user-namespace which is belongs to the reading
task are also shown.  But let's see if anyone wants that
first.

Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
Acked-by: David Howells <dhowells@redhat.com>
---
 security/keys/proc.c |   55 ++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/security/keys/proc.c b/security/keys/proc.c
index 7f508de..769f9bd 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -91,6 +91,28 @@ __initcall(key_proc_init);
  */
 #ifdef CONFIG_KEYS_DEBUG_PROC_KEYS
 
+static struct rb_node *__key_serial_next(struct rb_node *n)
+{
+	while (n) {
+		struct key *key = rb_entry(n, struct key, serial_node);
+		if (key->user->user_ns == current_user_ns())
+			break;
+		n = rb_next(n);
+	}
+	return n;
+}
+
+static struct rb_node *key_serial_next(struct rb_node *n)
+{
+	return __key_serial_next(rb_next(n));
+}
+
+static struct rb_node *key_serial_first(struct rb_root *r)
+{
+	struct rb_node *n = rb_first(r);
+	return __key_serial_next(n);
+}
+
 static int proc_keys_open(struct inode *inode, struct file *file)
 {
 	return seq_open(file, &proc_keys_ops);
@@ -104,10 +126,10 @@ static void *proc_keys_start(struct seq_file *p, loff_t *_pos)
 
 	spin_lock(&key_serial_lock);
 
-	_p = rb_first(&key_serial_tree);
+	_p = key_serial_first(&key_serial_tree);
 	while (pos > 0 && _p) {
 		pos--;
-		_p = rb_next(_p);
+		_p = key_serial_next(_p);
 	}
 
 	return _p;
@@ -117,7 +139,7 @@ static void *proc_keys_start(struct seq_file *p, loff_t *_pos)
 static void *proc_keys_next(struct seq_file *p, void *v, loff_t *_pos)
 {
 	(*_pos)++;
-	return rb_next((struct rb_node *) v);
+	return key_serial_next((struct rb_node *) v);
 
 }
 
@@ -203,6 +225,27 @@ static int proc_keys_show(struct seq_file *m, void *v)
 
 #endif /* CONFIG_KEYS_DEBUG_PROC_KEYS */
 
+static struct rb_node *__key_user_next(struct rb_node *n)
+{
+	while (n) {
+		struct key_user *user = rb_entry(n, struct key_user, node);
+		if (user->user_ns == current_user_ns())
+			break;
+		n = rb_next(n);
+	}
+	return n;
+}
+
+static struct rb_node *key_user_next(struct rb_node *n)
+{
+	return __key_user_next(rb_next(n));
+}
+
+static struct rb_node *key_user_first(struct rb_root *r)
+{
+	struct rb_node *n = rb_first(r);
+	return __key_user_next(n);
+}
 /*****************************************************************************/
 /*
  * implement "/proc/key-users" to provides a list of the key users
@@ -220,10 +263,10 @@ static void *proc_key_users_start(struct seq_file *p, loff_t *_pos)
 
 	spin_lock(&key_user_lock);
 
-	_p = rb_first(&key_user_tree);
+	_p = key_user_first(&key_user_tree);
 	while (pos > 0 && _p) {
 		pos--;
-		_p = rb_next(_p);
+		_p = key_user_next(_p);
 	}
 
 	return _p;
@@ -233,7 +276,7 @@ static void *proc_key_users_start(struct seq_file *p, loff_t *_pos)
 static void *proc_key_users_next(struct seq_file *p, void *v, loff_t *_pos)
 {
 	(*_pos)++;
-	return rb_next((struct rb_node *) v);
+	return key_user_next((struct rb_node *) v);
 
 }
 
-- 
1.5.4.3

Re: [PATCH 0/4] keys: work correctly with user namespaces

[James Morris]

On Thu, 26 Feb 2009, Serge E. Hallyn wrote:

> Hi James,
> 
> the following 4 patches apply on top of current security-testing-2.6 next
> branch.  They implement strict separation of keys in different user
> namespaces.  Do you mind sticking these in the security-testing tree?

No problem, once they're reviewed appropriately.

> 
> With these patches I can run the keyutils-tests simultaneously in two
> namespaces with no problems or failures.
> 
> thanks,
> -serge
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 0/4] keys: work correctly with user namespaces

[James Morris]

On Thu, 26 Feb 2009, Serge E. Hallyn wrote:

> Hi James,
> 
> the following 4 patches apply on top of current security-testing-2.6 next
> branch.  They implement strict separation of keys in different user
> namespaces.  Do you mind sticking these in the security-testing tree?
> 
> With these patches I can run the keyutils-tests simultaneously in two
> namespaces with no problems or failures.
> 

Applied.

-- 
James Morris
<jmorris@namei.org>

Re: [PATCH 0/4] keys: work correctly with user namespaces

[Serge E. Hallyn]

Quoting James Morris (jmorris@namei.org):
> On Thu, 26 Feb 2009, Serge E. Hallyn wrote:
> 
> > Hi James,
> > 
> > the following 4 patches apply on top of current security-testing-2.6 next
> > branch.  They implement strict separation of keys in different user
> > namespaces.  Do you mind sticking these in the security-testing tree?
> > 
> > With these patches I can run the keyutils-tests simultaneously in two
> > namespaces with no problems or failures.
> > 
> 
> Applied.
> 
> -- 
> James Morris
> <jmorris@namei.org>

Thanks, James.

-serge