kexec.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
@ 2022-05-12  7:01 Coiby Xu
  2022-05-12  7:01 ` [PATCH v8 1/4] kexec: clean up arch_kexec_kernel_verify_sig Coiby Xu
                   ` (4 more replies)
  0 siblings, 5 replies; 30+ messages in thread
From: Coiby Xu @ 2022-05-12  7:01 UTC (permalink / raw)
  To: kexec

Currently, a problem faced by arm64 is if a kernel image is signed by a
MOK key, loading it via the kexec_file_load() system call would be
rejected with the error "Lockdown: kexec: kexec of unsigned images is
restricted; see man kernel_lockdown.7". This happens because arm64 uses
only the primary keyring i.e. the .builtin_trusted_keys keyring that
contains only kernel built-in keys to verify the kexec kernel image. MOK
keys are loaded into the .platform keyring or/and .machine keyring. The
.machine keyring is linked to the secondary keyring i.e.
.secondary_trusted_keys keyring when the end-user chooses to trust MOK
keys. The platform keyring is exclusively used for kexec kernel image
verification and .secondary_trusted_keys together with
.builtin_trusted_keys are the system trusted keyrings. So obviously
there is no reason to not use .secondary_trusted_keys or .platform
keyring for kernel image signature verification. 

Similarly, s390 only uses platform keyring for kernel image signature
verification and built-in keys and secondary keyring are not used.

This patch set allows arm64 and s390 to use more system keyrings
including the .secondary_trusted_keys and .platform keyring to verify
kexec kernel image signature as x86 does.

The 3rd arm64 patch depends on the first two patches. The 4th s390 patch
can be applied independently.

v8:
 - drop "Cc: stable at vger.kernel.org" for the first two prerequisite
   patches [Baoquan]

v7:
 - drop the Fixes tag for the 2nd patch and add patch prerequisites
   [Baoquan]
 - improve cover letter

v6:
 - integrate the first three patches of "[PATCH 0/4] Unifrom keyring
   support across architectures and functions" from Michal [1]
 - improve commit message [Baoquan, Michal]
 - directly assign kexec_kernel_verify_pe_sig to
   kexec_file_ops->verify_sig [Michal]

v5:
 - improve commit message [Baoquan]

v4:
 - fix commit reference format issue and other checkpatch.pl warnings [Baoquan]

v3:
 - s/arch_kexec_kernel_verify_pe_sig/kexec_kernel_verify_pe_sig [Eric]
 - clean up arch_kexec_kernel_verify_sig [Eric]

v2:
 - only x86_64 and arm64 need to enable PE file signature check [Dave]

[1] https://lore.kernel.org/lkml/cover.1644953683.git.msuchanek at suse.de/

Coiby Xu (3):
  kexec: clean up arch_kexec_kernel_verify_sig
  kexec, KEYS: make the code in bzImage64_verify_sig generic
  arm64: kexec_file: use more system keyrings to verify kernel image
    signature

Michal Suchanek (1):
  kexec, KEYS, s390: Make use of built-in and secondary keyring for
    signature verification

 arch/arm64/kernel/kexec_image.c       | 11 +-----
 arch/s390/kernel/machine_kexec_file.c | 18 +++++++---
 arch/x86/kernel/kexec-bzimage64.c     | 20 +----------
 include/linux/kexec.h                 |  7 ++--
 kernel/kexec_file.c                   | 51 ++++++++++++++++-----------
 5 files changed, 50 insertions(+), 57 deletions(-)

-- 
2.35.3



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

* [PATCH v8 1/4] kexec: clean up arch_kexec_kernel_verify_sig
  2022-05-12  7:01 [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Coiby Xu
@ 2022-05-12  7:01 ` Coiby Xu
  2022-06-09 21:57   ` Mimi Zohar
  2022-05-12  7:01 ` [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic Coiby Xu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 30+ messages in thread
From: Coiby Xu @ 2022-05-12  7:01 UTC (permalink / raw)
  To: kexec

Currently there is no arch-specific implementation of
arch_kexec_kernel_verify_sig. Even if we want to add an implementation
for an architecture in the future, we can simply use "(struct
kexec_file_ops*)->verify_sig". So clean it up.

Suggested-by: Eric W. Biederman <ebiederm@xmission.com>
Reviewed-by: Michal Suchanek <msuchanek@suse.de>
Acked-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 include/linux/kexec.h |  4 ----
 kernel/kexec_file.c   | 34 +++++++++++++---------------------
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 58d1b58a971e..413235c6c797 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -202,10 +202,6 @@ int arch_kexec_apply_relocations(struct purgatory_info *pi,
 				 const Elf_Shdr *relsec,
 				 const Elf_Shdr *symtab);
 int arch_kimage_file_post_load_cleanup(struct kimage *image);
-#ifdef CONFIG_KEXEC_SIG
-int arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
-				 unsigned long buf_len);
-#endif
 int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
 
 extern int kexec_add_buffer(struct kexec_buf *kbuf);
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 8347fc158d2b..3720435807eb 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -89,25 +89,6 @@ int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
 	return kexec_image_post_load_cleanup_default(image);
 }
 
-#ifdef CONFIG_KEXEC_SIG
-static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
-					  unsigned long buf_len)
-{
-	if (!image->fops || !image->fops->verify_sig) {
-		pr_debug("kernel loader does not support signature verification.\n");
-		return -EKEYREJECTED;
-	}
-
-	return image->fops->verify_sig(buf, buf_len);
-}
-
-int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
-					unsigned long buf_len)
-{
-	return kexec_image_verify_sig_default(image, buf, buf_len);
-}
-#endif
-
 /*
  * arch_kexec_apply_relocations_add - apply relocations of type RELA
  * @pi:		Purgatory to be relocated.
@@ -184,13 +165,24 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 }
 
 #ifdef CONFIG_KEXEC_SIG
+static int kexec_image_verify_sig(struct kimage *image, void *buf,
+		unsigned long buf_len)
+{
+	if (!image->fops || !image->fops->verify_sig) {
+		pr_debug("kernel loader does not support signature verification.\n");
+		return -EKEYREJECTED;
+	}
+
+	return image->fops->verify_sig(buf, buf_len);
+}
+
 static int
 kimage_validate_signature(struct kimage *image)
 {
 	int ret;
 
-	ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
-					   image->kernel_buf_len);
+	ret = kexec_image_verify_sig(image, image->kernel_buf,
+			image->kernel_buf_len);
 	if (ret) {
 
 		if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
-- 
2.35.3



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

* [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic
  2022-05-12  7:01 [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Coiby Xu
  2022-05-12  7:01 ` [PATCH v8 1/4] kexec: clean up arch_kexec_kernel_verify_sig Coiby Xu
@ 2022-05-12  7:01 ` Coiby Xu
  2022-05-12  7:21   ` Baoquan He
  2022-06-09 22:18   ` Mimi Zohar
  2022-05-12  7:01 ` [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature Coiby Xu
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 30+ messages in thread
From: Coiby Xu @ 2022-05-12  7:01 UTC (permalink / raw)
  To: kexec

commit 278311e417be ("kexec, KEYS: Make use of platform keyring for
signature verify") adds platform keyring support on x86 kexec but not
arm64.

The code in bzImage64_verify_sig makes use of system keyrings including
.buitin_trusted_keys, .secondary_trusted_keys and .platform keyring to
verify signed kernel image as PE file. Make it generic so both x86_64
and arm64 can use it.

Cc: kexec at lists.infradead.org
Cc: keyrings at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Reviewed-by: Michal Suchanek <msuchanek@suse.de>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 arch/x86/kernel/kexec-bzimage64.c | 20 +-------------------
 include/linux/kexec.h             |  7 +++++++
 kernel/kexec_file.c               | 17 +++++++++++++++++
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd68b1f..f299b48f9c9f 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -17,7 +17,6 @@
 #include <linux/kernel.h>
 #include <linux/mm.h>
 #include <linux/efi.h>
-#include <linux/verification.h>
 
 #include <asm/bootparam.h>
 #include <asm/setup.h>
@@ -528,28 +527,11 @@ static int bzImage64_cleanup(void *loader_data)
 	return 0;
 }
 
-#ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
-static int bzImage64_verify_sig(const char *kernel, unsigned long kernel_len)
-{
-	int ret;
-
-	ret = verify_pefile_signature(kernel, kernel_len,
-				      VERIFY_USE_SECONDARY_KEYRING,
-				      VERIFYING_KEXEC_PE_SIGNATURE);
-	if (ret == -ENOKEY && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) {
-		ret = verify_pefile_signature(kernel, kernel_len,
-					      VERIFY_USE_PLATFORM_KEYRING,
-					      VERIFYING_KEXEC_PE_SIGNATURE);
-	}
-	return ret;
-}
-#endif
-
 const struct kexec_file_ops kexec_bzImage64_ops = {
 	.probe = bzImage64_probe,
 	.load = bzImage64_load,
 	.cleanup = bzImage64_cleanup,
 #ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
-	.verify_sig = bzImage64_verify_sig,
+	.verify_sig = kexec_kernel_verify_pe_sig,
 #endif
 };
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 413235c6c797..da83abfc628b 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -19,6 +19,7 @@
 #include <asm/io.h>
 
 #include <uapi/linux/kexec.h>
+#include <linux/verification.h>
 
 /* Location of a reserved region to hold the crash kernel.
  */
@@ -202,6 +203,12 @@ int arch_kexec_apply_relocations(struct purgatory_info *pi,
 				 const Elf_Shdr *relsec,
 				 const Elf_Shdr *symtab);
 int arch_kimage_file_post_load_cleanup(struct kimage *image);
+#ifdef CONFIG_KEXEC_SIG
+#ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
+int kexec_kernel_verify_pe_sig(const char *kernel,
+				    unsigned long kernel_len);
+#endif
+#endif
 int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
 
 extern int kexec_add_buffer(struct kexec_buf *kbuf);
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 3720435807eb..754885b96aab 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -165,6 +165,23 @@ void kimage_file_post_load_cleanup(struct kimage *image)
 }
 
 #ifdef CONFIG_KEXEC_SIG
+#ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
+int kexec_kernel_verify_pe_sig(const char *kernel, unsigned long kernel_len)
+{
+	int ret;
+
+	ret = verify_pefile_signature(kernel, kernel_len,
+				      VERIFY_USE_SECONDARY_KEYRING,
+				      VERIFYING_KEXEC_PE_SIGNATURE);
+	if (ret == -ENOKEY && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) {
+		ret = verify_pefile_signature(kernel, kernel_len,
+					      VERIFY_USE_PLATFORM_KEYRING,
+					      VERIFYING_KEXEC_PE_SIGNATURE);
+	}
+	return ret;
+}
+#endif
+
 static int kexec_image_verify_sig(struct kimage *image, void *buf,
 		unsigned long buf_len)
 {
-- 
2.35.3



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

* [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature
  2022-05-12  7:01 [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Coiby Xu
  2022-05-12  7:01 ` [PATCH v8 1/4] kexec: clean up arch_kexec_kernel_verify_sig Coiby Xu
  2022-05-12  7:01 ` [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic Coiby Xu
@ 2022-05-12  7:01 ` Coiby Xu
  2022-06-09 23:15   ` Mimi Zohar
  2022-05-12  7:01 ` [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification Coiby Xu
  2022-05-20 17:04 ` [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Mimi Zohar
  4 siblings, 1 reply; 30+ messages in thread
From: Coiby Xu @ 2022-05-12  7:01 UTC (permalink / raw)
  To: kexec

Currently, a problem faced by arm64 is if a kernel image is signed by a
MOK key, loading it via the kexec_file_load() system call would be
rejected with the error "Lockdown: kexec: kexec of unsigned images is
restricted; see man kernel_lockdown.7".

This happens because image_verify_sig uses only the primary keyring that
contains only kernel built-in keys to verify the kexec image.

This patch allows to verify arm64 kernel image signature using not only
.builtin_trusted_keys but also .platform and .secondary_trusted_keys
keyring.

Fixes: 732b7b93d849 ("arm64: kexec_file: add kernel signature verification support")
Cc: stable at vger.kernel.org # 34d5960af253: kexec: clean up arch_kexec_kernel_verify_sig
Cc: stable at vger.kernel.org # 83b7bb2d49ae: kexec, KEYS: make the code in bzImage64_verify_sig generic
Acked-by: Baoquan He <bhe@redhat.com>
Cc: kexec at lists.infradead.org
Cc: keyrings at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Co-developed-by: Michal Suchanek <msuchanek@suse.de>
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 arch/arm64/kernel/kexec_image.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
index 9ec34690e255..5ed6a585f21f 100644
--- a/arch/arm64/kernel/kexec_image.c
+++ b/arch/arm64/kernel/kexec_image.c
@@ -14,7 +14,6 @@
 #include <linux/kexec.h>
 #include <linux/pe.h>
 #include <linux/string.h>
-#include <linux/verification.h>
 #include <asm/byteorder.h>
 #include <asm/cpufeature.h>
 #include <asm/image.h>
@@ -130,18 +129,10 @@ static void *image_load(struct kimage *image,
 	return NULL;
 }
 
-#ifdef CONFIG_KEXEC_IMAGE_VERIFY_SIG
-static int image_verify_sig(const char *kernel, unsigned long kernel_len)
-{
-	return verify_pefile_signature(kernel, kernel_len, NULL,
-				       VERIFYING_KEXEC_PE_SIGNATURE);
-}
-#endif
-
 const struct kexec_file_ops kexec_image_ops = {
 	.probe = image_probe,
 	.load = image_load,
 #ifdef CONFIG_KEXEC_IMAGE_VERIFY_SIG
-	.verify_sig = image_verify_sig,
+	.verify_sig = kexec_kernel_verify_pe_sig,
 #endif
 };
-- 
2.35.3



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

* [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification
  2022-05-12  7:01 [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Coiby Xu
                   ` (2 preceding siblings ...)
  2022-05-12  7:01 ` [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature Coiby Xu
@ 2022-05-12  7:01 ` Coiby Xu
  2022-05-18 11:29   ` Heiko Carstens
  2022-05-20 17:04 ` [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Mimi Zohar
  4 siblings, 1 reply; 30+ messages in thread
From: Coiby Xu @ 2022-05-12  7:01 UTC (permalink / raw)
  To: kexec

From: Michal Suchanek <msuchanek@suse.de>

commit e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
adds support for KEXEC_SIG verification with keys from platform keyring
but the built-in keys and secondary keyring are not used.

Add support for the built-in keys and secondary keyring as x86 does.

Fixes: e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
Cc: stable at vger.kernel.org
Cc: Philipp Rudo <prudo@linux.ibm.com>
Cc: kexec at lists.infradead.org
Cc: keyrings at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
Acked-by: Baoquan He <bhe@redhat.com>
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 arch/s390/kernel/machine_kexec_file.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/arch/s390/kernel/machine_kexec_file.c b/arch/s390/kernel/machine_kexec_file.c
index 8f43575a4dd3..fc6d5f58debe 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -31,6 +31,7 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 	const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
 	struct module_signature *ms;
 	unsigned long sig_len;
+	int ret;
 
 	/* Skip signature verification when not secure IPLed. */
 	if (!ipl_secure_flag)
@@ -65,11 +66,18 @@ int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 		return -EBADMSG;
 	}
 
-	return verify_pkcs7_signature(kernel, kernel_len,
-				      kernel + kernel_len, sig_len,
-				      VERIFY_USE_PLATFORM_KEYRING,
-				      VERIFYING_MODULE_SIGNATURE,
-				      NULL, NULL);
+	ret = verify_pkcs7_signature(kernel, kernel_len,
+				     kernel + kernel_len, sig_len,
+				     VERIFY_USE_SECONDARY_KEYRING,
+				     VERIFYING_MODULE_SIGNATURE,
+				     NULL, NULL);
+	if (ret == -ENOKEY && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING))
+		ret = verify_pkcs7_signature(kernel, kernel_len,
+					     kernel + kernel_len, sig_len,
+					     VERIFY_USE_PLATFORM_KEYRING,
+					     VERIFYING_MODULE_SIGNATURE,
+					     NULL, NULL);
+	return ret;
 }
 #endif /* CONFIG_KEXEC_SIG */
 
-- 
2.35.3



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

* [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic
  2022-05-12  7:01 ` [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic Coiby Xu
@ 2022-05-12  7:21   ` Baoquan He
  2022-06-09 22:18   ` Mimi Zohar
  1 sibling, 0 replies; 30+ messages in thread
From: Baoquan He @ 2022-05-12  7:21 UTC (permalink / raw)
  To: kexec

On 05/12/22 at 03:01pm, Coiby Xu wrote:
> commit 278311e417be ("kexec, KEYS: Make use of platform keyring for
> signature verify") adds platform keyring support on x86 kexec but not
> arm64.
> 
> The code in bzImage64_verify_sig makes use of system keyrings including
> .buitin_trusted_keys, .secondary_trusted_keys and .platform keyring to
> verify signed kernel image as PE file. Make it generic so both x86_64
> and arm64 can use it.

LGTM,

Acked-by: Baoquan He <bhe@redhat.com>

> 
> Cc: kexec at lists.infradead.org
> Cc: keyrings at vger.kernel.org
> Cc: linux-security-module at vger.kernel.org
> Reviewed-by: Michal Suchanek <msuchanek@suse.de>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
> ---
>  arch/x86/kernel/kexec-bzimage64.c | 20 +-------------------
>  include/linux/kexec.h             |  7 +++++++
>  kernel/kexec_file.c               | 17 +++++++++++++++++
>  3 files changed, 25 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 170d0fd68b1f..f299b48f9c9f 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -17,7 +17,6 @@
>  #include <linux/kernel.h>
>  #include <linux/mm.h>
>  #include <linux/efi.h>
> -#include <linux/verification.h>
>  
>  #include <asm/bootparam.h>
>  #include <asm/setup.h>
> @@ -528,28 +527,11 @@ static int bzImage64_cleanup(void *loader_data)
>  	return 0;
>  }
>  
> -#ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
> -static int bzImage64_verify_sig(const char *kernel, unsigned long kernel_len)
> -{
> -	int ret;
> -
> -	ret = verify_pefile_signature(kernel, kernel_len,
> -				      VERIFY_USE_SECONDARY_KEYRING,
> -				      VERIFYING_KEXEC_PE_SIGNATURE);
> -	if (ret == -ENOKEY && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) {
> -		ret = verify_pefile_signature(kernel, kernel_len,
> -					      VERIFY_USE_PLATFORM_KEYRING,
> -					      VERIFYING_KEXEC_PE_SIGNATURE);
> -	}
> -	return ret;
> -}
> -#endif
> -
>  const struct kexec_file_ops kexec_bzImage64_ops = {
>  	.probe = bzImage64_probe,
>  	.load = bzImage64_load,
>  	.cleanup = bzImage64_cleanup,
>  #ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
> -	.verify_sig = bzImage64_verify_sig,
> +	.verify_sig = kexec_kernel_verify_pe_sig,
>  #endif
>  };
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 413235c6c797..da83abfc628b 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -19,6 +19,7 @@
>  #include <asm/io.h>
>  
>  #include <uapi/linux/kexec.h>
> +#include <linux/verification.h>
>  
>  /* Location of a reserved region to hold the crash kernel.
>   */
> @@ -202,6 +203,12 @@ int arch_kexec_apply_relocations(struct purgatory_info *pi,
>  				 const Elf_Shdr *relsec,
>  				 const Elf_Shdr *symtab);
>  int arch_kimage_file_post_load_cleanup(struct kimage *image);
> +#ifdef CONFIG_KEXEC_SIG
> +#ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
> +int kexec_kernel_verify_pe_sig(const char *kernel,
> +				    unsigned long kernel_len);
> +#endif
> +#endif
>  int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
>  
>  extern int kexec_add_buffer(struct kexec_buf *kbuf);
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index 3720435807eb..754885b96aab 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -165,6 +165,23 @@ void kimage_file_post_load_cleanup(struct kimage *image)
>  }
>  
>  #ifdef CONFIG_KEXEC_SIG
> +#ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
> +int kexec_kernel_verify_pe_sig(const char *kernel, unsigned long kernel_len)
> +{
> +	int ret;
> +
> +	ret = verify_pefile_signature(kernel, kernel_len,
> +				      VERIFY_USE_SECONDARY_KEYRING,
> +				      VERIFYING_KEXEC_PE_SIGNATURE);
> +	if (ret == -ENOKEY && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) {
> +		ret = verify_pefile_signature(kernel, kernel_len,
> +					      VERIFY_USE_PLATFORM_KEYRING,
> +					      VERIFYING_KEXEC_PE_SIGNATURE);
> +	}
> +	return ret;
> +}
> +#endif
> +
>  static int kexec_image_verify_sig(struct kimage *image, void *buf,
>  		unsigned long buf_len)
>  {
> -- 
> 2.35.3
> 



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

* [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification
  2022-05-12  7:01 ` [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification Coiby Xu
@ 2022-05-18 11:29   ` Heiko Carstens
  2022-05-19  0:39     ` Baoquan He
  0 siblings, 1 reply; 30+ messages in thread
From: Heiko Carstens @ 2022-05-18 11:29 UTC (permalink / raw)
  To: kexec

On Thu, May 12, 2022 at 03:01:23PM +0800, Coiby Xu wrote:
> From: Michal Suchanek <msuchanek@suse.de>
> 
> commit e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> adds support for KEXEC_SIG verification with keys from platform keyring
> but the built-in keys and secondary keyring are not used.
> 
> Add support for the built-in keys and secondary keyring as x86 does.
> 
> Fixes: e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> Cc: stable at vger.kernel.org
> Cc: Philipp Rudo <prudo@linux.ibm.com>
> Cc: kexec at lists.infradead.org
> Cc: keyrings at vger.kernel.org
> Cc: linux-security-module at vger.kernel.org
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
> Acked-by: Baoquan He <bhe@redhat.com>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
> ---
>  arch/s390/kernel/machine_kexec_file.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)

As far as I can tell this doesn't have any dependency to the other
patches in this series, so should I pick this up for the s390 tree, or
how will this go upstream?

In any case:
Acked-by: Heiko Carstens <hca@linux.ibm.com>


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

* [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification
  2022-05-18 11:29   ` Heiko Carstens
@ 2022-05-19  0:39     ` Baoquan He
  2022-05-19 11:56       ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Baoquan He @ 2022-05-19  0:39 UTC (permalink / raw)
  To: kexec

On 05/18/22 at 01:29pm, Heiko Carstens wrote:
> On Thu, May 12, 2022 at 03:01:23PM +0800, Coiby Xu wrote:
> > From: Michal Suchanek <msuchanek@suse.de>
> > 
> > commit e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> > adds support for KEXEC_SIG verification with keys from platform keyring
> > but the built-in keys and secondary keyring are not used.
> > 
> > Add support for the built-in keys and secondary keyring as x86 does.
> > 
> > Fixes: e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> > Cc: stable at vger.kernel.org
> > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > Cc: kexec at lists.infradead.org
> > Cc: keyrings at vger.kernel.org
> > Cc: linux-security-module at vger.kernel.org
> > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
> > Acked-by: Baoquan He <bhe@redhat.com>
> > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > ---
> >  arch/s390/kernel/machine_kexec_file.c | 18 +++++++++++++-----
> >  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> As far as I can tell this doesn't have any dependency to the other
> patches in this series, so should I pick this up for the s390 tree, or
> how will this go upstream?

Thanks, Heiko.

I want to ask Mimi if this can be taken into KEYS-ENCRYPTED tree.
Otherwise I will ask Andrew to help pick this whole series.

Surely, this patch 4 can be taken into s390 seperately since it's
independent, both looks good.

Thanks
Baoquan



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

* [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification
  2022-05-19  0:39     ` Baoquan He
@ 2022-05-19 11:56       ` Mimi Zohar
  2022-05-19 14:22         ` Baoquan He
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2022-05-19 11:56 UTC (permalink / raw)
  To: kexec

[Cc'ing Jarkko, linux-integrity]

On Thu, 2022-05-19 at 08:39 +0800, Baoquan He wrote:
> On 05/18/22 at 01:29pm, Heiko Carstens wrote:
> > On Thu, May 12, 2022 at 03:01:23PM +0800, Coiby Xu wrote:
> > > From: Michal Suchanek <msuchanek@suse.de>
> > > 
> > > commit e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> > > adds support for KEXEC_SIG verification with keys from platform keyring
> > > but the built-in keys and secondary keyring are not used.
> > > 
> > > Add support for the built-in keys and secondary keyring as x86 does.
> > > 
> > > Fixes: e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> > > Cc: stable at vger.kernel.org
> > > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > > Cc: kexec at lists.infradead.org
> > > Cc: keyrings at vger.kernel.org
> > > Cc: linux-security-module at vger.kernel.org
> > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
> > > Acked-by: Baoquan He <bhe@redhat.com>
> > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > > ---
> > >  arch/s390/kernel/machine_kexec_file.c | 18 +++++++++++++-----
> > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > 
> > As far as I can tell this doesn't have any dependency to the other
> > patches in this series, so should I pick this up for the s390 tree, or
> > how will this go upstream?
> 
> Thanks, Heiko.
> 
> I want to ask Mimi if this can be taken into KEYS-ENCRYPTED tree.
> Otherwise I will ask Andrew to help pick this whole series.
> 
> Surely, this patch 4 can be taken into s390 seperately since it's
> independent, both looks good.

KEYS-ENCRYTPED is a type of key, unrelated to using the .platform,
.builtin, .machine, or .secondary keyrings.  One of the main reasons
for this patch set is to use the new ".machine" keyring, which, if
enabled, is linked to the "secondary" keyring.  However, the only
reference to the ".machine" keyring is in the cover letter, not any of
the patch descriptions.  Since this is the basis for the system's
integrity, this seems like a pretty big omission.

From patch 2/4:
"The code in bzImage64_verify_sig makes use of system keyrings
including
.buitin_trusted_keys, .secondary_trusted_keys and .platform keyring to
verify signed kernel image as PE file..."

From patch 3/4:
"This patch allows to verify arm64 kernel image signature using not
only
.builtin_trusted_keys but also .platform and .secondary_trusted_keys
keyring."

From patch 4/4:
"... with keys from platform keyring but the built-in keys and
secondary keyring are not used."

This patch set could probably go through KEYS/KEYRINGS_INTEGRITY, but
it's kind of late to be asking.  Has it been in linux-next?  Should I
assume this patch set has been fully tested or can we get some "tags"?

thanks,

Mimi



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

* [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification
  2022-05-19 11:56       ` Mimi Zohar
@ 2022-05-19 14:22         ` Baoquan He
  2022-05-19 17:11           ` Michal =?unknown-8bit?q?Such=C3=A1nek?=
  0 siblings, 1 reply; 30+ messages in thread
From: Baoquan He @ 2022-05-19 14:22 UTC (permalink / raw)
  To: kexec

On 05/19/22 at 07:56am, Mimi Zohar wrote:
> [Cc'ing Jarkko, linux-integrity]
> 
> On Thu, 2022-05-19 at 08:39 +0800, Baoquan He wrote:
> > On 05/18/22 at 01:29pm, Heiko Carstens wrote:
> > > On Thu, May 12, 2022 at 03:01:23PM +0800, Coiby Xu wrote:
> > > > From: Michal Suchanek <msuchanek@suse.de>
> > > > 
> > > > commit e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> > > > adds support for KEXEC_SIG verification with keys from platform keyring
> > > > but the built-in keys and secondary keyring are not used.
> > > > 
> > > > Add support for the built-in keys and secondary keyring as x86 does.
> > > > 
> > > > Fixes: e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> > > > Cc: stable at vger.kernel.org
> > > > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > > > Cc: kexec at lists.infradead.org
> > > > Cc: keyrings at vger.kernel.org
> > > > Cc: linux-security-module at vger.kernel.org
> > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
> > > > Acked-by: Baoquan He <bhe@redhat.com>
> > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > > > ---
> > > >  arch/s390/kernel/machine_kexec_file.c | 18 +++++++++++++-----
> > > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > > 
> > > As far as I can tell this doesn't have any dependency to the other
> > > patches in this series, so should I pick this up for the s390 tree, or
> > > how will this go upstream?
> > 
> > Thanks, Heiko.
> > 
> > I want to ask Mimi if this can be taken into KEYS-ENCRYPTED tree.
> > Otherwise I will ask Andrew to help pick this whole series.
> > 
> > Surely, this patch 4 can be taken into s390 seperately since it's
> > independent, both looks good.
> 
> KEYS-ENCRYTPED is a type of key, unrelated to using the .platform,
> .builtin, .machine, or .secondary keyrings.  One of the main reasons
> for this patch set is to use the new ".machine" keyring, which, if
> enabled, is linked to the "secondary" keyring.  However, the only
> reference to the ".machine" keyring is in the cover letter, not any of
> the patch descriptions.  Since this is the basis for the system's
> integrity, this seems like a pretty big omission.
> 
> From patch 2/4:
> "The code in bzImage64_verify_sig makes use of system keyrings
> including
> .buitin_trusted_keys, .secondary_trusted_keys and .platform keyring to
> verify signed kernel image as PE file..."
> 
> From patch 3/4:
> "This patch allows to verify arm64 kernel image signature using not
> only
> .builtin_trusted_keys but also .platform and .secondary_trusted_keys
> keyring."
> 
> From patch 4/4:
> "... with keys from platform keyring but the built-in keys and
> secondary keyring are not used."
> 
> This patch set could probably go through KEYS/KEYRINGS_INTEGRITY, but
> it's kind of late to be asking.  Has it been in linux-next?  Should I
> assume this patch set has been fully tested or can we get some "tags"?

Right, it should be KEYS/KEYRINGS_INTEGRITY related, I made mistaken.
Now it got two ACKs from Michal and me. Michal met the same issue on
arm64 and posted another series of patches, finally Coiby integrated
Michal's patch and his to make this patchset. That would be great if
this can get reviewing from experts on key/keyring. Surely, Coiby need
update the patch log to add the '.machine' keyring into patch logs as
you pointed out.

IIRC, Coiby has tested it on x86_64/arm64, not sure if he took test on
s390. No, this hasn't been in linux-next.



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

* [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification
  2022-05-19 14:22         ` Baoquan He
@ 2022-05-19 17:11           ` Michal =?unknown-8bit?q?Such=C3=A1nek?=
  2022-06-16  1:46             ` Coiby Xu
  0 siblings, 1 reply; 30+ messages in thread
From: Michal =?unknown-8bit?q?Such=C3=A1nek?= @ 2022-05-19 17:11 UTC (permalink / raw)
  To: kexec

On Thu, May 19, 2022 at 10:22:15PM +0800, Baoquan He wrote:
> On 05/19/22 at 07:56am, Mimi Zohar wrote:
> > [Cc'ing Jarkko, linux-integrity]
> > 
> > On Thu, 2022-05-19 at 08:39 +0800, Baoquan He wrote:
> > > On 05/18/22 at 01:29pm, Heiko Carstens wrote:
> > > > On Thu, May 12, 2022 at 03:01:23PM +0800, Coiby Xu wrote:
> > > > > From: Michal Suchanek <msuchanek@suse.de>
> > > > > 
> > > > > commit e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> > > > > adds support for KEXEC_SIG verification with keys from platform keyring
> > > > > but the built-in keys and secondary keyring are not used.
> > > > > 
> > > > > Add support for the built-in keys and secondary keyring as x86 does.
> > > > > 
> > > > > Fixes: e23a8020ce4e ("s390/kexec_file: Signature verification prototype")
> > > > > Cc: stable at vger.kernel.org
> > > > > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > > > > Cc: kexec at lists.infradead.org
> > > > > Cc: keyrings at vger.kernel.org
> > > > > Cc: linux-security-module at vger.kernel.org
> > > > > Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> > > > > Reviewed-by: "Lee, Chun-Yi" <jlee@suse.com>
> > > > > Acked-by: Baoquan He <bhe@redhat.com>
> > > > > Signed-off-by: Coiby Xu <coxu@redhat.com>
> > > > > ---
> > > > >  arch/s390/kernel/machine_kexec_file.c | 18 +++++++++++++-----
> > > > >  1 file changed, 13 insertions(+), 5 deletions(-)
> > > > 
> > > > As far as I can tell this doesn't have any dependency to the other
> > > > patches in this series, so should I pick this up for the s390 tree, or
> > > > how will this go upstream?
> > > 
> > > Thanks, Heiko.
> > > 
> > > I want to ask Mimi if this can be taken into KEYS-ENCRYPTED tree.
> > > Otherwise I will ask Andrew to help pick this whole series.
> > > 
> > > Surely, this patch 4 can be taken into s390 seperately since it's
> > > independent, both looks good.
> > 
> > KEYS-ENCRYTPED is a type of key, unrelated to using the .platform,
> > .builtin, .machine, or .secondary keyrings.  One of the main reasons
> > for this patch set is to use the new ".machine" keyring, which, if
> > enabled, is linked to the "secondary" keyring.  However, the only
> > reference to the ".machine" keyring is in the cover letter, not any of
> > the patch descriptions.  Since this is the basis for the system's
> > integrity, this seems like a pretty big omission.
> > 
> > From patch 2/4:
> > "The code in bzImage64_verify_sig makes use of system keyrings
> > including
> > .buitin_trusted_keys, .secondary_trusted_keys and .platform keyring to
> > verify signed kernel image as PE file..."
> > 
> > From patch 3/4:
> > "This patch allows to verify arm64 kernel image signature using not
> > only
> > .builtin_trusted_keys but also .platform and .secondary_trusted_keys
> > keyring."
> > 
> > From patch 4/4:
> > "... with keys from platform keyring but the built-in keys and
> > secondary keyring are not used."
> > 
> > This patch set could probably go through KEYS/KEYRINGS_INTEGRITY, but
> > it's kind of late to be asking.  Has it been in linux-next?  Should I
> > assume this patch set has been fully tested or can we get some "tags"?
> 
> Right, it should be KEYS/KEYRINGS_INTEGRITY related, I made mistaken.
> Now it got two ACKs from Michal and me. Michal met the same issue on
> arm64 and posted another series of patches, finally Coiby integrated
> Michal's patch and his to make this patchset. That would be great if
> this can get reviewing from experts on key/keyring. Surely, Coiby need
> update the patch log to add the '.machine' keyring into patch logs as
> you pointed out.
> 
> IIRC, Coiby has tested it on x86_64/arm64, not sure if he took test on
> s390. No, this hasn't been in linux-next.

I used the s390 code on powerpc and there it did not work because the
built-in key was needed to verify the kernel.

I did not really run this on s390, only ported the fix I needed on
powerpc back to s390.

Thanks

Michal


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

* [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-05-12  7:01 [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Coiby Xu
                   ` (3 preceding siblings ...)
  2022-05-12  7:01 ` [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification Coiby Xu
@ 2022-05-20 17:04 ` Mimi Zohar
  2022-05-25  9:59   ` Coiby Xu
  4 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2022-05-20 17:04 UTC (permalink / raw)
  To: kexec

Hi Coiby,

On Thu, 2022-05-12 at 15:01 +0800, Coiby Xu wrote:

The cover letter should start out with an overall problem description
and then continue with the specifics.  In this case each of the arch's
use different keyrings to validate the kexec kernel image signature.  I
would continue with saying the MOK keys were originally loaded onto the
.platform keyring with the other EFI keys, but recently with the new
.machine keyring that changed.

The purpose of this patch set is a generic solution for the different
archs.


> Currently, a problem faced by arm64 is if a kernel image is signed by a
> MOK key, loading it via the kexec_file_load() system call would be
> rejected with the error "Lockdown: kexec: kexec of unsigned images is
> restricted; see man kernel_lockdown.7". This happens because arm64 uses
> only the primary keyring i.e. the .builtin_trusted_keys keyring that
> contains only kernel built-in keys to verify the kexec kernel image. MOK
> keys are loaded into the .platform keyring or/and .machine keyring. The
> .machine keyring is linked to the secondary keyring i.e.
> .secondary_trusted_keys keyring when the end-user chooses to trust MOK
> keys. The platform keyring is exclusively used for kexec kernel image
> verification and .secondary_trusted_keys together with
> .builtin_trusted_keys are the system trusted keyrings. So obviously
> there is no reason to not use .secondary_trusted_keys or .platform
> keyring for kernel image signature verification. 

Both the ".platform" and ".machine" keyring are linked to the
".secondary_trusted_keys" keyring.   The root of trust for these
keyrings are very different.  Instead of saying "So obviously there is
no reason to not use .secondary_trusted_keys" it would be more
beneficial to describe the root of trusts, allowing others to draw
their own conclusions for their usecase.

thanks,

Mimi

> 
> Similarly, s390 only uses platform keyring for kernel image signature
> verification and built-in keys and secondary keyring are not used.
> 
> This patch set allows arm64 and s390 to use more system keyrings
> including the .secondary_trusted_keys and .platform keyring to verify
> kexec kernel image signature as x86 does.
> 
> The 3rd arm64 patch depends on the first two patches. The 4th s390 patch
> can be applied independently.

> v8:
>  - drop "Cc: stable at vger.kernel.org" for the first two prerequisite
>    patches [Baoquan]
> 
> v7:
>  - drop the Fixes tag for the 2nd patch and add patch prerequisites
>    [Baoquan]
>  - improve cover letter
> 
> v6:
>  - integrate the first three patches of "[PATCH 0/4] Unifrom keyring
>    support across architectures and functions" from Michal [1]
>  - improve commit message [Baoquan, Michal]
>  - directly assign kexec_kernel_verify_pe_sig to
>    kexec_file_ops->verify_sig [Michal]
> 
> v5:
>  - improve commit message [Baoquan]
> 
> v4:
>  - fix commit reference format issue and other checkpatch.pl warnings [Baoquan]
> 
> v3:
>  - s/arch_kexec_kernel_verify_pe_sig/kexec_kernel_verify_pe_sig [Eric]
>  - clean up arch_kexec_kernel_verify_sig [Eric]
> 
> v2:
>  - only x86_64 and arm64 need to enable PE file signature check [Dave]
> 
> [1] https://lore.kernel.org/lkml/cover.1644953683.git.msuchanek at suse.de/
> 
> Coiby Xu (3):
>   kexec: clean up arch_kexec_kernel_verify_sig
>   kexec, KEYS: make the code in bzImage64_verify_sig generic
>   arm64: kexec_file: use more system keyrings to verify kernel image
>     signature
> 
> Michal Suchanek (1):
>   kexec, KEYS, s390: Make use of built-in and secondary keyring for
>     signature verification
> 
>  arch/arm64/kernel/kexec_image.c       | 11 +-----
>  arch/s390/kernel/machine_kexec_file.c | 18 +++++++---
>  arch/x86/kernel/kexec-bzimage64.c     | 20 +----------
>  include/linux/kexec.h                 |  7 ++--
>  kernel/kexec_file.c                   | 51 ++++++++++++++++-----------
>  5 files changed, 50 insertions(+), 57 deletions(-)
> 




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

* [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-05-20 17:04 ` [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Mimi Zohar
@ 2022-05-25  9:59   ` Coiby Xu
  2022-05-25 13:30     ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Coiby Xu @ 2022-05-25  9:59 UTC (permalink / raw)
  To: kexec

Hi Mimi,

On Fri, May 20, 2022 at 01:04:47PM -0400, Mimi Zohar wrote:
>Hi Coiby,
>
>On Thu, 2022-05-12 at 15:01 +0800, Coiby Xu wrote:
>
>The cover letter should start out with an overall problem description
>and then continue with the specifics.  In this case each of the arch's
>use different keyrings to validate the kexec kernel image signature.  I

Thanks for the suggestion! I will start with an overall problem
description in next version. For me, the bigger problem is arm kexec
can't make use of .secondary_trusted_keys or .platform to verify kernel
image signature. The by-product of the solution is it also helps
address the problem of inconsistent usage of keyrings by kexec between
different arches.

>would continue with saying the MOK keys were originally loaded onto the
>.platform keyring with the other EFI keys, but recently with the new
>.machine keyring that changed.

It seems I lack some background knowledge that makes me fail to
appreciate what change the new .machine keyring brings to kexec. As far
as I can understand, the new .machine keyring doesn't seem to change
much about kexec kernel image signature verification. kexec should be
able to use MOK keys to verify signature regardless of the keys being
loaded into .platform keyring or into the new .machine keyring. Because
the MOK keys have already be used to verify the 1st booting kernel's
image signature. To me, the significance of the new .machine keyring is
the end-users-enrolled keys can be also used to verify kernel modules
(the end users can also add his key to the .secondary_trusted_keys
keyring but the key needs to vouched by any existing key from the
.builtin_trusted_keys or .secondary_trusted_keys which is nearly
impossible).

>
>The purpose of this patch set is a generic solution for the different
>archs.
>
>
>> Currently, a problem faced by arm64 is if a kernel image is signed by a
>> MOK key, loading it via the kexec_file_load() system call would be
>> rejected with the error "Lockdown: kexec: kexec of unsigned images is
>> restricted; see man kernel_lockdown.7". This happens because arm64 uses
>> only the primary keyring i.e. the .builtin_trusted_keys keyring that
>> contains only kernel built-in keys to verify the kexec kernel image. MOK
>> keys are loaded into the .platform keyring or/and .machine keyring. The
>> .machine keyring is linked to the secondary keyring i.e.
>> .secondary_trusted_keys keyring when the end-user chooses to trust MOK
>> keys. The platform keyring is exclusively used for kexec kernel image
>> verification and .secondary_trusted_keys together with
>> .builtin_trusted_keys are the system trusted keyrings. So obviously
>> there is no reason to not use .secondary_trusted_keys or .platform
>> keyring for kernel image signature verification.
>
>Both the ".platform" and ".machine" keyring are linked to the
>".secondary_trusted_keys" keyring.   

I don't find any code that links the .platform keyring to the
.secondary_trusted_keys keyring and one [1] of your replies to "[PATCH
4/4] module, KEYS: Make use of platform keyring for signature
verification" is as follows,
   "Permission for loading the pre-OS keys onto the 'platform' keyring and
   using them is limited to verifying the kexec kernel image, nothing
   else."

[1] https://lore.kernel.org/linux-arm-kernel/3e39412657a4b0839bcf38544d591959e89877b8.camel at linux.ibm.com/

>The root of trust for these
>keyrings are very different.  Instead of saying "So obviously there is
>no reason to not use .secondary_trusted_keys" it would be more
>beneficial to describe the root of trusts, allowing others to draw
>their own conclusions for their usecase.

Thanks for the suggestion! I'll add the following text in v9, do it
looks good to you?

The root of trusts of the keys in the %.builtin_trusted_keys and
secondary_trusted_keys keyring is a Linux distribution vendor. The
.platform keyring could have both UEFI db keys and MOK keys or only UEFI
db keys. And the .machine keyring only have MOK keys. The root trust of
UEFI db keys is UEFI Platform Key which is shipped by an UEFI firmware
vendor. The root trust of MOK keys is a Linux distribution vendor (shim
has built-in MOK key) and the end user could also be another root of
trust if he/she enrolls his own key.

The %.builtin_trusted_keys keyring consists of the trustable keys built
into the kernel when a kernel is built by a developer.

The .secondary_trusted_keys keyring introduced by commit d3bfe84129f6
("certs: Add a secondary system keyring that can be added to
dynamically") consists of the trustable keys that an end-user can add
dynamically after booting the kernel. When this kernel enables the
.machine keyring and an end user chooses to trust Machine Owner Keys
(MOK) provided by shim, MOK keys will be added to the .machine
keyring and the .machine keyring is be linked to the
secondary_trusted_keys so keys contained in the .machine keyring will
automatically be searched when searching secondary_trusted_keys.

The .platform keyring introduced by commit 9dc92c45177a ("integrity:
Define a trusted platform keyring") consist of UEFI db and
MOK keys. If an end user chooses to Machine Owner Keys and the kernel
has the .machine keyring enabled, the .platform keyring only consists of
UEFI db keys since the MOK keys are added to the .machine keyring
instead.

>
>thanks,
>
>Mimi
>
>>
>> Similarly, s390 only uses platform keyring for kernel image signature
>> verification and built-in keys and secondary keyring are not used.
>>
>> This patch set allows arm64 and s390 to use more system keyrings
>> including the .secondary_trusted_keys and .platform keyring to verify
>> kexec kernel image signature as x86 does.
>>
>> The 3rd arm64 patch depends on the first two patches. The 4th s390 patch
>> can be applied independently.
>
>> v8:
>>  - drop "Cc: stable at vger.kernel.org" for the first two prerequisite
>>    patches [Baoquan]
>>
>> v7:
>>  - drop the Fixes tag for the 2nd patch and add patch prerequisites
>>    [Baoquan]
>>  - improve cover letter
>>
>> v6:
>>  - integrate the first three patches of "[PATCH 0/4] Unifrom keyring
>>    support across architectures and functions" from Michal [1]
>>  - improve commit message [Baoquan, Michal]
>>  - directly assign kexec_kernel_verify_pe_sig to
>>    kexec_file_ops->verify_sig [Michal]
>>
>> v5:
>>  - improve commit message [Baoquan]
>>
>> v4:
>>  - fix commit reference format issue and other checkpatch.pl warnings [Baoquan]
>>
>> v3:
>>  - s/arch_kexec_kernel_verify_pe_sig/kexec_kernel_verify_pe_sig [Eric]
>>  - clean up arch_kexec_kernel_verify_sig [Eric]
>>
>> v2:
>>  - only x86_64 and arm64 need to enable PE file signature check [Dave]
>>
>> [1] https://lore.kernel.org/lkml/cover.1644953683.git.msuchanek at suse.de/
>>
>> Coiby Xu (3):
>>   kexec: clean up arch_kexec_kernel_verify_sig
>>   kexec, KEYS: make the code in bzImage64_verify_sig generic
>>   arm64: kexec_file: use more system keyrings to verify kernel image
>>     signature
>>
>> Michal Suchanek (1):
>>   kexec, KEYS, s390: Make use of built-in and secondary keyring for
>>     signature verification
>>
>>  arch/arm64/kernel/kexec_image.c       | 11 +-----
>>  arch/s390/kernel/machine_kexec_file.c | 18 +++++++---
>>  arch/x86/kernel/kexec-bzimage64.c     | 20 +----------
>>  include/linux/kexec.h                 |  7 ++--
>>  kernel/kexec_file.c                   | 51 ++++++++++++++++-----------
>>  5 files changed, 50 insertions(+), 57 deletions(-)
>>
>
>

-- 
Best regards,
Coiby



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

* [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-05-25  9:59   ` Coiby Xu
@ 2022-05-25 13:30     ` Mimi Zohar
  2022-05-27 13:43       ` Coiby Xu
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2022-05-25 13:30 UTC (permalink / raw)
  To: kexec

On Wed, 2022-05-25 at 17:59 +0800, Coiby Xu wrote:
> Hi Mimi,
> 
> On Fri, May 20, 2022 at 01:04:47PM -0400, Mimi Zohar wrote:
> >Hi Coiby,
> >
> >On Thu, 2022-05-12 at 15:01 +0800, Coiby Xu wrote:
> >
> >The cover letter should start out with an overall problem description
> >and then continue with the specifics.  In this case each of the arch's
> >use different keyrings to validate the kexec kernel image signature.  I
> 
> Thanks for the suggestion! I will start with an overall problem
> description in next version. For me, the bigger problem is arm kexec
> can't make use of .secondary_trusted_keys or .platform to verify kernel
> image signature. The by-product of the solution is it also helps
> address the problem of inconsistent usage of keyrings by kexec between
> different arches.
> 
> >would continue with saying the MOK keys were originally loaded onto the
> >.platform keyring with the other EFI keys, but recently with the new
> >.machine keyring that changed.
> 
> It seems I lack some background knowledge that makes me fail to
> appreciate what change the new .machine keyring brings to kexec. As far
> as I can understand, the new .machine keyring doesn't seem to change
> much about kexec kernel image signature verification. kexec should be
> able to use MOK keys to verify signature regardless of the keys being
> loaded into .platform keyring or into the new .machine keyring. Because
> the MOK keys have already be used to verify the 1st booting kernel's
> image signature. To me, the significance of the new .machine keyring is
> the end-users-enrolled keys can be also used to verify kernel modules
> (the end users can also add his key to the .secondary_trusted_keys
> keyring but the key needs to vouched by any existing key from the
> .builtin_trusted_keys or .secondary_trusted_keys which is nearly
> impossible).

"the significance of the new .machine keyring is the end-users-enrolled 
keys can be also used to verify kernel modules" correct.   So any key
stored in MOK and loaded onto the .machine keyring, could also then be
used to verify the kexec'ed kernel image signature as well.

> 
> >
> >The purpose of this patch set is a generic solution for the different
> >archs.
> >
> >
> >> Currently, a problem faced by arm64 is if a kernel image is signed by a
> >> MOK key, loading it via the kexec_file_load() system call would be
> >> rejected with the error "Lockdown: kexec: kexec of unsigned images is
> >> restricted; see man kernel_lockdown.7". This happens because arm64 uses
> >> only the primary keyring i.e. the .builtin_trusted_keys keyring that
> >> contains only kernel built-in keys to verify the kexec kernel image. MOK
> >> keys are loaded into the .platform keyring or/and .machine keyring. The
> >> .machine keyring is linked to the secondary keyring i.e.
> >> .secondary_trusted_keys keyring when the end-user chooses to trust MOK
> >> keys. The platform keyring is exclusively used for kexec kernel image
> >> verification and .secondary_trusted_keys together with
> >> .builtin_trusted_keys are the system trusted keyrings. So obviously
> >> there is no reason to not use .secondary_trusted_keys or .platform
> >> keyring for kernel image signature verification.
> >
> >Both the ".platform" and ".machine" keyring are linked to the
> >".secondary_trusted_keys" keyring.
> 
> I don't find any code that links the .platform keyring to the
> .secondary_trusted_keys keyring and one [1] of your replies to "[PATCH
> 4/4] module, KEYS: Make use of platform keyring for signature
> verification" is as follows,
>    "Permission for loading the pre-OS keys onto the 'platform' keyring and
>    using them is limited to verifying the kexec kernel image, nothing
>    else."

Right, that should have been, "Both the .builtin_trusted_keys and
.machine keyrings are linked ..."

> 
> [1] https://lore.kernel.org/linux-arm-kernel/3e39412657a4b0839bcf38544d591959e89877b8.camel at linux.ibm.com/
> 
> >The root of trust for these
> >keyrings are very different.  Instead of saying "So obviously there is
> >no reason to not use .secondary_trusted_keys" it would be more
> >beneficial to describe the root of trusts, allowing others to draw
> >their own conclusions for their usecase.

Linking the .machine keyring to the .secondary keyring impacts the
root(s) of trust.

> 
> Thanks for the suggestion! I'll add the following text in v9, do it
> looks good to you?
> 
> The root of trusts of the keys in the %.builtin_trusted_keys and
> secondary_trusted_keys keyring is a Linux distribution vendor. 

The root of trust for each keyring should be described separately.

.builtin_trusted_keys:

For example,

Keys may be built into the kernel during build or inserted into memory
reserved for keys post build.  In both of these cases, trust is based
on verification of the kernel image signature.  On a physical system in
a secure boot environment, this trust is rooted in HW.

.machine:

< explanation >

.secondary_trusted_keys:

For example,

Certificates signed by keys on the .builtin_trusted_keys, .machine, or
existing keys on the .secondary_trusted_keys keryings may be loaded
onto the .secondary_trusted_keys keyring.  This establishes a signature
chain of trust based on keys loaded on either the .builtin_trusted_keys
or .machine keyrings, if configured and enabled.

.platform

< explanation >


thanks,

Mimi


> The
> .platform keyring could have both UEFI db keys and MOK keys or only UEFI
> db keys. And the .machine keyring only have MOK keys. The root trust of
> UEFI db keys is UEFI Platform Key which is shipped by an UEFI firmware
> vendor. The root trust of MOK keys is a Linux distribution vendor (shim
> has built-in MOK key) and the end user could also be another root of
> trust if he/she enrolls his own key.
> 
> The %.builtin_trusted_keys keyring consists of the trustable keys built
> into the kernel when a kernel is built by a developer.
> 
> The .secondary_trusted_keys keyring introduced by commit d3bfe84129f6
> ("certs: Add a secondary system keyring that can be added to
> dynamically") consists of the trustable keys that an end-user can add
> dynamically after booting the kernel. When this kernel enables the
> .machine keyring and an end user chooses to trust Machine Owner Keys
> (MOK) provided by shim, MOK keys will be added to the .machine
> keyring and the .machine keyring is be linked to the
> secondary_trusted_keys so keys contained in the .machine keyring will
> automatically be searched when searching secondary_trusted_keys.
> 
> The .platform keyring introduced by commit 9dc92c45177a ("integrity:
> Define a trusted platform keyring") consist of UEFI db and
> MOK keys. If an end user chooses to Machine Owner Keys and the kernel
> has the .machine keyring enabled, the .platform keyring only consists of
> UEFI db keys since the MOK keys are added to the .machine keyring
> instead.




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

* [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-05-25 13:30     ` Mimi Zohar
@ 2022-05-27 13:43       ` Coiby Xu
  2022-05-27 16:45         ` Mimi Zohar
  2022-06-09 15:35         ` Mimi Zohar
  0 siblings, 2 replies; 30+ messages in thread
From: Coiby Xu @ 2022-05-27 13:43 UTC (permalink / raw)
  To: kexec

Hi Mini,

It seems I need to only change cover letter and commit message i.e.
there is no concern about the code. So it's better to provide a
new cover letter here to collect new feedback from you thus we
can avoid unnecessary rounds of patch set.

Currently when loading a kernel image via the kexec_file_load() system
call, x86 can make use of three keyrings i.e. the .builtin_trusted_keys,
.secondary_trusted_keys and .platform keyrings to verify signature.
However, arm64 and s390 can only use the .builtin_trusted_keys and
.platform keyring respectively. For example, one resulting problem is
kexec'ing a kernel image  would be rejected with the error "Lockdown:
kexec: kexec of unsigned images is restricted; see man
kernel_lockdown.7".

This patch set enables arm64 and s390 to make use of the same keyrings
as x86 to very the signature kexec'ed kernel image.

The recently introduced .machine keyring impacts the roots of trust by
linking the .machine keyring to the .secondary keyring. The roots of
trust of different keyring are described as follows,

.builtin_trusted_keys:

Keys may be built into the kernel during build or inserted into memory
reserved for keys post build. The root of trust is the kernel build i.e.
a Linux distribution vendor. On a physical system in a secure boot
environment, this trust is rooted in hardware.

.machine:

If the end-users choose to trust the keys provided by first-stage UEFI
bootloader shim i.e. Machine Owner Keys (MOK keys), the keys will be
added to this keyring and this keyring is linked to the
.secondary_trusted_keys keyring as same as the .builtin_trusted_keys
keyring. Shim has built-in keys from a Linux distribution or the
end-users-enrolled keys. So the root of trust of this keyring is either
a Linux distribution vendor or the end-users.

.secondary_trusted_keys:

Certificates signed by keys on the .builtin_trusted_keys, .machine, or
existing keys on the .secondary_trusted_keys keryings may be loaded
onto the .secondary_trusted_keys keyring. This establishes a signature
chain of trust based on keys loaded on either the .builtin_trusted_keys
or .machine keyrings, if configured and enabled.

.platform:

The .platform keyring consist of UEFI db and MOK keys which are used by
shim to verify the first boot kernel's image signature. If end-users
choose to trust MOK keys and the kernel has the .machine keyring
enabled, the .platform keyring only consists of UEFI db keys since the
MOK keys are added to the .machine keyring instead. Because the
end-users could also enroll there own MOK keys, the root of trust could
be hardware or the end-users.


On Wed, May 25, 2022 at 09:30:40AM -0400, Mimi Zohar wrote:
>On Wed, 2022-05-25 at 17:59 +0800, Coiby Xu wrote:
>> Hi Mimi,
>>
[...]
>> It seems I lack some background knowledge that makes me fail to
>> appreciate what change the new .machine keyring brings to kexec. As far
>> as I can understand, the new .machine keyring doesn't seem to change
>> much about kexec kernel image signature verification. kexec should be
>> able to use MOK keys to verify signature regardless of the keys being
>> loaded into .platform keyring or into the new .machine keyring. Because
>> the MOK keys have already be used to verify the 1st booting kernel's
>> image signature. To me, the significance of the new .machine keyring is
>> the end-users-enrolled keys can be also used to verify kernel modules
>> (the end users can also add his key to the .secondary_trusted_keys
>> keyring but the key needs to vouched by any existing key from the
>> .builtin_trusted_keys or .secondary_trusted_keys which is nearly
>> impossible).
>
>"the significance of the new .machine keyring is the end-users-enrolled
>keys can be also used to verify kernel modules" correct.   So any key
>stored in MOK and loaded onto the .machine keyring, could also then be
>used to verify the kexec'ed kernel image signature as well.

Thanks for explanation!

>
[...]
>> >
>> >Both the ".platform" and ".machine" keyring are linked to the
>> >".secondary_trusted_keys" keyring.
>>
>> I don't find any code that links the .platform keyring to the
>> .secondary_trusted_keys keyring and one [1] of your replies to "[PATCH
>> 4/4] module, KEYS: Make use of platform keyring for signature
>> verification" is as follows,
>>    "Permission for loading the pre-OS keys onto the 'platform' keyring and
>>    using them is limited to verifying the kexec kernel image, nothing
>>    else."
>
>Right, that should have been, "Both the .builtin_trusted_keys and
>.machine keyrings are linked ..."

Thanks for the confirmation! I should have realized it's a typo.

>
>>
>> [1] https://lore.kernel.org/linux-arm-kernel/3e39412657a4b0839bcf38544d591959e89877b8.camel at linux.ibm.com/
>>
>> >The root of trust for these
>> >keyrings are very different.  Instead of saying "So obviously there is
>> >no reason to not use .secondary_trusted_keys" it would be more
>> >beneficial to describe the root of trusts, allowing others to draw
>> >their own conclusions for their usecase.
>
>Linking the .machine keyring to the .secondary keyring impacts the
>root(s) of trust.

Thanks for the clarification!

>
>>
>> Thanks for the suggestion! I'll add the following text in v9, do it
>> looks good to you?
>>
>> The root of trusts of the keys in the %.builtin_trusted_keys and
>> secondary_trusted_keys keyring is a Linux distribution vendor.
>
>The root of trust for each keyring should be described separately.
>
>.builtin_trusted_keys:
>
>For example,
>
>Keys may be built into the kernel during build or inserted into memory
>reserved for keys post build.  In both of these cases, trust is based
>on verification of the kernel image signature.  

Correct me if I'm wrong, without secure boot, there is no verification
of the kernel image signature so the root of trust should be trust on
the kernel builder.


> On a physical system in
>a secure boot environment, this trust is rooted in HW.
>
>.machine:
>
>< explanation >
>
>.secondary_trusted_keys:
>
>For example,
>
>Certificates signed by keys on the .builtin_trusted_keys, .machine, or
>existing keys on the .secondary_trusted_keys keryings may be loaded
>onto the .secondary_trusted_keys keyring.  This establishes a signature
>chain of trust based on keys loaded on either the .builtin_trusted_keys
>or .machine keyrings, if configured and enabled.
>
>.platform
>
>< explanation >
>
>
>thanks,
>
>Mimi

Thanks for providing the examples!

-- 
Best regards,
Coiby



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

* [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-05-27 13:43       ` Coiby Xu
@ 2022-05-27 16:45         ` Mimi Zohar
  2022-06-16  1:15           ` Coiby Xu
  2022-06-09 15:35         ` Mimi Zohar
  1 sibling, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2022-05-27 16:45 UTC (permalink / raw)
  To: kexec


On Fri, 2022-05-27 at 21:43 +0800, Coiby Xu wrote:
> Hi Mini,

Hi Coiby,

> new cover letter here to collect new feedback from you thus we
> can avoid unnecessary rounds of patch set.

Agreed.   Much better.  Just a couple of nits.

> Currently when loading a kernel image via the kexec_file_load() system
> call, x86 can make use of three keyrings i.e. the .builtin_trusted_keys,
> .secondary_trusted_keys and .platform keyrings to verify signature.

Either "a signature" or "signatures".

> However, arm64 and s390 can only use the .builtin_trusted_keys and
> .platform keyring respectively. For example, one resulting problem is
> kexec'ing a kernel image  would be rejected with the error "Lockdown:
> kexec: kexec of unsigned images is restricted; see man
> kernel_lockdown.7".
> 
> This patch set enables arm64 and s390 to make use of the same keyrings
> as x86 to very the signature kexec'ed kernel image.

  Fix "very".  Perhaps "verify the kexec'ed kernel image signature".
> 
> The recently introduced .machine keyring impacts the roots of trust by
> linking the .machine keyring to the .secondary keyring. The roots of
> trust of different keyring are described as follows,
> 
"of ... keyring" -> "for the ... keyrings"

> .builtin_trusted_keys:
> 
> Keys may be built into the kernel during build or inserted into memory
> reserved for keys post build. The root of trust is the kernel build i.e.
> a Linux distribution vendor. On a physical system in a secure boot
> environment, this trust is rooted in hardware.

Please look at my response to your question below.

> 
> .machine:
> 
> If the end-users choose to trust the keys provided by first-stage UEFI
> bootloader shim i.e. Machine Owner Keys (MOK keys), the keys will be
> added to this keyring and this keyring is linked to the

 Grammatically "and this"  needs to be fixed.

> .secondary_trusted_keys keyring as same as the .builtin_trusted_keys
> keyring. Shim has built-in keys from a Linux distribution or the
> end-users-enrolled keys. So the root of trust of this keyring is either
> a Linux distribution vendor or the end-users.
> 
> .secondary_trusted_keys:
> 
> Certificates signed by keys on the .builtin_trusted_keys, .machine, or
> existing keys on the .secondary_trusted_keys keryings may be loaded
> onto the .secondary_trusted_keys keyring. This establishes a signature
> chain of trust based on keys loaded on either the .builtin_trusted_keys
> or .machine keyrings, if configured and enabled.
> 
> .platform:
> 
> The .platform keyring consist of UEFI db and MOK keys which are used by
> shim to verify the first boot kernel's image signature. If end-users
> choose to trust MOK keys and the kernel has the .machine keyring
> enabled, the .platform keyring only consists of UEFI db keys since the
> MOK keys are added to the .machine keyring instead. Because the
> end-users could also enroll there own MOK keys, the root of trust could

"there" -> "their"

> be hardware or the end-users.

It's always "hardware".  "or" -> "and"?

<snip>

> >>
> >> The root of trusts of the keys in the %.builtin_trusted_keys and
> >> secondary_trusted_keys keyring is a Linux distribution vendor.
> >
> >The root of trust for each keyring should be described separately.
> >
> >.builtin_trusted_keys:
> >
> >For example,
> >
> >Keys may be built into the kernel during build or inserted into memory
> >reserved for keys post build.  In both of these cases, trust is based
> >on verification of the kernel image signature.  
> 
> Correct me if I'm wrong, without secure boot, there is no verification
> of the kernel image signature so the root of trust should be trust on
> the kernel builder.

No, basing the signature verification on secure boot could not have
been upstreamed.  IMA is based on policy, regardeless of the secure
boot mode.  A builtin policy may be specified on the boot command line,
but should be replaced with a more constrained custom policy [1]. 
Unlike the builtin policy rules, the architecture specific rules are
persistent[2].   The architecture specific rules are normally tied to
the secure boot modes.  On OpenPOWER, the architecture specific
"measure" rules are dependent on the trusted boot mode.

The current IMA policy rules can be viewed by cat'ing
<securityfs>/ima/policy.

[1] The builtin policies are not LSM aware.  The policy rules need to
be constrained to avoid integrity violations.

[2] arch specific policy rules:
security/integrity/ima/ima_efi.c, arch/powerpc/kernel/ima_arch.c

thanks,

Mimi



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

* Re: [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-05-27 13:43       ` Coiby Xu
  2022-05-27 16:45         ` Mimi Zohar
@ 2022-06-09 15:35         ` Mimi Zohar
  2022-06-16  1:21           ` Coiby Xu
  1 sibling, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2022-06-09 15:35 UTC (permalink / raw)
  To: Coiby Xu
  Cc: kexec, linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee

Hi Coiby,

On Fri, 2022-05-27 at 21:43 +0800, Coiby Xu wrote:
> It seems I need to only change cover letter and commit message i.e.
> there is no concern about the code. So it's better to provide a
> new cover letter here to collect new feedback from you thus we
> can avoid unnecessary rounds of patch set.

One other thing before re-posting, the existing Subject line is a bit
snarky.  Perhaps you could come with a more appropriate title?

thanks,

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 1/4] kexec: clean up arch_kexec_kernel_verify_sig
  2022-05-12  7:01 ` [PATCH v8 1/4] kexec: clean up arch_kexec_kernel_verify_sig Coiby Xu
@ 2022-06-09 21:57   ` Mimi Zohar
  0 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2022-06-09 21:57 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee, open list

On Thu, 2022-05-12 at 15:01 +0800, Coiby Xu wrote:
>  kimage_validate_signature(struct kimage *image)
>  {
>         int ret;
>  
> -       ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
> -                                          image->kernel_buf_len);
> +       ret = kexec_image_verify_sig(image, image->kernel_buf,
> +                       image->kernel_buf_len);

Please indent the above line to line up with the open parenthesis.

thanks,

Mimi

>         if (ret) {
>  
>                 if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic
  2022-05-12  7:01 ` [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic Coiby Xu
  2022-05-12  7:21   ` Baoquan He
@ 2022-06-09 22:18   ` Mimi Zohar
  2022-06-16  1:47     ` Coiby Xu
  1 sibling, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2022-06-09 22:18 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee, keyrings,
	linux-security-module, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)

Hi Coiby,

On Thu, 2022-05-12 at 15:01 +0800, Coiby Xu wrote:
> commit 278311e417be ("kexec, KEYS: Make use of platform keyring for
> signature verify") adds platform keyring support on x86 kexec but not
> arm64.
> 
> The code in bzImage64_verify_sig makes use of system keyrings including
> .buitin_trusted_keys, .secondary_trusted_keys and .platform keyring to
> verify signed kernel image as PE file. Make it generic so both x86_64
> and arm64 can use it.

^uses the keys on the .builtin_trusted_keys, .machine, if configured
and enabled, .secondary_trusted_keys, also if configured, and .platform
keyrings to verify the signed kernel image as PE file. 

> 
> @@ -202,6 +203,12 @@ int arch_kexec_apply_relocations(struct purgatory_info *pi,
>  				 const Elf_Shdr *relsec,
>  				 const Elf_Shdr *symtab);
>  int arch_kimage_file_post_load_cleanup(struct kimage *image);
> +#ifdef CONFIG_KEXEC_SIG
> +#ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
> +int kexec_kernel_verify_pe_sig(const char *kernel,
> +				    unsigned long kernel_len);

Please join this line with the previous one.

> +#endif
> +#endif
>  int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
>  
>  extern int kexec_add_buffer(struct kexec_buf *kbuf);

thanks,

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature
  2022-05-12  7:01 ` [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature Coiby Xu
@ 2022-06-09 23:15   ` Mimi Zohar
  2022-06-16  1:22     ` Coiby Xu
  2022-06-17  9:34     ` Michal Suchánek
  0 siblings, 2 replies; 30+ messages in thread
From: Mimi Zohar @ 2022-06-09 23:15 UTC (permalink / raw)
  To: Coiby Xu, kexec
  Cc: linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee, stable, keyrings,
	linux-security-module, Catalin Marinas, James Morse,
	AKASHI Takahiro, open list

On Thu, 2022-05-12 at 15:01 +0800, Coiby Xu wrote:
> Currently, a problem faced by arm64 is if a kernel image is signed by a
> MOK key, loading it via the kexec_file_load() system call would be
> rejected with the error "Lockdown: kexec: kexec of unsigned images is
> restricted; see man kernel_lockdown.7".
> 
> This happens because image_verify_sig uses only the primary keyring that
> contains only kernel built-in keys to verify the kexec image.

From the git history it's clear that .platform keyring was upstreamed
during the same open window as commit 732b7b93d849 ("arm64: kexec_file:
add kernel signature verification support").   Loading the MOK keys
onto the .platform keyring was upstreamed much later.  For this reason,
commit 732b7b93d849 only used keys on the  .builtin_trusted_keys
keyring.   This patch is now addressing it and the newly upstreamed
.machine keyring.

Only using the .builtin_trusted_keys is the problem statement, which
should be one of the first lines of the patch description, if not the
first line.

> 
> This patch allows to verify arm64 kernel image signature using not only
> .builtin_trusted_keys but also .platform and .secondary_trusted_keys
> keyring.

Please remember to update this to include the .machine keyring.

> 
> Fixes: 732b7b93d849 ("arm64: kexec_file: add kernel signature verification support")

Since the MOK keys weren't loaded onto the .platform keyring until much
later, I would not classify this as a fix.

thanks,

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-05-27 16:45         ` Mimi Zohar
@ 2022-06-16  1:15           ` Coiby Xu
  2022-06-17  3:57             ` Coiby Xu
  0 siblings, 1 reply; 30+ messages in thread
From: Coiby Xu @ 2022-06-16  1:15 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: kexec, linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee

Hi Mimi,

Thanks for carefully reviewing the covert letter and patches and
suggesting various improvements! And sorry for the late reply as I need
some time to learn more about secure boot, lockdown and IMA to better
make sense of what you mean. 

On Fri, May 27, 2022 at 12:45:54PM -0400, Mimi Zohar wrote:
>
>> new cover letter here to collect new feedback from you thus we
>> can avoid unnecessary rounds of patch set.
>
>Agreed.   Much better.  Just a couple of nits.

>
>> Currently when loading a kernel image via the kexec_file_load() system
>> call, x86 can make use of three keyrings i.e. the .builtin_trusted_keys,
>> .secondary_trusted_keys and .platform keyrings to verify signature.
>
>Either "a signature" or "signatures".
>
>> However, arm64 and s390 can only use the .builtin_trusted_keys and
>> .platform keyring respectively. For example, one resulting problem is
>> kexec'ing a kernel image  would be rejected with the error "Lockdown:
>> kexec: kexec of unsigned images is restricted; see man
>> kernel_lockdown.7".
>>
>> This patch set enables arm64 and s390 to make use of the same keyrings
>> as x86 to very the signature kexec'ed kernel image.
>
>  Fix "very".  Perhaps "verify the kexec'ed kernel image signature".
>>
>> The recently introduced .machine keyring impacts the roots of trust by
>> linking the .machine keyring to the .secondary keyring. The roots of
>> trust of different keyring are described as follows,
>>
>"of ... keyring" -> "for the ... keyrings"

Thanks for catching those typos and improving the wording!

>
>> .builtin_trusted_keys:
>>
>> Keys may be built into the kernel during build or inserted into memory
>> reserved for keys post build. The root of trust is the kernel build i.e.
>> a Linux distribution vendor. On a physical system in a secure boot
>> environment, this trust is rooted in hardware.
>
>Please look at my response to your question below.
>
>>
>> .machine:
>>
>> If the end-users choose to trust the keys provided by first-stage UEFI
>> bootloader shim i.e. Machine Owner Keys (MOK keys), the keys will be
>> added to this keyring and this keyring is linked to the
>
> Grammatically "and this"  needs to be fixed.

How about 
"the keys will be added to this keyring which  is linked to the..."?

>
>> .secondary_trusted_keys keyring as same as the .builtin_trusted_keys
>> keyring. Shim has built-in keys from a Linux distribution or the
>> end-users-enrolled keys. So the root of trust of this keyring is either
>> a Linux distribution vendor or the end-users.
>>
>> .secondary_trusted_keys:
>>
>> Certificates signed by keys on the .builtin_trusted_keys, .machine, or
>> existing keys on the .secondary_trusted_keys keryings may be loaded
>> onto the .secondary_trusted_keys keyring. This establishes a signature
>> chain of trust based on keys loaded on either the .builtin_trusted_keys
>> or .machine keyrings, if configured and enabled.
>>
>> .platform:
>>
>> The .platform keyring consist of UEFI db and MOK keys which are used by
>> shim to verify the first boot kernel's image signature. If end-users
>> choose to trust MOK keys and the kernel has the .machine keyring
>> enabled, the .platform keyring only consists of UEFI db keys since the
>> MOK keys are added to the .machine keyring instead. Because the
>> end-users could also enroll there own MOK keys, the root of trust could
>
>"there" -> "their"
>
>> be hardware or the end-users.
>
>It's always "hardware".  "or" -> "and"?

Thanks for catching these issues as well!

>
><snip>
>
>> >>
>> >> The root of trusts of the keys in the %.builtin_trusted_keys and
>> >> secondary_trusted_keys keyring is a Linux distribution vendor.
>> >
>> >The root of trust for each keyring should be described separately.
>> >
>> >.builtin_trusted_keys:
>> >
>> >For example,
>> >
>> >Keys may be built into the kernel during build or inserted into memory
>> >reserved for keys post build.  In both of these cases, trust is based
>> >on verification of the kernel image signature.
>>
>> Correct me if I'm wrong, without secure boot, there is no verification
>> of the kernel image signature so the root of trust should be trust on
>> the kernel builder.
>
>No, basing the signature verification on secure boot could not have
>been upstreamed.  

Thanks for correcting me! I was a bit confused by secure boot and
lockdown and also forgot enabling lockdown automatically when secure
boot is enabled is a downstream feature. Btw, when testing the 4th s390
patch, I found s390 skip signature validation when secure boot is not
enabled, is this a mistake?

// arch/s390/kernel/machine_kexec_file.c
#ifdef CONFIG_KEXEC_SIG
int s390_verify_sig(const char *kernel, unsigned long kernel_len)
{
	/* Skip signature verification when not secure IPLed. */
	if (!ipl_secure_flag)
		return 0;

> IMA is based on policy, regardeless of the secure
>boot mode.  A builtin policy may be specified on the boot command line,
>but should be replaced with a more constrained custom policy [1].
>Unlike the builtin policy rules, the architecture specific rules are
>persistent[2].   The architecture specific rules are normally tied to
>the secure boot modes.  On OpenPOWER, the architecture specific
>"measure" rules are dependent on the trusted boot mode.
>
>The current IMA policy rules can be viewed by cat'ing
><securityfs>/ima/policy.

Thanks for explaining IMA to me! There is still the question of what's
the root of trust for .builtin_trusted_keys when there is no real
signature verification. For example, when CONFIG_KEXEC_SIG is enabled,
the default IMA policy is to not appraise kexec image. Since lockdown is
not enabled by default, there is no real verification as
kimage_validate_signature succeeds even when kexec_image_verify_sig
fails. 

>
>[1] The builtin policies are not LSM aware.  The policy rules need to
>be constrained to avoid integrity violations.
>
>[2] arch specific policy rules:
>security/integrity/ima/ima_efi.c, arch/powerpc/kernel/ima_arch.c
>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-06-09 15:35         ` Mimi Zohar
@ 2022-06-16  1:21           ` Coiby Xu
  2022-06-17 12:06             ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Coiby Xu @ 2022-06-16  1:21 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: kexec, linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee

Hi Mimi,

On Thu, Jun 09, 2022 at 11:35:13AM -0400, Mimi Zohar wrote:
>Hi Coiby,
>
>On Fri, 2022-05-27 at 21:43 +0800, Coiby Xu wrote:
>> It seems I need to only change cover letter and commit message i.e.
>> there is no concern about the code. So it's better to provide a
>> new cover letter here to collect new feedback from you thus we
>> can avoid unnecessary rounds of patch set.
>
>One other thing before re-posting, the existing Subject line is a bit
>snarky.  Perhaps you could come with a more appropriate title?

How about "Using the same keyrings as x86 to verify kexec'ed kernel image
signature for arm64 and s390" or "Uniform keyrings to verify kexec'ed
kernel image signature across x86, arm64 and s390"?

>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature
  2022-06-09 23:15   ` Mimi Zohar
@ 2022-06-16  1:22     ` Coiby Xu
  2022-06-17  9:34     ` Michal Suchánek
  1 sibling, 0 replies; 30+ messages in thread
From: Coiby Xu @ 2022-06-16  1:22 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: kexec, linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee, stable, keyrings,
	linux-security-module, Catalin Marinas, James Morse,
	AKASHI Takahiro, open list

On Thu, Jun 09, 2022 at 07:15:27PM -0400, Mimi Zohar wrote:
>On Thu, 2022-05-12 at 15:01 +0800, Coiby Xu wrote:
>> Currently, a problem faced by arm64 is if a kernel image is signed by a
>> MOK key, loading it via the kexec_file_load() system call would be
>> rejected with the error "Lockdown: kexec: kexec of unsigned images is
>> restricted; see man kernel_lockdown.7".
>>
>> This happens because image_verify_sig uses only the primary keyring that
>> contains only kernel built-in keys to verify the kexec image.
>
>From the git history it's clear that .platform keyring was upstreamed
>during the same open window as commit 732b7b93d849 ("arm64: kexec_file:
>add kernel signature verification support").   Loading the MOK keys
>onto the .platform keyring was upstreamed much later.  For this reason,
>commit 732b7b93d849 only used keys on the  .builtin_trusted_keys
>keyring.   This patch is now addressing it and the newly upstreamed
>.machine keyring.

Thanks for explaining the history to me!

>
>Only using the .builtin_trusted_keys is the problem statement, which
>should be one of the first lines of the patch description, if not the
>first line.

Thanks for the suggestion! I've provided a new version of commit message
in the end.

>
>>
>> This patch allows to verify arm64 kernel image signature using not only
>> .builtin_trusted_keys but also .platform and .secondary_trusted_keys
>> keyring.
>
>Please remember to update this to include the .machine keyring.

Sure, thanks for the reminder!

>
>>
>> Fixes: 732b7b93d849 ("arm64: kexec_file: add kernel signature verification support")
>
>Since the MOK keys weren't loaded onto the .platform keyring until much
>later, I would not classify this as a fix.

For this Fixes tag, I mean arm64 couldn't make use of the secondary
keyring because a secondary system keyring was introduced long before
commit 732b7b93d849 ("arm64: kexec_file: add kernel signature
verification support"),

	commit d3bfe84129f65e0af2450743ebdab33d161d01c9
	Author: David Howells <dhowells@redhat.com>
	Date:   Wed Apr 6 16:14:27 2016 +0100
	
	    certs: Add a secondary system keyring that can be added to dynamically
	
	
	commit 732b7b93d849f8a44886ead563dfb6adec7f4419
	Author: AKASHI Takahiro <takahiro.akashi@linaro.org>
	Date:   Thu Nov 15 14:52:54 2018 +0900
	
	    arm64: kexec_file: add kernel signature verification support

Of course, my commit message is misleading since it talks about MOK key
in the first line. If stating the problem as arm64 only using the
.builtin_trusted_keys as suggested by you, I think this Fixes tag would
make sense. How about the following one?

Currently, when loading a kernel image via the kexec_file_load() system
call, arm64 can only use the .builtin_trusted_keys keyring to verify
a signature whereas x86 can use three more keyrings i.e.
.secondary_trusted_keys, .machine and .platform keyrings. For example,
one resulting problem is kexec'ing a kernel image  would be rejected
with the error "Lockdown: kexec: kexec of unsigned images is restricted;
see man kernel_lockdown.7".
  
This patch set enables arm64 to make use of the same keyrings as x86 to
verify the signature kexec'ed kernel image. 

Fixes: 732b7b93d849 ("arm64: kexec_file: add kernel signature verification support")


>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification
  2022-05-19 17:11           ` Michal =?unknown-8bit?q?Such=C3=A1nek?=
@ 2022-06-16  1:46             ` Coiby Xu
  0 siblings, 0 replies; 30+ messages in thread
From: Coiby Xu @ 2022-06-16  1:46 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Baoquan He, Michal Suchánek, Heiko Carstens, akpm, kexec,
	keyrings, linux-arm-kernel, Dave Young, Will Deacon,
	Eric W . Biederman, Chun-Yi Lee, stable, Philipp Rudo,
	linux-security-module, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, Martin Schwidefsky,
	open list:S390, open list, linux-integrity, Jarkko Sakkinen

Hi Mimi,

>> >
>> > This patch set could probably go through KEYS/KEYRINGS_INTEGRITY, but
>> > it's kind of late to be asking.  Has it been in linux-next?  Should I
>> > assume this patch set has been fully tested or can we get some "tags"?
>>
[...]
>>
>> IIRC, Coiby has tested it on x86_64/arm64, not sure if he took test on
>> s390. No, this hasn't been in linux-next.

For arm64, recently I did a new round of test and the patches works as
expected,
   1. Build 5.19.0-rc2
   2. generate keys and add them to .secondary_trusted_keys, MOK, UEFI
      db; 
   3. sign different kernel images with different keys including keys
      from .builtin_trusted_key, .secondary_trusted_keys keyring, UEFI db
      key and MOK key 
   4. Without lockdown, all kernel images can be kexec'ed; with lockdown
      enabled, only the kernel image signed by the key from
      .builtin_trusted_key can be kexec'ed

Then I build a new kernel with the patches applied and confirm all
kernel images can be kexec'ed.

>
>I used the s390 code on powerpc and there it did not work because the
>built-in key was needed to verify the kernel.
>
>I did not really run this on s390, only ported the fix I needed on
>powerpc back to s390.

For 390, I commented out the code that skips signature verification
when secure boot is not enabled since I couldn't find a machine that
supports secure boot and confirm before applying the patch, kernel
images signed by keys from .builtin_trusted_key, .secondary_trusted_keys
couldn't be kexec'ed when lockdown is enabled; after applying the
patch, those kernel images could be kexec'ed. 

>
>Thanks
>
>Michal
>

-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic
  2022-06-09 22:18   ` Mimi Zohar
@ 2022-06-16  1:47     ` Coiby Xu
  0 siblings, 0 replies; 30+ messages in thread
From: Coiby Xu @ 2022-06-16  1:47 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: kexec, linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee, keyrings,
	linux-security-module, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	H. Peter Anvin, open list:X86 ARCHITECTURE (32-BIT AND 64-BIT)

Hi Mimi,

On Thu, Jun 09, 2022 at 06:18:44PM -0400, Mimi Zohar wrote:
>Hi Coiby,
>
>On Thu, 2022-05-12 at 15:01 +0800, Coiby Xu wrote:
>> commit 278311e417be ("kexec, KEYS: Make use of platform keyring for
>> signature verify") adds platform keyring support on x86 kexec but not
>> arm64.
>>
>> The code in bzImage64_verify_sig makes use of system keyrings including
>> .buitin_trusted_keys, .secondary_trusted_keys and .platform keyring to
>> verify signed kernel image as PE file. Make it generic so both x86_64
>> and arm64 can use it.
>
>^uses the keys on the .builtin_trusted_keys, .machine, if configured
>and enabled, .secondary_trusted_keys, also if configured, and .platform
>keyrings to verify the signed kernel image as PE file.
>
>>
>> @@ -202,6 +203,12 @@ int arch_kexec_apply_relocations(struct purgatory_info *pi,
>>  				 const Elf_Shdr *relsec,
>>  				 const Elf_Shdr *symtab);
>>  int arch_kimage_file_post_load_cleanup(struct kimage *image);
>> +#ifdef CONFIG_KEXEC_SIG
>> +#ifdef CONFIG_SIGNED_PE_FILE_VERIFICATION
>> +int kexec_kernel_verify_pe_sig(const char *kernel,
>> +				    unsigned long kernel_len);
>
>Please join this line with the previous one.
>
>> +#endif
>> +#endif
>>  int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
>>
>>  extern int kexec_add_buffer(struct kexec_buf *kbuf);

I've applied the two suggestions, thanks!

>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-06-16  1:15           ` Coiby Xu
@ 2022-06-17  3:57             ` Coiby Xu
  2022-06-17 11:58               ` Mimi Zohar
  0 siblings, 1 reply; 30+ messages in thread
From: Coiby Xu @ 2022-06-17  3:57 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: kexec, linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee

On Thu, Jun 16, 2022 at 09:15:06AM +0800, Coiby Xu wrote:
>Hi Mimi,
>
>Thanks for carefully reviewing the covert letter and patches and
>suggesting various improvements! And sorry for the late reply as I need
>some time to learn more about secure boot, lockdown and IMA to better
>make sense of what you mean.
>
>On Fri, May 27, 2022 at 12:45:54PM -0400, Mimi Zohar wrote:
>>
>>>new cover letter here to collect new feedback from you thus we
>>>can avoid unnecessary rounds of patch set.
>>
>>Agreed.   Much better.  Just a couple of nits.
>
>>
>>>Currently when loading a kernel image via the kexec_file_load() system
>>>call, x86 can make use of three keyrings i.e. the .builtin_trusted_keys,
>>>.secondary_trusted_keys and .platform keyrings to verify signature.
>>
>>Either "a signature" or "signatures".
>>
>>>However, arm64 and s390 can only use the .builtin_trusted_keys and
>>>.platform keyring respectively. For example, one resulting problem is
>>>kexec'ing a kernel image  would be rejected with the error "Lockdown:
>>>kexec: kexec of unsigned images is restricted; see man
>>>kernel_lockdown.7".
>>>
>>>This patch set enables arm64 and s390 to make use of the same keyrings
>>>as x86 to very the signature kexec'ed kernel image.
>>
>> Fix "very".  Perhaps "verify the kexec'ed kernel image signature".
>>>
>>>The recently introduced .machine keyring impacts the roots of trust by
>>>linking the .machine keyring to the .secondary keyring. The roots of
>>>trust of different keyring are described as follows,
>>>
>>"of ... keyring" -> "for the ... keyrings"
>
>Thanks for catching those typos and improving the wording!
>
>>
>>>.builtin_trusted_keys:
>>>
>>>Keys may be built into the kernel during build or inserted into memory
>>>reserved for keys post build. The root of trust is the kernel build i.e.
>>>a Linux distribution vendor. On a physical system in a secure boot
>>>environment, this trust is rooted in hardware.
>>
>>Please look at my response to your question below.
>>
>>>
>>>.machine:
>>>
>>>If the end-users choose to trust the keys provided by first-stage UEFI
>>>bootloader shim i.e. Machine Owner Keys (MOK keys), the keys will be
>>>added to this keyring and this keyring is linked to the
>>
>>Grammatically "and this"  needs to be fixed.
>
>How about "the keys will be added to this keyring which  is linked to 
>the..."?
>
>>
>>>.secondary_trusted_keys keyring as same as the .builtin_trusted_keys
>>>keyring. Shim has built-in keys from a Linux distribution or the
>>>end-users-enrolled keys. So the root of trust of this keyring is either
>>>a Linux distribution vendor or the end-users.
>>>
>>>.secondary_trusted_keys:
>>>
>>>Certificates signed by keys on the .builtin_trusted_keys, .machine, or
>>>existing keys on the .secondary_trusted_keys keryings may be loaded
>>>onto the .secondary_trusted_keys keyring. This establishes a signature
>>>chain of trust based on keys loaded on either the .builtin_trusted_keys
>>>or .machine keyrings, if configured and enabled.
>>>
>>>.platform:
>>>
>>>The .platform keyring consist of UEFI db and MOK keys which are used by
>>>shim to verify the first boot kernel's image signature. If end-users
>>>choose to trust MOK keys and the kernel has the .machine keyring
>>>enabled, the .platform keyring only consists of UEFI db keys since the
>>>MOK keys are added to the .machine keyring instead. Because the
>>>end-users could also enroll there own MOK keys, the root of trust could
>>
>>"there" -> "their"
>>
>>>be hardware or the end-users.
>>
>>It's always "hardware".  "or" -> "and"?
>
>Thanks for catching these issues as well!
>
>>
>><snip>
>>
>>>>>
>>>>> The root of trusts of the keys in the %.builtin_trusted_keys and
>>>>> secondary_trusted_keys keyring is a Linux distribution vendor.
>>>>
>>>>The root of trust for each keyring should be described separately.
>>>>
>>>>.builtin_trusted_keys:
>>>>
>>>>For example,
>>>>
>>>>Keys may be built into the kernel during build or inserted into memory
>>>>reserved for keys post build.  In both of these cases, trust is based
>>>>on verification of the kernel image signature.
>>>
>>>Correct me if I'm wrong, without secure boot, there is no verification
>>>of the kernel image signature so the root of trust should be trust on
>>>the kernel builder.
>>
>>No, basing the signature verification on secure boot could not have
>>been upstreamed.
>
>Thanks for correcting me! I was a bit confused by secure boot and
>lockdown and also forgot enabling lockdown automatically when secure
>boot is enabled is a downstream feature. Btw, when testing the 4th s390
>patch, I found s390 skip signature validation when secure boot is not
>enabled, is this a mistake?
>
>// arch/s390/kernel/machine_kexec_file.c
>#ifdef CONFIG_KEXEC_SIG
>int s390_verify_sig(const char *kernel, unsigned long kernel_len)
>{
>	/* Skip signature verification when not secure IPLed. */
>	if (!ipl_secure_flag)
>		return 0;
>
>>IMA is based on policy, regardeless of the secure
>>boot mode.  A builtin policy may be specified on the boot command line,
>>but should be replaced with a more constrained custom policy [1].
>>Unlike the builtin policy rules, the architecture specific rules are
>>persistent[2].   The architecture specific rules are normally tied to
>>the secure boot modes.  On OpenPOWER, the architecture specific
>>"measure" rules are dependent on the trusted boot mode.
>>
>>The current IMA policy rules can be viewed by cat'ing
>><securityfs>/ima/policy.
>
>Thanks for explaining IMA to me! There is still the question of what's
>the root of trust for .builtin_trusted_keys when there is no real
>signature verification. For example, when CONFIG_KEXEC_SIG is enabled,
>the default IMA policy is to not appraise kexec image. Since lockdown is
>not enabled by default, there is no real verification as
>kimage_validate_signature succeeds even when kexec_image_verify_sig
>fails.

I realize my reasoning is incorrect. Actually the signature
verification which establishes the trust on the keys happens in the
bootloader. So IMA appraisal or kimage_validate_signature is irrelevant
to the question of the root of trust of .builtin_trusted_key. For GRUB,
it won't verify the signature by default when secure boot is not enabled.
Thus the question of what's root of trust when there is no signature
verification is still valid.

>
>>
>>[1] The builtin policies are not LSM aware.  The policy rules need to
>>be constrained to avoid integrity violations.
>>
>>[2] arch specific policy rules:
>>security/integrity/ima/ima_efi.c, arch/powerpc/kernel/ima_arch.c
>>
>>thanks,
>>
>>Mimi
>>
>
>-- 
>Best regards,
>Coiby

-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature
  2022-06-09 23:15   ` Mimi Zohar
  2022-06-16  1:22     ` Coiby Xu
@ 2022-06-17  9:34     ` Michal Suchánek
  1 sibling, 0 replies; 30+ messages in thread
From: Michal Suchánek @ 2022-06-17  9:34 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Coiby Xu, kexec, linux-arm-kernel, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee, stable, keyrings,
	linux-security-module, Catalin Marinas, James Morse,
	AKASHI Takahiro, open list

Hello,

On Thu, Jun 09, 2022 at 07:15:27PM -0400, Mimi Zohar wrote:
> On Thu, 2022-05-12 at 15:01 +0800, Coiby Xu wrote:
> > Currently, a problem faced by arm64 is if a kernel image is signed by a
> > MOK key, loading it via the kexec_file_load() system call would be
> > rejected with the error "Lockdown: kexec: kexec of unsigned images is
> > restricted; see man kernel_lockdown.7".
> > 
> > This happens because image_verify_sig uses only the primary keyring that
> > contains only kernel built-in keys to verify the kexec image.
> 
> From the git history it's clear that .platform keyring was upstreamed
> during the same open window as commit 732b7b93d849 ("arm64: kexec_file:
> add kernel signature verification support").   Loading the MOK keys
> onto the .platform keyring was upstreamed much later.  For this reason,
> commit 732b7b93d849 only used keys on the  .builtin_trusted_keys
> keyring.   This patch is now addressing it and the newly upstreamed
> .machine keyring.
> 
> Only using the .builtin_trusted_keys is the problem statement, which
> should be one of the first lines of the patch description, if not the
> first line.
> 
> > 
> > This patch allows to verify arm64 kernel image signature using not only
> > .builtin_trusted_keys but also .platform and .secondary_trusted_keys
> > keyring.
> 
> Please remember to update this to include the .machine keyring.
> 
> > 
> > Fixes: 732b7b93d849 ("arm64: kexec_file: add kernel signature verification support")
> 
> Since the MOK keys weren't loaded onto the .platform keyring until much
> later, I would not classify this as a fix.

It's definitely a fix

ea93102f32244e3f45c8b26260be77ed0cc1d16c v4.19-rc1 Fix kexec forbidding kernels signed with keys in the secondary keyring to boot
732b7b93d849f8a44886ead563dfb6adec7f4419 v5.0-rc1 arm64: kexec_file: add kernel signature verification support

the arm code does not reflect the preexisting use of the secondary keyring.

278311e417be60f7caef6fcb12bda4da2711ceff v5.1-rc1 kexec, KEYS: Make use of platform keyring for signature

This was added a bit later, indeed.

Cherry-picking the changes one by one to the arm code you can get
separate fixes tag for each change.

Replacing the arm code with the x86 code by code deduplication you get
whatever is the state of the art in the kernel in question. In 5.0 you
get the secondary keyring, and since 5.1 you also get the platform
keyring.

Nonetheless, it's a fix for the arm code which is deficient from the
very start.

Thanks

Michal


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-06-17  3:57             ` Coiby Xu
@ 2022-06-17 11:58               ` Mimi Zohar
  2022-06-20 13:14                 ` Coiby Xu
  0 siblings, 1 reply; 30+ messages in thread
From: Mimi Zohar @ 2022-06-17 11:58 UTC (permalink / raw)
  To: Coiby Xu
  Cc: kexec, linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee

On Fri, 2022-06-17 at 11:57 +0800, Coiby Xu wrote:
> >Thanks for explaining IMA to me! There is still the question of what's
> >the root of trust for .builtin_trusted_keys when there is no real
> >signature verification. For example, when CONFIG_KEXEC_SIG is enabled,
> >the default IMA policy is to not appraise kexec image. Since lockdown is
> >not enabled by default, there is no real verification as
> >kimage_validate_signature succeeds even when kexec_image_verify_sig
> >fails.
> 
> I realize my reasoning is incorrect. Actually the signature
> verification which establishes the trust on the keys happens in the
> bootloader. So IMA appraisal or kimage_validate_signature is irrelevant
> to the question of the root of trust of .builtin_trusted_key. For GRUB,
> it won't verify the signature by default when secure boot is not enabled.
> Thus the question of what's root of trust when there is no signature
> verification is still valid.

We're saying the same thing, just differently.  Your wording describes
secure boot, how it is established, and who/what is responsible for it.
I don't think those details are needed.  I originally said,

.builtin_trusted_keys:

For example,

Keys may be built into the kernel during build or inserted into memory
reserved for keys post build.  In both of these cases, trust is based
on verification of the kernel image signature.  On a physical system in
a secure boot environment, this trust is rooted in HW.

The last line should have said, "For example, on a physical system in a
...".

thanks,

Mimi


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-06-16  1:21           ` Coiby Xu
@ 2022-06-17 12:06             ` Mimi Zohar
  0 siblings, 0 replies; 30+ messages in thread
From: Mimi Zohar @ 2022-06-17 12:06 UTC (permalink / raw)
  To: Coiby Xu
  Cc: kexec, linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee

On Thu, 2022-06-16 at 09:21 +0800, Coiby Xu wrote:
> Hi Mimi,
> 
> On Thu, Jun 09, 2022 at 11:35:13AM -0400, Mimi Zohar wrote:
> >Hi Coiby,
> >
> >On Fri, 2022-05-27 at 21:43 +0800, Coiby Xu wrote:
> >> It seems I need to only change cover letter and commit message i.e.
> >> there is no concern about the code. So it's better to provide a
> >> new cover letter here to collect new feedback from you thus we
> >> can avoid unnecessary rounds of patch set.
> >
> >One other thing before re-posting, the existing Subject line is a bit
> >snarky.  Perhaps you could come with a more appropriate title?
> 
> How about "Using the same keyrings as x86 to verify kexec'ed kernel image
> signature for arm64 and s390" or "Uniform keyrings to verify kexec'ed
> kernel image signature across x86, arm64 and s390"?

Instead of using "Uniform", I would use the verb form "Unify".

Mimi
 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature
  2022-06-17 11:58               ` Mimi Zohar
@ 2022-06-20 13:14                 ` Coiby Xu
  0 siblings, 0 replies; 30+ messages in thread
From: Coiby Xu @ 2022-06-20 13:14 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: kexec, linux-arm-kernel, Michal Suchanek, Baoquan He, Dave Young,
	Will Deacon, Eric W . Biederman, Chun-Yi Lee

On Fri, Jun 17, 2022 at 07:58:37AM -0400, Mimi Zohar wrote:
>On Fri, 2022-06-17 at 11:57 +0800, Coiby Xu wrote:
>> >Thanks for explaining IMA to me! There is still the question of what's
>> >the root of trust for .builtin_trusted_keys when there is no real
>> >signature verification. For example, when CONFIG_KEXEC_SIG is enabled,
>> >the default IMA policy is to not appraise kexec image. Since lockdown is
>> >not enabled by default, there is no real verification as
>> >kimage_validate_signature succeeds even when kexec_image_verify_sig
>> >fails.
>>
>> I realize my reasoning is incorrect. Actually the signature
>> verification which establishes the trust on the keys happens in the
>> bootloader. So IMA appraisal or kimage_validate_signature is irrelevant
>> to the question of the root of trust of .builtin_trusted_key. For GRUB,
>> it won't verify the signature by default when secure boot is not enabled.
>> Thus the question of what's root of trust when there is no signature
>> verification is still valid.
>
>We're saying the same thing, just differently.  Your wording describes
>secure boot, how it is established, and who/what is responsible for it.
>I don't think those details are needed.  I originally said,

I think I'm addressing a different concern or case. If kexec_file_load is going
to verify a kernel image signature, what keys is it going to trust and why? I
believe explaining the root trust for different keyrings is to answer this
question. When a bootloader verifies a kernel image signature, the trust is
based on verification of the kernel image signature which we both agree. But
what if a bootloader doesn't do the verification? 

>
>.builtin_trusted_keys:
>
>For example,
>
>Keys may be built into the kernel during build or inserted into memory
>reserved for keys post build.  In both of these cases, trust is based
>on verification of the kernel image signature.  On a physical system in
>a secure boot environment, this trust is rooted in HW.
>
>The last line should have said, "For example, on a physical system in a
>...".
>
>thanks,
>
>Mimi
>

-- 
Best regards,
Coiby


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

end of thread, other threads:[~2022-06-20 13:26 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-12  7:01 [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Coiby Xu
2022-05-12  7:01 ` [PATCH v8 1/4] kexec: clean up arch_kexec_kernel_verify_sig Coiby Xu
2022-06-09 21:57   ` Mimi Zohar
2022-05-12  7:01 ` [PATCH v8 2/4] kexec, KEYS: make the code in bzImage64_verify_sig generic Coiby Xu
2022-05-12  7:21   ` Baoquan He
2022-06-09 22:18   ` Mimi Zohar
2022-06-16  1:47     ` Coiby Xu
2022-05-12  7:01 ` [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature Coiby Xu
2022-06-09 23:15   ` Mimi Zohar
2022-06-16  1:22     ` Coiby Xu
2022-06-17  9:34     ` Michal Suchánek
2022-05-12  7:01 ` [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification Coiby Xu
2022-05-18 11:29   ` Heiko Carstens
2022-05-19  0:39     ` Baoquan He
2022-05-19 11:56       ` Mimi Zohar
2022-05-19 14:22         ` Baoquan He
2022-05-19 17:11           ` Michal =?unknown-8bit?q?Such=C3=A1nek?=
2022-06-16  1:46             ` Coiby Xu
2022-05-20 17:04 ` [PATCH v8 0/4] use more system keyrings to verify arm64 and s390 kexec kernel image signature Mimi Zohar
2022-05-25  9:59   ` Coiby Xu
2022-05-25 13:30     ` Mimi Zohar
2022-05-27 13:43       ` Coiby Xu
2022-05-27 16:45         ` Mimi Zohar
2022-06-16  1:15           ` Coiby Xu
2022-06-17  3:57             ` Coiby Xu
2022-06-17 11:58               ` Mimi Zohar
2022-06-20 13:14                 ` Coiby Xu
2022-06-09 15:35         ` Mimi Zohar
2022-06-16  1:21           ` Coiby Xu
2022-06-17 12:06             ` Mimi Zohar

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