Re: [PATCH 08/30] kexec_file: Restrict at runtime if the kernel is locked down

2018-02-22 Thread Jiri Bohac
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

2018-02-22 Thread Jiri Bohac
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 08/30] kexec_file: Restrict at runtime if the kernel is locked down

2018-02-22 Thread David Howells
David Howells  wrote:

> I'm intending on inserting the attached patch before this one.

And replacing this patch with the attached.

David
---
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
kexec_file systemcall if the kernel is locked down unless IMA can be used
to validate the image.

[Modified by David Howells to fit with modifications to the previous patch
 and to return -EPERM if the kernel is locked down for consistency with
 other lockdowns]

Signed-off-by: Jiri Bohac 
Signed-off-by: David Howells 
Cc: Matthew Garrett 
cc: Chun-Yi Lee 
cc: ke...@lists.infradead.org

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index d5931e392050..c47c4de604cd 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -167,6 +167,14 @@ kimage_file_prepare_segments(struct kimage *image, int 
kernel_fd, int initrd_fd,
}
 
ret = 0;
+   if (is_ima_appraise_enabled())
+   break;
+
+   if (kernel_is_locked_down(reason)) {
+   ret = -EPERM;
+   goto out;
+   }
+
break;
 
/* All other errors are fatal, including nomem, unparseable
--
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

2018-02-22 Thread David Howells
I'm intending on inserting the attached patch before this one.

David
---
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
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 splits 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.

[Modified by David Howells such that:

 (1) verify_pefile_signature() differentiates between no-signature and
 sig-didn't-match in its returned errors.

 (2) kexec fails with EKEYREJECTED and logs an appropriate message if
 signature checking is enforced and an signature is not found, uses
 unsupported crypto or has no matching key.

 (3) kexec fails with EKEYREJECTED if there is a signature for which we
 have a key, but signature doesn't match - even if in non-forcing mode.

 (4) kexec fails with EBADMSG or some other error if there is a signature
 which cannot be parsed - even if in non-forcing mode.

 (5) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
 the signature - even if in non-forcing mode.

]

Signed-off-by: Jiri Bohac 
Signed-off-by: David Howells 
cc: Matthew Garrett 
cc: Chun-Yi Lee 
cc: ke...@lists.infradead.org

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c1236b187824..cb6e67b7442d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2019,20 +2019,30 @@ 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.
 
- In addition to that option, you need to enable signature
+ 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, though if
+ there's a signature that we can check, then it must be valid.
+
+ 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/crypto/asymmetric_keys/verify_pefile.c 
b/crypto/asymmetric_keys/verify_pefile.c
index d178650fd524..4473cea1e877 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -100,7 +100,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned 
int pelen,
 
if (!ddir->certs.virtual_address || !ddir->certs.size) {
pr_debug("Unsigned PE binary\n");
-   return -EKEYREJECTED;
+   return -ENODATA;
}
 
chkaddr(ctx->header_size, ddir->certs.virtual_address,
@@ -408,6 +408,8 @@ static int pefile_digest_pe(const void *pebuf, unsigned int 
pelen,
  *  (*) 0 if at least one signature chain intersects with the keys in the trust
  * keyring, or:
  *
+ *  (*) -ENODATA if there is no signature present.
+ *
  *  (*) -ENOPKG if a suitable crypto module couldn't be

Re: [PATCH 08/30] kexec_file: Restrict at runtime if the kernel is locked down

2018-01-17 Thread David Howells
Jiri Bohac  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
--
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

2018-01-11 Thread Jiri Bohac
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


Re: [PATCH 08/30] kexec_file: Restrict at runtime if the kernel is locked down

2018-01-11 Thread David Howells
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.

David
--
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

2018-01-11 Thread David Howells
Jiri Bohac  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
--
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

2018-01-11 Thread Jiri Bohac
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