Hi,
sorry for replying to such an old thread.
On Thu, Nov 09, 2017 at 05:31:38PM +0000, David Howells wrote:
> When KEXEC_VERIFY_SIG is not enabled, kernel should not load images through
> kexec_file systemcall if the kernel is locked down unless IMA can be used
> to validate the image.
I don't like the idea that the lockdown (which is a runtime
thing) requires a compile time option (KEXEC_VERIFY_SIG) that
forces the verification even when the kernel is then not locked
down at runtime.
Distribution kernels will then have KEXEC_VERIFY_SIG on and
everyone will need signed kexec images even when totally
uninterested in secureboot.
So instead of this patch, I propose the two followup patches that
split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE just as
we have with modules:
[PATCH 08a/30] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
[PATCH 08b/30] kexec_file: Restrict at runtime if the kernel is locked down
Lockdown would not require KEXEC_SIG_FORCE but when enabled it
would check the signature.
Thanks,
--
Jiri Bohac <[email protected]>
SUSE Labs, Prague, Czechia
This is a preparatory patch for kexec lockdown. A locked down kernel needs to
prevent unsigned kernel images to be loaded with kexec_file_load. Currently,
the only way to force the signature verification is compiling with
KEXEC_VERIFY_SIG. This prevents loading usigned images even when the kernel is
not locked down at runtime.
This patch spilts KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE.
Analogous to the MODULE_SIG and MODULE_SIG_FORCE for modules, KEXEC_SIG turns
on the signature verification but allows unsigned images to be loaded.
KEXEC_SIG_FORCE disallows images without a valid signature.
Signed-off-by: Jiri Bohac <[email protected]>
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 8eed3f94bfc7..f25facb0df96 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1951,20 +1951,28 @@ config KEXEC_FILE
for kernel and initramfs as opposed to list of segments as
accepted by previous system call.
-config KEXEC_VERIFY_SIG
+config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
---help---
- This option makes kernel signature verification mandatory for
- the kexec_file_load() syscall.
+ This option makes the kexec_file_load() syscall check for a valid
+ signature of the kernel image. The image can still be loaded without
+ a valid signature unless you also enable KEXEC_SIG_FORCE.
- In addition to that option, you need to enable signature
+ In addition to this option, you need to enable signature
verification for the corresponding kernel image type being
loaded in order for this to work.
+config KEXEC_SIG_FORCE
+ bool "Require a valid signature in kexec_file_load() syscall"
+ depends on KEXEC_SIG
+ ---help---
+ This option makes kernel signature verification mandatory for
+ the kexec_file_load() syscall.
+
config KEXEC_BZIMAGE_VERIFY_SIG
bool "Enable bzImage signature verification support"
- depends on KEXEC_VERIFY_SIG
+ depends on KEXEC_SIG
depends on SIGNED_PE_FILE_VERIFICATION
select SYSTEM_TRUSTED_KEYRING
---help---
diff --git a/arch/x86/kernel/machine_kexec_64.c b/arch/x86/kernel/machine_kexec_64.c
index 1f790cf9d38f..3fbe35b923ef 100644
--- a/arch/x86/kernel/machine_kexec_64.c
+++ b/arch/x86/kernel/machine_kexec_64.c
@@ -406,7 +406,7 @@ int arch_kimage_file_post_load_cleanup(struct kimage *image)
return image->fops->cleanup(image->image_loader_data);
}
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
int arch_kexec_kernel_verify_sig(struct kimage *image, void *kernel,
unsigned long kernel_len)
{
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index f16f6ceb3875..19652372f3ee 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -121,7 +121,7 @@ typedef void *(kexec_load_t)(struct kimage *image, char *kernel_buf,
unsigned long cmdline_len);
typedef int (kexec_cleanup_t)(void *loader_data);
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
typedef int (kexec_verify_sig_t)(const char *kernel_buf,
unsigned long kernel_len);
#endif
@@ -130,7 +130,7 @@ struct kexec_file_ops {
kexec_probe_t *probe;
kexec_load_t *load;
kexec_cleanup_t *cleanup;
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
kexec_verify_sig_t *verify_sig;
#endif
};
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -45,7 +45,7 @@ int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
return -EINVAL;
}
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
int __weak arch_kexec_kernel_verify_sig(struct kimage *image, void *buf,
unsigned long buf_len)
{
@@ -116,7 +116,7 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
const char __user *cmdline_ptr,
unsigned long cmdline_len, unsigned flags)
{
- int ret = 0;
+ int ret = 0, sig_err = -EPERM;
void *ldata;
loff_t size;
@@ -135,15 +135,20 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
if (ret)
goto out;
-#ifdef CONFIG_KEXEC_VERIFY_SIG
- ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
+#ifdef CONFIG_KEXEC_SIG
+ sig_err = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
image->kernel_buf_len);
- if (ret) {
+ if (sig_err)
pr_debug("kernel signature verification failed.\n");
+ else
+ pr_debug("kernel signature verification successful.\n");
+#endif
+
+ if (sig_err && IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
+ ret = sig_err;
goto out;
}
- pr_debug("kernel signature verification successful.\n");
-#endif
+
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
--
Jiri Bohac <[email protected]>
SUSE Labs, Prague, Czechia
When KEXEC_VERIFY_SIG is not enabled, kernel should not load images through
kexec_file systemcall if the kernel is locked down unless IMA can be used
to validate the image.
Signed-off-by: Jiri Bohac <[email protected]>
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -144,7 +144,13 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
pr_debug("kernel signature verification successful.\n");
#endif
- if (sig_err && IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
+ /* Don't permit images to be loaded into trusted kernels without
+ * a valid signature on them
+ */
+ if (sig_err &&
+ (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE) ||
+ (!is_ima_appraise_enabled() &&
+ kernel_is_locked_down("kexec of unsigned images")))) {
ret = sig_err;
goto out;
}
--
Jiri Bohac <[email protected]>
SUSE Labs, Prague, Czechia
Jiri Bohac <[email protected]> wrote:
> I don't like the idea that the lockdown (which is a runtime
> thing) requires a compile time option (KEXEC_VERIFY_SIG) that
> forces the verification even when the kernel is then not locked
> down at runtime.
It doesn't. The EPERM only triggers if:
(1) File signatures aren't mandatory (ie. CONFIG_KEXEC_VERIFY_SIG) is not
set, and
(2) you're not using IMA appraisal to validate the file contents, and
(3) lockdown mode is enabled.
If file signatures are mandatory or IMA appraisal is in use, then the lockdown
state doesn't need to be checked.
David
David Howells <[email protected]> wrote:
> > I don't like the idea that the lockdown (which is a runtime
> > thing) requires a compile time option (KEXEC_VERIFY_SIG) that
> > forces the verification even when the kernel is then not locked
> > down at runtime.
>
> It doesn't. The EPERM only triggers if:
>
> (1) File signatures aren't mandatory (ie. CONFIG_KEXEC_VERIFY_SIG) is not
> set, and
>
> (2) you're not using IMA appraisal to validate the file contents, and
>
> (3) lockdown mode is enabled.
>
> If file signatures are mandatory or IMA appraisal is in use, then the lockdown
> state doesn't need to be checked.
Having said that, I do see your point, I think. We should still let through
validly signed images, even if signatures aren't mandatory in lockdown mode.
David
On Thu, Jan 11, 2018 at 12:47:57PM +0000, David Howells wrote:
> > > I don't like the idea that the lockdown (which is a runtime
> > > thing) requires a compile time option (KEXEC_VERIFY_SIG) that
> > > forces the verification even when the kernel is then not locked
> > > down at runtime.
> >
> > It doesn't. The EPERM only triggers if:
> >
> > (1) File signatures aren't mandatory (ie. CONFIG_KEXEC_VERIFY_SIG) is not
> > set, and
> >
> > (2) you're not using IMA appraisal to validate the file contents, and
> >
> > (3) lockdown mode is enabled.
> >
> > If file signatures are mandatory or IMA appraisal is in use, then the lockdown
> > state doesn't need to be checked.
>
> Having said that, I do see your point, I think. We should still let through
> validly signed images, even if signatures aren't mandatory in lockdown mode.
yes, to be clear, the problem I'm trying to fix is:
- without CONFIG_KEXEC_VERIFY_SIG kexec in a locked down kernel
will not work at all -> every distro that wants to support
secureboot will need to enable CONFIG_KEXEC_VERIFY_SIG;
- once CONFIG_KEXEC_VERIFY_SIG is enabled, kexec images need to
be signed even if secureboot is not used
The problem is that CONFIG_KEXEC_VERIFY_SIG enables both the
implementation and the enforcement of the signature checking.
What I'm proposing are new config options that allow a kernel to
be compiled in such a way that:
- kexec works even without signatures if secureboot is off
- kexec works with secureboot but requires signed images
The semantics should be the same as with signed modules, because
requiring kexec signatures when you can load unsigned modules is
futile. But with your original patchset, that's exactly what
distro kernels will be doing when booted with secureboot off,
MODULE_SIG_FORCE=n and KEXEC_VERIFY_SIG=y.
Thanks,
--
Jiri Bohac <[email protected]>
SUSE Labs, Prague, Czechia
I think that your code isn't quite right. Looking at the patched code:
#ifdef CONFIG_KEXEC_SIG
sig_err = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
image->kernel_buf_len);
if (sig_err)
pr_debug("kernel signature verification failed.\n");
else
pr_debug("kernel signature verification successful.\n");
#endif
if (sig_err && IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
ret = sig_err;
goto out;
}
If the signature check fails because the signature is bad, but
CONFIG_KEXEC_SIG_FORCE=n then it now won't fail when it should.
If sig_err is -EKEYREJECTED, -EKEYEXPIRED or -EKEYREVOKED then it must fail,
even if the signature check isn't forced.
David
On Tue, Jan 16, 2018 at 04:31:51PM +0000, David Howells wrote:
> I think that your code isn't quite right. Looking at the patched code:
>
> #ifdef CONFIG_KEXEC_SIG
> sig_err = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
> image->kernel_buf_len);
> if (sig_err)
> pr_debug("kernel signature verification failed.\n");
> else
> pr_debug("kernel signature verification successful.\n");
> #endif
>
> if (sig_err && IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
> ret = sig_err;
> goto out;
> }
>
> If the signature check fails because the signature is bad, but
> CONFIG_KEXEC_SIG_FORCE=n then it now won't fail when it should.
>
> If sig_err is -EKEYREJECTED, -EKEYEXPIRED or -EKEYREVOKED then it must fail,
> even if the signature check isn't forced.
It wasn't my intention to fail in these cases. What additional
security does this bring? If simply stripping an invalid
signature from the image before loading will make it pass, why
should the image with an invalid signature be rejected?
Indeed, the module signing code, the semantics of which I wanted
to mimic, also won't load modules with invalid signatures. It
will load modules without any signatuire, it will load modules
with the MODULE_SIG_STRING modified and it will load modules with
either of MODULE_INIT_IGNORE_MODVERSIONS or
MODULE_INIT_IGNORE_VERMAGIC passed as flags to the finit_module
syscall. In all these cases, it will taint the kernel, which
might be something we want for kexec_file_load as well (?).
But I don't see why it distinguishes between ENOKEY and other
errors when it's so easy for the caller to strip the invalid
signature. And why kexec_file_load should do the same.
What am I missing?
Thanks,
--
Jiri Bohac <[email protected]>
SUSE Labs, Prague, Czechia
Jiri Bohac <[email protected]> wrote:
> > Having said that, I do see your point, I think. We should still let through
> > validly signed images, even if signatures aren't mandatory in lockdown mode.
>
> yes, to be clear, the problem I'm trying to fix is:
> - without CONFIG_KEXEC_VERIFY_SIG kexec in a locked down kernel
> will not work at all -> every distro that wants to support
> secureboot will need to enable CONFIG_KEXEC_VERIFY_SIG;
>
> - once CONFIG_KEXEC_VERIFY_SIG is enabled, kexec images need to
> be signed even if secureboot is not used
>
> The problem is that CONFIG_KEXEC_VERIFY_SIG enables both the
> implementation and the enforcement of the signature checking.
Yep. I understand that.
> What I'm proposing are new config options that allow a kernel to
> be compiled in such a way that:
> - kexec works even without signatures if secureboot is off
> - kexec works with secureboot but requires signed images
Agreed to both of those. I also agree with making it possible to
configurationally require signatures, which your first patch does.
> The semantics should be the same as with signed modules, because
> requiring kexec signatures when you can load unsigned modules is
> futile. But with your original patchset, that's exactly what
> distro kernels will be doing when booted with secureboot off,
> MODULE_SIG_FORCE=n and KEXEC_VERIFY_SIG=y.
I should fix that.
David
Jiri Bohac <[email protected]> wrote:
> > If sig_err is -EKEYREJECTED, -EKEYEXPIRED or -EKEYREVOKED then it must fail,
> > even if the signature check isn't forced.
>
> It wasn't my intention to fail in these cases. What additional
> security does this bring? If simply stripping an invalid
> signature from the image before loading will make it pass, why
> should the image with an invalid signature be rejected?
If there is a signature, then if we're checking signatures, in my opinion we
should check it - and fail if the signature can't be parsed, is revoked, we
have a key and the signature doesn't match - or even if we run out of memory.
The cases for which enforcement is required are when (a) there is no
signature, (b) we don't support the algorithms used, or (c) we don't have a
key.
If we're going to completely discard the result, why do your patches even
bother to check the signature at all?
David
On Wed, Jan 17, 2018 at 04:34:24PM +0000, David Howells wrote:
> Jiri Bohac <[email protected]> wrote:
>
> > > If sig_err is -EKEYREJECTED, -EKEYEXPIRED or -EKEYREVOKED then it must fail,
> > > even if the signature check isn't forced.
> >
> > It wasn't my intention to fail in these cases. What additional
> > security does this bring? If simply stripping an invalid
> > signature from the image before loading will make it pass, why
> > should the image with an invalid signature be rejected?
>
> If there is a signature, then if we're checking signatures, in my opinion we
> should check it - and fail if the signature can't be parsed, is revoked, we
> have a key and the signature doesn't match - or even if we run out of memory.
Key verification may and will fail for lots of reasons which is
just going to make a user's life harder. E.g. you want to kexec
an old kernel with an expired key. Or your date is just wrong and
you get -EKEYEXPIRED. And you don't care about the signing at
all; it's just compiled in because your distro also needs to work
with secureboot. As a user, you will have to debug what's wrong
for no good reason. And an actual attacker will just strip the
signature off the image and load it.
This makes no sense.
> The cases for which enforcement is required are when (a) there is no
> signature, (b) we don't support the algorithms used, or (c) we don't have a
> key.
>
> If we're going to completely discard the result, why do your patches even
> bother to check the signature at all?
I thought that the debug message might be useful. E.g. when
you're testing a kernel and you see "kernel signature
verification failed" in dmesg then you know this would fail on a
system with secure boot.
But if ignoring the return code seems like too bad a thing, I would
rather skip the signature checking if it's not going to be
enforced with lockdown or CONFIG_KEXEC_SIG_FORCE.
Also, only now I found that some of the error codes the crypto
code returns yield really confusing messages (e.g.
kexec_file_load of an unsigned kernel returns -ELIBBAD which
makes kexec exit with "kexec_file_load failed: Accessing a
corrupted shared library").
Maybe the error code could be unified to -EKEYREJECTED for all
sorts of key verification failures?
Thanks,
--
Jiri Bohac <[email protected]>
SUSE Labs, Prague, Czechia
Jiri Bohac <[email protected]> wrote:
> Key verification may and will fail for lots of reasons which is
> just going to make a user's life harder. E.g. you want to kexec
> an old kernel with an expired key. Or your date is just wrong and
> you get -EKEYEXPIRED.
Note that we can't check for expired keys as we can't trust the system clock
to be correct at this point.
> Also, only now I found that some of the error codes the crypto
> code returns yield really confusing messages (e.g.
> kexec_file_load of an unsigned kernel returns -ELIBBAD which
> makes kexec exit with "kexec_file_load failed: Accessing a
> corrupted shared library").
Yeah, that should be fixed.
> Maybe the error code could be unified to -EKEYREJECTED for all
> sorts of key verification failures?
Things like ENOMEM and EINTR definitely need to stay separate (not that I
allow interruption at the moment).
ENOKEY (couldn't find matching key), EINVAL (didn't recognise identifier),
ENOPKG (couldn't find a crypto algo) and EBADMSG (couldn't parse signature)
are arguable. I think there's a valid case for treating ENOKEY, EINVAL and
ENOPKG differently to EKEYREJECTED - more so for ENOKEY. In my opinion,
ENOKEY, EINVAL and ENOPKG are not fatal errors if we're not enforcing
signature checking, but EKEYREJECTED and EBADMSG are.
David