certs: Fix blacklist flag type confusion

[ Upstream commit 4993e1f9479a4161fd7d93e2b8b30b438f00cb0f ]

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: 734114f8782f ("KEYS: Add a system blacklist keyring")
Reported-by: Mickaël Salaün <mic@linux.microsoft.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Mickaël Salaün <mic@linux.microsoft.com>
cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/certs/blacklist.c b/certs/blacklist.c
index 3a507b9e2568a27e9d2e4359fc4fbdc3218dda4b..e9f3f81c51f96a17cb546ec559e319d0183138ce 100644 (file)
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -157,7 +157,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");

diff --git a/include/linux/key.h b/include/linux/key.h
index 8a15cabe928d0ee282742f0c44be26e7af01851d..8a66292090150a6ff6783386ecafa19c56638011 100644 (file)
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -248,6 +248,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);

diff --git a/security/integrity/ima/ima_mok.c b/security/integrity/ima/ima_mok.c
index 073ddc9bce5ba14ad9af18f2cc4f569043016b98..3e7a1523663b83c7d545f735fb276462e4f1062a 100644 (file)
--- a/security/integrity/ima/ima_mok.c
+++ b/security/integrity/ima/ima_mok.c
@@ -43,13 +43,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);

diff --git a/security/keys/key.c b/security/keys/key.c
index 5f4cb271464a06bbb3a46fb33ed746d46dae4e5a..0dec3c82dde95129873c0499ca67ed7520c359db 100644 (file)
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -305,6 +305,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;