Skip to content

Commit 8c0637e

Browse files
dhowellsdhowells
committed
keys: Make the KEY_NEED_* perms an enum rather than a mask
Since the meaning of combining the KEY_NEED_* constants is undefined, make it so that you can't do that by turning them into an enum. The enum is also given some extra values to represent special circumstances, such as: (1) The '0' value is reserved and causes a warning to trap the parameter being unset. (2) The key is to be unlinked and we require no permissions on it, only the keyring, (this replaces the KEY_LOOKUP_FOR_UNLINK flag). (3) An override due to CAP_SYS_ADMIN. (4) An override due to an instantiation token being present. (5) The permissions check is being deferred to later key_permission() calls. The extra values give the opportunity for LSMs to audit these situations. [Note: This really needs overhauling so that lookup_user_key() tells key_task_permission() and the LSM what operation is being done and leaves it to those functions to decide how to map that onto the available permits. However, I don't really want to make these change in the middle of the notifications patchset.] Signed-off-by: David Howells <[email protected]> cc: Jarkko Sakkinen <[email protected]> cc: Paul Moore <[email protected]> cc: Stephen Smalley <[email protected]> cc: Casey Schaufler <[email protected]> cc: [email protected] cc: [email protected]
1 parent e7d553d commit 8c0637e

File tree

9 files changed

+135
-74
lines changed

9 files changed

+135
-74
lines changed

‎include/linux/key.h

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,23 @@ struct net;
7171

7272
#define KEY_PERM_UNDEF 0xffffffff
7373

74+
/*
75+
* The permissions required on a key that we're looking up.
76+
*/
77+
enum key_need_perm {
78+
KEY_NEED_UNSPECIFIED, /* Needed permission unspecified */
79+
KEY_NEED_VIEW, /* Require permission to view attributes */
80+
KEY_NEED_READ, /* Require permission to read content */
81+
KEY_NEED_WRITE, /* Require permission to update / modify */
82+
KEY_NEED_SEARCH, /* Require permission to search (keyring) or find (key) */
83+
KEY_NEED_LINK, /* Require permission to link */
84+
KEY_NEED_SETATTR, /* Require permission to change attributes */
85+
KEY_NEED_UNLINK, /* Require permission to unlink key */
86+
KEY_SYSADMIN_OVERRIDE, /* Special: override by CAP_SYS_ADMIN */
87+
KEY_AUTHTOKEN_OVERRIDE, /* Special: override by possession of auth token */
88+
KEY_DEFER_PERM_CHECK, /* Special: permission check is deferred */
89+
};
90+
7491
struct seq_file;
7592
struct user_struct;
7693
struct signal_struct;
@@ -420,20 +437,9 @@ static inline key_serial_t key_serial(const struct key *key)
420437
extern void key_set_timeout(struct key *, unsigned);
421438

422439
extern key_ref_t lookup_user_key(key_serial_t id, unsigned long flags,
423-
key_perm_t perm);
440+
enum key_need_perm need_perm);
424441
extern void key_free_user_ns(struct user_namespace *);
425442

426-
/*
427-
* The permissions required on a key that we're looking up.
428-
*/
429-
#define KEY_NEED_VIEW 0x01 /* Require permission to view attributes */
430-
#define KEY_NEED_READ 0x02 /* Require permission to read content */
431-
#define KEY_NEED_WRITE 0x04 /* Require permission to update / modify */
432-
#define KEY_NEED_SEARCH 0x08 /* Require permission to search (keyring) or find (key) */
433-
#define KEY_NEED_LINK 0x10 /* Require permission to link */
434-
#define KEY_NEED_SETATTR 0x20 /* Require permission to change attributes */
435-
#define KEY_NEED_ALL 0x3f /* All the above permissions */
436-
437443
static inline short key_read_state(const struct key *key)
438444
{
439445
/* Barrier versus mark_key_instantiated(). */

‎include/linux/security.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,8 +1767,8 @@ static inline int security_path_chroot(const struct path *path)
17671767

17681768
int security_key_alloc(struct key *key, const struct cred *cred, unsigned long flags);
17691769
void security_key_free(struct key *key);
1770-
int security_key_permission(key_ref_t key_ref,
1771-
const struct cred *cred, unsigned perm);
1770+
int security_key_permission(key_ref_t key_ref, const struct cred *cred,
1771+
enum key_need_perm need_perm);
17721772
int security_key_getsecurity(struct key *key, char **_buffer);
17731773

17741774
#else
@@ -1786,7 +1786,7 @@ static inline void security_key_free(struct key *key)
17861786

17871787
static inline int security_key_permission(key_ref_t key_ref,
17881788
const struct cred *cred,
1789-
unsigned perm)
1789+
enum key_need_perm need_perm)
17901790
{
17911791
return 0;
17921792
}

‎security/keys/internal.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,6 @@ extern bool lookup_user_key_possessed(const struct key *key,
167167
const struct key_match_data *match_data);
168168
#define KEY_LOOKUP_CREATE 0x01
169169
#define KEY_LOOKUP_PARTIAL 0x02
170-
#define KEY_LOOKUP_FOR_UNLINK 0x04
171170

172171
extern long join_session_keyring(const char *name);
173172
extern void key_change_session_keyring(struct callback_head *twork);
@@ -183,7 +182,7 @@ extern void key_gc_keytype(struct key_type *ktype);
183182

184183
extern int key_task_permission(const key_ref_t key_ref,
185184
const struct cred *cred,
186-
key_perm_t perm);
185+
enum key_need_perm need_perm);
187186

188187
static inline void notify_key(struct key *key,
189188
enum key_notification_subtype subtype, u32 aux)
@@ -205,9 +204,10 @@ static inline void notify_key(struct key *key,
205204
/*
206205
* Check to see whether permission is granted to use a key in the desired way.
207206
*/
208-
static inline int key_permission(const key_ref_t key_ref, unsigned perm)
207+
static inline int key_permission(const key_ref_t key_ref,
208+
enum key_need_perm need_perm)
209209
{
210-
return key_task_permission(key_ref, current_cred(), perm);
210+
return key_task_permission(key_ref, current_cred(), need_perm);
211211
}
212212

213213
extern struct key_type key_type_request_key_auth;

‎security/keys/keyctl.c

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -434,7 +434,7 @@ long keyctl_invalidate_key(key_serial_t id)
434434

435435
/* Root is permitted to invalidate certain special keys */
436436
if (capable(CAP_SYS_ADMIN)) {
437-
key_ref = lookup_user_key(id, 0, 0);
437+
key_ref = lookup_user_key(id, 0, KEY_SYSADMIN_OVERRIDE);
438438
if (IS_ERR(key_ref))
439439
goto error;
440440
if (test_bit(KEY_FLAG_ROOT_CAN_INVAL,
@@ -479,7 +479,8 @@ long keyctl_keyring_clear(key_serial_t ringid)
479479

480480
/* Root is permitted to invalidate certain special keyrings */
481481
if (capable(CAP_SYS_ADMIN)) {
482-
keyring_ref = lookup_user_key(ringid, 0, 0);
482+
keyring_ref = lookup_user_key(ringid, 0,
483+
KEY_SYSADMIN_OVERRIDE);
483484
if (IS_ERR(keyring_ref))
484485
goto error;
485486
if (test_bit(KEY_FLAG_ROOT_CAN_CLEAR,
@@ -563,7 +564,7 @@ long keyctl_keyring_unlink(key_serial_t id, key_serial_t ringid)
563564
goto error;
564565
}
565566

566-
key_ref = lookup_user_key(id, KEY_LOOKUP_FOR_UNLINK, 0);
567+
key_ref = lookup_user_key(id, KEY_LOOKUP_PARTIAL, KEY_NEED_UNLINK);
567568
if (IS_ERR(key_ref)) {
568569
ret = PTR_ERR(key_ref);
569570
goto error2;
@@ -663,7 +664,7 @@ long keyctl_describe_key(key_serial_t keyid,
663664
key_put(instkey);
664665
key_ref = lookup_user_key(keyid,
665666
KEY_LOOKUP_PARTIAL,
666-
0);
667+
KEY_AUTHTOKEN_OVERRIDE);
667668
if (!IS_ERR(key_ref))
668669
goto okay;
669670
}
@@ -833,7 +834,7 @@ long keyctl_read_key(key_serial_t keyid, char __user *buffer, size_t buflen)
833834
size_t key_data_len;
834835

835836
/* find the key first */
836-
key_ref = lookup_user_key(keyid, 0, 0);
837+
key_ref = lookup_user_key(keyid, 0, KEY_DEFER_PERM_CHECK);
837838
if (IS_ERR(key_ref)) {
838839
ret = -ENOKEY;
839840
goto out;
@@ -1471,7 +1472,7 @@ long keyctl_set_timeout(key_serial_t id, unsigned timeout)
14711472
key_put(instkey);
14721473
key_ref = lookup_user_key(id,
14731474
KEY_LOOKUP_PARTIAL,
1474-
0);
1475+
KEY_AUTHTOKEN_OVERRIDE);
14751476
if (!IS_ERR(key_ref))
14761477
goto okay;
14771478
}
@@ -1579,7 +1580,8 @@ long keyctl_get_security(key_serial_t keyid,
15791580
return PTR_ERR(instkey);
15801581
key_put(instkey);
15811582

1582-
key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL, 0);
1583+
key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL,
1584+
KEY_AUTHTOKEN_OVERRIDE);
15831585
if (IS_ERR(key_ref))
15841586
return PTR_ERR(key_ref);
15851587
}

‎security/keys/permission.c

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* key_task_permission - Check a key can be used
1414
* @key_ref: The key to check.
1515
* @cred: The credentials to use.
16-
* @perm: The permissions to check for.
16+
* @need_perm: The permission required.
1717
*
1818
* Check to see whether permission is granted to use a key in the desired way,
1919
* but permit the security modules to override.
@@ -24,12 +24,30 @@
2424
* permissions bits or the LSM check.
2525
*/
2626
int key_task_permission(const key_ref_t key_ref, const struct cred *cred,
27-
unsigned perm)
27+
enum key_need_perm need_perm)
2828
{
2929
struct key *key;
30-
key_perm_t kperm;
30+
key_perm_t kperm, mask;
3131
int ret;
3232

33+
switch (need_perm) {
34+
default:
35+
WARN_ON(1);
36+
return -EACCES;
37+
case KEY_NEED_UNLINK:
38+
case KEY_SYSADMIN_OVERRIDE:
39+
case KEY_AUTHTOKEN_OVERRIDE:
40+
case KEY_DEFER_PERM_CHECK:
41+
goto lsm;
42+
43+
case KEY_NEED_VIEW: mask = KEY_OTH_VIEW; break;
44+
case KEY_NEED_READ: mask = KEY_OTH_READ; break;
45+
case KEY_NEED_WRITE: mask = KEY_OTH_WRITE; break;
46+
case KEY_NEED_SEARCH: mask = KEY_OTH_SEARCH; break;
47+
case KEY_NEED_LINK: mask = KEY_OTH_LINK; break;
48+
case KEY_NEED_SETATTR: mask = KEY_OTH_SETATTR; break;
49+
}
50+
3351
key = key_ref_to_ptr(key_ref);
3452

3553
/* use the second 8-bits of permissions for keys the caller owns */
@@ -64,13 +82,12 @@ int key_task_permission(const key_ref_t key_ref, const struct cred *cred,
6482
if (is_key_possessed(key_ref))
6583
kperm |= key->perm >> 24;
6684

67-
kperm = kperm & perm & KEY_NEED_ALL;
68-
69-
if (kperm != perm)
85+
if ((kperm & mask) != mask)
7086
return -EACCES;
7187

7288
/* let LSM be the final arbiter */
73-
return security_key_permission(key_ref, cred, perm);
89+
lsm:
90+
return security_key_permission(key_ref, cred, need_perm);
7491
}
7592
EXPORT_SYMBOL(key_task_permission);
7693

‎security/keys/process_keys.c

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -609,7 +609,7 @@ bool lookup_user_key_possessed(const struct key *key,
609609
* returned key reference.
610610
*/
611611
key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
612-
key_perm_t perm)
612+
enum key_need_perm need_perm)
613613
{
614614
struct keyring_search_context ctx = {
615615
.match_data.cmp = lookup_user_key_possessed,
@@ -773,35 +773,33 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
773773

774774
/* unlink does not use the nominated key in any way, so can skip all
775775
* the permission checks as it is only concerned with the keyring */
776-
if (lflags & KEY_LOOKUP_FOR_UNLINK) {
777-
ret = 0;
778-
goto error;
779-
}
780-
781-
if (!(lflags & KEY_LOOKUP_PARTIAL)) {
782-
ret = wait_for_key_construction(key, true);
783-
switch (ret) {
784-
case -ERESTARTSYS:
785-
goto invalid_key;
786-
default:
787-
if (perm)
776+
if (need_perm != KEY_NEED_UNLINK) {
777+
if (!(lflags & KEY_LOOKUP_PARTIAL)) {
778+
ret = wait_for_key_construction(key, true);
779+
switch (ret) {
780+
case -ERESTARTSYS:
781+
goto invalid_key;
782+
default:
783+
if (need_perm != KEY_AUTHTOKEN_OVERRIDE &&
784+
need_perm != KEY_DEFER_PERM_CHECK)
785+
goto invalid_key;
786+
case 0:
787+
break;
788+
}
789+
} else if (need_perm != KEY_DEFER_PERM_CHECK) {
790+
ret = key_validate(key);
791+
if (ret < 0)
788792
goto invalid_key;
789-
case 0:
790-
break;
791793
}
792-
} else if (perm) {
793-
ret = key_validate(key);
794-
if (ret < 0)
794+
795+
ret = -EIO;
796+
if (!(lflags & KEY_LOOKUP_PARTIAL) &&
797+
key_read_state(key) == KEY_IS_UNINSTANTIATED)
795798
goto invalid_key;
796799
}
797800

798-
ret = -EIO;
799-
if (!(lflags & KEY_LOOKUP_PARTIAL) &&
800-
key_read_state(key) == KEY_IS_UNINSTANTIATED)
801-
goto invalid_key;
802-
803801
/* check the permissions */
804-
ret = key_task_permission(key_ref, ctx.cred, perm);
802+
ret = key_task_permission(key_ref, ctx.cred, need_perm);
805803
if (ret < 0)
806804
goto invalid_key;
807805

‎security/security.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2398,10 +2398,10 @@ void security_key_free(struct key *key)
23982398
call_void_hook(key_free, key);
23992399
}
24002400

2401-
int security_key_permission(key_ref_t key_ref,
2402-
const struct cred *cred, unsigned perm)
2401+
int security_key_permission(key_ref_t key_ref, const struct cred *cred,
2402+
enum key_need_perm need_perm)
24032403
{
2404-
return call_int_hook(key_permission, 0, key_ref, cred, perm);
2404+
return call_int_hook(key_permission, 0, key_ref, cred, need_perm);
24052405
}
24062406

24072407
int security_key_getsecurity(struct key *key, char **_buffer)

‎security/selinux/hooks.c

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6561,20 +6561,43 @@ static void selinux_key_free(struct key *k)
65616561

65626562
static int selinux_key_permission(key_ref_t key_ref,
65636563
const struct cred *cred,
6564-
unsigned perm)
6564+
enum key_need_perm need_perm)
65656565
{
65666566
struct key *key;
65676567
struct key_security_struct *ksec;
6568-
u32 sid;
6568+
u32 perm, sid;
65696569

6570-
/* if no specific permissions are requested, we skip the
6571-
permission check. No serious, additional covert channels
6572-
appear to be created. */
6573-
if (perm == 0)
6570+
switch (need_perm) {
6571+
case KEY_NEED_VIEW:
6572+
perm = KEY__VIEW;
6573+
break;
6574+
case KEY_NEED_READ:
6575+
perm = KEY__READ;
6576+
break;
6577+
case KEY_NEED_WRITE:
6578+
perm = KEY__WRITE;
6579+
break;
6580+
case KEY_NEED_SEARCH:
6581+
perm = KEY__SEARCH;
6582+
break;
6583+
case KEY_NEED_LINK:
6584+
perm = KEY__LINK;
6585+
break;
6586+
case KEY_NEED_SETATTR:
6587+
perm = KEY__SETATTR;
6588+
break;
6589+
case KEY_NEED_UNLINK:
6590+
case KEY_SYSADMIN_OVERRIDE:
6591+
case KEY_AUTHTOKEN_OVERRIDE:
6592+
case KEY_DEFER_PERM_CHECK:
65746593
return 0;
6594+
default:
6595+
WARN_ON(1);
6596+
return -EPERM;
65756597

6576-
sid = cred_sid(cred);
6598+
}
65776599

6600+
sid = cred_sid(cred);
65786601
key = key_ref_to_ptr(key_ref);
65796602
ksec = key->security;
65806603

0 commit comments

Comments
 (0)