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

Dimitri John Ledkov xnox at ubuntu.com
Mon Sep 25 19:11:04 UTC 2017


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/pull/6876
Signed-off-by: Dimitri John Ledkov <xnox at ubuntu.com>
---

 Dear Ubuntu Kernel Team,

 I am consider to submit this patch upstream. Could you please review
 this patch, before I do so?

 I've generated the patch with the full function context, to point out
 that it does make_kuid/make_kgid using current_user_ns() but not the
 capability check. Which imho is silly.

 Regards,

 Dimitri.

 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





More information about the kernel-team mailing list