| Message ID | 20220512070123.29486-4-coxu@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
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
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 >
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
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 };
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(-)