[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