2022-02-15 20:43:02

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH 0/4] Unifrom keyring support across architectures and functions

While testing KEXEC_SIG on powerpc I noticed discrepancy in support for
different keyrings across architectures and between KEXEC_SIG and
MODULE_SIG. Fix this by enabling suport for the missing keyrings.

The latter two patches obviously conflict with the ongoing module code
cleanup. If they turn out desirable I will add them to the other series
dealing with KEXEC_SIG.

The arm patches can be merged independently.

Thanks

Michal

Michal Suchanek (4):
Fix arm64 kexec forbidding kernels signed with keys in the secondary
keyring to boot
kexec, KEYS, arm64: Make use of platform keyring for signature
verification
kexec, KEYS, s390: Make use of built-in and secondary keyring for
signature verification
module, KEYS: Make use of platform keyring for signature verification

arch/arm64/kernel/kexec_image.c | 13 +++++++++++--
arch/s390/kernel/machine_kexec_file.c | 18 +++++++++++++-----
kernel/module_signing.c | 14 ++++++++++----
3 files changed, 34 insertions(+), 11 deletions(-)

--
2.31.1


2022-02-15 22:01:31

by Michal Suchánek

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

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: Philipp Rudo <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Michal Suchanek <[email protected]>
---
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.31.1

2022-02-16 02:11:06

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH 1/4] Fix arm64 kexec forbidding kernels signed with keys in the secondary keyring to boot

commit d3bfe84129f6 ("certs: Add a secondary system keyring that can be added to dynamically")
split of .system_keyring into .builtin_trusted_keys and
.secondary_trusted_keys broke kexec, thereby preventing kernels signed by
keys which are now in the secondary keyring from being kexec'd.

Fix this by passing VERIFY_USE_SECONDARY_KEYRING to
verify_pefile_signature().

Cherry-picked from
commit ea93102f3224 ("Fix kexec forbidding kernels signed with keys in the secondary keyring to boot")

Fixes: 732b7b93d849 ("arm64: kexec_file: add kernel signature verification support")
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Michal Suchanek <[email protected]>
---
arch/arm64/kernel/kexec_image.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
index 9ec34690e255..1fbf2ee7c005 100644
--- a/arch/arm64/kernel/kexec_image.c
+++ b/arch/arm64/kernel/kexec_image.c
@@ -133,7 +133,8 @@ static void *image_load(struct kimage *image,
#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,
+ return verify_pefile_signature(kernel, kernel_len,
+ VERIFY_USE_SECONDARY_KEYRING,
VERIFYING_KEXEC_PE_SIGNATURE);
}
#endif
--
2.31.1

2022-02-16 06:58:32

by Michal Suchánek

[permalink] [raw]
Subject: [PATCH 4/4] module, KEYS: Make use of platform keyring for signature verification

Commit 278311e417be ("kexec, KEYS: Make use of platform keyring for signature verify")
adds support for use of platform keyring in kexec verification but
support for modules is missing.

Add support for verification of modules with keys from platform keyring
as well.

Fixes: 219a3e8676f3 ("integrity, KEYS: add a reference to platform keyring")
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Michal Suchanek <[email protected]>
---
kernel/module_signing.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 8723ae70ea1f..5e1624294874 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -38,8 +38,14 @@ int mod_verify_sig(const void *mod, struct load_info *info)
modlen -= sig_len + sizeof(ms);
info->len = modlen;

- return verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
- VERIFY_USE_SECONDARY_KEYRING,
- VERIFYING_MODULE_SIGNATURE,
- NULL, NULL);
+ ret = verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
+ VERIFY_USE_SECONDARY_KEYRING,
+ VERIFYING_MODULE_SIGNATURE,
+ NULL, NULL);
+ if (ret == -ENOKEY && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING))
+ ret = verify_pkcs7_signature(mod, modlen, mod + modlen, sig_len,
+ VERIFY_USE_PLATFORM_KEYRING,
+ VERIFYING_MODULE_SIGNATURE,
+ NULL, NULL);
+ return ret;
}
--
2.31.1

2022-03-28 20:57:34

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 4/4] module, KEYS: Make use of platform keyring for signature verification

On Mon, 2022-03-28 at 18:15 +0800, joeyli wrote:

Hi Joey,

> Hi Mimi,
>
> Sorry for bother you for this old topic.

Cc'ing Luis the kernel modules maintainer.

>
> On Tue, Feb 15, 2022 at 09:47:30PM +0100, Michal Such?nek wrote:
> > Hello,
> >
> > On Tue, Feb 15, 2022 at 03:08:18PM -0500, Mimi Zohar wrote:
> > > [Cc'ing Eric Snowberg]
> > >
> > > Hi Michal,
> > >
> > > On Tue, 2022-02-15 at 20:39 +0100, Michal Suchanek wrote:
> > > > Commit 278311e417be ("kexec, KEYS: Make use of platform keyring for signature verify")
> > > > adds support for use of platform keyring in kexec verification but
> > > > support for modules is missing.
> > > >
> > > > Add support for verification of modules with keys from platform keyring
> > > > as well.
> > >
> > > Permission for loading the pre-OS keys onto the "platform" keyring and
> > > using them is limited to verifying the kexec kernel image, nothing
> > > else.
> >
> > Why is the platform keyring limited to kexec, and nothing else?
> >
> > It should either be used for everything or for nothing. You have the
> > option to compile it in and then it should be used, and the option to
> > not compile it in and then it cannot be used.
> >
> > There are two basic use cases:
> >
> > (1) there is a vendor key which is very hard to use so you sign
> > something small and simple like shim with the vendor key, and sign your
> > kernel and modules with your own key that's typically enrolled with shim
> > MOK, and built into the kernel.
> >
> > (2) you import your key into the firmware, and possibly disable the
> > vendor key. You can load the kernel directly without shim, and then your
> > signing key is typically in the platform keyring and built into the
> > kernel.
> >
>
> In the second use case, if user can enroll their own key to db either before
> or after hardware shipping. And they don't need shim because they removed
> Microsoft or OEM/ODM keys. Why kernel can not provide a Kconfig option to
> them for trusting db keys for verifying kernel module, or for IMA (using CA
> in db)?
>
> In the above use case for distro, partner doesn't need to re-compiler distro
> kernel. They just need to re-sign distro kernel and modules. Which means
> that the partner trusted distro. Then the partner's key in db can be used to
> verify kernel image and also kernel module without shim involve.

From what I understand, distros don't want customers resigning their
kernels. If they did, then they could have enabled the
CONFIG_SYSTEM_EXTRA_CERTIFICATE, which would load the keys onto the
"builtin" keyring, and anything signed by those keys could be loaded
onto the secondary keyring. (Of course CONFIG_SYSTEM_EXTRA_CERTIFICATE
would need to be fixed/updated.)

We've gone through "what if" scenarios before. My response then, as
now, is post it as a patch with the real motivation for such a change.

thanks,

Mimi

2022-03-28 22:19:15

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH 4/4] module, KEYS: Make use of platform keyring for signature verification

Hello,

On Mon, Mar 28, 2022 at 02:44:30PM +0000, Eric Snowberg wrote:
>
>
> > On Mar 28, 2022, at 4:15 AM, joeyli <[email protected]> wrote:
> >
> > Hi Mimi,
> >
> > Sorry for bother you for this old topic.
> >
> > On Tue, Feb 15, 2022 at 09:47:30PM +0100, Michal Such?nek wrote:
> >> Hello,
> >>
> >> On Tue, Feb 15, 2022 at 03:08:18PM -0500, Mimi Zohar wrote:
> >>> [Cc'ing Eric Snowberg]
> >>>
> >>> Hi Michal,
> >>>
> >>> On Tue, 2022-02-15 at 20:39 +0100, Michal Suchanek wrote:
> >>>> Commit 278311e417be ("kexec, KEYS: Make use of platform keyring for signature verify")
> >>>> adds support for use of platform keyring in kexec verification but
> >>>> support for modules is missing.
> >>>>
> >>>> Add support for verification of modules with keys from platform keyring
> >>>> as well.
> >>>
> >>> Permission for loading the pre-OS keys onto the "platform" keyring and
> >>> using them is limited to verifying the kexec kernel image, nothing
> >>> else.
> >>
> >> Why is the platform keyring limited to kexec, and nothing else?
> >>
> >> It should either be used for everything or for nothing. You have the
> >> option to compile it in and then it should be used, and the option to
> >> not compile it in and then it cannot be used.
> >>
> >> There are two basic use cases:
> >>
> >> (1) there is a vendor key which is very hard to use so you sign
> >> something small and simple like shim with the vendor key, and sign your
> >> kernel and modules with your own key that's typically enrolled with shim
> >> MOK, and built into the kernel.
> >>
> >> (2) you import your key into the firmware, and possibly disable the
> >> vendor key. You can load the kernel directly without shim, and then your
> >> signing key is typically in the platform keyring and built into the
> >> kernel.
> >>
> >
> > In the second use case, if user can enroll their own key to db either before
> > or after hardware shipping. And they don't need shim because they removed
> > Microsoft or OEM/ODM keys. Why kernel can not provide a Kconfig option to
> > them for trusting db keys for verifying kernel module, or for IMA (using CA
> > in db)?
> >
> > In the above use case for distro, partner doesn't need to re-compiler distro
> > kernel. They just need to re-sign distro kernel and modules. Which means
> > that the partner trusted distro. Then the partner's key in db can be used to
> > verify kernel image and also kernel module without shim involve.
>
> If shim is used, the new machine keyring can be used to solve this problem.
> This pull request [1] allows additional certificates to be loaded into the MOKList
> without going through MokManager. Have the end-user/partner create a
> shim_certificate.efi containing their key. Then sign it with their DB key. When
> shim boots, it will validate shim_certificate.efi against the DB key and load the
> key contained within it into the MOKList. Now both module and kernel validation
> can be performed with this key, since it is contained within the machine keyring.

And why would you go through that when your platform keyring already has
the key and you don't need shim for anything? This sounds a lot like "I
have a hammer and all these look like nails" thinking.

Sure, there is use for the machine keyring in the case you need it and
have it regardless of the kernel making any use of it for anything.
Artifically adding it because the kernel fails to work with the platform
keyring sounds backwards, though.

Thanks

Michal

2022-03-28 22:35:24

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH 4/4] module, KEYS: Make use of platform keyring for signature verification

Hello,

On Mon, Mar 28, 2022 at 09:28:14AM -0400, Mimi Zohar wrote:
> On Mon, 2022-03-28 at 18:15 +0800, joeyli wrote:
>
> Hi Joey,
>
> > Hi Mimi,
> >
> > Sorry for bother you for this old topic.
>
> Cc'ing Luis the kernel modules maintainer.
>
> >
> > On Tue, Feb 15, 2022 at 09:47:30PM +0100, Michal Such?nek wrote:
> > > Hello,
> > >
> > > On Tue, Feb 15, 2022 at 03:08:18PM -0500, Mimi Zohar wrote:
> > > > [Cc'ing Eric Snowberg]
> > > >
> > > > Hi Michal,
> > > >
> > > > On Tue, 2022-02-15 at 20:39 +0100, Michal Suchanek wrote:
> > > > > Commit 278311e417be ("kexec, KEYS: Make use of platform keyring for signature verify")
> > > > > adds support for use of platform keyring in kexec verification but
> > > > > support for modules is missing.
> > > > >
> > > > > Add support for verification of modules with keys from platform keyring
> > > > > as well.
> > > >
> > > > Permission for loading the pre-OS keys onto the "platform" keyring and
> > > > using them is limited to verifying the kexec kernel image, nothing
> > > > else.
> > >
> > > Why is the platform keyring limited to kexec, and nothing else?
> > >
> > > It should either be used for everything or for nothing. You have the
> > > option to compile it in and then it should be used, and the option to
> > > not compile it in and then it cannot be used.
> > >
> > > There are two basic use cases:
> > >
> > > (1) there is a vendor key which is very hard to use so you sign
> > > something small and simple like shim with the vendor key, and sign your
> > > kernel and modules with your own key that's typically enrolled with shim
> > > MOK, and built into the kernel.
> > >
> > > (2) you import your key into the firmware, and possibly disable the
> > > vendor key. You can load the kernel directly without shim, and then your
> > > signing key is typically in the platform keyring and built into the
> > > kernel.
> > >
> >
> > In the second use case, if user can enroll their own key to db either before
> > or after hardware shipping. And they don't need shim because they removed
> > Microsoft or OEM/ODM keys. Why kernel can not provide a Kconfig option to
> > them for trusting db keys for verifying kernel module, or for IMA (using CA
> > in db)?
> >
> > In the above use case for distro, partner doesn't need to re-compiler distro
> > kernel. They just need to re-sign distro kernel and modules. Which means
> > that the partner trusted distro. Then the partner's key in db can be used to
> > verify kernel image and also kernel module without shim involve.
>
> From what I understand, distros don't want customers resigning their
> kernels. If they did, then they could have enabled the
> CONFIG_SYSTEM_EXTRA_CERTIFICATE, which would load the keys onto the
> "builtin" keyring, and anything signed by those keys could be loaded
> onto the secondary keyring. (Of course CONFIG_SYSTEM_EXTRA_CERTIFICATE
> would need to be fixed/updated.)

You don't need to re-sign. You can just import the distro key into the
firmware.

>
> We've gone through "what if" scenarios before. My response then, as
> now, is post it as a patch with the real motivation for such a change.

Then that's what this does. Both modules and kernel run on ring0 so
there is no practical distinction. For consistency verify both with the
same keys.

Either way if there should be a disctinction it should be explicit, not
implicit.

That is each option that imports keys should crate a basic keyring that
just has keys, and we should have 'kexec' and 'module' keyrings that
do not have keys, only link the keyrings that import keys from some
specific source. All of them by default but you can adjust this in
defconfigs depending on platform-typical usage.

Contrast to that the current 'secondary' keyring that randomly links
some key sources and not others, is used in some kexec implementations
and not others. Also if you list the keys in it do you get the keys
dynamically added at runtime, or also all the keys on the linked
keyrings? Whatever you get is misleading and unclear.

Thanks

Michal

2022-04-06 17:38:05

by joeyli

[permalink] [raw]
Subject: Re: [PATCH 1/4] Fix arm64 kexec forbidding kernels signed with keys in the secondary keyring to boot

On Tue, Feb 15, 2022 at 08:39:38PM +0100, Michal Suchanek wrote:
> commit d3bfe84129f6 ("certs: Add a secondary system keyring that can be added to dynamically")
> split of .system_keyring into .builtin_trusted_keys and
> .secondary_trusted_keys broke kexec, thereby preventing kernels signed by
> keys which are now in the secondary keyring from being kexec'd.
>
> Fix this by passing VERIFY_USE_SECONDARY_KEYRING to
> verify_pefile_signature().
>
> Cherry-picked from
> commit ea93102f3224 ("Fix kexec forbidding kernels signed with keys in the secondary keyring to boot")
>
> Fixes: 732b7b93d849 ("arm64: kexec_file: add kernel signature verification support")
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Michal Suchanek <[email protected]>

Reviewed-by: "Lee, Chun-Yi" <[email protected]>

> ---
> arch/arm64/kernel/kexec_image.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> index 9ec34690e255..1fbf2ee7c005 100644
> --- a/arch/arm64/kernel/kexec_image.c
> +++ b/arch/arm64/kernel/kexec_image.c
> @@ -133,7 +133,8 @@ static void *image_load(struct kimage *image,
> #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,
> + return verify_pefile_signature(kernel, kernel_len,
> + VERIFY_USE_SECONDARY_KEYRING,
> VERIFYING_KEXEC_PE_SIGNATURE);
> }
> #endif
> --
> 2.31.1

2022-04-06 17:42:25

by joeyli

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

On Tue, Feb 15, 2022 at 08:39:40PM +0100, Michal Suchanek wrote:
> 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: Philipp Rudo <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Michal Suchanek <[email protected]>

Reviewed-by: "Lee, Chun-Yi" <[email protected]>

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

2022-04-08 07:39:35

by Baoquan He

[permalink] [raw]
Subject: Re: [PATCH 1/4] Fix arm64 kexec forbidding kernels signed with keys in the secondary keyring to boot

Hi,

On 02/15/22 at 08:39pm, Michal Suchanek wrote:
> commit d3bfe84129f6 ("certs: Add a secondary system keyring that can be added to dynamically")
> split of .system_keyring into .builtin_trusted_keys and
> .secondary_trusted_keys broke kexec, thereby preventing kernels signed by
> keys which are now in the secondary keyring from being kexec'd.
>
> Fix this by passing VERIFY_USE_SECONDARY_KEYRING to
> verify_pefile_signature().
>
> Cherry-picked from
> commit ea93102f3224 ("Fix kexec forbidding kernels signed with keys in the secondary keyring to boot")

This line may need a line feed?

The patch 1~3 looks good to me. Coiby encountered the same issue
on arm64, and has posted a patch series to fix that and there's clean up
and code adjustment.

https://lore.kernel.org/all/[email protected]/T/#u

Hi Coiby,

Maybe you can check this patchset, and consider how to integrate your
patches based on this patch 1~/3?

For this patch itself, ack.

Acked-by: Baoquan He <[email protected]>

>
> Fixes: 732b7b93d849 ("arm64: kexec_file: add kernel signature verification support")
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Michal Suchanek <[email protected]>
> ---
> arch/arm64/kernel/kexec_image.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/kexec_image.c b/arch/arm64/kernel/kexec_image.c
> index 9ec34690e255..1fbf2ee7c005 100644
> --- a/arch/arm64/kernel/kexec_image.c
> +++ b/arch/arm64/kernel/kexec_image.c
> @@ -133,7 +133,8 @@ static void *image_load(struct kimage *image,
> #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,
> + return verify_pefile_signature(kernel, kernel_len,
> + VERIFY_USE_SECONDARY_KEYRING,
> VERIFYING_KEXEC_PE_SIGNATURE);
> }
> #endif
> --
> 2.31.1
>

2022-04-08 08:15:09

by Coiby Xu

[permalink] [raw]
Subject: Re: [PATCH 0/4] Unifrom keyring support across architectures and functions

Hi Michal,

As mentioned by Baoquan, I have a patch set "[PATCH v5 0/3] use more
system keyrings to verify arm64 kdump kernel image signature" [1]. The
differences between your patch set and mine are as follows,
- my patch set only adds support for arm64 while yours also extends to
s390
- I made the code for verifying signed kernel image as PE file in x86
public so arm64 can reuse the code as well which seems to be better
approach
- I also cleaned up clean up arch_kexec_kernel_verify_sig

Would you mind if I integrate your first 3 patches with mine as follows
- for arm64, I'll use my version
- for s390, I'll use your version

For your last patch which allows to use of platform keyring for
signature verification of kernel module, I'll leave it to yourself. How
do you think about it?


[1] https://lore.kernel.org/all/[email protected]/

On Tue, Feb 15, 2022 at 08:39:37PM +0100, Michal Suchanek wrote:
>While testing KEXEC_SIG on powerpc I noticed discrepancy in support for
>different keyrings across architectures and between KEXEC_SIG and
>MODULE_SIG. Fix this by enabling suport for the missing keyrings.
>
>The latter two patches obviously conflict with the ongoing module code
>cleanup. If they turn out desirable I will add them to the other series
>dealing with KEXEC_SIG.
>
>The arm patches can be merged independently.
>
>Thanks
>
>Michal
>
>Michal Suchanek (4):
> Fix arm64 kexec forbidding kernels signed with keys in the secondary
> keyring to boot
> kexec, KEYS, arm64: Make use of platform keyring for signature
> verification
> kexec, KEYS, s390: Make use of built-in and secondary keyring for
> signature verification
> module, KEYS: Make use of platform keyring for signature verification
>
> arch/arm64/kernel/kexec_image.c | 13 +++++++++++--
> arch/s390/kernel/machine_kexec_file.c | 18 +++++++++++++-----
> kernel/module_signing.c | 14 ++++++++++----
> 3 files changed, 34 insertions(+), 11 deletions(-)
>
>--
>2.31.1
>

--
Best regards,
Coiby

2022-04-08 10:04:59

by Michal Suchánek

[permalink] [raw]
Subject: Re: [PATCH 0/4] Unifrom keyring support across architectures and functions

On Fri, Apr 08, 2022 at 03:47:04PM +0800, Coiby Xu wrote:
> Hi Michal,
>
> As mentioned by Baoquan, I have a patch set "[PATCH v5 0/3] use more
> system keyrings to verify arm64 kdump kernel image signature" [1]. The
> differences between your patch set and mine are as follows, - my patch set
> only adds support for arm64 while yours also extends to
> s390
> - I made the code for verifying signed kernel image as PE file in x86
> public so arm64 can reuse the code as well which seems to be better
> approach
> - I also cleaned up clean up arch_kexec_kernel_verify_sig
>
> Would you mind if I integrate your first 3 patches with mine as follows
> - for arm64, I'll use my version
> - for s390, I'll use your version

Great

less code duplication is always good.

Thanks

Michal

>
> For your last patch which allows to use of platform keyring for
> signature verification of kernel module, I'll leave it to yourself. How
> do you think about it?
>
>
> [1] https://lore.kernel.org/all/[email protected]/
>
> On Tue, Feb 15, 2022 at 08:39:37PM +0100, Michal Suchanek wrote:
> > While testing KEXEC_SIG on powerpc I noticed discrepancy in support for
> > different keyrings across architectures and between KEXEC_SIG and
> > MODULE_SIG. Fix this by enabling suport for the missing keyrings.
> >
> > The latter two patches obviously conflict with the ongoing module code
> > cleanup. If they turn out desirable I will add them to the other series
> > dealing with KEXEC_SIG.
> >
> > The arm patches can be merged independently.
> >
> > Thanks
> >
> > Michal
> >
> > Michal Suchanek (4):
> > Fix arm64 kexec forbidding kernels signed with keys in the secondary
> > keyring to boot
> > kexec, KEYS, arm64: Make use of platform keyring for signature
> > verification
> > kexec, KEYS, s390: Make use of built-in and secondary keyring for
> > signature verification
> > module, KEYS: Make use of platform keyring for signature verification
> >
> > arch/arm64/kernel/kexec_image.c | 13 +++++++++++--
> > arch/s390/kernel/machine_kexec_file.c | 18 +++++++++++++-----
> > kernel/module_signing.c | 14 ++++++++++----
> > 3 files changed, 34 insertions(+), 11 deletions(-)
> >
> > --
> > 2.31.1
> >
>
> --
> Best regards,
> Coiby
>