Re: [PATCH 08/30] kexec_file: Restrict at runtime if the kernel is locked down
On Thu, Feb 22, 2018 at 02:21:53PM +, David Howells wrote: > commit ed0424c531d7dd25adebdec0ee6a78a5784f207a > Author: David Howells > Date: Thu Feb 22 14:01:49 2018 + > > kexec_file: Restrict at runtime if the kernel is locked down > > When KEXEC_VERIFY_SIG is not enabled, kernel should not load images > through s/KEXEC_VERIFY_SIG/KEXEC_SIG/ Again, my mistake :/ Other than that, looks OK. Much cleaner than my version. Thanks! Reviewed-by: Jiri Bohac -- Jiri Bohac SUSE Labs, Prague, Czechia -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/30] kexec_file: Restrict at runtime if the kernel is locked down
On Thu, Feb 22, 2018 at 02:20:43PM +, David Howells wrote: > commit 87a39b258eca2e15884ee90c3fcd5758d6057b17 > Author: David Howells > Date: Thu Feb 22 13:42:04 2018 + > > kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE > > This is a preparatory patch for kexec_file_load() lockdown. A locked down > kernel needs to prevent unsigned kernel images to be loaded with s/to be loaded/from being loaded/ (my own mistake :-)) Otherwise looks good. Thanks for improving my idea. Reviewed-by: Jiri Bohac -- Jiri Bohac SUSE Labs, Prague, Czechia -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/30] Enforce module signatures if the kernel is locked down
On Thu, Feb 22, 2018 at 01:07:57PM +, David Howells wrote: > I'm considering folding the attached changes into this patch. > > It adjusts the errors generated: > > (1) If there's no signature (ENODATA) or we can't check it (ENOPKG, ENOKEY), > then: > > (a) If signatures are enforced then EKEYREJECTED is returned. > > (b) If IMA will have validated the image, return 0 (okay). > > (c) If there's no signature or we can't check it, but the kernel is >locked down then EPERM is returned (this is then consistent with >other lockdown cases). > > (2) If the signature is unparseable (EBADMSG, EINVAL), the signature fails > the check (EKEYREJECTED) or a system error occurs (eg. ENOMEM), we return > the error we got. > > Note that the X.509 code doesn't check for key expiry as the RTC might not be > valid or might not have been transferred to the kernel's clock yet. Looks good. Reviewed-by: Jiri Bohac -- Jiri Bohac SUSE Labs, Prague, Czechia -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08a/30] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
On Wed, Jan 17, 2018 at 04:34:24PM +, David Howells wrote: > Jiri Bohac 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 SUSE Labs, Prague, Czechia -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08a/30] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
On Tue, Jan 16, 2018 at 04:31:51PM +, 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
SUSE Labs, Prague, Czechia
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/30] kexec_file: Restrict at runtime if the kernel is locked down
On Thu, Jan 11, 2018 at 12:47:57PM +, 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 SUSE Labs, Prague, Czechia -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08b/30] kexec_file: Restrict at runtime if the kernel is locked down
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
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
SUSE Labs, Prague, Czechia
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 08a/30] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
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
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 (
Re: [PATCH 08/30] kexec_file: Restrict at runtime if the kernel is locked down
Hi, sorry for replying to such an old thread. On Thu, Nov 09, 2017 at 05:31:38PM +, 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 SUSE Labs, Prague, Czechia -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
