[PATCH 0/3] keys: play nicely with user namespaces

[PATCH 0/3] keys: play nicely with user namespaces

[Serge E. Hallyn]

Hi David,

so here is a first attempt at getting keys and uid namespaces
to play nice.  The semantics need some discussion.  As I recall
Eric and yourself appeared to agree that some keyrings should
be inherited into child user namespaces.  I segragate them
cleanly bc that appears to be the simplest thing to do especially
given the use of i.e. lookup_by_name("uid.500").  IMO it shouldn't
be a big problem - userspace can always list keys it wants into a
file, start a new user namespace, then re-read them out of the
tempfile...

Comments appreciated.

thanks,
-serge

[PATCH 1/3] 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-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 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 6608a3d..fe326a6 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 7c72baa..db4c029 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/3] 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-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 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/3] 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-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---
 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

Re: [PATCH 0/3] keys: play nicely with user namespaces

[David Howells]

Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:

> so here is a first attempt at getting keys and uid namespaces
> to play nice.  The semantics need some discussion.  As I recall
> Eric and yourself appeared to agree that some keyrings should
> be inherited into child user namespaces.

Ummm...  If keys are effectively a per-user-namespace resource, then they
can't be shared between namespaces.  We could either duplicate all the keys
and keyrings, or we could just start afresh.  The latter is certainly the
easiest.

> I segragate them cleanly bc that appears to be the simplest thing to do
> especially given the use of i.e. lookup_by_name("uid.500").

Sounds reasonable.

> IMO it shouldn't be a big problem - userspace can always list keys it wants
> into a file, start a new user namespace, then re-read them out of the
> tempfile...

Not so.  You aren't necessarily permitted to read a key - the read function
has to be supported by the key type for a start, and then you have to pass all
the security checks.

David

Re: [PATCH 0/3] keys: play nicely with user namespaces

[Serge E. Hallyn]

Quoting David Howells (dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > so here is a first attempt at getting keys and uid namespaces
> > to play nice.  The semantics need some discussion.  As I recall
> > Eric and yourself appeared to agree that some keyrings should
> > be inherited into child user namespaces.
> 
> Ummm...  If keys are effectively a per-user-namespace resource, then they
> can't be shared between namespaces.  We could either duplicate all the keys
> and keyrings, or we could just start afresh.  The latter is certainly the
> easiest.
> 
> > I segragate them cleanly bc that appears to be the simplest thing to do
> > especially given the use of i.e. lookup_by_name("uid.500").
> 
> Sounds reasonable.
> 
> > IMO it shouldn't be a big problem - userspace can always list keys it wants
> > into a file, start a new user namespace, then re-read them out of the
> > tempfile...
> 
> Not so.  You aren't necessarily permitted to read a key - the read function
> has to be supported by the key type for a start, and then you have to pass all
> the security checks.

I guess the question is what sorts of keys would you want a child
user-namespace to inherit (that perhaps it couldn't)?  The primary
ones I can think of are keys for an encrypted fs.  Are there any sorts
of keys X uses?

Anyway if this set of patches does the segration correctly, I can float
a patch on top of these to copy the keyrings.  But should the (automatic
in-kernel) copy then still go through the security checks?  (If not, is
that safe, and if so, is there any advantage?)

Do you have an automated testsuite for the keyrings?  I just played
around with keyctl to test, since there was nothing in ltp.

thanks,
-serge

Re: [PATCH 0/3] keys: play nicely with user namespaces

[David Howells]

Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:

> I guess the question is what sorts of keys would you want a child
> user-namespace to inherit (that perhaps it couldn't)?  The primary
> ones I can think of are keys for an encrypted fs.

Yeah.  But it can always ask for them.

> Are there any sorts of keys X uses?

Not at the moment.

> Anyway if this set of patches does the segration correctly, I can float
> a patch on top of these to copy the keyrings.

Each key type would need to provide an operation for copying its keys.

> But should the (automatic in-kernel) copy then still go through the security
> checks?  (If not, is that safe, and if so, is there any advantage?)

I'm not sure, and that raises an interesting point.  How do you alter the UID
and GID of keys that you're copying?  You may have a set of keys with
different UIDs, for example.

> Do you have an automated testsuite for the keyrings?  I just played
> around with keyctl to test, since there was nothing in ltp.

Yes.

	http://people.redhat.com/~dhowells/keys/keyutils/keyutils-tests.tar.bz2

which may need:

	http://people.redhat.com/~dhowells/keys/keyutils/rhts_environment.sh

The tests are designed to run under RH's automated test environment.  All my
tests are shell scripts that wrap the keyctl program.

David

Re: [PATCH 0/3] keys: play nicely with user namespaces

[David Howells]

Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:

> > I'm not sure, and that raises an interesting point.  How do you alter the
> > UID and GID of keys that you're copying?  You may have a set of keys with
> > different UIDs, for example.
> 
> In fact that's the expectation, else why bother creating a new user
> namespace :)
> 
> Ok so my preference is to keep them segragated and always empty on
> clone(CLONE_NEWUSER), and it sounds like that's the sanest thing right
> now.  Please shout if I'm misunderstanding.

I think you're misunderstanding.

You can have, say, a keyring owned by UID 1, with three keys owned by UIDs 2,
3 and 4, respectively, and you could be, say, running as UID 5.

If you want to copy this keyring and these keys, do you just set the ownership
of the copies to your new UID?  That might give you extra privileges.

David

Re: [PATCH 0/3] keys: play nicely with user namespaces

[Serge E. Hallyn]

Quoting David Howells (dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > > I'm not sure, and that raises an interesting point.  How do you alter the
> > > UID and GID of keys that you're copying?  You may have a set of keys with
> > > different UIDs, for example.
> > 
> > In fact that's the expectation, else why bother creating a new user
> > namespace :)
> > 
> > Ok so my preference is to keep them segragated and always empty on
> > clone(CLONE_NEWUSER), and it sounds like that's the sanest thing right
> > now.  Please shout if I'm misunderstanding.
> 
> I think you're misunderstanding.
> 
> You can have, say, a keyring owned by UID 1, with three keys owned by UIDs 2,
> 3 and 4, respectively, and you could be, say, running as UID 5.
> 
> If you want to copy this keyring and these keys, do you just set the ownership
> of the copies to your new UID?  That might give you extra privileges.

Well no, I don't want to change any ownerships.

You're assuming I am UID 1 and own that keyring, right?  And now I do a
clone(CLONE_NEWUSER).  The new task will have UID 0 and no access to any
of those keys by virtue of being in a new user namespace.

So now, if I as UID 1 in the parent ns had access to the data loaded
into those keys, I can reload them into my new keyring.  Just as I could
do anyway.  And if I want to, since I own the new user namespace, I can
instantiate uid 2 in my new user namespace and make a key owned by UID
2.  Doesn't matter.

-serge

Re: [PATCH 0/3] keys: play nicely with user namespaces

[David Howells]

Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:

> > You can have, say, a keyring owned by UID 1, with three keys owned by UIDs
> > 2, 3 and 4, respectively, and you could be, say, running as UID 5.
> > 
> > If you want to copy this keyring and these keys, do you just set the
> > ownership of the copies to your new UID?  That might give you extra
> > privileges.
> 
> Well no, I don't want to change any ownerships.
> 
> You're assuming I am UID 1 and own that keyring, right?

Actually, I said "you could be, say, running as UID 5".  In which case, you
wouldn't own the keyring or any of the keys.  Perhaps that's a bad example.

Assume instead you're UID 1, and so are given owner rights on the keyring (as
opposed to group or other rights).

> And now I do a clone(CLONE_NEWUSER).  The new task will have UID 0 and no
> access to any of those keys by virtue of being in a new user namespace.

Yes.

> So now, if I as UID 1 in the parent ns had access to the data loaded
> into those keys, I can reload them into my new keyring.

I'm not sure what you mean by 'reload them'.

The kernel could copy the keys from the old keyring when the namespace is
cloned, but what UID should it put on the new keys?  Your new one (UID 0)?  If
so, that might grant you extra rights you didn't have before; also it means
these keys now come out of your quota, whereas they didn't previously.

> Just as I could do anyway.  And if I want to, since I own the new user
> namespace, I can instantiate uid 2 in my new user namespace and make a key
> owned by UID 2.  Doesn't matter.

That's not what I meant.  I think we might be talking at cross-purposes.  Are
you thinking of having userspace copy the keys across?

David

Re: [PATCH 0/3] keys: play nicely with user namespaces

[Serge E. Hallyn]

Quoting David Howells (dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > > You can have, say, a keyring owned by UID 1, with three keys owned by UIDs
> > > 2, 3 and 4, respectively, and you could be, say, running as UID 5.
> > > 
> > > If you want to copy this keyring and these keys, do you just set the
> > > ownership of the copies to your new UID?  That might give you extra
> > > privileges.
> > 
> > Well no, I don't want to change any ownerships.
> > 
> > You're assuming I am UID 1 and own that keyring, right?
> 
> Actually, I said "you could be, say, running as UID 5".  In which case, you

oops

> wouldn't own the keyring or any of the keys.  Perhaps that's a bad example.
> 
> Assume instead you're UID 1, and so are given owner rights on the keyring (as
> opposed to group or other rights).
> 
> > And now I do a clone(CLONE_NEWUSER).  The new task will have UID 0 and no
> > access to any of those keys by virtue of being in a new user namespace.
> 
> Yes.
> 
> > So now, if I as UID 1 in the parent ns had access to the data loaded
> > into those keys, I can reload them into my new keyring.
> 
> I'm not sure what you mean by 'reload them'.
> 
> The kernel could copy the keys from the old keyring when the namespace is
> cloned, but what UID should it put on the new keys?  Your new one (UID 0)?  If
> so, that might grant you extra rights you didn't have before; also it means
> these keys now come out of your quota, whereas they didn't previously.
> 
> > Just as I could do anyway.  And if I want to, since I own the new user
> > namespace, I can instantiate uid 2 in my new user namespace and make a key
> > owned by UID 2.  Doesn't matter.
> 
> That's not what I meant.  I think we might be talking at cross-purposes.  Are
> you thinking of having userspace copy the keys across?

Right, the kernel just segregates based on user namespaces, and if
userspace wants a task in a child user namespace to have keys, it
can load them.

-serge

Re: [PATCH 0/3] keys: play nicely with user namespaces

[Serge E. Hallyn]

Quoting David Howells (dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > I guess the question is what sorts of keys would you want a child
> > user-namespace to inherit (that perhaps it couldn't)?  The primary
> > ones I can think of are keys for an encrypted fs.
> 
> Yeah.  But it can always ask for them.
> 
> > Are there any sorts of keys X uses?
> 
> Not at the moment.
> 
> > Anyway if this set of patches does the segration correctly, I can float
> > a patch on top of these to copy the keyrings.
> 
> Each key type would need to provide an operation for copying its keys.
> 
> > But should the (automatic in-kernel) copy then still go through the security
> > checks?  (If not, is that safe, and if so, is there any advantage?)
> 
> I'm not sure, and that raises an interesting point.  How do you alter the UID
> and GID of keys that you're copying?  You may have a set of keys with
> different UIDs, for example.

In fact that's the expectation, else why bother creating a new user
namespace :)

Ok so my preference is to keep them segragated and always empty on
clone(CLONE_NEWUSER), and it sounds like that's the sanest thing right
now.  Please shout if I'm misunderstanding.

> > Do you have an automated testsuite for the keyrings?  I just played
> > around with keyctl to test, since there was nothing in ltp.
> 
> Yes.
> 
> 	http://people.redhat.com/~dhowells/keys/keyutils/keyutils-tests.tar.bz2
> 
> which may need:
> 
> 	http://people.redhat.com/~dhowells/keys/keyutils/rhts_environment.sh
> 
> The tests are designed to run under RH's automated test environment.  All my
> tests are shell scripts that wrap the keyctl program.

Cool, thanks, I'll test with those.

-serge

Re: [PATCH 0/3] keys: play nicely with user namespaces

[Serge E. Hallyn]

Quoting David Howells (dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:
> > Do you have an automated testsuite for the keyrings?  I just played
> > around with keyctl to test, since there was nothing in ltp.
> 
> Yes.
> 
> 	http://people.redhat.com/~dhowells/keys/keyutils/keyutils-tests.tar.bz2
> 
> which may need:
> 
> 	http://people.redhat.com/~dhowells/keys/keyutils/rhts_environment.sh
> 
> The tests are designed to run under RH's automated test environment.  All my
> tests are shell scripts that wrap the keyctl program.

With a plain security-next tree, I get the following output (as well as
with a patched one).  Expected?

root@sergelap:/mnt/userns-test/keyutils-tests# make runtest
make[1]: Entering directory `/mnt/userns-test/keyutils-tests/keyctl/add/noargs'
sh ./runtest.sh
Running with session keyring RHTS/keyctl/7732
Joined session keyring: 1022953145
+++ ADD NO ARGS
+++ ADD ONE ARG
+++ ADD TWO ARGS
+++ ADD THREE ARGS
+++ ADD FIVE ARGS
Reporting test DevelopersOwnTest as PASS
/usr/bin/rhts_environment.sh: line 24: rhts_db_submit_result: command not found
make[1]: Leaving directory `/mnt/userns-test/keyutils-tests/keyctl/add/noargs'
make[1]: Entering directory `/mnt/userns-test/keyutils-tests/keyctl/add/bad-args'
sh ./runtest.sh
Running with session keyring RHTS/keyctl/7764
Joined session keyring: 730770469
+++ CHECK EMPTY KEY TYPE
+++ CHECK UNSUPPORTED KEY TYPE
+++ CHECK INVALID KEY TYPE
+++ CHECK MAXLEN KEY TYPE
+++ CHECK OVERLONG KEY TYPE
+++ CHECK ADD KEYRING WITH PAYLOAD
+++ CHECK MAXLEN DESC
+++ CHECK OVERLONG DESC
+++ CHECK BAD KEY ID
Reporting test DevelopersOwnTest as PASS
/usr/bin/rhts_environment.sh: line 24: rhts_db_submit_result: command not found
make[1]: Leaving directory `/mnt/userns-test/keyutils-tests/keyctl/add/bad-args'
make[1]: Entering directory `/mnt/userns-test/keyutils-tests/keyctl/add/useradd'
sh ./runtest.sh
Running with session keyring RHTS/keyctl/7818
Joined session keyring: 788468022
+++ ADD USER KEY
+++ PRINT PAYLOAD
+++ UPDATE USER KEY
+++ PRINT UPDATED PAYLOAD
+++ ADD KEY TO NON-KEYRING
+++ UNLINK KEY
Session Keyring
       -3 --alswrv      0     0  keyring: RHTS/keyctl/7818
Reporting test DevelopersOwnTest as PASS
/usr/bin/rhts_environment.sh: line 24: rhts_db_submit_result: command not found
make[1]: Leaving directory `/mnt/userns-test/keyutils-tests/keyctl/add/useradd'
make[1]: Entering directory `/mnt/userns-test/keyutils-tests/keyctl/pupdate/noargs'
sh ./runtest.sh
Running with session keyring RHTS/keyctl/7860
Joined session keyring: 703700630
+++ PUPDATE NO ARGS
+++ PUPDATE TWO ARGS
Reporting test DevelopersOwnTest as PASS
/usr/bin/rhts_environment.sh: line 24: rhts_db_submit_result: command not found
make[1]: Leaving directory `/mnt/userns-test/keyutils-tests/keyctl/pupdate/noargs'
make[1]: Entering directory `/mnt/userns-test/keyutils-tests/keyctl/pupdate/userupdate'
sh ./runtest.sh
Running with session keyring RHTS/keyctl/7889
Joined session keyring: 538541192
+++ ADD USER KEY
+++ PRINT PAYLOAD
+++ PUPDATE USER KEY
+++ PRINT UPDATED PAYLOAD
+++ UNLINK KEY
Reporting test DevelopersOwnTest as PASS
/usr/bin/rhts_environment.sh: line 24: rhts_db_submit_result: command not found
make[1]: Leaving directory `/mnt/userns-test/keyutils-tests/keyctl/pupdate/userupdate'
make[1]: Entering directory `/mnt/userns-test/keyutils-tests/keyctl/pupdate/bad-args'
sh ./runtest.sh
Running with session keyring RHTS/keyctl/7926
Joined session keyring: 74932378
+++ CHECK UPDATE SESSION KEYRING
+++ CHECK UPDATE INVALID KEY
+++ ADD USER KEY
+++ UNLINK KEY
+++ UPDATE UNLINKED KEY
Reporting test DevelopersOwnTest as PASS
/usr/bin/rhts_environment.sh: line 24: rhts_db_submit_result: command not found
make[1]: Leaving directory `/mnt/userns-test/keyutils-tests/keyctl/pupdate/bad-args'
make[1]: Entering directory `/mnt/userns-test/keyutils-tests/keyctl/instantiating/noargs'
sh ./runtest.sh
Running with session keyring RHTS/keyctl/7971
Joined session keyring: 346603144
+++ NO ARGS
+++ ONE ARG
+++ TWO ARGS
+++ THREE ARGS
+++ FOUR ARGS
Reporting test DevelopersOwnTest as PASS
/usr/bin/rhts_environment.sh: line 24: rhts_db_submit_result: command not found
make[1]: Leaving directory `/mnt/userns-test/keyutils-tests/keyctl/instantiating/noargs'
make[1]: Entering directory `/mnt/userns-test/keyutils-tests/keyctl/instantiating/bad-args'
sh ./runtest.sh
Running with session keyring RHTS/keyctl/8009
Joined session keyring: 154433922
+++ CHECK BAD KEY ID
+++ CREATE KEY
+++ CHECK ALREADY INSTANTIATED KEY
+++ CHECK NEGATE TIMEOUT
+++ UNLINK KEY
+++ CHECK NON-EXISTENT KEY ID
Reporting test DevelopersOwnTest as PASS
/usr/bin/rhts_environment.sh: line 24: rhts_db_submit_result: command not found
make[1]: Leaving directory `/mnt/userns-test/keyutils-tests/keyctl/instantiating/bad-args'
make[1]: Entering directory `/mnt/userns-test/keyutils-tests/keyctl/permitting/valid'
sh ./runtest.sh
Running with session keyring RHTS/keyctl/8073
Joined session keyring: 97798558
+++ ADD KEYRING
+++ ADD KEY
+++ CHOWN
FAILED
+++ CHGRP
+++ ITERATE PERMISSIONS
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
FAILED
+++ VIEW GROUP PERMISSIONS
FAILED
+++ VIEW OTHER PERMISSIONS
FAILED
+++ REMOVE SETATTR
FAILED
FAILED
FAILED
+++ REINSTATE SETATTR
+++ UNLINK KEYRING
Reporting test DevelopersOwnTest as FAIL
/usr/bin/rhts_environment.sh: line 24: rhts_db_submit_result: command not found
make[1]: *** [runtest] Error 1
make[1]: Leaving directory `/mnt/userns-test/keyutils-tests/keyctl/permitting/valid'

Re: [PATCH 0/3] keys: play nicely with user namespaces

[David Howells]

Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:

> +++ CHOWN
> FAILED

Ah.  My web pages have an old version of the testsuite; one that expects the
keyctl chown op to fail with ENOTSUPP.  Unfortunately, keyctl chown now works,
so the test firstly finds chown failing to fail, and then because it expects
it to fail, it doesn't chown the key back again, and thus the remaining tests
fail.

I'll dig the newer version of the tests out tomorrow.

David

Re: [PATCH 0/3] keys: play nicely with user namespaces

[David Howells]

Try:

	http://people.redhat.com/~dhowells/keys/keyutils/keyutils-tests.tar.bz2

There were three updates required:

 (1) chown is now supported.

 (2) keyctl unlink does old keyring pointer block destruction lazily, and so a
     wait is required for the key being unlinked to be destroyed.

 (3) Anonymous session keyrings are now called "_ses" rather than "_ses.<pid>".

I've also made the following changes:

 (4) The name of the output file for each test is printed:

	=== /mnt/testarea/tmp.y9MVa88S ===

 (5) If a failure occurs, 'keyctl show' is dumped into the output file.

David

Re: [PATCH 0/3] keys: play nicely with user namespaces

[Serge E. Hallyn]

Quoting David Howells (dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> 
> Try:
> 
> 	http://people.redhat.com/~dhowells/keys/keyutils/keyutils-tests.tar.bz2
> 
> There were three updates required:
> 
>  (1) chown is now supported.
> 
>  (2) keyctl unlink does old keyring pointer block destruction lazily, and so a
>      wait is required for the key being unlinked to be destroyed.
> 
>  (3) Anonymous session keyrings are now called "_ses" rather than "_ses.<pid>".
> 
> I've also made the following changes:
> 
>  (4) The name of the output file for each test is printed:
> 
> 	=== /mnt/testarea/tmp.y9MVa88S ===
> 
>  (5) If a failure occurs, 'keyctl show' is dumped into the output file.

Cool, thanks.  I needed the following change to toolbox.inc.sh.orig in
order to be able to run with >1 user namespaces:

36c36
< maxsquota=`grep '^ *0': /proc/key-users | sed s@.*/@@`
---
> maxsquota=`grep '^ *0': /proc/key-users | sed s@.*/@@ | head -1`

since /proc/key-users then lists multiple entries.  Otherwise,
all tests pass when running the testsuite in a child-user-ns.

So I'll just sit on these patches waiting for an acked-by (or nack),
then ask for these three patches in linux-next.

thanks,
-serge

Re: [PATCH 0/3] keys: play nicely with user namespaces

[David Howells]

Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:

> Cool, thanks.  I needed the following change to toolbox.inc.sh.orig in
> order to be able to run with >1 user namespaces:
> 
> 36c36
> < maxsquota=`grep '^ *0': /proc/key-users | sed s@.*/@@`
> ---
> > maxsquota=`grep '^ *0': /proc/key-users | sed s@.*/@@ | head -1`
> 
> since /proc/key-users then lists multiple entries.  Otherwise,
> all tests pass when running the testsuite in a child-user-ns.

Should you only get key-users from the set of users in your namespace, I
wonder?

David

Re: [PATCH 0/3] keys: play nicely with user namespaces

[Serge E. Hallyn]

Quoting David Howells (dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:
> 
> > Cool, thanks.  I needed the following change to toolbox.inc.sh.orig in
> > order to be able to run with >1 user namespaces:
> > 
> > 36c36
> > < maxsquota=`grep '^ *0': /proc/key-users | sed s@.*/@@`
> > ---
> > > maxsquota=`grep '^ *0': /proc/key-users | sed s@.*/@@ | head -1`
> > 
> > since /proc/key-users then lists multiple entries.  Otherwise,
> > all tests pass when running the testsuite in a child-user-ns.
> 
> Should you only get key-users from the set of users in your namespace, I
> wonder?
> 
> David

Yup - patch coming (probably next week) for that, but there's the
question, given that user namespaces are hierarchical, of whether,
if pidns B is a child of pidns A created by userid 500, a task in
pidns A should see keys in userns B (listed as belonging to userid
500).

-serge

Re: [PATCH 0/3] keys: play nicely with user namespaces

[David Howells]

Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:

> Yup - patch coming (probably next week) for that,

Thanks.

> but there's the question, given that user namespaces are hierarchical, of
> whether, if pidns B is a child of pidns A created by userid 500, a task in
> pidns A should see keys in userns B (listed as belonging to userid 500).

Does that mean all the UIDs of B should be part of A?  Or is just UID 500
inherited?  Or is UID 0 in B the same as UID 500 in A?

David

Re: [PATCH 0/3] keys: play nicely with user namespaces

[Eric W. Biederman]

David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:

> Serge E. Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org> wrote:
>
>> Yup - patch coming (probably next week) for that,
>
> Thanks.
>
>> but there's the question, given that user namespaces are hierarchical, of
>> whether, if pidns B is a child of pidns A created by userid 500, a task in
>> pidns A should see keys in userns B (listed as belonging to userid 500).
>
> Does that mean all the UIDs of B should be part of A?  Or is just UID 500
> inherited?  Or is UID 0 in B the same as UID 500 in A?

So far the design is that user namespaces are disjoint with one specific exception.

The user who creates the user namespace is expected to have god like powers over
all users in the created user namespace.

When carefully implemented will allow a user namespace to be created
with normal user permissions and for the user that created user
namespace to manage the resources owned by users in that user
namespace.

Eric

Re: [PATCH 0/3] keys: play nicely with user namespaces

[David Howells]

Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:

> So far the design is that user namespaces are disjoint with one specific
> exception.
> 
> The user who creates the user namespace is expected to have god like powers
> over all users in the created user namespace.

I see.

> When carefully implemented will allow a user namespace to be created with
> normal user permissions and for the user that created user namespace to
> manage the resources owned by users in that user namespace.

I'm not sure how to deal with this wrt keys.  There are two problems to
consider:

 (1) Should a key with UID 500 from namespace A in Serge's example be visible
     in namespace B?

     If such a key should show up in namespace B, should its UID be given as 0
     to userspace?

 (2) How is the quota controlled?  Do new keys made up under the domain of
     namespace B go to namespace B UID 0's quota?  Or do they come out of
     namespace A's UID 500 quota?

David

Re: [PATCH 0/3] keys: play nicely with user namespaces

[Serge E. Hallyn]

Quoting David Howells (dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org):
> Eric W. Biederman <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
> 
> > So far the design is that user namespaces are disjoint with one specific
> > exception.
> > 
> > The user who creates the user namespace is expected to have god like powers
> > over all users in the created user namespace.
> 
> I see.
> 
> > When carefully implemented will allow a user namespace to be created with
> > normal user permissions and for the user that created user namespace to
> > manage the resources owned by users in that user namespace.
> 
> I'm not sure how to deal with this wrt keys.  There are two problems to
> consider:
> 
>  (1) Should a key with UID 500 from namespace A in Serge's example be visible
>      in namespace B?

Conceptually that'd be nice, but it seems unnecessary since (IIUC)
userspace in the child userns can always reload the key.  So then
we can leverage the intricate fs controls (where in fact a file may
be owned by (A,500) and (B,0) simultaneoushly, or by (A,500) with
no visibility by (B,0), while keeping the keys access controls simpler.

That's why I'm keeping them strictly separate in the current patchset.
If that turns out to be too simplistic, then we can always enable
cross-userns visibility later.

>      If such a key should show up in namespace B, should its UID be given as 0
>      to userspace?

That is the model we want for filesystem, but I'm really hoping we can
keep the keys model simpler.

>  (2) How is the quota controlled?  Do new keys made up under the domain of
>      namespace B go to namespace B UID 0's quota?  Or do they come out of
>      namespace A's UID 500 quota?

Ideally it would be both.  Since the namespaces are hierarchical, the
requirement of safe unprivileged user-namespace cloning should hold
in a child user-ns too, so at each level we want keys being counted
toward the quota of the acting (uid,userns) as well as that of each
(uidX,usernsX) up the creator chain.

-serge