linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 00/12] Enroll kernel keys thru MOK
@ 2021-08-19  0:20 Eric Snowberg
  2021-08-19  0:20 ` [PATCH v4 01/12] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK) Eric Snowberg
                   ` (12 more replies)
  0 siblings, 13 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:20 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, konrad.wilk

Many UEFI Linux distributions boot using shim.  The UEFI shim provides
what is called Machine Owner Keys (MOK).  Shim uses both the UEFI Secure
Boot DB and MOK keys to validate the next step in the boot chain.  The
MOK facility can be used to import user generated keys.  These keys can
be used to sign an end-user development kernel build.  When Linux boots,
pre-boot keys (both UEFI Secure Boot DB and MOK keys) get loaded in the
Linux .platform keyring.  

Currently, pre-boot keys are not trusted within the Linux trust boundary
[1]. These platform keys 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 [2].  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 introduces a new Linux kernel keyring containing the Machine
Owner Keys (MOK) called .mok. It also 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
[3]. This would work similar to how the kernel uses MOK variables 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
mok keyring instead of the platform keyring. Mimi has suggested that
only CA keys be loaded into this keyring. All other certs will load 
into the platform keyring instead.

The .mok 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 all keys are added into the .mok keyring,
they are linked to the secondary trusted keyring.  After the link is 
created, keys contained in the .mok keyring will automatically be 
searched when searching the secondary trusted keys.

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.

V2 changes:
- The .mok keyring persists past boot
- Removed the unrestricted move into the secondary keyring
- Removed the keyring move bypass patch
- Added restrictions to allow the .mok to be linked to either the
  builtin or secondary keyrings
- Secondary keyring dependency has been removed

V3 changes:
- Only CA keys contained in the MOKList are loaded, nothing else
- Support for kernels built without the secondary trusted keyring
  has been dropped.

V4 changes:
- Add new Kconfig INTEGRITY_MOK_KEYRING and move all mok keyring
  code behind it
- Changed patch series ordering
- Consolidated a few patches

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

Eric Snowberg (12):
  integrity: Introduce a Linux keyring for the Machine Owner Key (MOK)
  integrity: Do not allow mok keyring updates following init
  KEYS: CA link restriction
  integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_ca
  integrity: add new keyring handler for mok keys
  KEYS: add a reference to mok keyring
  KEYS: Introduce link restriction to include builtin, secondary and mok
    keys
  KEYS: integrity: change link restriction to trust the mok keyring
  KEYS: link secondary_trusted_keys to mok trusted keys
  integrity: store reference to mok keyring
  integrity: Trust MOK keys if MokListTrustedRT found
  integrity: Only use mok keyring when uefi_check_trust_mok_keys is true

 certs/system_keyring.c                        | 40 ++++++++-
 crypto/asymmetric_keys/restrict.c             | 40 +++++++++
 include/crypto/public_key.h                   |  5 ++
 include/keys/system_keyring.h                 | 14 +++
 security/integrity/Kconfig                    | 11 +++
 security/integrity/Makefile                   |  1 +
 security/integrity/digsig.c                   | 18 +++-
 security/integrity/integrity.h                | 17 +++-
 .../platform_certs/keyring_handler.c          | 17 +++-
 .../platform_certs/keyring_handler.h          |  5 ++
 security/integrity/platform_certs/load_uefi.c |  4 +-
 .../integrity/platform_certs/mok_keyring.c    | 85 +++++++++++++++++++
 12 files changed, 249 insertions(+), 8 deletions(-)
 create mode 100644 security/integrity/platform_certs/mok_keyring.c


base-commit: 7c60610d476766e128cc4284bb6349732cbd6606
-- 
2.18.4


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

* [PATCH v4 01/12] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK)
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
@ 2021-08-19  0:20 ` Eric Snowberg
  2021-08-19  0:20 ` [PATCH v4 02/12] integrity: Do not allow mok keyring updates following init Eric Snowberg
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:20 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, konrad.wilk

Many UEFI Linux distributions boot using shim.  The UEFI shim provides
what is called Machine Owner Keys (MOK). Shim uses both the UEFI Secure
Boot DB and MOK keys to validate the next step in the boot chain.  The
MOK facility can be used to import user generated keys.  These keys can
be used to sign an end-users development kernel build.  When Linux
boots, both UEFI Secure Boot DB and MOK keys get loaded in the Linux
.platform keyring.

Add a new Linux keyring called .mok.  This keyring shall contain just
MOK CA keys and not the remaining keys in the platform keyring. This new
.mok keyring will be used in follow on patches.  Unlike keys in the
platform keyring, keys contained in the .mok keyring will be trusted
within the kernel if the end-user has chosen to do so.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Removed destory keyring code
v3: Unmodified from v2
v4: Add Kconfig, merged in "integrity: add add_to_mok_keyring" 
---
 security/integrity/Kconfig                    | 11 +++++
 security/integrity/Makefile                   |  1 +
 security/integrity/digsig.c                   |  1 +
 security/integrity/integrity.h                | 12 +++++-
 .../integrity/platform_certs/mok_keyring.c    | 42 +++++++++++++++++++
 5 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 security/integrity/platform_certs/mok_keyring.c

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 71f0177e8716..7a69021e2d42 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -62,6 +62,17 @@ config INTEGRITY_PLATFORM_KEYRING
          provided by the platform for verifying the kexec'ed kerned image
          and, possibly, the initramfs signature.
 
+config INTEGRITY_MOK_KEYRING
+	bool "Provide a keyring to which CA Machine Owner Keys may be added"
+	depends on SECONDARY_TRUSTED_KEYRING
+	depends on INTEGRITY_ASYMMETRIC_KEYS
+	depends on SYSTEM_BLACKLIST_KEYRING
+	help
+	 If set, provide a keyring to which CA Machine Owner Keys (MOK) may
+	 be added. This keyring shall contain just CA MOK keys.  Unlike keys
+	 in the platform keyring, keys contained in the .mok keyring will be
+	 trusted within the kernel.
+
 config LOAD_UEFI_KEYS
        depends on INTEGRITY_PLATFORM_KEYRING
        depends on EFI
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 7ee39d66cf16..e3f4588a069c 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -10,6 +10,7 @@ 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_MOK_KEYRING) += 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 3b06a01bd0fd..e07334504ef1 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 547425c20e11..be56ba49dc19 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;
 
@@ -283,3 +284,12 @@ static inline void __init add_to_platform_keyring(const char *source,
 {
 }
 #endif
+
+#ifdef CONFIG_INTEGRITY_MOK_KEYRING
+void __init add_to_mok_keyring(const char *source, const void *data, size_t len);
+#else
+static inline 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
new file mode 100644
index 000000000000..bcd9ac78ce3b
--- /dev/null
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -0,0 +1,42 @@
+// 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 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);
+
+	/*
+	 * Some MOKList keys may not pass the mok keyring restrictions.
+	 * If the restriction check does not pass and the platform keyring
+	 * is configured, try to add it into that 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);
+}
-- 
2.18.4


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

* [PATCH v4 02/12] integrity: Do not allow mok keyring updates following init
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
  2021-08-19  0:20 ` [PATCH v4 01/12] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK) Eric Snowberg
@ 2021-08-19  0:20 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 03/12] KEYS: CA link restriction Eric Snowberg
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:20 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, konrad.wilk

The mok keyring is setup during init.  No additional keys should be allowed
to be added afterwards.  Leave the permission as read only.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
v4: Unmodified from v2
---
 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 e07334504ef1..5cad38e6f56a 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -140,7 +140,8 @@ int __init integrity_init_keyring(const unsigned int id)
 		return -ENOMEM;
 
 	restriction->check = restrict_link_to_ima;
-	perm |= KEY_USR_WRITE;
+	if (id != INTEGRITY_KEYRING_MOK)
+		perm |= KEY_USR_WRITE;
 
 out:
 	return __integrity_init_keyring(id, perm, restriction);
-- 
2.18.4


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

* [PATCH v4 03/12] KEYS: CA link restriction
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
  2021-08-19  0:20 ` [PATCH v4 01/12] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK) Eric Snowberg
  2021-08-19  0:20 ` [PATCH v4 02/12] integrity: Do not allow mok keyring updates following init Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 04/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_ca Eric Snowberg
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

Add a new link restriction.  Restrict the addition of keys in a keyring
based on the key to be added being a CA (self-signed).

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Removed secondary keyring references
v3: Removed restrict_link_by_system_trusted_or_ca
    Simplify restrict_link_by_ca - only see if the key is a CA
    Did not add __init in front of restrict_link_by_ca in case
      restriction could be resued in the future
v4: Unmodified from v3
---
 crypto/asymmetric_keys/restrict.c | 40 +++++++++++++++++++++++++++++++
 include/crypto/public_key.h       |  5 ++++
 2 files changed, 45 insertions(+)

diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
index 84cefe3b3585..9ae43d3f862b 100644
--- a/crypto/asymmetric_keys/restrict.c
+++ b/crypto/asymmetric_keys/restrict.c
@@ -108,6 +108,46 @@ int restrict_link_by_signature(struct key *dest_keyring,
 	return ret;
 }
 
+/**
+ * restrict_link_by_ca - Restrict additions to a ring of CA keys
+ * @dest_keyring: Keyring being linked to.
+ * @type: The type of key being added.
+ * @payload: The payload of the new key.
+ * @trusted: Unused.
+ *
+ * Check if the new certificate is a CA. If it is a CA, 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;
+
+	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;
+
+	return public_key_verify_signature(pkey, sig);
+}
+
 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 *);
 
-- 
2.18.4


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

* [PATCH v4 04/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_ca
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (2 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 03/12] KEYS: CA link restriction Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 05/12] integrity: add new keyring handler for mok keys Eric Snowberg
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

Set the restriction check for INTEGRITY_KEYRING_MOK keys to
restrict_link_by_ca.  This will only allow CA keys into the mok
keyring.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Added !IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING check so mok
    keyring gets created even when it isn't enabled
v3: Rename restrict_link_by_system_trusted_or_ca to restrict_link_by_ca
v4: removed unnecessary restriction->check set
---
 security/integrity/digsig.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 5cad38e6f56a..1f410242752c 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -132,14 +132,18 @@ int __init integrity_init_keyring(const unsigned int id)
 		goto out;
 	}
 
-	if (!IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING))
+	if (!IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING) && id != INTEGRITY_KEYRING_MOK)
 		return 0;
 
 	restriction = kzalloc(sizeof(struct key_restriction), GFP_KERNEL);
 	if (!restriction)
 		return -ENOMEM;
 
-	restriction->check = restrict_link_to_ima;
+	if (id == INTEGRITY_KEYRING_MOK)
+		restriction->check = restrict_link_by_ca;
+	else
+		restriction->check = restrict_link_to_ima;
+
 	if (id != INTEGRITY_KEYRING_MOK)
 		perm |= KEY_USR_WRITE;
 
-- 
2.18.4


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

* [PATCH v4 05/12] integrity: add new keyring handler for mok keys
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (3 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 04/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_ca Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 06/12] KEYS: add a reference to mok keyring Eric Snowberg
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

Currently both Secure Boot DB and Machine Owner Keys (MOK) go through
the same keyring handler (get_handler_for_db). With the addition of the
new mok keyring, the end-user may choose to trust MOK keys.

Introduce a new keyring handler specific for mok keys.  If mok keys are
trusted by the end-user, use the new keyring handler instead.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v3: Only change the keyring handler if the secondary is enabled
v4: Removed trust_moklist check
---
 .../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..fc4ad85d9223 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_INTEGRITY_MOK_KEYRING))
+			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 f290f78c3f30..c1bfd1cd7cc3 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 related	[flat|nested] 33+ messages in thread

* [PATCH v4 06/12] KEYS: add a reference to mok keyring
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (4 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 05/12] integrity: add new keyring handler for mok keys Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 07/12] KEYS: Introduce link restriction to include builtin, secondary and mok keys Eric Snowberg
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

Expose the .mok keyring created in integrity code by adding
a reference.  This makes the mok keyring accessible for keyring
restrictions in the future.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
v3: set_mok_trusted_keys only available when secondary is enabled
v4: Moved code under CONFIG_INTEGRITY_MOK_KEYRING
---
 certs/system_keyring.c        | 9 +++++++++
 include/keys/system_keyring.h | 8 ++++++++
 2 files changed, 17 insertions(+)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 692365dee2bd..94af3fe107b4 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -22,6 +22,9 @@ static struct key *builtin_trusted_keys;
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
 static struct key *secondary_trusted_keys;
 #endif
+#ifdef CONFIG_INTEGRITY_MOK_KEYRING
+static struct key *mok_trusted_keys;
+#endif
 #ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
 static struct key *platform_trusted_keys;
 #endif
@@ -91,6 +94,12 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
 	return restriction;
 }
 #endif
+#ifdef CONFIG_INTEGRITY_MOK_KEYRING
+void __init set_mok_trusted_keys(struct key *keyring)
+{
+	mok_trusted_keys = keyring;
+}
+#endif
 
 /*
  * Create the trusted keyrings
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 6acd3cf13a18..059e32e36b3a 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -38,6 +38,14 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
 #define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
 #endif
 
+#ifdef CONFIG_INTEGRITY_MOK_KEYRING
+extern void __init set_mok_trusted_keys(struct key *keyring);
+#else
+static inline void __init set_mok_trusted_keys(struct key *keyring)
+{
+}
+#endif
+
 extern struct pkcs7_message *pkcs7;
 #ifdef CONFIG_SYSTEM_BLACKLIST_KEYRING
 extern int mark_hash_blacklisted(const char *hash);
-- 
2.18.4


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

* [PATCH v4 07/12] KEYS: Introduce link restriction to include builtin, secondary and mok keys
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (5 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 06/12] KEYS: add a reference to mok keyring Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 08/12] KEYS: integrity: change link restriction to trust the mok keyring Eric Snowberg
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

Introduce a new link restriction that includes the trusted builtin,
secondary and mok keys. The restriction is based on the key to be added
being vouched for by a key in any of these three keyrings.

Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v3: Initial version
v4: moved code under CONFIG_INTEGRITY_MOK_KEYRING
---
 certs/system_keyring.c        | 23 +++++++++++++++++++++++
 include/keys/system_keyring.h |  6 ++++++
 2 files changed, 29 insertions(+)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 94af3fe107b4..a75c815a42c8 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -99,6 +99,29 @@ void __init set_mok_trusted_keys(struct key *keyring)
 {
 	mok_trusted_keys = keyring;
 }
+
+/**
+ * restrict_link_by_builtin_secondary_and_ca_trusted
+ *
+ * Restrict the addition of keys into a keyring based on the key-to-be-added
+ * being vouched for by a key in either the built-in, the secondary, or
+ * the mok keyrings.
+ */
+int restrict_link_by_builtin_secondary_and_ca_trusted(
+	struct key *dest_keyring,
+	const struct key_type *type,
+	const union key_payload *payload,
+	struct key *restrict_key)
+{
+	if (mok_trusted_keys && type == &key_type_keyring &&
+	    dest_keyring == secondary_trusted_keys &&
+	    payload == &mok_trusted_keys->payload)
+		/* Allow the mok keyring to be added to the secondary */
+		return 0;
+
+	return restrict_link_by_builtin_and_secondary_trusted(dest_keyring, type,
+							      payload, restrict_key);
+}
 #endif
 
 /*
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 059e32e36b3a..8cc9606a6cab 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -39,8 +39,14 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
 #endif
 
 #ifdef CONFIG_INTEGRITY_MOK_KEYRING
+extern int restrict_link_by_builtin_secondary_and_ca_trusted(
+	struct key *dest_keyring,
+	const struct key_type *type,
+	const union key_payload *payload,
+	struct key *restrict_key);
 extern void __init set_mok_trusted_keys(struct key *keyring);
 #else
+#define restrict_link_by_builtin_secondary_and_ca_trusted restrict_link_by_builtin_trusted
 static inline void __init set_mok_trusted_keys(struct key *keyring)
 {
 }
-- 
2.18.4


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

* [PATCH v4 08/12] KEYS: integrity: change link restriction to trust the mok keyring
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (6 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 07/12] KEYS: Introduce link restriction to include builtin, secondary and mok keys Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 09/12] KEYS: link secondary_trusted_keys to mok trusted keys Eric Snowberg
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

With the introduction of the mok keyring, the end-user may choose to
trust Machine Owner Keys (MOK) within the kernel. If they have chosen to
trust them, the .mok keyring will contain these keys.  If not, the mok
keyring will always be empty.  Update the restriction check to allow the
secondary trusted keyring and ima keyring to also trust mok keys.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v4: Initial version (consolidated two previous patches)
---
 certs/system_keyring.c      | 5 ++++-
 security/integrity/digsig.c | 4 ++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index a75c815a42c8..1c39af137cf1 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -89,7 +89,10 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
 	if (!restriction)
 		panic("Can't allocate secondary trusted keyring restriction\n");
 
-	restriction->check = restrict_link_by_builtin_and_secondary_trusted;
+	if (IS_ENABLED(CONFIG_INTEGRITY_MOK_KEYRING))
+		restriction->check = restrict_link_by_builtin_secondary_and_ca_trusted;
+	else
+		restriction->check = restrict_link_by_builtin_and_secondary_trusted;
 
 	return restriction;
 }
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 1f410242752c..a93d558b795b 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -34,7 +34,11 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
 };
 
 #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
+#ifdef CONFIG_INTEGRITY_MOK_KEYRING
+#define restrict_link_to_ima restrict_link_by_builtin_secondary_and_ca_trusted
+#else
 #define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
+#endif
 #else
 #define restrict_link_to_ima restrict_link_by_builtin_trusted
 #endif
-- 
2.18.4


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

* [PATCH v4 09/12] KEYS: link secondary_trusted_keys to mok trusted keys
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (7 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 08/12] KEYS: integrity: change link restriction to trust the mok keyring Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 10/12] integrity: store reference to mok keyring Eric Snowberg
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

Allow the .mok keyring to be linked to the secondary_trusted_keys.  After
the link is created, keys contained in the .mok keyring will automatically
be searched when searching secondary_trusted_keys.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v3: Initial version
v4: Unmodified from v3
---
 certs/system_keyring.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 1c39af137cf1..4ce39b4ccc04 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -101,6 +101,9 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
 void __init set_mok_trusted_keys(struct key *keyring)
 {
 	mok_trusted_keys = keyring;
+
+	if (key_link(secondary_trusted_keys, mok_trusted_keys) < 0)
+		panic("Can't link (mok) trusted keyrings\n");
 }
 
 /**
-- 
2.18.4


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

* [PATCH v4 10/12] integrity: store reference to mok keyring
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (8 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 09/12] KEYS: link secondary_trusted_keys to mok trusted keys Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 11/12] integrity: Trust MOK keys if MokListTrustedRT found Eric Snowberg
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

Store a reference to the mok keyring in system keyring code. The
system keyring code needs this to complete the keyring link to
to mok keyring.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
v3: Unmodified from v2
v4: Removed trust_moklist check
---
 security/integrity/digsig.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index a93d558b795b..0f14ffef9c43 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -116,6 +116,8 @@ static int __init __integrity_init_keyring(const unsigned int id,
 	} else {
 		if (id == INTEGRITY_KEYRING_PLATFORM)
 			set_platform_trusted_keys(keyring[id]);
+		if (id == INTEGRITY_KEYRING_MOK)
+			set_mok_trusted_keys(keyring[id]);
 		if (id == INTEGRITY_KEYRING_IMA)
 			load_module_cert(keyring[id]);
 	}
-- 
2.18.4


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

* [PATCH v4 11/12] integrity: Trust MOK keys if MokListTrustedRT found
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (9 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 10/12] integrity: store reference to mok keyring Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19  0:21 ` [PATCH v4 12/12] integrity: Only use mok keyring when uefi_check_trust_mok_keys is true Eric Snowberg
  2021-08-19 11:38 ` [PATCH v4 00/12] Enroll kernel keys thru MOK Jarkko Sakkinen
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

A new Machine Owner Key (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>
---
v1: Initial version
v2: Removed mok_keyring_trust_setup function
v4: Unmodified from v2
---
 .../integrity/platform_certs/mok_keyring.c    | 27 +++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index bcd9ac78ce3b..bcfab894a9dc 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2021, Oracle and/or its affiliates.
  */
 
+#include <linux/efi.h>
 #include "../integrity.h"
 
 static __init int mok_keyring_init(void)
@@ -40,3 +41,29 @@ void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
 	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
+ * 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)));
+}
-- 
2.18.4


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

* [PATCH v4 12/12] integrity: Only use mok keyring when uefi_check_trust_mok_keys is true
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (10 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 11/12] integrity: Trust MOK keys if MokListTrustedRT found Eric Snowberg
@ 2021-08-19  0:21 ` Eric Snowberg
  2021-08-19 11:38 ` [PATCH v4 00/12] Enroll kernel keys thru MOK Jarkko Sakkinen
  12 siblings, 0 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19  0:21 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, konrad.wilk

With the introduction of uefi_check_trust_mok_keys, it signifies the end-
user wants to trust the mok keyring as trusted keys.  If they have chosen 
to trust the mok keyring, load the qualifying keys into it during boot, 
then link it to the secondary keyring .  If the user has not chosen to 
trust the mok keyring, it will be empty and not linked to the secondary 
keyring.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v4: Initial version
---
 security/integrity/digsig.c                      |  2 +-
 security/integrity/integrity.h                   |  5 +++++
 .../integrity/platform_certs/keyring_handler.c   |  2 +-
 security/integrity/platform_certs/mok_keyring.c  | 16 ++++++++++++++++
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 0f14ffef9c43..fd255e5b6293 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -116,7 +116,7 @@ static int __init __integrity_init_keyring(const unsigned int id,
 	} else {
 		if (id == INTEGRITY_KEYRING_PLATFORM)
 			set_platform_trusted_keys(keyring[id]);
-		if (id == INTEGRITY_KEYRING_MOK)
+		if (id == INTEGRITY_KEYRING_MOK && trust_moklist())
 			set_mok_trusted_keys(keyring[id]);
 		if (id == INTEGRITY_KEYRING_IMA)
 			load_module_cert(keyring[id]);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index be56ba49dc19..57683fdea2af 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -287,9 +287,14 @@ static inline void __init add_to_platform_keyring(const char *source,
 
 #ifdef CONFIG_INTEGRITY_MOK_KEYRING
 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_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/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
index fc4ad85d9223..471bf103b444 100644
--- a/security/integrity/platform_certs/keyring_handler.c
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -82,7 +82,7 @@ __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
 __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_INTEGRITY_MOK_KEYRING))
+		if (IS_ENABLED(CONFIG_INTEGRITY_MOK_KEYRING) && trust_moklist())
 			return add_to_mok_keyring;
 		else
 			return add_to_platform_keyring;
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index bcfab894a9dc..3dbb6d17e17d 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -8,6 +8,8 @@
 #include <linux/efi.h>
 #include "../integrity.h"
 
+bool trust_mok;
+
 static __init int mok_keyring_init(void)
 {
 	int rc;
@@ -67,3 +69,17 @@ static __init bool uefi_check_trust_mok_keys(void)
 	 */
 	return (status == EFI_SUCCESS && (!(attr & EFI_VARIABLE_NON_VOLATILE)));
 }
+
+bool __init trust_moklist(void)
+{
+	static bool initialized;
+
+	if (!initialized) {
+		initialized = true;
+
+		if (uefi_check_trust_mok_keys())
+			trust_mok = true;
+	}
+
+	return trust_mok;
+}
-- 
2.18.4


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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
                   ` (11 preceding siblings ...)
  2021-08-19  0:21 ` [PATCH v4 12/12] integrity: Only use mok keyring when uefi_check_trust_mok_keys is true Eric Snowberg
@ 2021-08-19 11:38 ` Jarkko Sakkinen
  2021-08-19 13:10   ` Mimi Zohar
  12 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2021-08-19 11:38 UTC (permalink / raw)
  To: Eric Snowberg, keyrings, linux-integrity, zohar, dhowells, dwmw2,
	herbert, davem, jmorris, serge
  Cc: keescook, gregkh, torvalds, scott.branden, weiyongjun1, nayna,
	ebiggers, ardb, nramas, lszubowi, linux-kernel, linux-crypto,
	linux-security-module, James.Bottomley, pjones, konrad.wilk

On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
> Many UEFI Linux distributions boot using shim.  The UEFI shim provides
> what is called Machine Owner Keys (MOK).  Shim uses both the UEFI Secure
> Boot DB and MOK keys to validate the next step in the boot chain.  The
> MOK facility can be used to import user generated keys.  These keys can
> be used to sign an end-user development kernel build.  When Linux boots,
> pre-boot keys (both UEFI Secure Boot DB and MOK keys) get loaded in the
> Linux .platform keyring.  
> 
> Currently, pre-boot keys are not trusted within the Linux trust boundary
> [1]. These platform keys 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 [2].  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.

As of today, I can use a prebuilt kernel, crate my own MOK key and sign
modules. What will be different?

> 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 introduces a new Linux kernel keyring containing the Machine
> Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.

I would name it as ".machine" because it is more "re-usable" name, e.g.
could be used for similar things as MOK. ".mok" is a bad name because
it binds directly to a single piece of user space software.

/Jarkko

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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-19 11:38 ` [PATCH v4 00/12] Enroll kernel keys thru MOK Jarkko Sakkinen
@ 2021-08-19 13:10   ` Mimi Zohar
  2021-08-19 15:23     ` Eric Snowberg
  2021-08-23 17:35     ` Jarkko Sakkinen
  0 siblings, 2 replies; 33+ messages in thread
From: Mimi Zohar @ 2021-08-19 13:10 UTC (permalink / raw)
  To: Jarkko Sakkinen, Eric Snowberg, keyrings, linux-integrity,
	dhowells, dwmw2, herbert, davem, jmorris, serge
  Cc: keescook, gregkh, torvalds, scott.branden, weiyongjun1, nayna,
	ebiggers, ardb, nramas, lszubowi, linux-kernel, linux-crypto,
	linux-security-module, James.Bottomley, pjones, konrad.wilk,
	Patrick Uiterwijk

On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
> On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
> > Many UEFI Linux distributions boot using shim.  The UEFI shim provides
> > what is called Machine Owner Keys (MOK).  Shim uses both the UEFI Secure
> > Boot DB and MOK keys to validate the next step in the boot chain.  The
> > MOK facility can be used to import user generated keys.  These keys can
> > be used to sign an end-user development kernel build.  When Linux boots,
> > pre-boot keys (both UEFI Secure Boot DB and MOK keys) get loaded in the
> > Linux .platform keyring.  
> > 
> > Currently, pre-boot keys are not trusted within the Linux trust boundary
> > [1]. These platform keys 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 [2].  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.
> 
> As of today, I can use a prebuilt kernel, crate my own MOK key and sign
> modules. What will be different?

The UEFI db and MOK keys are being loaded onto the .platform keyring,
which is suppose to be limited to verifying the kexec kernel image
signature.  With a downstream patch, kernel modules are being verified
as well.

Initially Patrick Uiterwijk's "[PATCH 0/3] Load keys from TPM2 NV Index
on IMA keyring" patch set attempted to define a new root of trust based
on a key stored in the TPM.  This patch set is similarly attempting to
define a new root of trust based on CA keys stored in the MOK db.

The purpose of this patch set is to define a new, safe trust source
parallel to the builtin keyring, without relying on a downstream patch.
With the new root of trust, the end user could sign his own kernel
modules, sign third party keys, and load keys onto the IMA keyring,
which can be used for signing the IMA policy and other files.

> 
> > 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 introduces a new Linux kernel keyring containing the Machine
> > Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
> 
> I would name it as ".machine" because it is more "re-usable" name, e.g.
> could be used for similar things as MOK. ".mok" is a bad name because
> it binds directly to a single piece of user space software.

Nayna previously said,
   "I believe the underlying source from where CA keys are loaded might vary 
   based on the architecture (".mok" is UEFI specific.). The key part is 
   that this new keyring should contain only CA keys which can be later 
   used to vouch for user keys loaded onto IMA or secondary keyring at 
   runtime. It would be good to have a "ca" in the name, like .xxxx-ca, 
   where xxxx can be machine, owner, or system. I prefer .system-ca."

The CA keys on the MOK db is simply the first root of trust being
defined, but other roots of trust are sure to follow.  For this reason,
I agree naming the new keyring "mok" should be avoided.

thanks,

Mimi


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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-19 13:10   ` Mimi Zohar
@ 2021-08-19 15:23     ` Eric Snowberg
  2021-08-19 17:32       ` Mimi Zohar
  2021-08-23 17:37       ` Jarkko Sakkinen
  2021-08-23 17:35     ` Jarkko Sakkinen
  1 sibling, 2 replies; 33+ messages in thread
From: Eric Snowberg @ 2021-08-19 15:23 UTC (permalink / raw)
  To: Mimi Zohar, Jarkko Sakkinen, David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, James Bottomley, pjones,
	konrad.wilk, Patrick Uiterwijk


> On Aug 19, 2021, at 7:10 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
>> On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
>>> 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 introduces a new Linux kernel keyring containing the Machine
>>> Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
>> 
>> I would name it as ".machine" because it is more "re-usable" name, e.g.
>> could be used for similar things as MOK. ".mok" is a bad name because
>> it binds directly to a single piece of user space software.
> 
> Nayna previously said,
>   "I believe the underlying source from where CA keys are loaded might vary 
>   based on the architecture (".mok" is UEFI specific.). The key part is 
>   that this new keyring should contain only CA keys which can be later 
>   used to vouch for user keys loaded onto IMA or secondary keyring at 
>   runtime. It would be good to have a "ca" in the name, like .xxxx-ca, 
>   where xxxx can be machine, owner, or system. I prefer .system-ca."
> 
> The CA keys on the MOK db is simply the first root of trust being
> defined, but other roots of trust are sure to follow.  For this reason,
> I agree naming the new keyring "mok" should be avoided.

As I said previously, I’m open to renaming, I just would like to have an 
agreement on the new name before changing everything.  The current proposed 
names I have heard are “.machine" and ".system-ca".  Is there a preference 
the maintainers feel is appropriate?  If so, please let me know and I’ll 
rename it. Thanks.


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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-19 15:23     ` Eric Snowberg
@ 2021-08-19 17:32       ` Mimi Zohar
  2021-08-23 17:51         ` Jarkko Sakkinen
  2021-08-23 17:37       ` Jarkko Sakkinen
  1 sibling, 1 reply; 33+ messages in thread
From: Mimi Zohar @ 2021-08-19 17:32 UTC (permalink / raw)
  To: Eric Snowberg, Jarkko Sakkinen, David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, James Bottomley, pjones,
	konrad.wilk, Patrick Uiterwijk

On Thu, 2021-08-19 at 09:23 -0600, Eric Snowberg wrote:
> > On Aug 19, 2021, at 7:10 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
> >> On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
> >>> 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 introduces a new Linux kernel keyring containing the Machine
> >>> Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
> >> 
> >> I would name it as ".machine" because it is more "re-usable" name, e.g.
> >> could be used for similar things as MOK. ".mok" is a bad name because
> >> it binds directly to a single piece of user space software.
> > 
> > Nayna previously said,
> >   "I believe the underlying source from where CA keys are loaded might vary 
> >   based on the architecture (".mok" is UEFI specific.). The key part is 
> >   that this new keyring should contain only CA keys which can be later 
> >   used to vouch for user keys loaded onto IMA or secondary keyring at 
> >   runtime. It would be good to have a "ca" in the name, like .xxxx-ca, 
> >   where xxxx can be machine, owner, or system. I prefer .system-ca."
> > 
> > The CA keys on the MOK db is simply the first root of trust being
> > defined, but other roots of trust are sure to follow.  For this reason,
> > I agree naming the new keyring "mok" should be avoided.
> 
> As I said previously, I’m open to renaming, I just would like to have an 
> agreement on the new name before changing everything.  The current proposed 
> names I have heard are “.machine" and ".system-ca".  Is there a preference 
> the maintainers feel is appropriate?  If so, please let me know and I’ll 
> rename it. Thanks.
> 

Jarkko, I think the emphasis should not be on "machine" from Machine
Owner Key (MOK), but on "owner".  Whereas Nayna is focusing more on the
"_ca" aspect of the name.   Perhaps consider naming it
"system_owner_ca" or something along those lines.

thanks,

Mimi



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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-19 13:10   ` Mimi Zohar
  2021-08-19 15:23     ` Eric Snowberg
@ 2021-08-23 17:35     ` Jarkko Sakkinen
  2021-08-23 17:48       ` Eric Snowberg
  1 sibling, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2021-08-23 17:35 UTC (permalink / raw)
  To: Mimi Zohar, Eric Snowberg, keyrings, linux-integrity, dhowells,
	dwmw2, herbert, davem, jmorris, serge
  Cc: keescook, gregkh, torvalds, scott.branden, weiyongjun1, nayna,
	ebiggers, ardb, nramas, lszubowi, linux-kernel, linux-crypto,
	linux-security-module, James.Bottomley, pjones, konrad.wilk,
	Patrick Uiterwijk

On Thu, 2021-08-19 at 09:10 -0400, Mimi Zohar wrote:
> On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
> > On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
> > > Many UEFI Linux distributions boot using shim.  The UEFI shim provides
> > > what is called Machine Owner Keys (MOK).  Shim uses both the UEFI Secure
> > > Boot DB and MOK keys to validate the next step in the boot chain.  The
> > > MOK facility can be used to import user generated keys.  These keys can
> > > be used to sign an end-user development kernel build.  When Linux boots,
> > > pre-boot keys (both UEFI Secure Boot DB and MOK keys) get loaded in the
> > > Linux .platform keyring.  
> > > 
> > > Currently, pre-boot keys are not trusted within the Linux trust boundary
> > > [1]. These platform keys 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 [2].  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.
> > 
> > As of today, I can use a prebuilt kernel, crate my own MOK key and sign
> > modules. What will be different?
> 
> The UEFI db and MOK keys are being loaded onto the .platform keyring,
> which is suppose to be limited to verifying the kexec kernel image
> signature.  With a downstream patch, kernel modules are being verified
> as well.
> 
> Initially Patrick Uiterwijk's "[PATCH 0/3] Load keys from TPM2 NV Index
> on IMA keyring" patch set attempted to define a new root of trust based
> on a key stored in the TPM.  This patch set is similarly attempting to
> define a new root of trust based on CA keys stored in the MOK db.
> 
> The purpose of this patch set is to define a new, safe trust source
> parallel to the builtin keyring, without relying on a downstream patch.
> With the new root of trust, the end user could sign his own kernel
> modules, sign third party keys, and load keys onto the IMA keyring,
> which can be used for signing the IMA policy and other files.

I can, as of today, generate my own mok key and sign my LKM's, and
kernel will verify my LKM's.

What is different?


/Jarkko

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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-19 15:23     ` Eric Snowberg
  2021-08-19 17:32       ` Mimi Zohar
@ 2021-08-23 17:37       ` Jarkko Sakkinen
  1 sibling, 0 replies; 33+ messages in thread
From: Jarkko Sakkinen @ 2021-08-23 17:37 UTC (permalink / raw)
  To: Eric Snowberg, Mimi Zohar, David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, James Bottomley, pjones,
	konrad.wilk, Patrick Uiterwijk

On Thu, 2021-08-19 at 09:23 -0600, Eric Snowberg wrote:
> > On Aug 19, 2021, at 7:10 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
> > > On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
> > > > 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 introduces a new Linux kernel keyring containing the Machine
> > > > Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
> > > 
> > > I would name it as ".machine" because it is more "re-usable" name, e.g.
> > > could be used for similar things as MOK. ".mok" is a bad name because
> > > it binds directly to a single piece of user space software.
> > 
> > Nayna previously said,
> >   "I believe the underlying source from where CA keys are loaded might vary 
> >   based on the architecture (".mok" is UEFI specific.). The key part is 
> >   that this new keyring should contain only CA keys which can be later 
> >   used to vouch for user keys loaded onto IMA or secondary keyring at 
> >   runtime. It would be good to have a "ca" in the name, like .xxxx-ca, 
> >   where xxxx can be machine, owner, or system. I prefer .system-ca."
> > 
> > The CA keys on the MOK db is simply the first root of trust being
> > defined, but other roots of trust are sure to follow.  For this reason,
> > I agree naming the new keyring "mok" should be avoided.
> 
> As I said previously, I’m open to renaming, I just would like to have an 
> agreement on the new name before changing everything.  The current proposed 
> names I have heard are “.machine" and ".system-ca".  Is there a preference 
> the maintainers feel is appropriate?  If so, please let me know and I’ll 
> rename it. Thanks.


Just ".system" would be good. It's informative enough.


/Jarkko

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

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


> On Aug 23, 2021, at 11:35 AM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> 
> On Thu, 2021-08-19 at 09:10 -0400, Mimi Zohar wrote:
>> On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
>>> On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
>>>> Many UEFI Linux distributions boot using shim.  The UEFI shim provides
>>>> what is called Machine Owner Keys (MOK).  Shim uses both the UEFI Secure
>>>> Boot DB and MOK keys to validate the next step in the boot chain.  The
>>>> MOK facility can be used to import user generated keys.  These keys can
>>>> be used to sign an end-user development kernel build.  When Linux boots,
>>>> pre-boot keys (both UEFI Secure Boot DB and MOK keys) get loaded in the
>>>> Linux .platform keyring.  
>>>> 
>>>> Currently, pre-boot keys are not trusted within the Linux trust boundary
>>>> [1]. These platform keys 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 [2].  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.
>>> 
>>> As of today, I can use a prebuilt kernel, crate my own MOK key and sign
>>> modules. What will be different?
>> 
>> The UEFI db and MOK keys are being loaded onto the .platform keyring,
>> which is suppose to be limited to verifying the kexec kernel image
>> signature.  With a downstream patch, kernel modules are being verified
>> as well.
>> 
>> Initially Patrick Uiterwijk's "[PATCH 0/3] Load keys from TPM2 NV Index
>> on IMA keyring" patch set attempted to define a new root of trust based
>> on a key stored in the TPM.  This patch set is similarly attempting to
>> define a new root of trust based on CA keys stored in the MOK db.
>> 
>> The purpose of this patch set is to define a new, safe trust source
>> parallel to the builtin keyring, without relying on a downstream patch.
>> With the new root of trust, the end user could sign his own kernel
>> modules, sign third party keys, and load keys onto the IMA keyring,
>> which can be used for signing the IMA policy and other files.
> 
> I can, as of today, generate my own mok key and sign my LKM's, and
> kernel will verify my LKM's.
> 
> What is different?

Are you sure your kernel doesn’t contain some version of the rejected 
patch referenced in the cover letter  [1]?

https://lore.kernel.org/lkml/1556221605.24945.3.camel@HansenPartnership.com/


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

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

On Thu, 2021-08-19 at 13:32 -0400, Mimi Zohar wrote:
> On Thu, 2021-08-19 at 09:23 -0600, Eric Snowberg wrote:
> > > On Aug 19, 2021, at 7:10 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > 
> > > On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
> > > > On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
> > > > > 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 introduces a new Linux kernel keyring containing the Machine
> > > > > Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
> > > > 
> > > > I would name it as ".machine" because it is more "re-usable" name, e.g.
> > > > could be used for similar things as MOK. ".mok" is a bad name because
> > > > it binds directly to a single piece of user space software.
> > > 
> > > Nayna previously said,
> > >   "I believe the underlying source from where CA keys are loaded might vary 
> > >   based on the architecture (".mok" is UEFI specific.). The key part is 
> > >   that this new keyring should contain only CA keys which can be later 
> > >   used to vouch for user keys loaded onto IMA or secondary keyring at 
> > >   runtime. It would be good to have a "ca" in the name, like .xxxx-ca, 
> > >   where xxxx can be machine, owner, or system. I prefer .system-ca."
> > > 
> > > The CA keys on the MOK db is simply the first root of trust being
> > > defined, but other roots of trust are sure to follow.  For this reason,
> > > I agree naming the new keyring "mok" should be avoided.
> > 
> > As I said previously, I’m open to renaming, I just would like to have an 
> > agreement on the new name before changing everything.  The current proposed 
> > names I have heard are “.machine" and ".system-ca".  Is there a preference 
> > the maintainers feel is appropriate?  If so, please let me know and I’ll 
> > rename it. Thanks.
> > 
> 
> Jarkko, I think the emphasis should not be on "machine" from Machine
> Owner Key (MOK), but on "owner".  Whereas Nayna is focusing more on the
> "_ca" aspect of the name.   Perhaps consider naming it
> "system_owner_ca" or something along those lines.

What do you gain such overly long identifier? Makes no sense. What 
is "ca aspect of the name" anyway?

/Jarkko

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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-23 17:51         ` Jarkko Sakkinen
@ 2021-08-23 20:48           ` Nayna
  2021-08-24 14:34             ` Mimi Zohar
  0 siblings, 1 reply; 33+ messages in thread
From: Nayna @ 2021-08-23 20:48 UTC (permalink / raw)
  To: Jarkko Sakkinen, Mimi Zohar, Eric Snowberg, David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, James Bottomley, pjones,
	konrad.wilk, Patrick Uiterwijk


On 8/23/21 1:51 PM, Jarkko Sakkinen wrote:
> On Thu, 2021-08-19 at 13:32 -0400, Mimi Zohar wrote:
>> On Thu, 2021-08-19 at 09:23 -0600, Eric Snowberg wrote:
>>>> On Aug 19, 2021, at 7:10 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>>>
>>>> On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
>>>>> On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
>>>>>> 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 introduces a new Linux kernel keyring containing the Machine
>>>>>> Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
>>>>> I would name it as ".machine" because it is more "re-usable" name, e.g.
>>>>> could be used for similar things as MOK. ".mok" is a bad name because
>>>>> it binds directly to a single piece of user space software.
>>>> Nayna previously said,
>>>>    "I believe the underlying source from where CA keys are loaded might vary
>>>>    based on the architecture (".mok" is UEFI specific.). The key part is
>>>>    that this new keyring should contain only CA keys which can be later
>>>>    used to vouch for user keys loaded onto IMA or secondary keyring at
>>>>    runtime. It would be good to have a "ca" in the name, like .xxxx-ca,
>>>>    where xxxx can be machine, owner, or system. I prefer .system-ca."
>>>>
>>>> The CA keys on the MOK db is simply the first root of trust being
>>>> defined, but other roots of trust are sure to follow.  For this reason,
>>>> I agree naming the new keyring "mok" should be avoided.
>>> As I said previously, I’m open to renaming, I just would like to have an
>>> agreement on the new name before changing everything.  The current proposed
>>> names I have heard are “.machine" and ".system-ca".  Is there a preference
>>> the maintainers feel is appropriate?  If so, please let me know and I’ll
>>> rename it. Thanks.
>>>
>> Jarkko, I think the emphasis should not be on "machine" from Machine
>> Owner Key (MOK), but on "owner".  Whereas Nayna is focusing more on the
>> "_ca" aspect of the name.   Perhaps consider naming it
>> "system_owner_ca" or something along those lines.
> What do you gain such overly long identifier? Makes no sense. What
> is "ca aspect of the name" anyway?

As I mentioned previously, the main usage of this new keyring is that it 
should contain only CA keys which can be later used to vouch for user 
keys loaded onto secondary or IMA keyring at runtime. Having ca in the 
name like .xxxx_ca, would make the keyring name self-describing. Since 
you preferred .system, we can call it .system_ca.

Thanks & Regards,

        - Nayna


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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-23 20:48           ` Nayna
@ 2021-08-24 14:34             ` Mimi Zohar
  2021-08-25 22:21               ` Jarkko Sakkinen
  0 siblings, 1 reply; 33+ messages in thread
From: Mimi Zohar @ 2021-08-24 14:34 UTC (permalink / raw)
  To: Nayna, Jarkko Sakkinen, Eric Snowberg, David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, James Bottomley, pjones,
	konrad.wilk, Patrick Uiterwijk


> >> Jarkko, I think the emphasis should not be on "machine" from Machine
> >> Owner Key (MOK), but on "owner".  Whereas Nayna is focusing more on the
> >> "_ca" aspect of the name.   Perhaps consider naming it
> >> "system_owner_ca" or something along those lines.

> > What do you gain such overly long identifier? Makes no sense. What
> > is "ca aspect of the name" anyway?
> 
> As I mentioned previously, the main usage of this new keyring is that it 
> should contain only CA keys which can be later used to vouch for user 
> keys loaded onto secondary or IMA keyring at runtime. Having ca in the 
> name like .xxxx_ca, would make the keyring name self-describing. Since 
> you preferred .system, we can call it .system_ca.

Sounds good to me.  Jarkko?

thanks,

Mimi


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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-24 14:34             ` Mimi Zohar
@ 2021-08-25 22:21               ` Jarkko Sakkinen
  2021-08-25 22:27                 ` James Bottomley
  0 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2021-08-25 22:21 UTC (permalink / raw)
  To: Mimi Zohar, Nayna, Eric Snowberg, David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, James Bottomley, pjones,
	konrad.wilk, Patrick Uiterwijk

On Tue, 2021-08-24 at 10:34 -0400, Mimi Zohar wrote:
> > > > Jarkko, I think the emphasis should not be on "machine" from Machine
> > > > Owner Key (MOK), but on "owner".  Whereas Nayna is focusing more on the
> > > > "_ca" aspect of the name.   Perhaps consider naming it
> > > > "system_owner_ca" or something along those lines.
> > > What do you gain such overly long identifier? Makes no sense. What
> > > is "ca aspect of the name" anyway?
> > 
> > As I mentioned previously, the main usage of this new keyring is that it 
> > should contain only CA keys which can be later used to vouch for user 
> > keys loaded onto secondary or IMA keyring at runtime. Having ca in the 
> > name like .xxxx_ca, would make the keyring name self-describing. Since 
> > you preferred .system, we can call it .system_ca.
> 
> Sounds good to me.  Jarkko?
> 
> thanks,
> 
> Mimi

I just wonder what you exactly gain with "_ca"?

/Jarkko

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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-25 22:21               ` Jarkko Sakkinen
@ 2021-08-25 22:27                 ` James Bottomley
  2021-08-27 20:44                   ` Nayna
  0 siblings, 1 reply; 33+ messages in thread
From: James Bottomley @ 2021-08-25 22:27 UTC (permalink / raw)
  To: Jarkko Sakkinen, Mimi Zohar, Nayna, Eric Snowberg, David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, pjones, konrad.wilk,
	Patrick Uiterwijk

On Thu, 2021-08-26 at 01:21 +0300, Jarkko Sakkinen wrote:
> On Tue, 2021-08-24 at 10:34 -0400, Mimi Zohar wrote:
> > > > > Jarkko, I think the emphasis should not be on "machine" from
> > > > > Machine Owner Key (MOK), but on "owner".  Whereas Nayna is
> > > > > focusing more on the "_ca" aspect of the name.   Perhaps
> > > > > consider naming it "system_owner_ca" or something along those
> > > > > lines.
> > > > What do you gain such overly long identifier? Makes no sense.
> > > > What is "ca aspect of the name" anyway?
> > > 
> > > As I mentioned previously, the main usage of this new keyring is
> > > that it should contain only CA keys which can be later used to
> > > vouch for user keys loaded onto secondary or IMA keyring at
> > > runtime. Having ca in the  name like .xxxx_ca, would make the
> > > keyring name self-describing. Since you preferred .system, we can
> > > call it .system_ca.
> > 
> > Sounds good to me.  Jarkko?
> > 
> > thanks,
> > 
> > Mimi
> 
> I just wonder what you exactly gain with "_ca"?

Remember, a CA cert is a self signed cert with the CA:TRUE basic
constraint.  Pretty much no secure boot key satisfies this (secure boot
chose deliberately NOT to use CA certificates, so they're all some type
of intermediate or leaf), so the design seems to be only to pick out
the CA certificates you put in the MOK keyring.  Adding the _ca suffix
may deflect some of the "why aren't all my MOK certificates in the
keyring" emails ...

James



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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-25 22:27                 ` James Bottomley
@ 2021-08-27 20:44                   ` Nayna
  2021-08-30 17:39                     ` Eric Snowberg
  2021-09-01  4:34                     ` Jarkko Sakkinen
  0 siblings, 2 replies; 33+ messages in thread
From: Nayna @ 2021-08-27 20:44 UTC (permalink / raw)
  To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, Eric Snowberg,
	David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, pjones, konrad.wilk,
	Patrick Uiterwijk


On 8/25/21 6:27 PM, James Bottomley wrote:
> On Thu, 2021-08-26 at 01:21 +0300, Jarkko Sakkinen wrote:
>> On Tue, 2021-08-24 at 10:34 -0400, Mimi Zohar wrote:
>>>>>> Jarkko, I think the emphasis should not be on "machine" from
>>>>>> Machine Owner Key (MOK), but on "owner".  Whereas Nayna is
>>>>>> focusing more on the "_ca" aspect of the name.   Perhaps
>>>>>> consider naming it "system_owner_ca" or something along those
>>>>>> lines.
>>>>> What do you gain such overly long identifier? Makes no sense.
>>>>> What is "ca aspect of the name" anyway?
>>>> As I mentioned previously, the main usage of this new keyring is
>>>> that it should contain only CA keys which can be later used to
>>>> vouch for user keys loaded onto secondary or IMA keyring at
>>>> runtime. Having ca in the  name like .xxxx_ca, would make the
>>>> keyring name self-describing. Since you preferred .system, we can
>>>> call it .system_ca.
>>> Sounds good to me.  Jarkko?
>>>
>>> thanks,
>>>
>>> Mimi
>> I just wonder what you exactly gain with "_ca"?
> Remember, a CA cert is a self signed cert with the CA:TRUE basic
> constraint.  Pretty much no secure boot key satisfies this (secure boot
> chose deliberately NOT to use CA certificates, so they're all some type
> of intermediate or leaf), so the design seems to be only to pick out
> the CA certificates you put in the MOK keyring.  Adding the _ca suffix
> may deflect some of the "why aren't all my MOK certificates in the
> keyring" emails ...


My understanding is the .system_ca keyring should not be restricted only 
to self-signed CAs (Root CA). Any cert that can qualify as Root or 
Intermediate CA with Basic Constraints CA:TRUE should be allowed. In 
fact, the intermediate CA certificates closest to the leaf nodes would 
be best.

Thanks for bringing up that adding the _ca suffix may deflect some of 
the "why aren't all my MOK certificates in the keyring" emails.

Thanks & Regards,

     - Nayna


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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-27 20:44                   ` Nayna
@ 2021-08-30 17:39                     ` Eric Snowberg
  2021-09-01  0:52                       ` Nayna
  2021-09-01  4:34                     ` Jarkko Sakkinen
  1 sibling, 1 reply; 33+ messages in thread
From: Eric Snowberg @ 2021-08-30 17:39 UTC (permalink / raw)
  To: Nayna, Mimi Zohar
  Cc: James Bottomley, Jarkko Sakkinen, David Howells, keyrings,
	linux-integrity, David Woodhouse, Herbert Xu, David S . Miller,
	James Morris, Serge E . Hallyn, keescook, gregkh, torvalds,
	scott.branden, weiyongjun1, nayna, ebiggers, ardb,
	Lakshmi Ramasubramanian, lszubowi, linux-kernel, linux-crypto,
	linux-security-module, pjones, konrad.wilk, Patrick Uiterwijk


> On Aug 27, 2021, at 2:44 PM, Nayna <nayna@linux.vnet.ibm.com> wrote:
> On 8/25/21 6:27 PM, James Bottomley wrote:
>> 
>> Remember, a CA cert is a self signed cert with the CA:TRUE basic
>> constraint.  Pretty much no secure boot key satisfies this (secure boot
>> chose deliberately NOT to use CA certificates, so they're all some type
>> of intermediate or leaf), so the design seems to be only to pick out
>> the CA certificates you put in the MOK keyring.  Adding the _ca suffix
>> may deflect some of the "why aren't all my MOK certificates in the
>> keyring" emails ...
> 
> 
> My understanding is the .system_ca keyring should not be restricted only
> to self-signed CAs (Root CA). Any cert that can qualify as Root or
> Intermediate CA with Basic Constraints CA:TRUE should be allowed. In
> fact, the intermediate CA certificates closest to the leaf nodes would be
> best.

With an intermediate containing CA:TRUE, the intermediate cert would not 
be self signed. Just for my clarification, does this mean I should remove
the check that validates if it is self signed and instead somehow check if 
the CA flag is set?  Wouldn’t this potentially allow improperly signed certs 
into this new keyring?


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

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


On 8/30/21 1:39 PM, Eric Snowberg wrote:
>> On Aug 27, 2021, at 2:44 PM, Nayna <nayna@linux.vnet.ibm.com> wrote:
>> On 8/25/21 6:27 PM, James Bottomley wrote:
>>> Remember, a CA cert is a self signed cert with the CA:TRUE basic
>>> constraint.  Pretty much no secure boot key satisfies this (secure boot
>>> chose deliberately NOT to use CA certificates, so they're all some type
>>> of intermediate or leaf), so the design seems to be only to pick out
>>> the CA certificates you put in the MOK keyring.  Adding the _ca suffix
>>> may deflect some of the "why aren't all my MOK certificates in the
>>> keyring" emails ...
>>
>> My understanding is the .system_ca keyring should not be restricted only
>> to self-signed CAs (Root CA). Any cert that can qualify as Root or
>> Intermediate CA with Basic Constraints CA:TRUE should be allowed. In
>> fact, the intermediate CA certificates closest to the leaf nodes would be
>> best.
> With an intermediate containing CA:TRUE, the intermediate cert would not
> be self signed. Just for my clarification, does this mean I should remove
> the check that validates if it is self signed and instead somehow check if
> the CA flag is set?  Wouldn’t this potentially allow improperly signed certs
> into this new keyring?
>
In this model, we are relying on the admin to ensure the authenticity of 
the certificate(s) being loaded onto the new keyring. It is similar to 
trusting the admin to enable the variable and add keys to MOK. Following 
are the checks that must pass before adding it to .system_ca keyring.

1. Check against revocation_list.
2. Check Basic Constraints: CA=TRUE.
3. Check keyUsage = keyCertSign.

Thanks & Regards,

        - Nayna


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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-09-01  0:52                       ` Nayna
@ 2021-09-01  1:51                         ` Eric Snowberg
  2021-09-02 10:18                           ` Mimi Zohar
  0 siblings, 1 reply; 33+ messages in thread
From: Eric Snowberg @ 2021-09-01  1:51 UTC (permalink / raw)
  To: Nayna, Mimi Zohar
  Cc: James Bottomley, Jarkko Sakkinen, David Howells, keyrings,
	linux-integrity, David Woodhouse, Herbert Xu, David S . Miller,
	James Morris, Serge E . Hallyn, keescook, gregkh, torvalds,
	scott.branden, weiyongjun1, nayna, ebiggers, ardb,
	Lakshmi Ramasubramanian, lszubowi, linux-kernel, linux-crypto,
	linux-security-module, pjones, konrad.wilk, Patrick Uiterwijk


> On Aug 31, 2021, at 6:52 PM, Nayna <nayna@linux.vnet.ibm.com> wrote:
> 
> 
> On 8/30/21 1:39 PM, Eric Snowberg wrote:
>>> On Aug 27, 2021, at 2:44 PM, Nayna <nayna@linux.vnet.ibm.com> wrote:
>>> On 8/25/21 6:27 PM, James Bottomley wrote:
>>>> Remember, a CA cert is a self signed cert with the CA:TRUE basic
>>>> constraint.  Pretty much no secure boot key satisfies this (secure boot
>>>> chose deliberately NOT to use CA certificates, so they're all some type
>>>> of intermediate or leaf), so the design seems to be only to pick out
>>>> the CA certificates you put in the MOK keyring.  Adding the _ca suffix
>>>> may deflect some of the "why aren't all my MOK certificates in the
>>>> keyring" emails ...
>>> 
>>> My understanding is the .system_ca keyring should not be restricted only
>>> to self-signed CAs (Root CA). Any cert that can qualify as Root or
>>> Intermediate CA with Basic Constraints CA:TRUE should be allowed. In
>>> fact, the intermediate CA certificates closest to the leaf nodes would be
>>> best.
>> With an intermediate containing CA:TRUE, the intermediate cert would not
>> be self signed. Just for my clarification, does this mean I should remove
>> the check that validates if it is self signed and instead somehow check if
>> the CA flag is set?  Wouldn’t this potentially allow improperly signed certs
>> into this new keyring?
>> 
> In this model, we are relying on the admin to ensure the authenticity of the certificate(s) being loaded onto the new keyring. It is similar to trusting the admin to enable the variable and add keys to MOK. Following are the checks that must pass before adding it to .system_ca keyring.
> 
> 1. Check against revocation_list.
> 2. Check Basic Constraints: CA=TRUE.
> 3. Check keyUsage = keyCertSign.

Originally I thought the request to only load CA certs into this new keyring 
was so root of trust could be validated for the entire chain.  If a portion
of the model now relies on the admin to ensure authenticity, and the complete
chain is not needed, why not have the admin also check for #2 and #3? Meaning,
when the Kconfig option is enabled and the new MokListTrustedRT UEFI is set, 
whatever the admin has placed in the MOKList goes into this new keyring.


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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-08-27 20:44                   ` Nayna
  2021-08-30 17:39                     ` Eric Snowberg
@ 2021-09-01  4:34                     ` Jarkko Sakkinen
  2021-09-01  4:36                       ` Jarkko Sakkinen
  1 sibling, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2021-09-01  4:34 UTC (permalink / raw)
  To: Nayna, James Bottomley, Mimi Zohar, Eric Snowberg, David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, pjones, konrad.wilk,
	Patrick Uiterwijk

On Fri, 2021-08-27 at 16:44 -0400, Nayna wrote:
> On 8/25/21 6:27 PM, James Bottomley wrote:
> > On Thu, 2021-08-26 at 01:21 +0300, Jarkko Sakkinen wrote:
> > > On Tue, 2021-08-24 at 10:34 -0400, Mimi Zohar wrote:
> > > > > > > Jarkko, I think the emphasis should not be on "machine" from
> > > > > > > Machine Owner Key (MOK), but on "owner".  Whereas Nayna is
> > > > > > > focusing more on the "_ca" aspect of the name.   Perhaps
> > > > > > > consider naming it "system_owner_ca" or something along those
> > > > > > > lines.
> > > > > > What do you gain such overly long identifier? Makes no sense.
> > > > > > What is "ca aspect of the name" anyway?
> > > > > As I mentioned previously, the main usage of this new keyring is
> > > > > that it should contain only CA keys which can be later used to
> > > > > vouch for user keys loaded onto secondary or IMA keyring at
> > > > > runtime. Having ca in the  name like .xxxx_ca, would make the
> > > > > keyring name self-describing. Since you preferred .system, we can
> > > > > call it .system_ca.
> > > > Sounds good to me.  Jarkko?
> > > > 
> > > > thanks,
> > > > 
> > > > Mimi
> > > I just wonder what you exactly gain with "_ca"?
> > Remember, a CA cert is a self signed cert with the CA:TRUE basic
> > constraint.  Pretty much no secure boot key satisfies this (secure boot
> > chose deliberately NOT to use CA certificates, so they're all some type
> > of intermediate or leaf), so the design seems to be only to pick out
> > the CA certificates you put in the MOK keyring.  Adding the _ca suffix
> > may deflect some of the "why aren't all my MOK certificates in the
> > keyring" emails ...
> 
> My understanding is the .system_ca keyring should not be restricted only 
> to self-signed CAs (Root CA). Any cert that can qualify as Root or 
> Intermediate CA with Basic Constraints CA:TRUE should be allowed. In 
> fact, the intermediate CA certificates closest to the leaf nodes would 
> be best.
> 
> Thanks for bringing up that adding the _ca suffix may deflect some of 
> the "why aren't all my MOK certificates in the keyring" emails.

What the heck is the pragamatic gain of adding such a suffix? Makes
zero sense.

/Jarkko

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

* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
  2021-09-01  4:34                     ` Jarkko Sakkinen
@ 2021-09-01  4:36                       ` Jarkko Sakkinen
  2021-09-01  4:46                         ` Jarkko Sakkinen
  0 siblings, 1 reply; 33+ messages in thread
From: Jarkko Sakkinen @ 2021-09-01  4:36 UTC (permalink / raw)
  To: Nayna, James Bottomley, Mimi Zohar, Eric Snowberg, David Howells
  Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
	David S . Miller, James Morris, Serge E . Hallyn, keescook,
	gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
	ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
	linux-crypto, linux-security-module, pjones, konrad.wilk,
	Patrick Uiterwijk

On Wed, 2021-09-01 at 07:34 +0300, Jarkko Sakkinen wrote:
> On Fri, 2021-08-27 at 16:44 -0400, Nayna wrote:
> > On 8/25/21 6:27 PM, James Bottomley wrote:
> > > On Thu, 2021-08-26 at 01:21 +0300, Jarkko Sakkinen wrote:
> > > > On Tue, 2021-08-24 at 10:34 -0400, Mimi Zohar wrote:
> > > > > > > > Jarkko, I think the emphasis should not be on "machine" from
> > > > > > > > Machine Owner Key (MOK), but on "owner".  Whereas Nayna is
> > > > > > > > focusing more on the "_ca" aspect of the name.   Perhaps
> > > > > > > > consider naming it "system_owner_ca" or something along those
> > > > > > > > lines.
> > > > > > > What do you gain such overly long identifier? Makes no sense.
> > > > > > > What is "ca aspect of the name" anyway?
> > > > > > As I mentioned previously, the main usage of this new keyring is
> > > > > > that it should contain only CA keys which can be later used to
> > > > > > vouch for user keys loaded onto secondary or IMA keyring at
> > > > > > runtime. Having ca in the  name like .xxxx_ca, would make the
> > > > > > keyring name self-describing. Since you preferred .system, we can
> > > > > > call it .system_ca.
> > > > > Sounds good to me.  Jarkko?
> > > > > 
> > > > > thanks,
> > > > > 
> > > > > Mimi
> > > > I just wonder what you exactly gain with "_ca"?
> > > Remember, a CA cert is a self signed cert with the CA:TRUE basic
> > > constraint.  Pretty much no secure boot key satisfies this (secure boot
> > > chose deliberately NOT to use CA certificates, so they're all some type
> > > of intermediate or leaf), so the design seems to be only to pick out
> > > the CA certificates you put in the MOK keyring.  Adding the _ca suffix
> > > may deflect some of the "why aren't all my MOK certificates in the
> > > keyring" emails ...
> > 
> > My understanding is the .system_ca keyring should not be restricted only 
> > to self-signed CAs (Root CA). Any cert that can qualify as Root or 
> > Intermediate CA with Basic Constraints CA:TRUE should be allowed. In 
> > fact, the intermediate CA certificates closest to the leaf nodes would 
> > be best.
> > 
> > Thanks for bringing up that adding the _ca suffix may deflect some of 
> > the "why aren't all my MOK certificates in the keyring" emails.
> 
> What the heck is the pragamatic gain of adding such a suffix? Makes
> zero sense

If this series needs both "system" and "system_ca" keyrings, then
there would be some sanity in this.

Also, I still *fully* lack understanding of the use of word system.

Why MOK is not SOK then??

/Jarkko

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

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

On Wed, 2021-09-01 at 07:36 +0300, Jarkko Sakkinen wrote:
> On Wed, 2021-09-01 at 07:34 +0300, Jarkko Sakkinen wrote:
> > On Fri, 2021-08-27 at 16:44 -0400, Nayna wrote:
> > > On 8/25/21 6:27 PM, James Bottomley wrote:
> > > > On Thu, 2021-08-26 at 01:21 +0300, Jarkko Sakkinen wrote:
> > > > > On Tue, 2021-08-24 at 10:34 -0400, Mimi Zohar wrote:
> > > > > > > > > Jarkko, I think the emphasis should not be on "machine" from
> > > > > > > > > Machine Owner Key (MOK), but on "owner".  Whereas Nayna is
> > > > > > > > > focusing more on the "_ca" aspect of the name.   Perhaps
> > > > > > > > > consider naming it "system_owner_ca" or something along those
> > > > > > > > > lines.
> > > > > > > > What do you gain such overly long identifier? Makes no sense.
> > > > > > > > What is "ca aspect of the name" anyway?
> > > > > > > As I mentioned previously, the main usage of this new keyring is
> > > > > > > that it should contain only CA keys which can be later used to
> > > > > > > vouch for user keys loaded onto secondary or IMA keyring at
> > > > > > > runtime. Having ca in the  name like .xxxx_ca, would make the
> > > > > > > keyring name self-describing. Since you preferred .system, we can
> > > > > > > call it .system_ca.
> > > > > > Sounds good to me.  Jarkko?
> > > > > > 
> > > > > > thanks,
> > > > > > 
> > > > > > Mimi
> > > > > I just wonder what you exactly gain with "_ca"?
> > > > Remember, a CA cert is a self signed cert with the CA:TRUE basic
> > > > constraint.  Pretty much no secure boot key satisfies this (secure boot
> > > > chose deliberately NOT to use CA certificates, so they're all some type
> > > > of intermediate or leaf), so the design seems to be only to pick out
> > > > the CA certificates you put in the MOK keyring.  Adding the _ca suffix
> > > > may deflect some of the "why aren't all my MOK certificates in the
> > > > keyring" emails ...
> > > 
> > > My understanding is the .system_ca keyring should not be restricted only 
> > > to self-signed CAs (Root CA). Any cert that can qualify as Root or 
> > > Intermediate CA with Basic Constraints CA:TRUE should be allowed. In 
> > > fact, the intermediate CA certificates closest to the leaf nodes would 
> > > be best.
> > > 
> > > Thanks for bringing up that adding the _ca suffix may deflect some of 
> > > the "why aren't all my MOK certificates in the keyring" emails.
> > 
> > What the heck is the pragamatic gain of adding such a suffix? Makes
> > zero sense
> 
> If this series needs both "system" and "system_ca" keyrings, then
> there would be some sanity in this.
> 
> Also, I still *fully* lack understanding of the use of word system.
> 
> Why MOK is not SOK then??

Please just call it "machine". You have machines that hold the keyring.

"system" does not mean anything concrete. I don't know what a "system"
is.

/Jarkko

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

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

On Tue, 2021-08-31 at 19:51 -0600, Eric Snowberg wrote:
> > On Aug 31, 2021, at 6:52 PM, Nayna <nayna@linux.vnet.ibm.com> wrote:
> > On 8/30/21 1:39 PM, Eric Snowberg wrote:
> >>> On Aug 27, 2021, at 2:44 PM, Nayna <nayna@linux.vnet.ibm.com> wrote:
> >>> On 8/25/21 6:27 PM, James Bottomley wrote:
> >>>> Remember, a CA cert is a self signed cert with the CA:TRUE basic
> >>>> constraint.  Pretty much no secure boot key satisfies this (secure boot
> >>>> chose deliberately NOT to use CA certificates, so they're all some type
> >>>> of intermediate or leaf), so the design seems to be only to pick out
> >>>> the CA certificates you put in the MOK keyring.  Adding the _ca suffix
> >>>> may deflect some of the "why aren't all my MOK certificates in the
> >>>> keyring" emails ...
> >>> 
> >>> My understanding is the .system_ca keyring should not be restricted only
> >>> to self-signed CAs (Root CA). Any cert that can qualify as Root or
> >>> Intermediate CA with Basic Constraints CA:TRUE should be allowed. In
> >>> fact, the intermediate CA certificates closest to the leaf nodes would be
> >>> best.
> >> With an intermediate containing CA:TRUE, the intermediate cert would not
> >> be self signed. Just for my clarification, does this mean I should remove
> >> the check that validates if it is self signed and instead somehow check if
> >> the CA flag is set?  Wouldn’t this potentially allow improperly signed certs
> >> into this new keyring?
> >> 
> > In this model, we are relying on the admin to ensure the authenticity of the certificate(s) being loaded onto the new keyring. It is similar to trusting the admin to enable the variable and add keys to MOK. Following are the checks that must pass before adding it to .system_ca keyring.
> > 
> > 1. Check against revocation_list.
> > 2. Check Basic Constraints: CA=TRUE.
> > 3. Check keyUsage = keyCertSign.
> 
> Originally I thought the request to only load CA certs into this new keyring 
> was so root of trust could be validated for the entire chain.  If a portion
> of the model now relies on the admin to ensure authenticity, and the complete
> chain is not needed, why not have the admin also check for #2 and #3? Meaning,
> when the Kconfig option is enabled and the new MokListTrustedRT UEFI is set, 
> whatever the admin has placed in the MOKList goes into this new keyring.

The root of trust for the new "machine" keyring, at least in the UEFI
use case, is registering keys in the MOK db, which requires physical
presence.  So we're trusting the MOK db, which means we're really
trusting both the admin and UEFI to do the right things.  There is no
harm in verifying the CA assumption when loading the certs onto the
"machine" keyring.

From an IMA perspective, all that is needed to sign an IMA custom
policy and local code is the ability to load a single self-signed CA
certificate.  So the self-signed CA restriction is fine.  Obviously
other use cases are being discussed here.  If the other use cases want
to relax the self-signed CA restriction to allow intermediary CA's, it
should be explicitly called out in a separate patch, with its own patch
description, providing the motivation.

thanks,

Mimi


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

end of thread, other threads:[~2021-09-02 10:19 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-19  0:20 [PATCH v4 00/12] Enroll kernel keys thru MOK Eric Snowberg
2021-08-19  0:20 ` [PATCH v4 01/12] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK) Eric Snowberg
2021-08-19  0:20 ` [PATCH v4 02/12] integrity: Do not allow mok keyring updates following init Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 03/12] KEYS: CA link restriction Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 04/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_ca Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 05/12] integrity: add new keyring handler for mok keys Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 06/12] KEYS: add a reference to mok keyring Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 07/12] KEYS: Introduce link restriction to include builtin, secondary and mok keys Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 08/12] KEYS: integrity: change link restriction to trust the mok keyring Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 09/12] KEYS: link secondary_trusted_keys to mok trusted keys Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 10/12] integrity: store reference to mok keyring Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 11/12] integrity: Trust MOK keys if MokListTrustedRT found Eric Snowberg
2021-08-19  0:21 ` [PATCH v4 12/12] integrity: Only use mok keyring when uefi_check_trust_mok_keys is true Eric Snowberg
2021-08-19 11:38 ` [PATCH v4 00/12] Enroll kernel keys thru MOK Jarkko Sakkinen
2021-08-19 13:10   ` Mimi Zohar
2021-08-19 15:23     ` Eric Snowberg
2021-08-19 17:32       ` Mimi Zohar
2021-08-23 17:51         ` Jarkko Sakkinen
2021-08-23 20:48           ` Nayna
2021-08-24 14:34             ` Mimi Zohar
2021-08-25 22:21               ` Jarkko Sakkinen
2021-08-25 22:27                 ` James Bottomley
2021-08-27 20:44                   ` Nayna
2021-08-30 17:39                     ` Eric Snowberg
2021-09-01  0:52                       ` Nayna
2021-09-01  1:51                         ` Eric Snowberg
2021-09-02 10:18                           ` Mimi Zohar
2021-09-01  4:34                     ` Jarkko Sakkinen
2021-09-01  4:36                       ` Jarkko Sakkinen
2021-09-01  4:46                         ` Jarkko Sakkinen
2021-08-23 17:37       ` Jarkko Sakkinen
2021-08-23 17:35     ` Jarkko Sakkinen
2021-08-23 17:48       ` Eric Snowberg

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