[PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature

[PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature

[Coiby Xu]

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@vger.kernel.org # 34d5960af253: kexec: clean up arch_kexec_kernel_verify_sig
Cc: stable@vger.kernel.org # 83b7bb2d49ae: kexec, KEYS: make the code in bzImage64_verify_sig generic
Acked-by: Baoquan He <bhe@redhat.com>
Cc: kexec@lists.infradead.org
Cc: keyrings@vger.kernel.org
Cc: linux-security-module@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

Re: [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature

[Mimi Zohar]

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

Re: [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature

[Coiby Xu]

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

Re: [PATCH v8 3/4] arm64: kexec_file: use more system keyrings to verify kernel image signature

[Michal Suchánek]

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

[PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification

[Coiby Xu]

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@vger.kernel.org
Cc: Philipp Rudo <prudo@linux.ibm.com>
Cc: kexec@lists.infradead.org
Cc: keyrings@vger.kernel.org
Cc: linux-security-module@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

Re: [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification

[Heiko Carstens]

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@vger.kernel.org
> Cc: Philipp Rudo <prudo@linux.ibm.com>
> Cc: kexec@lists.infradead.org
> Cc: keyrings@vger.kernel.org
> Cc: linux-security-module@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>

Re: [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification

[Baoquan He]

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@vger.kernel.org
> > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > Cc: kexec@lists.infradead.org
> > Cc: keyrings@vger.kernel.org
> > Cc: linux-security-module@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

Re: [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification

[Mimi Zohar]

[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@vger.kernel.org
> > > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > > Cc: kexec@lists.infradead.org
> > > Cc: keyrings@vger.kernel.org
> > > Cc: linux-security-module@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

Re: [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification

[Baoquan He]

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@vger.kernel.org
> > > > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > > > Cc: kexec@lists.infradead.org
> > > > Cc: keyrings@vger.kernel.org
> > > > Cc: linux-security-module@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.

Re: [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification

[Michal Suchánek]

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@vger.kernel.org
> > > > > Cc: Philipp Rudo <prudo@linux.ibm.com>
> > > > > Cc: kexec@lists.infradead.org
> > > > > Cc: keyrings@vger.kernel.org
> > > > > Cc: linux-security-module@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

Re: [PATCH v8 4/4] kexec, KEYS, s390: Make use of built-in and secondary keyring for signature verification

[Coiby Xu]

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