diff mbox series

[RFC,05/12] integrity: Introduce mok keyring

Message ID 20210707024403.1083977-6-eric.snowberg@oracle.com (mailing list archive)
State New, archived
Headers show
Series Enroll kernel keys thru MOK | expand

Commit Message

Eric Snowberg July 7, 2021, 2:43 a.m. UTC
Introduce a new keyring called mok.  This keyring will be used during
boot. Afterwards it will be destroyed.

Follow on patches will use this keyring to load trusted MOK keys.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/integrity/Makefile                   |  3 ++-
 security/integrity/digsig.c                   |  1 +
 security/integrity/integrity.h                |  7 ++++-
 security/integrity/platform_certs/load_uefi.c |  1 +
 .../integrity/platform_certs/mok_keyring.c    | 26 +++++++++++++++++++
 5 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 security/integrity/platform_certs/mok_keyring.c

Comments

Linus Torvalds July 7, 2021, 7:31 p.m. UTC | #1
On Tue, Jul 6, 2021 at 7:45 PM Eric Snowberg <eric.snowberg@oracle.com> wrote:
>
> Introduce a new keyring called mok.  This keyring will be used during
> boot. Afterwards it will be destroyed.

Already discussed elsewhere, but yeah, when using TLA's, unless they
are universally understood (like "CPU" or "TLB" or whatever), please
spell them out somewhere for people who don't have the background.

I saw that you said elsewhere that MOK is "Machine Owner Key", but
please let's just have that in the sources and commit messages at
least for the original new code cases.

Maybe it becomes obvious over time as there is more history to the
code, but when you literally introduce a new concept, please spell it
out.

           Linus
Jarkko Sakkinen July 7, 2021, 9:26 p.m. UTC | #2
On Wed, Jul 07, 2021 at 12:31:23PM -0700, Linus Torvalds wrote:
> On Tue, Jul 6, 2021 at 7:45 PM Eric Snowberg <eric.snowberg@oracle.com> wrote:
> >
> > Introduce a new keyring called mok.  This keyring will be used during
> > boot. Afterwards it will be destroyed.
> 
> Already discussed elsewhere, but yeah, when using TLA's, unless they
> are universally understood (like "CPU" or "TLB" or whatever), please
> spell them out somewhere for people who don't have the background.
> 
> I saw that you said elsewhere that MOK is "Machine Owner Key", but
> please let's just have that in the sources and commit messages at
> least for the original new code cases.
> 
> Maybe it becomes obvious over time as there is more history to the
> code, but when you literally introduce a new concept, please spell it
> out.
> 
>            Linus
> 
I'd suggest for the short summary:

"integrity: Introduce a Linux keyring for the Machine Owner Key (MOK)"

Given that "keyring" is such a saturated and ambiguous word, and this not a
subsystem patch for keyring itself, it should be explicit what is meant by
a keyring.

/Jarkko
Eric Snowberg July 7, 2021, 10:32 p.m. UTC | #3
> On Jul 7, 2021, at 3:26 PM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> 
> On Wed, Jul 07, 2021 at 12:31:23PM -0700, Linus Torvalds wrote:
>> On Tue, Jul 6, 2021 at 7:45 PM Eric Snowberg <eric.snowberg@oracle.com> wrote:
>>> 
>>> Introduce a new keyring called mok.  This keyring will be used during
>>> boot. Afterwards it will be destroyed.
>> 
>> Already discussed elsewhere, but yeah, when using TLA's, unless they
>> are universally understood (like "CPU" or "TLB" or whatever), please
>> spell them out somewhere for people who don't have the background.
>> 
>> I saw that you said elsewhere that MOK is "Machine Owner Key", but
>> please let's just have that in the sources and commit messages at
>> least for the original new code cases.
>> 
>> Maybe it becomes obvious over time as there is more history to the
>> code, but when you literally introduce a new concept, please spell it
>> out.
>> 
>>           Linus
>> 
> I'd suggest for the short summary:
> 
> "integrity: Introduce a Linux keyring for the Machine Owner Key (MOK)"
> 
> Given that "keyring" is such a saturated and ambiguous word, and this not a
> subsystem patch for keyring itself, it should be explicit what is meant by
> a keyring.

If we can go in this direction, I will update the heading as Jarkko has 
suggested in a follow on series.  I will also include a better summary in 
this patch, along with a MOK explanation at the beginning. Thanks.
diff mbox series

Patch

diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 7ee39d66cf16..8e2e98cba1f6 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -9,7 +9,8 @@  integrity-y := iint.o
 integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
 integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
 integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
-integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o
+integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o \
+						  platform_certs/mok_keyring.o
 integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
 				      platform_certs/load_uefi.o \
 				      platform_certs/keyring_handler.o
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index a8436c6b93ec..56800a5f1e10 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -30,6 +30,7 @@  static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
 	".ima",
 #endif
 	".platform",
+	".mok",
 };
 
 #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index f801b2076f01..5126c80bd0d4 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -151,7 +151,8 @@  int integrity_kernel_read(struct file *file, loff_t offset,
 #define INTEGRITY_KEYRING_EVM		0
 #define INTEGRITY_KEYRING_IMA		1
 #define INTEGRITY_KEYRING_PLATFORM	2
-#define INTEGRITY_KEYRING_MAX		3
+#define INTEGRITY_KEYRING_MOK		3
+#define INTEGRITY_KEYRING_MAX		4
 
 extern struct dentry *integrity_dir;
 
@@ -282,9 +283,13 @@  integrity_audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type)
 #ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
 void __init add_to_platform_keyring(const char *source, const void *data,
 				    size_t len);
+void __init destroy_mok_keyring(void);
 #else
 static inline void __init add_to_platform_keyring(const char *source,
 						  const void *data, size_t len)
 {
 }
+static inline void __init destroy_mok_keyring(void)
+{
+}
 #endif
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index f290f78c3f30..94faa4b32441 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -193,6 +193,7 @@  static int __init load_uefi_certs(void)
 
 	/* Load the MokListRT certs */
 	rc = load_moklist_certs();
+	destroy_mok_keyring();
 
 	return rc;
 }
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
new file mode 100644
index 000000000000..2b0d17caf8fd
--- /dev/null
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -0,0 +1,26 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MOK keyring routines.
+ *
+ * Copyright (c) 2021, Oracle and/or its affiliates.
+ */
+
+#include "../integrity.h"
+
+static __init int mok_keyring_init(void)
+{
+	int rc;
+
+	rc = integrity_init_keyring(INTEGRITY_KEYRING_MOK);
+	if (rc)
+		return rc;
+
+	pr_notice("MOK Keyring initialized\n");
+	return 0;
+}
+device_initcall(mok_keyring_init);
+
+void __init destroy_mok_keyring(void)
+{
+	return integrity_destroy_keyring(INTEGRITY_KEYRING_MOK);
+}