IMA: Use the system trusted keyrings instead of .ima_mok (update)

PATCH In review Mimi Zohar 2 participants 6 messages Apr 6, 2016 – Apr 7, 2016

Subsystems (4)

Security (LSM) Keys/keyrings Integrity measurement architecture (IMA) Extended Verification Module (EVM)

lore

Share & tools

lore

Mimi Zohar

author

2016-04-06 04:10 UTC #0 lore raw

PATCH 1/1 IMA: Use the system trusted keyrings instead of .ima_mok (update)

Hi David,

Please feel free to squash this patch with yours of the same name (12/12).

Just replace the .ima_mok keyring with the .secondary_trusted_keys.

Signed-off-by Mimi Zohar <[email protected]>

---
 include/keys/system_keyring.h    |  4 +--
 security/integrity/digsig.c      |  9 ++----
 security/integrity/ima/Kconfig   | 61 +++++++++++-----------------------------
 security/integrity/ima/Makefile  |  2 +-
 security/integrity/ima/ima_mok.c |  4 +--
 5 files changed, 24 insertions(+), 56 deletions(-)

include/keys/system_keyring.h +2 -2

index 87eeea4..fbd4647 100644
@@ -33,7 +33,7 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
 #define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
 #endif
 
-#ifdef CONFIG_IMA_MOK_KEYRING
+#ifdef CONFIG_IMA_BLACKLIST_KEYRING
 extern struct key *ima_blacklist_keyring;
 
 static inline struct key *get_ima_blacklist_keyring(void)

@@ -45,7 +45,7 @@ static inline struct key *get_ima_blacklist_keyring(void)
 {
 	return NULL;
 }
-#endif /* CONFIG_IMA_MOK_KEYRING */
+#endif /* CONFIG_IMA_BLACKLIST_KEYRING */
 
 
 #endif /* _KEYS_SYSTEM_KEYRING_H */

security/integrity/digsig.c +3 -6

index ef2f911..4304372 100644
@@ -42,12 +42,10 @@ static bool init_keyring __initdata = true;
 static bool init_keyring __initdata;
 #endif
 
-#if defined(CONFIG_IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN)
-#define restrict_link_to_ima restrict_link_by_builtin_trusted
-#elif defined(CONFIG_IMA_KEYRINGS_ADD_IF_SIGNED_BY_BUILTIN_OR_SECONDARY)
+#ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
 #define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
 #else
-#define restrict_link_to_ima restrict_link_reject
+#define restrict_link_to_ima restrict_link_by_builtin_trusted
 #endif
 
 int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,

@@ -94,8 +92,7 @@ int __init integrity_init_keyring(const unsigned int id)
 				     KEY_USR_VIEW | KEY_USR_READ |
 				     KEY_USR_WRITE | KEY_USR_SEARCH),
 				    KEY_ALLOC_NOT_IN_QUOTA,
-				    restrict_link_to_ima,
-				    NULL);
+				    restrict_link_to_ima, NULL);
 	if (IS_ERR(keyring[id])) {
 		err = PTR_ERR(keyring[id]);
 		pr_info("Can't allocate %s keyring (%d)\n",

security/integrity/ima/Kconfig +16 ~~-45~~

77 lines not on this page · raw message

security/integrity/ima/Makefile +1 -1

index a8539f9..9aeaeda 100644
@@ -8,4 +8,4 @@ obj-$(CONFIG_IMA) += ima.o
 ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
 	 ima_policy.o ima_template.o ima_template_lib.o
 ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
-obj-$(CONFIG_IMA_MOK_KEYRING) += ima_mok.o
+obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o

security/integrity/ima/ima_mok.c +2 -2

index 1480f68..74a27995 100644
@@ -27,7 +27,7 @@ struct key *ima_blacklist_keyring;
  */
 __init int ima_mok_init(void)
 {
-	pr_notice("Allocating IMA blacklist keyrings.\n");
+	pr_notice("Allocating IMA blacklist keyring.\n");
 
 	ima_blacklist_keyring = keyring_alloc(".ima_blacklist",
 				KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),

@@ -38,7 +38,7 @@ __init int ima_mok_init(void)
 				restrict_link_by_builtin_trusted, NULL);
 
 	if (IS_ERR(ima_blacklist_keyring))
-		panic("Can't allocate IMA MOK or blacklist keyrings.");
+		panic("Can't allocate IMA blacklist keyring.");
 
 	set_bit(KEY_FLAG_KEEP, &ima_blacklist_keyring->flags);
 	return 0;

Signature · 1 line

2.1.0

David Howells

maintainer

2016-04-06 16:24 UTC #1 lore raw

Looking in digsig.c, I see:

	#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
	static bool init_keyring __initdata = true;
	#else
	static bool init_keyring __initdata;
	#endif

Since this doesn't ever appear to be altered, should integrity_init_keyring() just be made conditionally compiled?

David

Mimi Zohar

author

2016-04-06 17:06 UTC #2 lore raw

Quoted 10 lines · David Howells

Looking in digsig.c, I see:
	#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
	static bool init_keyring __initdata = true;
	#else
	static bool init_keyring __initdata;
	#endif
Since this doesn't ever appear to be altered, should integrity_init_keyring() just be made conditionally compiled?

I'm not sure what you're asking. If you're asking if the whole file can be include based on whether this option is enabled, then no.

Mimi

David Howells

maintainer

2016-04-06 18:10 UTC #3 lore raw

Quoted 2 lines · Mimi Zohar

I'm not sure what you're asking. If you're asking if the whole file can be include based on whether this option is enabled, then no.

No - but integrity_init_keyring() just returns if init_keyring is false - but this is a variable and is assigned storage, despite the fact that its value is only set at compile time as far as I can see.

David

Mimi Zohar

author

2016-04-06 18:51 UTC #4 lore raw

Quoted 8 lines · David Howells

Quoted 2 lines · Mimi Zohar
I'm not sure what you're asking. If you're asking if the whole file can be include based on whether this option is enabled, then no.
No - but integrity_init_keyring() just returns if init_keyring is false - but this is a variable and is assigned storage, despite the fact that its value is only set at compile time as far as I can see.

Originally userspace created the original IMA and EVM keyrings, while the dot prefixed trusted keyrings were created by the kernel. The kernel could just as well create the original underscore prefixed IMA and EVM keyrings.

Mimi

David Howells

maintainer

2016-04-07 09:01 UTC #5 lore raw

In reply to Mimi Zohar

Quoted 5 lines · Mimi Zohar

Please feel free to squash this patch with yours of the same name (12/12).
Just replace the .ima_mok keyring with the .secondary_trusted_keys.
Signed-off-by Mimi Zohar <[email protected]>

Okay, I've squashed it and am posting a new set. How do you want your changes attributing to you?

Do you have any objections to my patch set beginning:

[RFC PATCH 0/7] KEYS: Adjust public key signature handling

also at:

http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-sig

upon which the trusted patches are based?

David