Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

security: keys: perform capable check only on privileged operations

If the current task fails the check for the queried capability via
`capable(CAP_SYS_ADMIN)` LSMs like SELinux generate a denial message.
Issuing such denial messages unnecessarily can lead to a policy author
granting more privileges to a subject than needed to silence them.

Reorder CAP_SYS_ADMIN checks after the check whether the operation is
actually privileged.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>

authored by

Christian Göttsche and committed by
Jarkko Sakkinen
2d7f105e 57012c57

+8 -3
+8 -3
security/keys/keyctl.c
··· 980 980 ret = -EACCES; 981 981 down_write(&key->sem); 982 982 983 - if (!capable(CAP_SYS_ADMIN)) { 983 + { 984 + bool is_privileged_op = false; 985 + 984 986 /* only the sysadmin can chown a key to some other UID */ 985 987 if (user != (uid_t) -1 && !uid_eq(key->uid, uid)) 986 - goto error_put; 988 + is_privileged_op = true; 987 989 988 990 /* only the sysadmin can set the key's GID to a group other 989 991 * than one of those that the current process subscribes to */ 990 992 if (group != (gid_t) -1 && !gid_eq(gid, key->gid) && !in_group_p(gid)) 993 + is_privileged_op = true; 994 + 995 + if (is_privileged_op && !capable(CAP_SYS_ADMIN)) 991 996 goto error_put; 992 997 } 993 998 ··· 1093 1088 down_write(&key->sem); 1094 1089 1095 1090 /* if we're not the sysadmin, we can only change a key that we own */ 1096 - if (capable(CAP_SYS_ADMIN) || uid_eq(key->uid, current_fsuid())) { 1091 + if (uid_eq(key->uid, current_fsuid()) || capable(CAP_SYS_ADMIN)) { 1097 1092 key->perm = perm; 1098 1093 notify_key(key, NOTIFY_KEY_SETATTR, 0); 1099 1094 ret = 0;