linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Mickaël Salaün" <mic@digikod.net>
To: David Howells <dhowells@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Jarkko Sakkinen <jarkko@kernel.org>
Cc: "Mickaël Salaün" <mic@digikod.net>,
	"David S . Miller" <davem@davemloft.net>,
	"Herbert Xu" <herbert@gondor.apana.org.au>,
	"James Morris" <jmorris@namei.org>,
	"Mickaël Salaün" <mic@linux.microsoft.com>,
	"Mimi Zohar" <zohar@linux.ibm.com>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	"Tyler Hicks" <tyhicks@linux.microsoft.com>,
	keyrings@vger.kernel.org, linux-crypto@vger.kernel.org,
	linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	"Mimi Zohar" <zohar@linux.vnet.ibm.com>
Subject: [PATCH v4 04/10] certs: Fix blacklist flag type confusion
Date: Thu, 21 Jan 2021 16:55:07 +0100	[thread overview]
Message-ID: <20210121155513.539519-5-mic@digikod.net> (raw)
In-Reply-To: <20210121155513.539519-1-mic@digikod.net>

From: David Howells <dhowells@redhat.com>

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().

ima_mok_init() allocates another blacklist keyring (with different
properties) dedicated to IMA.  We can also use KEY_ALLOC_SET_KEEP 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 or modified.

Fixes: 734114f8782f ("KEYS: Add a system blacklist keyring")
Cc: Mimi Zohar <zohar@linux.vnet.ibm.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Reported-by: Mickaël Salaün <mic@linux.microsoft.com>
Signed-off-by: David Howells <dhowells@redhat.com>
[mic@linux.microsoft.com: Fix argument bit-ORing in ima_mok_init() and
extend commit description]
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
---

Changes since v3:
* Extend ima_mok_init() description (suggested by Jarkko Sakkinen).
* Add Acked-by Jarkko Sakkinen.

Changes since v2:
* Cherry-pick rewritten v1 patch from
  https://lore.kernel.org/lkml/2659836.1607940186@warthog.procyon.org.uk/
  to rebase on v5.11-rc3 and fix ima_mok_init().
---
 certs/blacklist.c                | 2 +-
 include/linux/key.h              | 1 +
 security/integrity/ima/ima_mok.c | 4 +---
 security/keys/key.c              | 2 ++
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/certs/blacklist.c b/certs/blacklist.c
index 4e1a58170d5c..446818e90b54 100644
--- a/certs/blacklist.c
+++ b/certs/blacklist.c
@@ -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");
diff --git a/include/linux/key.h b/include/linux/key.h
index 0f2e24f13c2b..eed3ce139a32 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -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);
diff --git a/security/integrity/ima/ima_mok.c b/security/integrity/ima/ima_mok.c
index 36cadadbfba4..5594dd38ab04 100644
--- a/security/integrity/ima/ima_mok.c
+++ b/security/integrity/ima/ima_mok.c
@@ -38,13 +38,11 @@ __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 ebe752b137aa..c45afdd1dfbb 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -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;
-- 
2.30.0


  parent reply	other threads:[~2021-01-21 15:59 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-21 15:55 [PATCH v4 00/10] Enable root to update the blacklist keyring Mickaël Salaün
2021-01-21 15:55 ` [PATCH v4 01/10] tools/certs: Add print-cert-tbs-hash.sh Mickaël Salaün
2021-01-21 15:55 ` [PATCH v4 02/10] certs: Check that builtin blacklist hashes are valid Mickaël Salaün
2021-01-21 15:55 ` [PATCH v4 03/10] certs: Fix blacklisted hexadecimal hash string check Mickaël Salaün
2021-01-21 15:55 ` Mickaël Salaün [this message]
2021-01-21 15:55 ` [PATCH v4 05/10] certs: Replace K{U,G}IDT_INIT() with GLOBAL_ROOT_{U,G}ID Mickaël Salaün
2021-01-21 15:55 ` [PATCH v4 06/10] certs: Make blacklist_vet_description() more strict Mickaël Salaün
2021-01-21 15:55 ` [PATCH v4 07/10] certs/blacklist: fix kernel doc interface issue Mickaël Salaün
2021-01-21 15:55 ` [PATCH v4 08/10] certs: Factor out the blacklist hash creation Mickaël Salaün
2021-01-21 15:55 ` [PATCH v4 09/10] PKCS#7: Fix missing include Mickaël Salaün
2021-01-21 15:55 ` [PATCH v4 10/10] certs: Allow root user to append signed hashes to the blacklist keyring Mickaël Salaün
2021-01-28 16:52 ` [PATCH v4 00/10] Enable root to update " David Howells
2021-01-28 17:38   ` Mickaël Salaün
2021-01-28 18:00     ` Mickaël Salaün
2021-01-30 19:39   ` Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210121155513.539519-5-mic@digikod.net \
    --to=mic@digikod.net \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=jarkko@kernel.org \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mic@linux.microsoft.com \
    --cc=serge@hallyn.com \
    --cc=tyhicks@linux.microsoft.com \
    --cc=zohar@linux.ibm.com \
    --cc=zohar@linux.vnet.ibm.com \
    --subject='Re: [PATCH v4 04/10] certs: Fix blacklist flag type confusion' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).