From: Dave Young <dyoung@redhat.com>
To: Lianbo Jiang <lijiang@redhat.com>
Cc: linux-kernel@vger.kernel.org, kexec@lists.infradead.org,
ebiederm@xmission.com, jbohac@suse.cz, jmorris@namei.org,
mjg59@google.com, bhe@redhat.com
Subject: Re: [PATCH] kexec: Do not verify the signature without the lockdown or mandatory signature
Date: Tue, 26 May 2020 13:04:51 +0800 [thread overview]
Message-ID: <20200526050451.GA74334@dhcp-128-65.nay.redhat.com> (raw)
In-Reply-To: <20200525052351.24134-1-lijiang@redhat.com>
On 05/25/20 at 01:23pm, Lianbo Jiang wrote:
> Signature verification is an important security feature, to protect
> system from being attacked with a kernel of unknown origin. Kexec
> rebooting is a way to replace the running kernel, hence need be
> secured carefully.
>
> In the current code of handling signature verification of kexec kernel,
> the logic is very twisted. It mixes signature verification, IMA signature
> appraising and kexec lockdown.
>
> If there is no KEXEC_SIG_FORCE, kexec kernel image doesn't have one of
> signature, the supported crypto, and key, we don't think this is wrong,
> Unless kexec lockdown is executed. IMA is considered as another kind of
> signature appraising method.
>
> If kexec kernel image has signature/crypto/key, it has to go through the
> signature verification and pass. Otherwise it's seen as verification
> failure, and won't be loaded.
>
> Seems kexec kernel image with an unqualified signature is even worse than
> those w/o signature at all, this sounds very unreasonable. E.g. If people
> get a unsigned kernel to load, or a kernel signed with expired key, which
> one is more dangerous?
>
> So, here, let's simplify the logic to improve code readability. If the
> KEXEC_SIG_FORCE enabled or kexec lockdown enabled, signature verification
> is mandated. Otherwise, we lift the bar for any kernel image.
>
> Signed-off-by: Lianbo Jiang <lijiang@redhat.com>
> ---
> kernel/kexec_file.c | 37 ++++++-------------------------------
> 1 file changed, 6 insertions(+), 31 deletions(-)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index faa74d5f6941..e4bdf0c42f35 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -181,52 +181,27 @@ void kimage_file_post_load_cleanup(struct kimage *image)
> static int
> kimage_validate_signature(struct kimage *image)
> {
> - const char *reason;
> int ret;
>
> ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
> image->kernel_buf_len);
> - switch (ret) {
> - case 0:
> - break;
> + if (ret) {
> + pr_debug("kernel signature verification failed (%d).\n", ret);
>
> - /* Certain verification errors are non-fatal if we're not
> - * checking errors, provided we aren't mandating that there
> - * must be a valid signature.
> - */
> - case -ENODATA:
> - reason = "kexec of unsigned image";
> - goto decide;
> - case -ENOPKG:
> - reason = "kexec of image with unsupported crypto";
> - goto decide;
> - case -ENOKEY:
> - reason = "kexec of image with unavailable key";
> - decide:
> - if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
> - pr_notice("%s rejected\n", reason);
> + if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE))
> return ret;
> - }
>
> - /* If IMA is guaranteed to appraise a signature on the kexec
> + /*
> + * If IMA is guaranteed to appraise a signature on the kexec
> * image, permit it even if the kernel is otherwise locked
> * down.
> */
> if (!ima_appraise_signature(READING_KEXEC_IMAGE) &&
> security_locked_down(LOCKDOWN_KEXEC))
> return -EPERM;
> -
> - return 0;
> -
> - /* All other errors are fatal, including nomem, unparseable
> - * signatures and signature check failures - even if signatures
> - * aren't required.
> - */
> - default:
> - pr_notice("kernel signature verification failed (%d).\n", ret);
> }
>
> - return ret;
> + return 0;
> }
> #endif
>
> --
> 2.17.1
>
Acked-by: Dave Young <dyoung@redhat.com>
Thanks
Dave
next prev parent reply other threads:[~2020-05-26 5:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-25 5:23 [PATCH] kexec: Do not verify the signature without the lockdown or mandatory signature Lianbo Jiang
2020-05-26 5:04 ` Dave Young [this message]
2020-05-26 13:59 ` Jiri Bohac
2020-05-27 3:15 ` lijiang
2020-05-27 4:08 ` lijiang
2020-05-27 10:16 ` Jiri Bohac
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200526050451.GA74334@dhcp-128-65.nay.redhat.com \
--to=dyoung@redhat.com \
--cc=bhe@redhat.com \
--cc=ebiederm@xmission.com \
--cc=jbohac@suse.cz \
--cc=jmorris@namei.org \
--cc=kexec@lists.infradead.org \
--cc=lijiang@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mjg59@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).