linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 00/12] Enroll kernel keys thru MOK
@ 2021-07-07  2:43 Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 01/12] KEYS: Add KEY_ALLOC_BYPASS_RESTRICTION option to key_move Eric Snowberg
                   ` (13 more replies)
  0 siblings, 14 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:43 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

This is a follow up to the "Add additional MOK vars" [1] series I 
previously sent.  This series incorporates the feedback given 
both publicly on the mailing list and privately from Mimi. This 
series just focuses on getting end-user keys into the kernel trust 
boundary.

Currently, pre-boot keys are not trusted within the Linux boundary [2].
Pre-boot keys include UEFI Secure Boot DB keys and MOKList keys. These
keys are loaded into the platform keyring and can only be used for kexec.
If an end-user wants to use their own key within the Linux trust
boundary, they must either compile it into the kernel themselves or use
the insert-sys-cert script. Both options present a problem. Many
end-users do not want to compile their own kernels. With the
insert-sys-cert option, there are missing upstream changes [3].  Also,
with the insert-sys-cert option, the end-user must re-sign their kernel
again with their own key, and then insert that key into the MOK db.
Another problem with insert-sys-cert is that only a single key can be
inserted into a compressed kernel.

Having the ability to insert a key into the Linux trust boundary opens
up various possibilities.  The end-user can use a pre-built kernel and
sign their own kernel modules.  It also opens up the ability for an
end-user to more easily use digital signature based IMA-appraisal.  To
get a key into the ima keyring, it must be signed by a key within the
Linux trust boundary.

Downstream Linux distros try to have a single signed kernel for each
architecture.  Each end-user may use this kernel in entirely different
ways.  Some downstream kernels have chosen to always trust platform keys
within the Linux trust boundary for kernel module signing.  These
kernels have no way of using digital signature base IMA appraisal. 

This series adds a new MOK variable to shim.  This variable allows the
end-user to decide if they want to trust keys enrolled in the MOK within
the Linux trust boundary.  By default, nothing changes; MOK keys are
not trusted within the Linux kernel.  They are only trusted after the 
end-user makes the decision themselves.  The end-user would set this
through mokutil using a new --trust-mok option [4]. This would work
similar to how the kernel uses MOK variable to enable/disable signature
validation as well as use/ignore the db.

When shim boots, it mirrors the new MokTML Boot Services variable to a new
MokListTrustedRT Runtime Services variable and extends PCR14. 
MokListTrustedRT is written without EFI_VARIABLE_NON_VOLATILE set,
preventing an end-user from setting it after booting and doing a kexec.

When the kernel boots, if MokListTrustedRT is set and
EFI_VARIABLE_NON_VOLATILE is not set, the MokListRT is loaded into the
secondary trusted keyring instead of the platform keyring. Mimi has
suggested that only CA keys or keys that can be vouched for by other
kernel keys be loaded. All other certs will load into the platform
keyring instead.

This is done by introducing a new .mok keyring.  This keyring is only
used during boot.  After booting it is destroyed and not visible to the
end-user after booting completes.  This keyring contains a new keyring
permission that only allows CA keys to be loaded. If the permission
fails, the key is later loaded into the platform keyring.  After keys
are added into the .mok keyring, they are moved into the secondary
trusted keyring.

Secure Boot keys will never be trusted.  They will always be loaded
into the platform keyring.  If an end-user wanted to trust one, they
would need to enroll it into the MOK.

I have included links to both the mokutil [3] and shim [4] changes I
have made to support this new functionality.

Thank you and looking forward to hearing your reviews.

[1] https://lore.kernel.org/linux-integrity/20210517225714.498032-1-eric.snowberg@oracle.com/
[2] https://lore.kernel.org/lkml/1556221605.24945.3.camel@HansenPartnership.com/
[3] https://lore.kernel.org/patchwork/cover/902768/
[4] https://github.com/esnowberg/mokutil/tree/0.3.0-mokvars-v2
[5] https://github.com/esnowberg/shim/tree/mokvars-v2

Eric Snowberg (12):
  KEYS: Add KEY_ALLOC_BYPASS_RESTRICTION option to key_move
  KEYS: Allow unrestricted keys to be moved to the secondary keyring
  KEYS: CA link restriction
  integrity: add integrity_destroy_keyring
  integrity: Introduce mok keyring
  integrity: Trust mok keys if MokListTrustedRT found
  integrity: add add_to_mok_keyring
  integrity: restrict INTEGRITY_KEYRING_MOK to
    restrict_link_by_secondary_trusted_or_ca
  integrity: accessor function to get trust_moklist
  integrity: add new keyring handler
  integrity: move keys from the mok keyring into the secondary keyring
  integrity: Suppress error message for keys added to the mok keyring

 certs/system_keyring.c                        | 43 +++++++++
 crypto/asymmetric_keys/restrict.c             | 60 +++++++++++++
 include/crypto/public_key.h                   |  5 ++
 include/keys/system_keyring.h                 | 21 +++++
 security/integrity/Makefile                   |  3 +-
 security/integrity/digsig.c                   | 26 +++++-
 security/integrity/integrity.h                | 21 ++++-
 .../platform_certs/keyring_handler.c          | 17 +++-
 .../platform_certs/keyring_handler.h          |  5 ++
 security/integrity/platform_certs/load_uefi.c |  5 +-
 .../integrity/platform_certs/mok_keyring.c    | 90 +++++++++++++++++++
 security/keys/keyring.c                       | 10 ++-
 12 files changed, 294 insertions(+), 12 deletions(-)
 create mode 100644 security/integrity/platform_certs/mok_keyring.c


base-commit: 13311e74253fe64329390df80bed3f07314ddd61
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 01/12] KEYS: Add KEY_ALLOC_BYPASS_RESTRICTION option to key_move
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
@ 2021-07-07  2:43 ` Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 02/12] KEYS: Allow unrestricted keys to be moved to the secondary keyring Eric Snowberg
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:43 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Callers of key_create_or_update can pass KEY_ALLOC_BYPASS_RESTRICTION to
suppress the restrictions check. Add the same support to key_move to
bypass restrictions on the destination keyring.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/keys/keyring.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 5e6a90760753..56ea2b78d2e5 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1585,7 +1585,7 @@ EXPORT_SYMBOL(key_unlink);
  *
  * It is assumed that the caller has checked that it is permitted for a link to
  * be made (the keyring should have Write permission and the key Link
- * permission).
+ * permission). It can be overridden by passing KEY_ALLOC_BYPASS_RESTRICTION.
  */
 int key_move(struct key *key,
 	     struct key *from_keyring,
@@ -1618,9 +1618,11 @@ int key_move(struct key *key,
 	if (to_edit->dead_leaf && (flags & KEYCTL_MOVE_EXCL))
 		goto error;
 
-	ret = __key_link_check_restriction(to_keyring, key);
-	if (ret < 0)
-		goto error;
+	if (!(flags & KEY_ALLOC_BYPASS_RESTRICTION)) {
+		ret = __key_link_check_restriction(to_keyring, key);
+		if (ret < 0)
+			goto error;
+	}
 	ret = __key_link_check_live_key(to_keyring, key);
 	if (ret < 0)
 		goto error;
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 02/12] KEYS: Allow unrestricted keys to be moved to the secondary keyring
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 01/12] KEYS: Add KEY_ALLOC_BYPASS_RESTRICTION option to key_move Eric Snowberg
@ 2021-07-07  2:43 ` Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 03/12] KEYS: CA link restriction Eric Snowberg
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:43 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Allow keys to be moved into the secondary keyring without checking its
trust chain.  This is available only during kernel initialization.

This will allow keys in the MOK list to be added during boot.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 certs/system_keyring.c        | 25 +++++++++++++++++++++++++
 include/keys/system_keyring.h |  7 +++++++
 2 files changed, 32 insertions(+)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 692365dee2bd..f02bc5832684 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -90,6 +90,31 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
 
 	return restriction;
 }
+
+/**
+ * move_to_trusted_secondary_keyring - Move to the secondary trusted
+ * keyring with no validation.
+ * @key: The key to add to the secondary trusted keyring
+ * @from_keyring: The keyring containing the key to move from
+ *
+ * Move key to the secondary keyring without checking its trust chain.  This
+ * is available only during kernel initialization.
+ */
+__init int move_to_trusted_secondary_keyring(struct key *key, struct key *from_keyring)
+{
+	int ret;
+
+	ret = key_move(key, from_keyring, secondary_trusted_keys,
+		       KEY_ALLOC_BYPASS_RESTRICTION);
+
+	if (ret)
+		pr_err("Problem loading X.509 certificate %d\n", ret);
+	else
+		pr_notice("Loaded X.509 cert '%s' linked to secondary sys keyring\n",
+			  key->description);
+
+	return ret;
+}
 #endif
 
 /*
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 6acd3cf13a18..f40837026d6d 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -34,8 +34,15 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
 	const struct key_type *type,
 	const union key_payload *payload,
 	struct key *restriction_key);
+extern __init int move_to_trusted_secondary_keyring(struct key *key,
+						    struct key *from_keyring);
 #else
 #define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
+static inline __init int move_to_trusted_secondary_keyring(struct key *key,
+							   struct key *from_keyring)
+{
+	return -EKEYREVOKED;
+}
 #endif
 
 extern struct pkcs7_message *pkcs7;
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 03/12] KEYS: CA link restriction
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 01/12] KEYS: Add KEY_ALLOC_BYPASS_RESTRICTION option to key_move Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 02/12] KEYS: Allow unrestricted keys to be moved to the secondary keyring Eric Snowberg
@ 2021-07-07  2:43 ` Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 04/12] integrity: add integrity_destroy_keyring Eric Snowberg
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:43 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Restrict the addition of keys in a keyring based on the key to be
added being a CA (self-signed) or by being vouched for by a key in
either the built-in or the secondary trusted keyrings.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 certs/system_keyring.c            | 18 ++++++++++
 crypto/asymmetric_keys/restrict.c | 60 +++++++++++++++++++++++++++++++
 include/crypto/public_key.h       |  5 +++
 include/keys/system_keyring.h     | 14 ++++++++
 4 files changed, 97 insertions(+)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index f02bc5832684..b4c82276bba5 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -73,6 +73,24 @@ int restrict_link_by_builtin_and_secondary_trusted(
 					  secondary_trusted_keys);
 }
 
+/**
+ * restrict_link_by_secondary_trusted_or_ca - Restrict keyring
+ *   addition by being a CA or vouched by the secondary keyrings.
+ *
+ *  Restrict the addition of keys in a keyring based on the key-to-be-added
+ *  being a CA (self signed) or by being vouched for by a key in either
+ *  the built-in or the secondary system keyrings.
+ */
+int restrict_link_by_secondary_trusted_or_ca(
+	struct key *dest_keyring,
+	const struct key_type *type,
+	const union key_payload *payload,
+	struct key *restrict_key)
+{
+	return restrict_link_by_ca(dest_keyring, type, payload,
+				   secondary_trusted_keys);
+}
+
 /**
  * Allocate a struct key_restriction for the "builtin and secondary trust"
  * keyring. Only for use in system_trusted_keyring_init().
diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
index 84cefe3b3585..75e4379226e8 100644
--- a/crypto/asymmetric_keys/restrict.c
+++ b/crypto/asymmetric_keys/restrict.c
@@ -108,6 +108,66 @@ int restrict_link_by_signature(struct key *dest_keyring,
 	return ret;
 }
 
+/**
+ * restrict_link_by_ca - Restrict additions to a ring of public keys
+ * based on it being a CA
+ * @dest_keyring: Keyring being linked to.
+ * @type: The type of key being added.
+ * @payload: The payload of the new key.
+ * @trusted: A key or ring of keys that can be used to vouch for the new cert.
+ *
+ * Check if the new certificate is a CA or if they key can be vouched for
+ * by keys already linked in the destination keyring or the trusted
+ * keyring.  If one of those is the signing key or it is self signed, then
+ * mark the new certificate as being ok to link.
+ *
+ * Returns 0 if the new certificate was accepted, -ENOKEY if we could not find
+ * a matching parent certificate in the trusted list.  -ENOPKG if the signature
+ * uses unsupported crypto, or some other error if there is a matching
+ * certificate  but the signature check cannot be performed.
+ */
+int restrict_link_by_ca(struct key *dest_keyring,
+			const struct key_type *type,
+			const union key_payload *payload,
+			struct key *trust_keyring)
+{
+	const struct public_key_signature *sig;
+	const struct public_key *pkey;
+	struct key *key;
+	int ret;
+
+	if (type != &key_type_asymmetric)
+		return -EOPNOTSUPP;
+
+	sig = payload->data[asym_auth];
+	if (!sig)
+		return -ENOPKG;
+
+	if (!sig->auth_ids[0] && !sig->auth_ids[1])
+		return -ENOKEY;
+
+	pkey = payload->data[asym_crypto];
+	if (!pkey)
+		return -ENOPKG;
+
+	ret = public_key_verify_signature(pkey, sig);
+	if (!ret)
+		return 0;
+
+	if (!trust_keyring)
+		return -ENOKEY;
+
+	key = find_asymmetric_key(trust_keyring,
+				  sig->auth_ids[0], sig->auth_ids[1],
+				  false);
+	if (IS_ERR(key))
+		return -ENOKEY;
+
+	ret = verify_signature(key, sig);
+	key_put(key);
+	return ret;
+}
+
 static bool match_either_id(const struct asymmetric_key_ids *pair,
 			    const struct asymmetric_key_id *single)
 {
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 47accec68cb0..545af1ea57de 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -71,6 +71,11 @@ extern int restrict_link_by_key_or_keyring_chain(struct key *trust_keyring,
 						 const union key_payload *payload,
 						 struct key *trusted);
 
+extern int restrict_link_by_ca(struct key *dest_keyring,
+			       const struct key_type *type,
+			       const union key_payload *payload,
+			       struct key *trust_keyring);
+
 extern int query_asymmetric_key(const struct kernel_pkey_params *,
 				struct kernel_pkey_query *);
 
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index f40837026d6d..43c76fba9481 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -34,10 +34,24 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
 	const struct key_type *type,
 	const union key_payload *payload,
 	struct key *restriction_key);
+extern int restrict_link_by_secondary_trusted_or_ca(
+	struct key *dest_keyring,
+	const struct key_type *type,
+	const union key_payload *payload,
+	struct key *restrict_key);
 extern __init int move_to_trusted_secondary_keyring(struct key *key,
 						    struct key *from_keyring);
 #else
 #define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
+static inline int restrict_link_by_secondary_trusted_or_ca(
+	struct key *dest_keyring,
+	const struct key_type *type,
+	const union key_payload *payload,
+	struct key *restrict_key)
+{
+	return -ENOKEY;
+}
+
 static inline __init int move_to_trusted_secondary_keyring(struct key *key,
 							   struct key *from_keyring)
 {
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 04/12] integrity: add integrity_destroy_keyring
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (2 preceding siblings ...)
  2021-07-07  2:43 ` [PATCH RFC 03/12] KEYS: CA link restriction Eric Snowberg
@ 2021-07-07  2:43 ` Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 05/12] integrity: Introduce mok keyring Eric Snowberg
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:43 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Not all kernel keyrings need to survive past boot. Add a destroy
function to remove a keyring no longer needed.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/integrity/digsig.c    | 8 ++++++++
 security/integrity/integrity.h | 5 +++++
 2 files changed, 13 insertions(+)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 3b06a01bd0fd..a8436c6b93ec 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -145,6 +145,14 @@ int __init integrity_init_keyring(const unsigned int id)
 	return __integrity_init_keyring(id, perm, restriction);
 }
 
+void __init integrity_destroy_keyring(const unsigned int id)
+{
+	if (id >= INTEGRITY_KEYRING_MAX)
+		return;
+	key_put(keyring[id]);
+	keyring[id] = NULL;
+}
+
 static int __init integrity_add_key(const unsigned int id, const void *data,
 				    off_t size, key_perm_t perm)
 {
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 547425c20e11..f801b2076f01 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -164,6 +164,7 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 int integrity_modsig_verify(unsigned int id, const struct modsig *modsig);
 
 int __init integrity_init_keyring(const unsigned int id);
+void __init integrity_destroy_keyring(const unsigned int id);
 int __init integrity_load_x509(const unsigned int id, const char *path);
 int __init integrity_load_cert(const unsigned int id, const char *source,
 			       const void *data, size_t len, key_perm_t perm);
@@ -187,6 +188,10 @@ static inline int integrity_init_keyring(const unsigned int id)
 	return 0;
 }
 
+static inline void __init integrity_destroy_keyring(const unsigned int id)
+{
+}
+
 static inline int __init integrity_load_cert(const unsigned int id,
 					     const char *source,
 					     const void *data, size_t len,
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 05/12] integrity: Introduce mok keyring
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (3 preceding siblings ...)
  2021-07-07  2:43 ` [PATCH RFC 04/12] integrity: add integrity_destroy_keyring Eric Snowberg
@ 2021-07-07  2:43 ` Eric Snowberg
  2021-07-07 19:31   ` Linus Torvalds
  2021-07-07  2:43 ` [PATCH RFC 06/12] integrity: Trust mok keys if MokListTrustedRT found Eric Snowberg
                   ` (8 subsequent siblings)
  13 siblings, 1 reply; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:43 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

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

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);
+}
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 06/12] integrity: Trust mok keys if MokListTrustedRT found
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (4 preceding siblings ...)
  2021-07-07  2:43 ` [PATCH RFC 05/12] integrity: Introduce mok keyring Eric Snowberg
@ 2021-07-07  2:43 ` Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 07/12] integrity: add add_to_mok_keyring Eric Snowberg
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:43 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

A new MOK variable called MokListTrustedRT has been introduced in shim.
When this UEFI variable is set, it indicates the end-user has made the
decision themself that they wish to trust MOK keys within the Linux
trust boundary.  It is not an error if this variable does not exist. If
it does not exist, the MOK keys should not be trusted within the kernel.

MOK variables are mirrored from Boot Services to Runtime Services.  When
shim sees the new MokTML BS variable, it will create a new variable
(before Exit Boot Services is called) called MokListTrustedRT without
EFI_VARIABLE_NON_VOLATILE set.  Following Exit Boot Services, UEFI
variables can only be set and created with SetVariable if both
EFI_VARIABLE_RUNTIME_ACCESS & EFI_VARIABLE_NON_VOLATILE are set.
Therefore, this can not be defeated by simply creating a MokListTrustedRT
variable from Linux, the existence of EFI_VARIABLE_NON_VOLATILE will cause
uefi_check_trust_mok_keys to return false.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 .../integrity/platform_certs/mok_keyring.c    | 38 +++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index 2b0d17caf8fd..666fa355996d 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -5,8 +5,11 @@
  * Copyright (c) 2021, Oracle and/or its affiliates.
  */
 
+#include <linux/efi.h>
 #include "../integrity.h"
 
+bool trust_mok;
+
 static __init int mok_keyring_init(void)
 {
 	int rc;
@@ -24,3 +27,38 @@ void __init destroy_mok_keyring(void)
 {
 	return integrity_destroy_keyring(INTEGRITY_KEYRING_MOK);
 }
+
+/*
+ * Try to load the MokListTrustedRT UEFI variable to see if we should trust
+ * the mok keys within the kernel. It is not an error if this variable
+ * does not exist.  If it does not exist, mok keys should not be trusted
+ * within the kernel.
+ */
+static __init bool uefi_check_trust_mok_keys(void)
+{
+	efi_status_t status;
+	unsigned int mtrust = 0;
+	unsigned long size = sizeof(mtrust);
+	efi_guid_t guid = EFI_SHIM_LOCK_GUID;
+	u32 attr;
+
+	status = efi.get_variable(L"MokListTrustedRT", &guid, &attr, &size, &mtrust);
+
+	/*
+	 * The EFI_VARIABLE_NON_VOLATILE check is to verify MokListTrustedRT
+	 * was set thru shim mirrioring and not by a user from the host os.
+	 * According to the UEFI spec, once EBS is performed, SetVariable()
+	 * will succeed only when both EFI_VARIABLE_RUNTIME_ACCESS &
+	 * EFI_VARIABLE_NON_VOLATILE are set.
+	 */
+	return (status == EFI_SUCCESS && (!(attr & EFI_VARIABLE_NON_VOLATILE)));
+}
+
+static __init int mok_keyring_trust_setup(void)
+{
+	if (uefi_check_trust_mok_keys())
+		trust_mok = true;
+	return 0;
+}
+
+late_initcall(mok_keyring_trust_setup);
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 07/12] integrity: add add_to_mok_keyring
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (5 preceding siblings ...)
  2021-07-07  2:43 ` [PATCH RFC 06/12] integrity: Trust mok keys if MokListTrustedRT found Eric Snowberg
@ 2021-07-07  2:43 ` Eric Snowberg
  2021-07-07  2:43 ` [PATCH RFC 08/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_secondary_trusted_or_ca Eric Snowberg
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:43 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Add the ability to load MOK keys to the mok keyring. If the permssions
do not allow the key to be added to the MOK keyring this is not an
error, add it to the platform keyring instead.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/integrity/integrity.h                |  4 ++++
 .../integrity/platform_certs/mok_keyring.c    | 21 +++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 5126c80bd0d4..68720fa6454f 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -284,6 +284,7 @@ integrity_audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type)
 void __init add_to_platform_keyring(const char *source, const void *data,
 				    size_t len);
 void __init destroy_mok_keyring(void);
+void __init add_to_mok_keyring(const char *source, const void *data, size_t len);
 #else
 static inline void __init add_to_platform_keyring(const char *source,
 						  const void *data, size_t len)
@@ -292,4 +293,7 @@ static inline void __init add_to_platform_keyring(const char *source,
 static inline void __init destroy_mok_keyring(void)
 {
 }
+void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
+{
+}
 #endif
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index 666fa355996d..a5644a8a834c 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -28,6 +28,27 @@ void __init destroy_mok_keyring(void)
 	return integrity_destroy_keyring(INTEGRITY_KEYRING_MOK);
 }
 
+void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
+{
+	key_perm_t perm;
+	int rc;
+
+	perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW;
+	rc = integrity_load_cert(INTEGRITY_KEYRING_MOK, source, data, len, perm);
+
+	/*
+	 * If the mok keyring restrictions prevented the cert from loading,
+	 * this is not an error.  Just load it into the platform keyring
+	 * instead.
+	 */
+	if (rc)
+		rc = integrity_load_cert(INTEGRITY_KEYRING_PLATFORM, source,
+					 data, len, perm);
+
+	if (rc)
+		pr_info("Error adding keys to mok keyring %s\n", source);
+}
+
 /*
  * Try to load the MokListTrustedRT UEFI variable to see if we should trust
  * the mok keys within the kernel. It is not an error if this variable
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 08/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_secondary_trusted_or_ca
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (6 preceding siblings ...)
  2021-07-07  2:43 ` [PATCH RFC 07/12] integrity: add add_to_mok_keyring Eric Snowberg
@ 2021-07-07  2:43 ` Eric Snowberg
  2021-07-07  2:44 ` [PATCH RFC 09/12] integrity: accessor function to get trust_moklist Eric Snowberg
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:43 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Set the restriction check for INTEGRITY_KEYRING_MOK keys to
restrict_link_by_secondary_trusted_or_ca.  This will only allow keys
into the mok keyring that are either a CA or trusted by a key contained
within the secondary trusted keyring.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/integrity/digsig.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 56800a5f1e10..07547f1a4806 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -140,6 +140,11 @@ int __init integrity_init_keyring(const unsigned int id)
 		return -ENOMEM;
 
 	restriction->check = restrict_link_to_ima;
+	if (id == INTEGRITY_KEYRING_MOK)
+		restriction->check = restrict_link_by_secondary_trusted_or_ca;
+	else
+		restriction->check = restrict_link_to_ima;
+
 	perm |= KEY_USR_WRITE;
 
 out:
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 09/12] integrity: accessor function to get trust_moklist
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (7 preceding siblings ...)
  2021-07-07  2:43 ` [PATCH RFC 08/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_secondary_trusted_or_ca Eric Snowberg
@ 2021-07-07  2:44 ` Eric Snowberg
  2021-07-07  2:44 ` [PATCH RFC 10/12] integrity: add new keyring handler Eric Snowberg
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:44 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Add an accessor function to see if the mok list should be trusted.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/integrity/integrity.h                  | 5 +++++
 security/integrity/platform_certs/mok_keyring.c | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 68720fa6454f..a5f7af825f9b 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -285,6 +285,7 @@ void __init add_to_platform_keyring(const char *source, const void *data,
 				    size_t len);
 void __init destroy_mok_keyring(void);
 void __init add_to_mok_keyring(const char *source, const void *data, size_t len);
+bool __init trust_moklist(void);
 #else
 static inline void __init add_to_platform_keyring(const char *source,
 						  const void *data, size_t len)
@@ -296,4 +297,8 @@ static inline void __init destroy_mok_keyring(void)
 void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
 {
 }
+static inline bool __init trust_moklist(void)
+{
+	return false;
+}
 #endif
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index a5644a8a834c..7d23772a1135 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -83,3 +83,8 @@ static __init int mok_keyring_trust_setup(void)
 }
 
 late_initcall(mok_keyring_trust_setup);
+
+bool __init trust_moklist(void)
+{
+	return trust_mok;
+}
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 10/12] integrity: add new keyring handler
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (8 preceding siblings ...)
  2021-07-07  2:44 ` [PATCH RFC 09/12] integrity: accessor function to get trust_moklist Eric Snowberg
@ 2021-07-07  2:44 ` Eric Snowberg
  2021-07-07  2:44 ` [PATCH RFC 11/12] integrity: move keys from the mok keyring into the secondary keyring Eric Snowberg
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:44 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Add a new keyring handler for the mok keyring.  If the Secondary trusted
keyring is enabled and the end-user trusts the MOK keys, this new
keyring handler is used.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 .../integrity/platform_certs/keyring_handler.c  | 17 ++++++++++++++++-
 .../integrity/platform_certs/keyring_handler.h  |  5 +++++
 security/integrity/platform_certs/load_uefi.c   |  4 ++--
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
index 5604bd57c990..b6daeb1e3de5 100644
--- a/security/integrity/platform_certs/keyring_handler.c
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -66,7 +66,7 @@ static __init void uefi_revocation_list_x509(const char *source,
 
 /*
  * Return the appropriate handler for particular signature list types found in
- * the UEFI db and MokListRT tables.
+ * the UEFI db tables.
  */
 __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
 {
@@ -75,6 +75,21 @@ __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
 	return 0;
 }
 
+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the MokListRT tables.
+ */
+__init efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type)
+{
+	if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) {
+		if (IS_ENABLED(CONFIG_SECONDARY_TRUSTED_KEYRING) && trust_moklist())
+			return add_to_mok_keyring;
+		else
+			return add_to_platform_keyring;
+	}
+	return 0;
+}
+
 /*
  * Return the appropriate handler for particular signature list types found in
  * the UEFI dbx and MokListXRT tables.
diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 2462bfa08fe3..284558f30411 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -24,6 +24,11 @@ void blacklist_binary(const char *source, const void *data, size_t len);
  */
 efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type);
 
+/*
+ * Return the handler for particular signature list types found in the mok.
+ */
+efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type);
+
 /*
  * Return the handler for particular signature list types found in the dbx.
  */
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index 94faa4b32441..f021dd81f080 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -94,7 +94,7 @@ static int __init load_moklist_certs(void)
 		rc = parse_efi_signature_list("UEFI:MokListRT (MOKvar table)",
 					      mokvar_entry->data,
 					      mokvar_entry->data_size,
-					      get_handler_for_db);
+					      get_handler_for_mok);
 		/* All done if that worked. */
 		if (!rc)
 			return rc;
@@ -109,7 +109,7 @@ static int __init load_moklist_certs(void)
 	mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
 	if (mok) {
 		rc = parse_efi_signature_list("UEFI:MokListRT",
-					      mok, moksize, get_handler_for_db);
+					      mok, moksize, get_handler_for_mok);
 		kfree(mok);
 		if (rc)
 			pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 11/12] integrity: move keys from the mok keyring into the secondary keyring
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (9 preceding siblings ...)
  2021-07-07  2:44 ` [PATCH RFC 10/12] integrity: add new keyring handler Eric Snowberg
@ 2021-07-07  2:44 ` Eric Snowberg
  2021-07-07  2:44 ` [PATCH RFC 12/12] integrity: Suppress error message for keys added to the mok keyring Eric Snowberg
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:44 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Keys added to the mok keyring are only stored there temporarily. After
passing the permissions check, move the key from the mok keyring into
the secondary trusted keyring.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/integrity/digsig.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 07547f1a4806..e301cee037bf 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -175,8 +175,13 @@ static int __init integrity_add_key(const unsigned int id, const void *data,
 		rc = PTR_ERR(key);
 		pr_err("Problem loading X.509 certificate %d\n", rc);
 	} else {
-		pr_notice("Loaded X.509 cert '%s'\n",
-			  key_ref_to_ptr(key)->description);
+		if (id == INTEGRITY_KEYRING_MOK)
+			rc = move_to_trusted_secondary_keyring(key_ref_to_ptr(key),
+							       keyring[id]);
+		else
+			pr_notice("Loaded X.509 cert '%s'\n",
+				  key_ref_to_ptr(key)->description);
+
 		key_ref_put(key);
 	}
 
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH RFC 12/12] integrity: Suppress error message for keys added to the mok keyring
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (10 preceding siblings ...)
  2021-07-07  2:44 ` [PATCH RFC 11/12] integrity: move keys from the mok keyring into the secondary keyring Eric Snowberg
@ 2021-07-07  2:44 ` Eric Snowberg
  2021-07-07  6:46 ` [PATCH RFC 00/12] Enroll kernel keys thru MOK Christoph Hellwig
  2021-07-07 12:39 ` Mimi Zohar
  13 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07  2:44 UTC (permalink / raw)
  To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge
  Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Suppress the error message for keys added to the mok keyring. If an
error occurs, the key will be added to the platform keyring instead.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
 security/integrity/digsig.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index e301cee037bf..50bdf839fa44 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -173,7 +173,8 @@ static int __init integrity_add_key(const unsigned int id, const void *data,
 				   KEY_ALLOC_NOT_IN_QUOTA);
 	if (IS_ERR(key)) {
 		rc = PTR_ERR(key);
-		pr_err("Problem loading X.509 certificate %d\n", rc);
+		if (id != INTEGRITY_KEYRING_MOK)
+			pr_err("Problem loading X.509 certificate %d\n", rc);
 	} else {
 		if (id == INTEGRITY_KEYRING_MOK)
 			rc = move_to_trusted_secondary_keyring(key_ref_to_ptr(key),
-- 
2.18.4


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (11 preceding siblings ...)
  2021-07-07  2:44 ` [PATCH RFC 12/12] integrity: Suppress error message for keys added to the mok keyring Eric Snowberg
@ 2021-07-07  6:46 ` Christoph Hellwig
  2021-07-07 16:23   ` Eric Snowberg
  2021-07-07 12:39 ` Mimi Zohar
  13 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2021-07-07  6:46 UTC (permalink / raw)
  To: Eric Snowberg
  Cc: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert,
	davem, jarkko, jmorris, serge, keescook, gregkh, torvalds,
	scott.branden, weiyongjun1, nayna, ebiggers, ardb, nramas,
	lszubowi, linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

On Tue, Jul 06, 2021 at 10:43:51PM -0400, Eric Snowberg wrote:
> This is a follow up to the "Add additional MOK vars" [1] series I 
> previously sent.  This series incorporates the feedback given 
> both publicly on the mailing list and privately from Mimi. This 
> series just focuses on getting end-user keys into the kernel trust 
> boundary.

WTF is MOK?

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (12 preceding siblings ...)
  2021-07-07  6:46 ` [PATCH RFC 00/12] Enroll kernel keys thru MOK Christoph Hellwig
@ 2021-07-07 12:39 ` Mimi Zohar
  2021-07-07 16:28   ` Eric Snowberg
  13 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2021-07-07 12:39 UTC (permalink / raw)
  To: Eric Snowberg, keyrings, linux-integrity, dhowells, dwmw2,
	herbert, davem, jarkko, jmorris, serge
  Cc: keescook, gregkh, torvalds, scott.branden, weiyongjun1, nayna,
	ebiggers, ardb, nramas, lszubowi, linux-kernel, linux-crypto,
	linux-security-module, James.Bottomley, pjones, glin,
	konrad.wilk

On Tue, 2021-07-06 at 22:43 -0400, Eric Snowberg wrote:
> This is a follow up to the "Add additional MOK vars" [1] series I 
> previously sent.  This series incorporates the feedback given 
> both publicly on the mailing list and privately from Mimi. This 
> series just focuses on getting end-user keys into the kernel trust 
> boundary.
> 
> Currently, pre-boot keys are not trusted within the Linux boundary [2].
> Pre-boot keys include UEFI Secure Boot DB keys and MOKList keys. These
> keys are loaded into the platform keyring and can only be used for kexec.
> If an end-user wants to use their own key within the Linux trust
> boundary, they must either compile it into the kernel themselves or use
> the insert-sys-cert script. Both options present a problem. Many
> end-users do not want to compile their own kernels. With the
> insert-sys-cert option, there are missing upstream changes [3].  Also,
> with the insert-sys-cert option, the end-user must re-sign their kernel
> again with their own key, and then insert that key into the MOK db.
> Another problem with insert-sys-cert is that only a single key can be
> inserted into a compressed kernel.
> 
> Having the ability to insert a key into the Linux trust boundary opens
> up various possibilities.  The end-user can use a pre-built kernel and
> sign their own kernel modules.  It also opens up the ability for an
> end-user to more easily use digital signature based IMA-appraisal.  To
> get a key into the ima keyring, it must be signed by a key within the
> Linux trust boundary.
> 
> Downstream Linux distros try to have a single signed kernel for each
> architecture.  Each end-user may use this kernel in entirely different
> ways.  Some downstream kernels have chosen to always trust platform keys
> within the Linux trust boundary for kernel module signing.  These
> kernels have no way of using digital signature base IMA appraisal. 
> 
> This series adds a new MOK variable to shim.  This variable allows the
> end-user to decide if they want to trust keys enrolled in the MOK within
> the Linux trust boundary.  By default, nothing changes; MOK keys are
> not trusted within the Linux kernel.  They are only trusted after the 
> end-user makes the decision themselves.  The end-user would set this
> through mokutil using a new --trust-mok option [4]. This would work
> similar to how the kernel uses MOK variable to enable/disable signature
> validation as well as use/ignore the db.
> 
> When shim boots, it mirrors the new MokTML Boot Services variable to a new
> MokListTrustedRT Runtime Services variable and extends PCR14. 
> MokListTrustedRT is written without EFI_VARIABLE_NON_VOLATILE set,
> preventing an end-user from setting it after booting and doing a kexec.
> 
> When the kernel boots, if MokListTrustedRT is set and
> EFI_VARIABLE_NON_VOLATILE is not set, the MokListRT is loaded into the
> secondary trusted keyring instead of the platform keyring. Mimi has
> suggested that only CA keys or keys that can be vouched for by other
> kernel keys be loaded. All other certs will load into the platform
> keyring instead.

Loading MOK CA keys onto the "secondary" keyring would need to be an
exception.   Once CA keys are loaded onto the "secondary" keyring,  any
certificates signed by those CA keys may be loaded normally, without
needing an exception, onto the "secondary" keyring.  The kernel MAY
load those keys onto the "secondary" keyring, but really doesn't need
to be involved.

Loading ALL of the MOK db keys onto either the "secondary" or
"platform" keyrings makes the code a lot more complicated.  Is it
really necessary?

thanks,

Mimi

> 
> This is done by introducing a new .mok keyring.  This keyring is only
> used during boot.  After booting it is destroyed and not visible to the
> end-user after booting completes.  This keyring contains a new keyring
> permission that only allows CA keys to be loaded. If the permission
> fails, the key is later loaded into the platform keyring.  After keys
> are added into the .mok keyring, they are moved into the secondary
> trusted keyring.
> 
> Secure Boot keys will never be trusted.  They will always be loaded
> into the platform keyring.  If an end-user wanted to trust one, they
> would need to enroll it into the MOK.
> 
> I have included links to both the mokutil [3] and shim [4] changes I
> have made to support this new functionality.
> 
> Thank you and looking forward to hearing your reviews.
> 
> [1] https://lore.kernel.org/linux-integrity/20210517225714.498032-1-eric.snowberg@oracle.com/
> [2] https://lore.kernel.org/lkml/1556221605.24945.3.camel@HansenPartnership.com/
> [3] https://lore.kernel.org/patchwork/cover/902768/
> [4] https://github.com/esnowberg/mokutil/tree/0.3.0-mokvars-v2
> [5] https://github.com/esnowberg/shim/tree/mokvars-v2
> 
> Eric Snowberg (12):
>   KEYS: Add KEY_ALLOC_BYPASS_RESTRICTION option to key_move
>   KEYS: Allow unrestricted keys to be moved to the secondary keyring
>   KEYS: CA link restriction
>   integrity: add integrity_destroy_keyring
>   integrity: Introduce mok keyring
>   integrity: Trust mok keys if MokListTrustedRT found
>   integrity: add add_to_mok_keyring
>   integrity: restrict INTEGRITY_KEYRING_MOK to
>     restrict_link_by_secondary_trusted_or_ca
>   integrity: accessor function to get trust_moklist
>   integrity: add new keyring handler
>   integrity: move keys from the mok keyring into the secondary keyring
>   integrity: Suppress error message for keys added to the mok keyring
> 
>  certs/system_keyring.c                        | 43 +++++++++
>  crypto/asymmetric_keys/restrict.c             | 60 +++++++++++++
>  include/crypto/public_key.h                   |  5 ++
>  include/keys/system_keyring.h                 | 21 +++++
>  security/integrity/Makefile                   |  3 +-
>  security/integrity/digsig.c                   | 26 +++++-
>  security/integrity/integrity.h                | 21 ++++-
>  .../platform_certs/keyring_handler.c          | 17 +++-
>  .../platform_certs/keyring_handler.h          |  5 ++
>  security/integrity/platform_certs/load_uefi.c |  5 +-
>  .../integrity/platform_certs/mok_keyring.c    | 90 +++++++++++++++++++
>  security/keys/keyring.c                       | 10 ++-
>  12 files changed, 294 insertions(+), 12 deletions(-)
>  create mode 100644 security/integrity/platform_certs/mok_keyring.c
> 
> 
> base-commit: 13311e74253fe64329390df80bed3f07314ddd61



^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-07  6:46 ` [PATCH RFC 00/12] Enroll kernel keys thru MOK Christoph Hellwig
@ 2021-07-07 16:23   ` Eric Snowberg
  2021-07-07 16:28     ` Christoph Hellwig
  0 siblings, 1 reply; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07 16:23 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: keyrings, linux-integrity, Mimi Zohar, David Howells,
	David Woodhouse, herbert, davem, jarkko, jmorris, serge,
	keescook, gregkh, torvalds, scott.branden, weiyongjun1, nayna,
	ebiggers, ardb, nramas, lszubowi, linux-kernel, linux-crypto,
	linux-security-module, James.Bottomley, pjones, glin,
	konrad.wilk


> On Jul 7, 2021, at 12:46 AM, Christoph Hellwig <hch@infradead.org> wrote:
> 
> On Tue, Jul 06, 2021 at 10:43:51PM -0400, Eric Snowberg wrote:
>> This is a follow up to the "Add additional MOK vars" [1] series I 
>> previously sent.  This series incorporates the feedback given 
>> both publicly on the mailing list and privately from Mimi. This 
>> series just focuses on getting end-user keys into the kernel trust 
>> boundary.
> 
> WTF is MOK?

MOK stands for Machine Owner Key.   The MOK facility can be used to 
import keys that you use to sign your own development kernel build, 
so that it is able to boot with UEFI Secure Boot enabled. Many Linux 
distributions have implemented UEFI Secure Boot using these keys 
as well as the ones Secure Boot provides.  It allows the end-user 
a choice, instead of locking them into only being able to use keys 
their hardware manufacture provided, or forcing them to enroll keys 
through their BIOS.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-07 12:39 ` Mimi Zohar
@ 2021-07-07 16:28   ` Eric Snowberg
  2021-07-07 17:00     ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07 16:28 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
	Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
	serge, keescook, gregkh, torvalds, scott.branden, weiyongjun1,
	nayna, ebiggers, ardb, nramas, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, James.Bottomley, pjones,
	glin, konrad.wilk


> On Jul 7, 2021, at 6:39 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Tue, 2021-07-06 at 22:43 -0400, Eric Snowberg wrote:
>> This is a follow up to the "Add additional MOK vars" [1] series I 
>> previously sent.  This series incorporates the feedback given 
>> both publicly on the mailing list and privately from Mimi. This 
>> series just focuses on getting end-user keys into the kernel trust 
>> boundary.
>> 
>> Currently, pre-boot keys are not trusted within the Linux boundary [2].
>> Pre-boot keys include UEFI Secure Boot DB keys and MOKList keys. These
>> keys are loaded into the platform keyring and can only be used for kexec.
>> If an end-user wants to use their own key within the Linux trust
>> boundary, they must either compile it into the kernel themselves or use
>> the insert-sys-cert script. Both options present a problem. Many
>> end-users do not want to compile their own kernels. With the
>> insert-sys-cert option, there are missing upstream changes [3].  Also,
>> with the insert-sys-cert option, the end-user must re-sign their kernel
>> again with their own key, and then insert that key into the MOK db.
>> Another problem with insert-sys-cert is that only a single key can be
>> inserted into a compressed kernel.
>> 
>> Having the ability to insert a key into the Linux trust boundary opens
>> up various possibilities.  The end-user can use a pre-built kernel and
>> sign their own kernel modules.  It also opens up the ability for an
>> end-user to more easily use digital signature based IMA-appraisal.  To
>> get a key into the ima keyring, it must be signed by a key within the
>> Linux trust boundary.
>> 
>> Downstream Linux distros try to have a single signed kernel for each
>> architecture.  Each end-user may use this kernel in entirely different
>> ways.  Some downstream kernels have chosen to always trust platform keys
>> within the Linux trust boundary for kernel module signing.  These
>> kernels have no way of using digital signature base IMA appraisal. 
>> 
>> This series adds a new MOK variable to shim.  This variable allows the
>> end-user to decide if they want to trust keys enrolled in the MOK within
>> the Linux trust boundary.  By default, nothing changes; MOK keys are
>> not trusted within the Linux kernel.  They are only trusted after the 
>> end-user makes the decision themselves.  The end-user would set this
>> through mokutil using a new --trust-mok option [4]. This would work
>> similar to how the kernel uses MOK variable to enable/disable signature
>> validation as well as use/ignore the db.
>> 
>> When shim boots, it mirrors the new MokTML Boot Services variable to a new
>> MokListTrustedRT Runtime Services variable and extends PCR14. 
>> MokListTrustedRT is written without EFI_VARIABLE_NON_VOLATILE set,
>> preventing an end-user from setting it after booting and doing a kexec.
>> 
>> When the kernel boots, if MokListTrustedRT is set and
>> EFI_VARIABLE_NON_VOLATILE is not set, the MokListRT is loaded into the
>> secondary trusted keyring instead of the platform keyring. Mimi has
>> suggested that only CA keys or keys that can be vouched for by other
>> kernel keys be loaded. All other certs will load into the platform
>> keyring instead.
> 
> Loading MOK CA keys onto the "secondary" keyring would need to be an
> exception.   Once CA keys are loaded onto the "secondary" keyring,  any
> certificates signed by those CA keys may be loaded normally, without
> needing an exception, onto the "secondary" keyring.  The kernel MAY
> load those keys onto the "secondary" keyring, but really doesn't need
> to be involved.
> 
> Loading ALL of the MOK db keys onto either the "secondary" or
> "platform" keyrings makes the code a lot more complicated.  Is it
> really necessary?

Today all keys are loaded into the platform keyring. For kexec_file_load, 
the platform and secondary keys are trusted the same.  If this series were 
not to load them all into either keyring, it would be a kexec_file_load 
regression, since keys that previously loaded into the platform keyring 
could be missing.  The complexity arises from the CA key restriction.  
If that requirement was removed, this series would be much smaller.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-07 16:23   ` Eric Snowberg
@ 2021-07-07 16:28     ` Christoph Hellwig
  2021-07-07 16:45       ` Eric Snowberg
  0 siblings, 1 reply; 30+ messages in thread
From: Christoph Hellwig @ 2021-07-07 16:28 UTC (permalink / raw)
  To: Eric Snowberg
  Cc: Christoph Hellwig, keyrings, linux-integrity, Mimi Zohar,
	David Howells, David Woodhouse, herbert, davem, jarkko, jmorris,
	serge, keescook, gregkh, torvalds, scott.branden, weiyongjun1,
	nayna, ebiggers, ardb, nramas, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, James.Bottomley, pjones,
	glin, konrad.wilk

On Wed, Jul 07, 2021 at 10:23:04AM -0600, Eric Snowberg wrote:
> 
> > On Jul 7, 2021, at 12:46 AM, Christoph Hellwig <hch@infradead.org> wrote:
> > 
> > On Tue, Jul 06, 2021 at 10:43:51PM -0400, Eric Snowberg wrote:
> >> This is a follow up to the "Add additional MOK vars" [1] series I 
> >> previously sent.  This series incorporates the feedback given 
> >> both publicly on the mailing list and privately from Mimi. This 
> >> series just focuses on getting end-user keys into the kernel trust 
> >> boundary.
> > 
> > WTF is MOK?
> 
> MOK stands for Machine Owner Key.   The MOK facility can be used to 
> import keys that you use to sign your own development kernel build, 
> so that it is able to boot with UEFI Secure Boot enabled. Many Linux 
> distributions have implemented UEFI Secure Boot using these keys 
> as well as the ones Secure Boot provides.  It allows the end-user 
> a choice, instead of locking them into only being able to use keys 
> their hardware manufacture provided, or forcing them to enroll keys 
> through their BIOS.

Please spell this out in your cover letters and commit logs.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-07 16:28     ` Christoph Hellwig
@ 2021-07-07 16:45       ` Eric Snowberg
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07 16:45 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: keyrings, linux-integrity, Mimi Zohar, David Howells,
	David Woodhouse, herbert, davem, jarkko, jmorris, serge,
	keescook, gregkh, torvalds, scott.branden, weiyongjun1, nayna,
	ebiggers, ardb, nramas, lszubowi, linux-kernel, linux-crypto,
	linux-security-module, James.Bottomley, pjones, glin,
	konrad.wilk


> On Jul 7, 2021, at 10:28 AM, Christoph Hellwig <hch@infradead.org> wrote:
> 
> On Wed, Jul 07, 2021 at 10:23:04AM -0600, Eric Snowberg wrote:
>> 
>>> On Jul 7, 2021, at 12:46 AM, Christoph Hellwig <hch@infradead.org> wrote:
>>> 
>>> On Tue, Jul 06, 2021 at 10:43:51PM -0400, Eric Snowberg wrote:
>>>> This is a follow up to the "Add additional MOK vars" [1] series I 
>>>> previously sent.  This series incorporates the feedback given 
>>>> both publicly on the mailing list and privately from Mimi. This 
>>>> series just focuses on getting end-user keys into the kernel trust 
>>>> boundary.
>>> 
>>> WTF is MOK?
>> 
>> MOK stands for Machine Owner Key.   The MOK facility can be used to 
>> import keys that you use to sign your own development kernel build, 
>> so that it is able to boot with UEFI Secure Boot enabled. Many Linux 
>> distributions have implemented UEFI Secure Boot using these keys 
>> as well as the ones Secure Boot provides.  It allows the end-user 
>> a choice, instead of locking them into only being able to use keys 
>> their hardware manufacture provided, or forcing them to enroll keys 
>> through their BIOS.
> 
> Please spell this out in your cover letters and commit logs.

I will add it in the future, thanks.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-07 16:28   ` Eric Snowberg
@ 2021-07-07 17:00     ` Mimi Zohar
  2021-07-07 22:10       ` Eric Snowberg
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2021-07-07 17:00 UTC (permalink / raw)
  To: Eric Snowberg
  Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
	Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
	serge, keescook, gregkh, torvalds, scott.branden, weiyongjun1,
	nayna, ebiggers, ardb, nramas, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, James.Bottomley, pjones,
	glin, konrad.wilk

On Wed, 2021-07-07 at 10:28 -0600, Eric Snowberg wrote:
> > On Jul 7, 2021, at 6:39 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Tue, 2021-07-06 at 22:43 -0400, Eric Snowberg wrote:
> >> This is a follow up to the "Add additional MOK vars" [1] series I 
> >> previously sent.  This series incorporates the feedback given 
> >> both publicly on the mailing list and privately from Mimi. This 
> >> series just focuses on getting end-user keys into the kernel trust 
> >> boundary.
> >> 
> >> Currently, pre-boot keys are not trusted within the Linux boundary [2].
> >> Pre-boot keys include UEFI Secure Boot DB keys and MOKList keys. These
> >> keys are loaded into the platform keyring and can only be used for kexec.
> >> If an end-user wants to use their own key within the Linux trust
> >> boundary, they must either compile it into the kernel themselves or use
> >> the insert-sys-cert script. Both options present a problem. Many
> >> end-users do not want to compile their own kernels. With the
> >> insert-sys-cert option, there are missing upstream changes [3].  Also,
> >> with the insert-sys-cert option, the end-user must re-sign their kernel
> >> again with their own key, and then insert that key into the MOK db.
> >> Another problem with insert-sys-cert is that only a single key can be
> >> inserted into a compressed kernel.
> >> 
> >> Having the ability to insert a key into the Linux trust boundary opens
> >> up various possibilities.  The end-user can use a pre-built kernel and
> >> sign their own kernel modules.  It also opens up the ability for an
> >> end-user to more easily use digital signature based IMA-appraisal.  To
> >> get a key into the ima keyring, it must be signed by a key within the
> >> Linux trust boundary.
> >> 
> >> Downstream Linux distros try to have a single signed kernel for each
> >> architecture.  Each end-user may use this kernel in entirely different
> >> ways.  Some downstream kernels have chosen to always trust platform keys
> >> within the Linux trust boundary for kernel module signing.  These
> >> kernels have no way of using digital signature base IMA appraisal. 
> >> 
> >> This series adds a new MOK variable to shim.  This variable allows the
> >> end-user to decide if they want to trust keys enrolled in the MOK within
> >> the Linux trust boundary.  By default, nothing changes; MOK keys are
> >> not trusted within the Linux kernel.  They are only trusted after the 
> >> end-user makes the decision themselves.  The end-user would set this
> >> through mokutil using a new --trust-mok option [4]. This would work
> >> similar to how the kernel uses MOK variable to enable/disable signature
> >> validation as well as use/ignore the db.
> >> 
> >> When shim boots, it mirrors the new MokTML Boot Services variable to a new
> >> MokListTrustedRT Runtime Services variable and extends PCR14. 
> >> MokListTrustedRT is written without EFI_VARIABLE_NON_VOLATILE set,
> >> preventing an end-user from setting it after booting and doing a kexec.
> >> 
> >> When the kernel boots, if MokListTrustedRT is set and
> >> EFI_VARIABLE_NON_VOLATILE is not set, the MokListRT is loaded into the
> >> secondary trusted keyring instead of the platform keyring. Mimi has
> >> suggested that only CA keys or keys that can be vouched for by other
> >> kernel keys be loaded. All other certs will load into the platform
> >> keyring instead.
> > 
> > Loading MOK CA keys onto the "secondary" keyring would need to be an
> > exception.   Once CA keys are loaded onto the "secondary" keyring,  any
> > certificates signed by those CA keys may be loaded normally, without
> > needing an exception, onto the "secondary" keyring.  The kernel MAY
> > load those keys onto the "secondary" keyring, but really doesn't need
> > to be involved.
> > 
> > Loading ALL of the MOK db keys onto either the "secondary" or
> > "platform" keyrings makes the code a lot more complicated.  Is it
> > really necessary?
> 
> Today all keys are loaded into the platform keyring. For kexec_file_load, 
> the platform and secondary keys are trusted the same.  If this series were 
> not to load them all into either keyring, it would be a kexec_file_load 
> regression, since keys that previously loaded into the platform keyring 
> could be missing. The complexity arises from the CA key restriction.  
> If that requirement was removed, this series would be much smaller.

To prevent the regression, allow the the existing firmware/UEFI keys to
continue to be loaded on the platform keyring, as it is currently being
done.  The new code would load just the MOK db CA keys onto the
secondary keyring, based on the new UEFI variable.  This is the only
code that would require a
"restrict_link_by_builtin_and_secondary_trusted" exemption.  The code
duplication would be minimal in comparison to the complexity being
introduced.

thanks,

Mimi


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 05/12] integrity: Introduce mok keyring
  2021-07-07  2:43 ` [PATCH RFC 05/12] integrity: Introduce mok keyring Eric Snowberg
@ 2021-07-07 19:31   ` Linus Torvalds
  2021-07-07 21:26     ` Jarkko Sakkinen
  0 siblings, 1 reply; 30+ messages in thread
From: Linus Torvalds @ 2021-07-07 19:31 UTC (permalink / raw)
  To: Eric Snowberg
  Cc: keyrings, linux-integrity, Mimi Zohar, David Howells,
	David Woodhouse, Herbert Xu, David Miller, Jarkko Sakkinen,
	James Morris James Morris, Serge E. Hallyn, Kees Cook,
	Greg Kroah-Hartman, scott.branden, Wei Yongjun, Nayna Jain,
	Eric Biggers, Ard Biesheuvel, nramas, Lenny Szubowicz,
	Linux Kernel Mailing List, Linux Crypto Mailing List, LSM List,
	James Bottomley, Peter Jones, Gary Lin, Konrad Rzeszutek Wilk

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 05/12] integrity: Introduce mok keyring
  2021-07-07 19:31   ` Linus Torvalds
@ 2021-07-07 21:26     ` Jarkko Sakkinen
  2021-07-07 22:32       ` Eric Snowberg
  0 siblings, 1 reply; 30+ messages in thread
From: Jarkko Sakkinen @ 2021-07-07 21:26 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Eric Snowberg, keyrings, linux-integrity, Mimi Zohar,
	David Howells, David Woodhouse, Herbert Xu, David Miller,
	James Morris James Morris, Serge E. Hallyn, Kees Cook,
	Greg Kroah-Hartman, scott.branden, Wei Yongjun, Nayna Jain,
	Eric Biggers, Ard Biesheuvel, nramas, Lenny Szubowicz,
	Linux Kernel Mailing List, Linux Crypto Mailing List, LSM List,
	James Bottomley, Peter Jones, Gary Lin, Konrad Rzeszutek Wilk

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-07 17:00     ` Mimi Zohar
@ 2021-07-07 22:10       ` Eric Snowberg
  2021-07-08 13:56         ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07 22:10 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
	Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
	Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk


> On Jul 7, 2021, at 11:00 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Wed, 2021-07-07 at 10:28 -0600, Eric Snowberg wrote:
>>> On Jul 7, 2021, at 6:39 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> On Tue, 2021-07-06 at 22:43 -0400, Eric Snowberg wrote:
>>>> This is a follow up to the "Add additional MOK vars" [1] series I 
>>>> previously sent.  This series incorporates the feedback given 
>>>> both publicly on the mailing list and privately from Mimi. This 
>>>> series just focuses on getting end-user keys into the kernel trust 
>>>> boundary.
>>>> 
>>>> Currently, pre-boot keys are not trusted within the Linux boundary [2].
>>>> Pre-boot keys include UEFI Secure Boot DB keys and MOKList keys. These
>>>> keys are loaded into the platform keyring and can only be used for kexec.
>>>> If an end-user wants to use their own key within the Linux trust
>>>> boundary, they must either compile it into the kernel themselves or use
>>>> the insert-sys-cert script. Both options present a problem. Many
>>>> end-users do not want to compile their own kernels. With the
>>>> insert-sys-cert option, there are missing upstream changes [3].  Also,
>>>> with the insert-sys-cert option, the end-user must re-sign their kernel
>>>> again with their own key, and then insert that key into the MOK db.
>>>> Another problem with insert-sys-cert is that only a single key can be
>>>> inserted into a compressed kernel.
>>>> 
>>>> Having the ability to insert a key into the Linux trust boundary opens
>>>> up various possibilities.  The end-user can use a pre-built kernel and
>>>> sign their own kernel modules.  It also opens up the ability for an
>>>> end-user to more easily use digital signature based IMA-appraisal.  To
>>>> get a key into the ima keyring, it must be signed by a key within the
>>>> Linux trust boundary.
>>>> 
>>>> Downstream Linux distros try to have a single signed kernel for each
>>>> architecture.  Each end-user may use this kernel in entirely different
>>>> ways.  Some downstream kernels have chosen to always trust platform keys
>>>> within the Linux trust boundary for kernel module signing.  These
>>>> kernels have no way of using digital signature base IMA appraisal. 
>>>> 
>>>> This series adds a new MOK variable to shim.  This variable allows the
>>>> end-user to decide if they want to trust keys enrolled in the MOK within
>>>> the Linux trust boundary.  By default, nothing changes; MOK keys are
>>>> not trusted within the Linux kernel.  They are only trusted after the 
>>>> end-user makes the decision themselves.  The end-user would set this
>>>> through mokutil using a new --trust-mok option [4]. This would work
>>>> similar to how the kernel uses MOK variable to enable/disable signature
>>>> validation as well as use/ignore the db.
>>>> 
>>>> When shim boots, it mirrors the new MokTML Boot Services variable to a new
>>>> MokListTrustedRT Runtime Services variable and extends PCR14. 
>>>> MokListTrustedRT is written without EFI_VARIABLE_NON_VOLATILE set,
>>>> preventing an end-user from setting it after booting and doing a kexec.
>>>> 
>>>> When the kernel boots, if MokListTrustedRT is set and
>>>> EFI_VARIABLE_NON_VOLATILE is not set, the MokListRT is loaded into the
>>>> secondary trusted keyring instead of the platform keyring. Mimi has
>>>> suggested that only CA keys or keys that can be vouched for by other
>>>> kernel keys be loaded. All other certs will load into the platform
>>>> keyring instead.
>>> 
>>> Loading MOK CA keys onto the "secondary" keyring would need to be an
>>> exception.   Once CA keys are loaded onto the "secondary" keyring,  any
>>> certificates signed by those CA keys may be loaded normally, without
>>> needing an exception, onto the "secondary" keyring.  The kernel MAY
>>> load those keys onto the "secondary" keyring, but really doesn't need
>>> to be involved.
>>> 
>>> Loading ALL of the MOK db keys onto either the "secondary" or
>>> "platform" keyrings makes the code a lot more complicated.  Is it
>>> really necessary?
>> 
>> Today all keys are loaded into the platform keyring. For kexec_file_load, 
>> the platform and secondary keys are trusted the same.  If this series were 
>> not to load them all into either keyring, it would be a kexec_file_load 
>> regression, since keys that previously loaded into the platform keyring 
>> could be missing. The complexity arises from the CA key restriction.  
>> If that requirement was removed, this series would be much smaller.
> 
> To prevent the regression, allow the the existing firmware/UEFI keys to
> continue to be loaded on the platform keyring, as it is currently being
> done.  The new code would load just the MOK db CA keys onto the
> secondary keyring, based on the new UEFI variable.  This is the only
> code that would require a
> "restrict_link_by_builtin_and_secondary_trusted" exemption.  The code
> duplication would be minimal in comparison to the complexity being
> introduced.

This series was written with the following three requirements in mind:

1. Only CA keys that were originally bound for the platform keyring 
can enter the secondary keyring.

2. No key in the UEFI Secure Boot DB, CA or not, may enter the 
secondary keyring, only MOKList keys may be trusted.

3. A new MOK variable is added to signify the user wants to trust 
MOKList keys.

Given these requirements, I started down the path I think you are 
suggesting.  However I found it to be more complex.  If we load all 
keys into the platform keyring first and later try to load only CA keys, 
we don’t have a way of knowing where the platform key came from.  
Platform keys can originate from the UEFI Secure Boot DB or the MOKList. 
This would violate the second requirement. This caused me to need to 
create a new keyring handler. [PATCH RFC 10/12] integrity: add new 
keyring handler.  

To satisfy the first requirement a new restriction is required.  This 
is contained in [PATCH RFC 03/12] KEYS: CA link restriction.

To satisfy the third requirement, we must read the new MOK var.  This 
is contained in [PATCH RFC 06/12] integrity: Trust mok keys if 
MokListTrustedRT found.

The patches above make up a majority of the new code.  

The remaining code of creating a new .mok keyring was done with code 
reuse in mind. Many of the required functions necessary to add this 
capability is already contained in integrity_ functions.  If the 
operation was done directly on the secondary keyring, similar code 
would need to be added to certs/system_keyring.c. Just like how the 
platform keyring is created within integrity code, the mok keyring 
is created in the same fashion.  When the platform keyring has 
completed initialization and loaded all its keys, the keyring is set 
into system_keyring code using set_platform_trusted_keys.  Instead of 
setting the mok keyring, I’m moving the keys directly into the secondary 
keyring, while bypassing the current restriction placed on this keyring.
Basically I'm trying to follow the same design pattern. 

If requirements #1, #2 or both (#1 and #2) could be dropped, most of 
this series would not be necessary.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 05/12] integrity: Introduce mok keyring
  2021-07-07 21:26     ` Jarkko Sakkinen
@ 2021-07-07 22:32       ` Eric Snowberg
  0 siblings, 0 replies; 30+ messages in thread
From: Eric Snowberg @ 2021-07-07 22:32 UTC (permalink / raw)
  To: Jarkko Sakkinen, Linus Torvalds
  Cc: keyrings, linux-integrity, Mimi Zohar, David Howells,
	David Woodhouse, Herbert Xu, David Miller,
	James Morris James Morris, Serge E. Hallyn, Kees Cook,
	Greg Kroah-Hartman, scott.branden, Wei Yongjun, Nayna Jain,
	Eric Biggers, Ard Biesheuvel, Lakshmi Ramasubramanian,
	Lenny Szubowicz, Linux Kernel Mailing List,
	Linux Crypto Mailing List, LSM List, James Bottomley,
	Peter Jones, Gary Lin, konrad.wilk


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


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-07 22:10       ` Eric Snowberg
@ 2021-07-08 13:56         ` Mimi Zohar
  2021-07-08 17:59           ` Eric Snowberg
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2021-07-08 13:56 UTC (permalink / raw)
  To: Eric Snowberg
  Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
	Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
	Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

On Wed, 2021-07-07 at 16:10 -0600, Eric Snowberg wrote:
> > On Jul 7, 2021, at 11:00 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Wed, 2021-07-07 at 10:28 -0600, Eric Snowberg wrote:
> >>> On Jul 7, 2021, at 6:39 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>> 
> >>> On Tue, 2021-07-06 at 22:43 -0400, Eric Snowberg wrote:
> >>>> This is a follow up to the "Add additional MOK vars" [1] series I 
> >>>> previously sent.  This series incorporates the feedback given 
> >>>> both publicly on the mailing list and privately from Mimi. This 
> >>>> series just focuses on getting end-user keys into the kernel trust 
> >>>> boundary.
> >>>> 
> >>>> Currently, pre-boot keys are not trusted within the Linux boundary [2].
> >>>> Pre-boot keys include UEFI Secure Boot DB keys and MOKList keys. These
> >>>> keys are loaded into the platform keyring and can only be used for kexec.
> >>>> If an end-user wants to use their own key within the Linux trust
> >>>> boundary, they must either compile it into the kernel themselves or use
> >>>> the insert-sys-cert script. Both options present a problem. Many
> >>>> end-users do not want to compile their own kernels. With the
> >>>> insert-sys-cert option, there are missing upstream changes [3].  Also,
> >>>> with the insert-sys-cert option, the end-user must re-sign their kernel
> >>>> again with their own key, and then insert that key into the MOK db.
> >>>> Another problem with insert-sys-cert is that only a single key can be
> >>>> inserted into a compressed kernel.
> >>>> 
> >>>> Having the ability to insert a key into the Linux trust boundary opens
> >>>> up various possibilities.  The end-user can use a pre-built kernel and
> >>>> sign their own kernel modules.  It also opens up the ability for an
> >>>> end-user to more easily use digital signature based IMA-appraisal.  To
> >>>> get a key into the ima keyring, it must be signed by a key within the
> >>>> Linux trust boundary.
> >>>> 
> >>>> Downstream Linux distros try to have a single signed kernel for each
> >>>> architecture.  Each end-user may use this kernel in entirely different
> >>>> ways.  Some downstream kernels have chosen to always trust platform keys
> >>>> within the Linux trust boundary for kernel module signing.  These
> >>>> kernels have no way of using digital signature base IMA appraisal. 
> >>>> 
> >>>> This series adds a new MOK variable to shim.  This variable allows the
> >>>> end-user to decide if they want to trust keys enrolled in the MOK within
> >>>> the Linux trust boundary.  By default, nothing changes; MOK keys are
> >>>> not trusted within the Linux kernel.  They are only trusted after the 
> >>>> end-user makes the decision themselves.  The end-user would set this
> >>>> through mokutil using a new --trust-mok option [4]. This would work
> >>>> similar to how the kernel uses MOK variable to enable/disable signature
> >>>> validation as well as use/ignore the db.
> >>>> 
> >>>> When shim boots, it mirrors the new MokTML Boot Services variable to a new
> >>>> MokListTrustedRT Runtime Services variable and extends PCR14. 
> >>>> MokListTrustedRT is written without EFI_VARIABLE_NON_VOLATILE set,
> >>>> preventing an end-user from setting it after booting and doing a kexec.
> >>>> 
> >>>> When the kernel boots, if MokListTrustedRT is set and
> >>>> EFI_VARIABLE_NON_VOLATILE is not set, the MokListRT is loaded into the
> >>>> secondary trusted keyring instead of the platform keyring. Mimi has
> >>>> suggested that only CA keys or keys that can be vouched for by other
> >>>> kernel keys be loaded. All other certs will load into the platform
> >>>> keyring instead.
> >>> 
> >>> Loading MOK CA keys onto the "secondary" keyring would need to be an
> >>> exception.   Once CA keys are loaded onto the "secondary" keyring,  any
> >>> certificates signed by those CA keys may be loaded normally, without
> >>> needing an exception, onto the "secondary" keyring.  The kernel MAY
> >>> load those keys onto the "secondary" keyring, but really doesn't need
> >>> to be involved.
> >>> 
> >>> Loading ALL of the MOK db keys onto either the "secondary" or
> >>> "platform" keyrings makes the code a lot more complicated.  Is it
> >>> really necessary?
> >> 
> >> Today all keys are loaded into the platform keyring. For kexec_file_load, 
> >> the platform and secondary keys are trusted the same.  If this series were 
> >> not to load them all into either keyring, it would be a kexec_file_load 
> >> regression, since keys that previously loaded into the platform keyring 
> >> could be missing. The complexity arises from the CA key restriction.  
> >> If that requirement was removed, this series would be much smaller.
> > 
> > To prevent the regression, allow the the existing firmware/UEFI keys to
> > continue to be loaded on the platform keyring, as it is currently being
> > done.  The new code would load just the MOK db CA keys onto the
> > secondary keyring, based on the new UEFI variable.  This is the only
> > code that would require a
> > "restrict_link_by_builtin_and_secondary_trusted" exemption.  The code
> > duplication would be minimal in comparison to the complexity being
> > introduced.
> 
> This series was written with the following three requirements in mind:
> 
> 1. Only CA keys that were originally bound for the platform keyring 
> can enter the secondary keyring.
> 
> 2. No key in the UEFI Secure Boot DB, CA or not, may enter the 
> secondary keyring, only MOKList keys may be trusted.
> 
> 3. A new MOK variable is added to signify the user wants to trust 
> MOKList keys.

Sounds good!

> 
> Given these requirements, I started down the path I think you are 
> suggesting.  However I found it to be more complex.  If we load all 
> keys into the platform keyring first and later try to load only CA keys, 
> we don’t have a way of knowing where the platform key came from.  
> Platform keys can originate from the UEFI Secure Boot DB or the MOKList. 
> This would violate the second requirement. This caused me to need to 
> create a new keyring handler. [PATCH RFC 10/12] integrity: add new 
> keyring handler.

To prevent the regression you mentioned, I was suggesting reading the
MOK DB twice.  One time loading all the keys onto the platform keyring.
The other time loading only the CA keys onto the secondary keyring.

> 
> To satisfy the first requirement a new restriction is required.  This 
> is contained in [PATCH RFC 03/12] KEYS: CA link restriction.
> 
> To satisfy the third requirement, we must read the new MOK var.  This 
> is contained in [PATCH RFC 06/12] integrity: Trust mok keys if 
> MokListTrustedRT found.
> 
> The patches above make up a majority of the new code.  
> 
> The remaining code of creating a new .mok keyring was done with code 
> reuse in mind. Many of the required functions necessary to add this 
> capability is already contained in integrity_ functions.  If the 
> operation was done directly on the secondary keyring, similar code 
> would need to be added to certs/system_keyring.c. Just like how the 
> platform keyring is created within integrity code, the mok keyring 
> is created in the same fashion.  When the platform keyring has 
> completed initialization and loaded all its keys, the keyring is set 
> into system_keyring code using set_platform_trusted_keys.  Instead of 
> setting the mok keyring, I’m moving the keys directly into the secondary 
> keyring, while bypassing the current restriction placed on this keyring.
> Basically I'm trying to follow the same design pattern. 
> 
> If requirements #1, #2 or both (#1 and #2) could be dropped, most of 
> this series would not be necessary.

But without these requirements, the source of trust is unclear.

Is there a reason why the MOK keyring is temporary?   Asumming a
function similar to "restrict_link_by_builtin_and_secondary_trusted" is
defined to include the MOK keyring, the CA keys in the MOK db would be
loaded onto the MOK keyring, the other keys that meet the secondary
keyring restriction would be loaded directly onto the secondary
keyring[1], and as you currently have, the remaining keys onto the
platform keyring.

This eliminates the exemption needed for loading keys onto the
secondary keyring.  The MOK keyring, containing just CA keys, becomes a
new trust source.

thanks,

Mimi

[1] Refer to the comment in
restrict_link_by_builtin_and_secondary_trusted() on linking the builtin
keyring to the secondary keyring.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-08 13:56         ` Mimi Zohar
@ 2021-07-08 17:59           ` Eric Snowberg
  2021-07-08 19:31             ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Eric Snowberg @ 2021-07-08 17:59 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
	Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
	Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk


> On Jul 8, 2021, at 7:56 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Wed, 2021-07-07 at 16:10 -0600, Eric Snowberg wrote:
>>> On Jul 7, 2021, at 11:00 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> On Wed, 2021-07-07 at 10:28 -0600, Eric Snowberg wrote:
>>>>> On Jul 7, 2021, at 6:39 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>> 
>>>>> On Tue, 2021-07-06 at 22:43 -0400, Eric Snowberg wrote:
>>>>>> This is a follow up to the "Add additional MOK vars" [1] series I 
>>>>>> previously sent.  This series incorporates the feedback given 
>>>>>> both publicly on the mailing list and privately from Mimi. This 
>>>>>> series just focuses on getting end-user keys into the kernel trust 
>>>>>> boundary.
>>>>>> 
>>>>>> Currently, pre-boot keys are not trusted within the Linux boundary [2].
>>>>>> Pre-boot keys include UEFI Secure Boot DB keys and MOKList keys. These
>>>>>> keys are loaded into the platform keyring and can only be used for kexec.
>>>>>> If an end-user wants to use their own key within the Linux trust
>>>>>> boundary, they must either compile it into the kernel themselves or use
>>>>>> the insert-sys-cert script. Both options present a problem. Many
>>>>>> end-users do not want to compile their own kernels. With the
>>>>>> insert-sys-cert option, there are missing upstream changes [3].  Also,
>>>>>> with the insert-sys-cert option, the end-user must re-sign their kernel
>>>>>> again with their own key, and then insert that key into the MOK db.
>>>>>> Another problem with insert-sys-cert is that only a single key can be
>>>>>> inserted into a compressed kernel.
>>>>>> 
>>>>>> Having the ability to insert a key into the Linux trust boundary opens
>>>>>> up various possibilities.  The end-user can use a pre-built kernel and
>>>>>> sign their own kernel modules.  It also opens up the ability for an
>>>>>> end-user to more easily use digital signature based IMA-appraisal.  To
>>>>>> get a key into the ima keyring, it must be signed by a key within the
>>>>>> Linux trust boundary.
>>>>>> 
>>>>>> Downstream Linux distros try to have a single signed kernel for each
>>>>>> architecture.  Each end-user may use this kernel in entirely different
>>>>>> ways.  Some downstream kernels have chosen to always trust platform keys
>>>>>> within the Linux trust boundary for kernel module signing.  These
>>>>>> kernels have no way of using digital signature base IMA appraisal. 
>>>>>> 
>>>>>> This series adds a new MOK variable to shim.  This variable allows the
>>>>>> end-user to decide if they want to trust keys enrolled in the MOK within
>>>>>> the Linux trust boundary.  By default, nothing changes; MOK keys are
>>>>>> not trusted within the Linux kernel.  They are only trusted after the 
>>>>>> end-user makes the decision themselves.  The end-user would set this
>>>>>> through mokutil using a new --trust-mok option [4]. This would work
>>>>>> similar to how the kernel uses MOK variable to enable/disable signature
>>>>>> validation as well as use/ignore the db.
>>>>>> 
>>>>>> When shim boots, it mirrors the new MokTML Boot Services variable to a new
>>>>>> MokListTrustedRT Runtime Services variable and extends PCR14. 
>>>>>> MokListTrustedRT is written without EFI_VARIABLE_NON_VOLATILE set,
>>>>>> preventing an end-user from setting it after booting and doing a kexec.
>>>>>> 
>>>>>> When the kernel boots, if MokListTrustedRT is set and
>>>>>> EFI_VARIABLE_NON_VOLATILE is not set, the MokListRT is loaded into the
>>>>>> secondary trusted keyring instead of the platform keyring. Mimi has
>>>>>> suggested that only CA keys or keys that can be vouched for by other
>>>>>> kernel keys be loaded. All other certs will load into the platform
>>>>>> keyring instead.
>>>>> 
>>>>> Loading MOK CA keys onto the "secondary" keyring would need to be an
>>>>> exception.   Once CA keys are loaded onto the "secondary" keyring,  any
>>>>> certificates signed by those CA keys may be loaded normally, without
>>>>> needing an exception, onto the "secondary" keyring.  The kernel MAY
>>>>> load those keys onto the "secondary" keyring, but really doesn't need
>>>>> to be involved.
>>>>> 
>>>>> Loading ALL of the MOK db keys onto either the "secondary" or
>>>>> "platform" keyrings makes the code a lot more complicated.  Is it
>>>>> really necessary?
>>>> 
>>>> Today all keys are loaded into the platform keyring. For kexec_file_load, 
>>>> the platform and secondary keys are trusted the same.  If this series were 
>>>> not to load them all into either keyring, it would be a kexec_file_load 
>>>> regression, since keys that previously loaded into the platform keyring 
>>>> could be missing. The complexity arises from the CA key restriction.  
>>>> If that requirement was removed, this series would be much smaller.
>>> 
>>> To prevent the regression, allow the the existing firmware/UEFI keys to
>>> continue to be loaded on the platform keyring, as it is currently being
>>> done.  The new code would load just the MOK db CA keys onto the
>>> secondary keyring, based on the new UEFI variable.  This is the only
>>> code that would require a
>>> "restrict_link_by_builtin_and_secondary_trusted" exemption.  The code
>>> duplication would be minimal in comparison to the complexity being
>>> introduced.
>> 
>> This series was written with the following three requirements in mind:
>> 
>> 1. Only CA keys that were originally bound for the platform keyring 
>> can enter the secondary keyring.
>> 
>> 2. No key in the UEFI Secure Boot DB, CA or not, may enter the 
>> secondary keyring, only MOKList keys may be trusted.
>> 
>> 3. A new MOK variable is added to signify the user wants to trust 
>> MOKList keys.
> 
> Sounds good!
> 
>> 
>> Given these requirements, I started down the path I think you are 
>> suggesting.  However I found it to be more complex.  If we load all 
>> keys into the platform keyring first and later try to load only CA keys, 
>> we don’t have a way of knowing where the platform key came from.  
>> Platform keys can originate from the UEFI Secure Boot DB or the MOKList. 
>> This would violate the second requirement. This caused me to need to 
>> create a new keyring handler. [PATCH RFC 10/12] integrity: add new 
>> keyring handler.
> 
> To prevent the regression you mentioned, I was suggesting reading the
> MOK DB twice.  One time loading all the keys onto the platform keyring.
> The other time loading only the CA keys onto the secondary keyring.
> 
>> 
>> To satisfy the first requirement a new restriction is required.  This 
>> is contained in [PATCH RFC 03/12] KEYS: CA link restriction.
>> 
>> To satisfy the third requirement, we must read the new MOK var.  This 
>> is contained in [PATCH RFC 06/12] integrity: Trust mok keys if 
>> MokListTrustedRT found.
>> 
>> The patches above make up a majority of the new code.  
>> 
>> The remaining code of creating a new .mok keyring was done with code 
>> reuse in mind. Many of the required functions necessary to add this 
>> capability is already contained in integrity_ functions.  If the 
>> operation was done directly on the secondary keyring, similar code 
>> would need to be added to certs/system_keyring.c. Just like how the 
>> platform keyring is created within integrity code, the mok keyring 
>> is created in the same fashion.  When the platform keyring has 
>> completed initialization and loaded all its keys, the keyring is set 
>> into system_keyring code using set_platform_trusted_keys.  Instead of 
>> setting the mok keyring, I’m moving the keys directly into the secondary 
>> keyring, while bypassing the current restriction placed on this keyring.
>> Basically I'm trying to follow the same design pattern. 
>> 
>> If requirements #1, #2 or both (#1 and #2) could be dropped, most of 
>> this series would not be necessary.
> 
> But without these requirements, the source of trust is unclear.
> 
> Is there a reason why the MOK keyring is temporary?  

I suppose it doesn't have to be temporary.  I was trying not to introduce
another keyring within system_keyring code.

>  Asumming a
> function similar to "restrict_link_by_builtin_and_secondary_trusted" is
> defined to include the MOK keyring, the CA keys in the MOK db would be
> loaded onto the MOK keyring, the other keys that meet the secondary
> keyring restriction would be loaded directly onto the secondary
> keyring[1], and as you currently have, the remaining keys onto the
> platform keyring.
> 
> This eliminates the exemption needed for loading keys onto the
> secondary keyring.  The MOK keyring, containing just CA keys, becomes a
> new trust source.

I just want to make sure I understand. If we kept the .mok keyring around, 
we would store it into the system_keyring code, just like the platform 
keyring is stored.  This would allow the move exemption code to be removed.
If the mok keyring is a new trust source, whenever the secondary keyring 
is referenced in verify_ code, the mok keyring will be checked too.  If 
I have this right, let me know and I’ll work on a v2.  Thanks.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-08 17:59           ` Eric Snowberg
@ 2021-07-08 19:31             ` Mimi Zohar
  2021-07-08 23:17               ` Eric Snowberg
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2021-07-08 19:31 UTC (permalink / raw)
  To: Eric Snowberg
  Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
	Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
	Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

Hi Eric,

On Thu, 2021-07-08 at 11:59 -0600, Eric Snowberg wrote:
> 
> >  Asumming a
> > function similar to "restrict_link_by_builtin_and_secondary_trusted" is
> > defined to include the MOK keyring, the CA keys in the MOK db would be
> > loaded onto the MOK keyring, the other keys that meet the secondary
> > keyring restriction would be loaded directly onto the secondary
> > keyring[1], and as you currently have, the remaining keys onto the
> > platform keyring.
> > 
> > This eliminates the exemption needed for loading keys onto the
> > secondary keyring.  The MOK keyring, containing just CA keys, becomes a
> > new trust source.
> 
> I just want to make sure I understand. If we kept the .mok keyring around, 
> we would store it into the system_keyring code, just like the platform 
> keyring is stored.  This would allow the move exemption code to be removed.
> If the mok keyring is a new trust source, whenever the secondary keyring 
> is referenced in verify_ code, the mok keyring will be checked too.  If 
> I have this right, let me know and I’ll work on a v2.  Thanks.

All the firmware keys are loaded onto the "platform" keyring, without
any restriction.  Your reference point should be the "builtin" and
"secondary" keyrings, not the "platform" keyring.

Changes:
- defining a new keyring restriction which only allows CA keys to be
loaded onto the MOK keyring.
- defining a new keyring restriction something along the lines of
"restrict_link_by_builtin_mok_and_secondary_trusted()".

In the case of "restrict_link_by_builtin_and_secondary_trusted()", it's
based on a build time option.  In the case of MOK, it might be both a
build time and runtime firmware variable option.  There are quite a few
permutations that will somehow need to be addressed:  secondary keyring
not defined, but MOK keyring defined, and the reverse.

Once all the CA keys in the MOK db are loaded onto the MOK keyring,
there will be no need for moving keys to the secondary keyring.  The
secondary keyring restriction will just work.  The main question is
whether there will need to be two passes.   One pass to first load all
the CA keys onto the MOK keyring.  A second pass to load the keys onto
the secondary keyring, based on the keyring restriction, and the
remaining ones onto the "platform" keyring to avoid the regression.

[Once the CA keys are loaded onto the MOK keyring, userspace will be
able to load certificates, signed by a key on the MOK keyring, onto the
secondary keyring.]

thanks,

Mimi


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-08 19:31             ` Mimi Zohar
@ 2021-07-08 23:17               ` Eric Snowberg
  2021-07-09  1:10                 ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Eric Snowberg @ 2021-07-08 23:17 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
	Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
	Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk


> On Jul 8, 2021, at 1:31 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Thu, 2021-07-08 at 11:59 -0600, Eric Snowberg wrote:
>> 
>>> Asumming a
>>> function similar to "restrict_link_by_builtin_and_secondary_trusted" is
>>> defined to include the MOK keyring, the CA keys in the MOK db would be
>>> loaded onto the MOK keyring, the other keys that meet the secondary
>>> keyring restriction would be loaded directly onto the secondary
>>> keyring[1], and as you currently have, the remaining keys onto the
>>> platform keyring.
>>> 
>>> This eliminates the exemption needed for loading keys onto the
>>> secondary keyring.  The MOK keyring, containing just CA keys, becomes a
>>> new trust source.
>> 
>> I just want to make sure I understand. If we kept the .mok keyring around, 
>> we would store it into the system_keyring code, just like the platform 
>> keyring is stored.  This would allow the move exemption code to be removed.
>> If the mok keyring is a new trust source, whenever the secondary keyring 
>> is referenced in verify_ code, the mok keyring will be checked too.  If 
>> I have this right, let me know and I’ll work on a v2.  Thanks.
> 
> All the firmware keys are loaded onto the "platform" keyring, without
> any restriction.  Your reference point should be the "builtin" and
> "secondary" keyrings, not the "platform" keyring.
> 
> Changes:
> - defining a new keyring restriction which only allows CA keys to be
> loaded onto the MOK keyring.
> - defining a new keyring restriction something along the lines of
> "restrict_link_by_builtin_mok_and_secondary_trusted()".
> 
> In the case of "restrict_link_by_builtin_and_secondary_trusted()", it's
> based on a build time option.  In the case of MOK, it might be both a
> build time and runtime firmware variable option.  There are quite a few
> permutations that will somehow need to be addressed:  secondary keyring
> not defined, but MOK keyring defined, and the reverse.
> 
> Once all the CA keys in the MOK db are loaded onto the MOK keyring,

To avoid confusion with the new keyring name, would it be more appropriate 
to change what we are calling the .mok keyring to the .trusted_platform 
keyring instead? Or just leave it as .mok?

> there will be no need for moving keys to the secondary keyring.  The
> secondary keyring restriction will just work.  The main question is
> whether there will need to be two passes.   One pass to first load all
> the CA keys onto the MOK keyring.  A second pass to load the keys onto
> the secondary keyring, based on the keyring restriction, and the
> remaining ones onto the "platform" keyring to avoid the regression.
> 
> [Once the CA keys are loaded onto the MOK keyring, userspace will be
> able to load certificates, signed by a key on the MOK keyring, onto the
> secondary keyring.]

Other than that, I think I got it, I’ll start working on a v2.  Thanks.


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-08 23:17               ` Eric Snowberg
@ 2021-07-09  1:10                 ` Mimi Zohar
  2021-07-09 15:59                   ` Nayna
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2021-07-09  1:10 UTC (permalink / raw)
  To: Eric Snowberg
  Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
	Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
	Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk

On Thu, 2021-07-08 at 17:17 -0600, Eric Snowberg wrote:
> > Once all the CA keys in the MOK db are loaded onto the MOK keyring,
> 
> To avoid confusion with the new keyring name, would it be more appropriate 
> to change what we are calling the .mok keyring to the .trusted_platform 
> keyring instead? Or just leave it as .mok?

Definitely not ".trusted_platform" keyring, as it would be too
confusing with the existing "trusted" key type [1].  At least for now,
leave it as ".mok".

thanks,

Mimi

[1] Documentation/security/keys/trusted-encrypted.rst


^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK
  2021-07-09  1:10                 ` Mimi Zohar
@ 2021-07-09 15:59                   ` Nayna
  0 siblings, 0 replies; 30+ messages in thread
From: Nayna @ 2021-07-09 15:59 UTC (permalink / raw)
  To: Mimi Zohar, Eric Snowberg
  Cc: keyrings, linux-integrity, David Howells, David Woodhouse,
	Herbert Xu, David S . Miller, Jarkko Sakkinen, James Morris,
	Serge E . Hallyn, keescook, gregkh, torvalds, scott.branden,
	weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
	linux-kernel, linux-crypto, linux-security-module,
	James.Bottomley, pjones, glin, konrad.wilk


On 7/8/21 9:10 PM, Mimi Zohar wrote:
> Definitely not ".trusted_platform" keyring, as it would be too
> confusing with the existing "trusted" key type [1].  At least for now,
> leave it as ".mok".

Since this keyring is meant only for "CA" keys, can we name it as ".ca" ?

Thanks & Regards,

     - Nayna


^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2021-07-09 16:00 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-07  2:43 [PATCH RFC 00/12] Enroll kernel keys thru MOK Eric Snowberg
2021-07-07  2:43 ` [PATCH RFC 01/12] KEYS: Add KEY_ALLOC_BYPASS_RESTRICTION option to key_move Eric Snowberg
2021-07-07  2:43 ` [PATCH RFC 02/12] KEYS: Allow unrestricted keys to be moved to the secondary keyring Eric Snowberg
2021-07-07  2:43 ` [PATCH RFC 03/12] KEYS: CA link restriction Eric Snowberg
2021-07-07  2:43 ` [PATCH RFC 04/12] integrity: add integrity_destroy_keyring Eric Snowberg
2021-07-07  2:43 ` [PATCH RFC 05/12] integrity: Introduce mok keyring Eric Snowberg
2021-07-07 19:31   ` Linus Torvalds
2021-07-07 21:26     ` Jarkko Sakkinen
2021-07-07 22:32       ` Eric Snowberg
2021-07-07  2:43 ` [PATCH RFC 06/12] integrity: Trust mok keys if MokListTrustedRT found Eric Snowberg
2021-07-07  2:43 ` [PATCH RFC 07/12] integrity: add add_to_mok_keyring Eric Snowberg
2021-07-07  2:43 ` [PATCH RFC 08/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_secondary_trusted_or_ca Eric Snowberg
2021-07-07  2:44 ` [PATCH RFC 09/12] integrity: accessor function to get trust_moklist Eric Snowberg
2021-07-07  2:44 ` [PATCH RFC 10/12] integrity: add new keyring handler Eric Snowberg
2021-07-07  2:44 ` [PATCH RFC 11/12] integrity: move keys from the mok keyring into the secondary keyring Eric Snowberg
2021-07-07  2:44 ` [PATCH RFC 12/12] integrity: Suppress error message for keys added to the mok keyring Eric Snowberg
2021-07-07  6:46 ` [PATCH RFC 00/12] Enroll kernel keys thru MOK Christoph Hellwig
2021-07-07 16:23   ` Eric Snowberg
2021-07-07 16:28     ` Christoph Hellwig
2021-07-07 16:45       ` Eric Snowberg
2021-07-07 12:39 ` Mimi Zohar
2021-07-07 16:28   ` Eric Snowberg
2021-07-07 17:00     ` Mimi Zohar
2021-07-07 22:10       ` Eric Snowberg
2021-07-08 13:56         ` Mimi Zohar
2021-07-08 17:59           ` Eric Snowberg
2021-07-08 19:31             ` Mimi Zohar
2021-07-08 23:17               ` Eric Snowberg
2021-07-09  1:10                 ` Mimi Zohar
2021-07-09 15:59                   ` Nayna

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