[PATCH] KEYS: allow changing key ownership with CAP_SYS_ADMIN in a NS

[Dimitri John Ledkov]

Currently, changing key ownership from one namespaced uid/gid to
another namespaced uid/gid is only allowed by processes that have
CAP_SYS_ADMIN in the intial namespace. Fix the capability check to
also check the capability in the current capability.

Fixes: https://github.com/systemd/systemd/issues/6281
Signed-off-by: Dimitri John Ledkov <xnox at ubuntu.com>
---

 Dear containers mailing list,

 There is now userspace code that uses kernel keyring, in
 user-namespaces, and tries to chown the keyrings from one namespaced
 uid/gid to another. See systemd keyring code to store invocation id
 in src/core/execute.c.

 This code fails under system user-namespace containers, such as
 OpenVZ, LXC, LXD.

 Setup for a reproducer:
 ## enter a user-names, however you like. Using lxd as an example
 $ lxc init ubunt-daily:a test-keyring
 $ lxc exec test-keyring bash
 # apt install keyutils

 Reproducer:
 # keyctl session
 Joined session keyring: 556756508
 # keyctl chown 556756508 1000
 keyctl_chown: Permission denied
 # keyctl chown 556756508 0
 # echo $?

 The permission denied is unexpected, and this patch resolves
 this. I've tested this patch by recompiling Ubuntu kernel with this
 patch applied and testing above in a VM.

 Some prior art. When user namespaces were written in 2011, code to
 switch from capable to ns_capable was written, but eventually not
 merged due to siding on a being more conservative, as mentioned in
 the pull request in 2012. Given that user-namespaces are more mature
 now, and actually in active use, it is time to lax capability checks
 to probe user namespace rather than initial naspace.

 Please consider applying this patch.

 References to prior art:
 https://lists.onap.org/pipermail/containers/2011-September/028137.html
 https://github.com/torvalds/linux/commit/437589a74b6a590d175f86cf9f7b2efcee7765e7

      Ultimately there will be work to relax privlige checks from
     "capable(FOO)" to "ns_capable(user_ns, FOO)" where it is safe
     allowing root in a user names to do those things that today we
     only forbid to non-root users because it will confuse suid root
     applications.

 security/keys/keyctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index ab0b337c84b4..dc554bb80325 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -822,65 +822,65 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)
 	struct key_user *newowner, *zapowner = NULL;
 	struct key *key;
 	key_ref_t key_ref;
 	long ret;
 	kuid_t uid;
 	kgid_t gid;
 
 	uid = make_kuid(current_user_ns(), user);
 	gid = make_kgid(current_user_ns(), group);
 	ret = -EINVAL;
 	if ((user != (uid_t) -1) && !uid_valid(uid))
 		goto error;
 	if ((group != (gid_t) -1) && !gid_valid(gid))
 		goto error;
 
 	ret = 0;
 	if (user == (uid_t) -1 && group == (gid_t) -1)
 		goto error;
 
 	key_ref = lookup_user_key(id, KEY_LOOKUP_CREATE | KEY_LOOKUP_PARTIAL,
 				  KEY_NEED_SETATTR);
 	if (IS_ERR(key_ref)) {
 		ret = PTR_ERR(key_ref);
 		goto error;
 	}
 
 	key = key_ref_to_ptr(key_ref);
 
 	/* make the changes with the locks held to prevent chown/chown races */
 	ret = -EACCES;
 	down_write(&key->sem);
 
-	if (!capable(CAP_SYS_ADMIN)) {
+	if (!ns_capable(current_user_ns(), CAP_SYS_ADMIN)) {
 		/* only the sysadmin can chown a key to some other UID */
 		if (user != (uid_t) -1 && !uid_eq(key->uid, uid))
 			goto error_put;
 
 		/* only the sysadmin can set the key's GID to a group other
 		 * than one of those that the current process subscribes to */
 		if (group != (gid_t) -1 && !gid_eq(gid, key->gid) && !in_group_p(gid))
 			goto error_put;
 	}
 
 	/* change the UID */
 	if (user != (uid_t) -1 && !uid_eq(uid, key->uid)) {
 		ret = -ENOMEM;
 		newowner = key_user_lookup(uid);
 		if (!newowner)
 			goto error_put;
 
 		/* transfer the quota burden to the new user */
 		if (test_bit(KEY_FLAG_IN_QUOTA, &key->flags)) {
 			unsigned maxkeys = uid_eq(uid, GLOBAL_ROOT_UID) ?
 				key_quota_root_maxkeys : key_quota_maxkeys;
 			unsigned maxbytes = uid_eq(uid, GLOBAL_ROOT_UID) ?
 				key_quota_root_maxbytes : key_quota_maxbytes;
 
 			spin_lock(&newowner->lock);
 			if (newowner->qnkeys + 1 >= maxkeys ||
 			    newowner->qnbytes + key->quotalen >= maxbytes ||
 			    newowner->qnbytes + key->quotalen <
 			    newowner->qnbytes)
 				goto quota_overrun;
 
 			newowner->qnkeys++;
-- 
2.14.1