Skip to content

Commit

Permalink
certs: Fix blacklist flag type confusion
Browse files Browse the repository at this point in the history
KEY_FLAG_KEEP is not meant to be passed to keyring_alloc() or key_alloc(),
as these only take KEY_ALLOC_* flags.  KEY_FLAG_KEEP has the same value as
KEY_ALLOC_BYPASS_RESTRICTION, but fortunately only key_create_or_update()
uses it.  LSMs using the key_alloc hook don't check that flag.

KEY_FLAG_KEEP is then ignored but fortunately (again) the root user cannot
write to the blacklist keyring, so it is not possible to remove a key/hash
from it.

Fix this by adding a KEY_ALLOC_SET_KEEP flag that tells key_alloc() to set
KEY_FLAG_KEEP on the new key.  blacklist_init() can then, correctly, pass
this to keyring_alloc().

We can also use this in ima_mok_init() rather than setting the flag
manually.

Note that this doesn't fix an observable bug with the current
implementation but it is required to allow addition of new hashes to the
blacklist in the future without making it possible for them to be removed.

Fixes: 734114f ("KEYS: Add a system blacklist keyring")
Reported-by: Mickaël Salaün <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Mickaël Salaün <[email protected]>
cc: Mimi Zohar <[email protected]>
Cc: David Woodhouse <[email protected]>
  • Loading branch information
dhowells committed Jan 21, 2021
1 parent f14602c commit 4993e1f
Show file tree
Hide file tree
Showing 4 changed files with 6 additions and 4 deletions.
2 changes: 1 addition & 1 deletion certs/blacklist.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ static int __init blacklist_init(void)
KEY_USR_VIEW | KEY_USR_READ |
KEY_USR_SEARCH,
KEY_ALLOC_NOT_IN_QUOTA |
KEY_FLAG_KEEP,
KEY_ALLOC_SET_KEEP,
NULL, NULL);
if (IS_ERR(blacklist_keyring))
panic("Can't allocate system blacklist keyring\n");
Expand Down
1 change: 1 addition & 0 deletions include/linux/key.h
Original file line number Diff line number Diff line change
Expand Up @@ -289,6 +289,7 @@ extern struct key *key_alloc(struct key_type *type,
#define KEY_ALLOC_BUILT_IN 0x0004 /* Key is built into kernel */
#define KEY_ALLOC_BYPASS_RESTRICTION 0x0008 /* Override the check on restricted keyrings */
#define KEY_ALLOC_UID_KEYRING 0x0010 /* allocating a user or user session keyring */
#define KEY_ALLOC_SET_KEEP 0x0020 /* Set the KEEP flag on the key/keyring */

extern void key_revoke(struct key *key);
extern void key_invalidate(struct key *key);
Expand Down
5 changes: 2 additions & 3 deletions security/integrity/ima/ima_mok.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@ __init int ima_mok_init(void)
(KEY_POS_ALL & ~KEY_POS_SETATTR) |
KEY_USR_VIEW | KEY_USR_READ |
KEY_USR_WRITE | KEY_USR_SEARCH,
KEY_ALLOC_NOT_IN_QUOTA,
KEY_ALLOC_NOT_IN_QUOTA |
KEY_ALLOC_SET_KEEP,
restriction, NULL);

if (IS_ERR(ima_blacklist_keyring))
panic("Can't allocate IMA blacklist keyring.");

set_bit(KEY_FLAG_KEEP, &ima_blacklist_keyring->flags);
return 0;
}
device_initcall(ima_mok_init);
2 changes: 2 additions & 0 deletions security/keys/key.c
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,8 @@ struct key *key_alloc(struct key_type *type, const char *desc,
key->flags |= 1 << KEY_FLAG_BUILTIN;
if (flags & KEY_ALLOC_UID_KEYRING)
key->flags |= 1 << KEY_FLAG_UID_KEYRING;
if (flags & KEY_ALLOC_SET_KEEP)
key->flags |= 1 << KEY_FLAG_KEEP;

#ifdef KEY_DEBUGGING
key->magic = KEY_DEBUG_MAGIC;
Expand Down

0 comments on commit 4993e1f

Please sign in to comment.