When changing key ownership with keyctl_chown_key, continue to allow initial namespace sysadmin to perform any changes. Whitelist namespaced sysadmin to also make the changes, if * key uid & gid are mapped to the current namespace * CAP_SYS_ADMIN capability is present in the same namespace * the key is already possessed This ensures that namespace sysadmin cannot move a key across user namespaces, and cannot operate on keys that use a different uid_map. The specific usecase here, is for the priviledged supervisor to pre-create a session keyring and pass it onto a new process. However, this does not fix the desired issue at hand. Since "containers" typically are in a different pid namespace, yet may use an identical uid_map. Thus resulting in initial namespace CAP_SYS_ADMIN process not able to CHOWN keys belonging to a userid namespace, but two sibling namespace CAP_SYS_ADMIN process can chown each other keys. To fix this properly, the kernel keyring keys need to grow additional references, e.g. to the upcoming container id stuff, or to hook onto pid namespace somehow. After doing this work, I relalised that https://github.com/systemd/systemd/issues/6281 can be solved elsehow. Specifically I only needed to create they keys with the right owner, and thus setreuid/setregid was enough to create keyring with the right owner. However, I feel there is still value to post this patch to the original mailing list thread, with an extra NAK, explaining slightly more caveats around opening up permission to the chown operation. But otherwise the code below is just a NAK on this current approach, as it will yield bad results. Signed-off-by: Dimitri John Ledkov <xnox@xxxxxxxxxx> --- security/keys/internal.h | 3 +++ security/keys/keyctl.c | 6 +++++- security/keys/permission.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/security/keys/internal.h b/security/keys/internal.h index 9f8208dc0e55..c8444d413f6a 100644 --- a/security/keys/internal.h +++ b/security/keys/internal.h @@ -188,6 +188,9 @@ static inline int key_permission(const key_ref_t key_ref, unsigned perm) return key_task_permission(key_ref, current_cred(), perm); } +extern bool privileged_wrt_key_uidgid(struct user_namespace *ns, const key_ref_t key_ref); +extern bool capable_wrt_possessed_key_uidgid(const key_ref_t key_ref, int cap); + /* * Authorisation record for request_key(). */ diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c index 1ffe60bb2845..ddacd048c0cf 100644 --- a/security/keys/keyctl.c +++ b/security/keys/keyctl.c @@ -830,6 +830,7 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group) kuid_t uid; kgid_t gid; + /* below ensures that new uid & gid belong to the current namespace */ uid = make_kuid(current_user_ns(), user); gid = make_kgid(current_user_ns(), group); ret = -EINVAL; @@ -855,7 +856,10 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group) ret = -EACCES; down_write(&key->sem); - if (!capable(CAP_SYS_ADMIN)) { + /* sysadmin can do anything. */ + /* namespace sysadmin can do anything, if the key is already possessed. */ + if (!(capable(CAP_SYS_ADMIN) || + capable_wrt_possessed_key_uidgid(key_ref, 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; diff --git a/security/keys/permission.c b/security/keys/permission.c index f68dc04d614e..2c99da790ec4 100644 --- a/security/keys/permission.c +++ b/security/keys/permission.c @@ -108,3 +108,34 @@ int key_validate(const struct key *key) return 0; } EXPORT_SYMBOL(key_validate); + +/** + * privileged_wrt_key_uidgid - Do capabilities in the namespace work over the key? + * @ns: The user namespace in question + * @key_ref: The key in question + * + * Return true if the key's uid and gid are within the namespace. + */ +bool privileged_wrt_key_uidgid(struct user_namespace *ns, const key_ref_t key_ref) +{ + struct key *key = key_ref_to_ptr(key_ref); + + return kuid_has_mapping(ns, key->uid) && + kgid_has_mapping(ns, key->gid); +} + +/** + * capable_wrt_possessed_key_uidgid - Check nsown_capable and uid and gid mapped + * @key_ref: The key in question + * @cap: The capability in question + * + * Return true if the current task has the given capability targeted at + * its own user namespace and that the given key's uid and gid are + * mapped into the current user namespace and the key is possessed. + */ +bool capable_wrt_possessed_key_uidgid(const key_ref_t key_ref, int cap) +{ + struct user_namespace *ns = current_user_ns(); + + return is_key_possessed(key_ref) && ns_capable(ns, cap) && privileged_wrt_key_uidgid(ns, key_ref); +} -- 2.19.1 _______________________________________________ Containers mailing list Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/containers
