linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Matthew Garrett <matthewgarrett@google.com>
Cc: jmorris@namei.org, linux-security@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
	David Howells <dhowells@redhat.com>, Jessica Yu <jeyu@kernel.org>
Subject: Re: [PATCH V33 04/30] Enforce module signatures if the kernel is locked down
Date: Thu, 20 Jun 2019 20:46:12 -0700	[thread overview]
Message-ID: <201906202045.793323FC@keescook> (raw)
In-Reply-To: <20190621011941.186255-5-matthewgarrett@google.com>

On Thu, Jun 20, 2019 at 06:19:15PM -0700, Matthew Garrett wrote:
> From: David Howells <dhowells@redhat.com>
> 
> If the kernel is locked down, require that all modules have valid
> signatures that we can verify.
> 
> I have adjusted 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 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.
> 
>  [Modified by Matthew Garrett to remove the IMA integration. This will
>   be replaced with integration with the IMA architecture policy
>   patchset.]
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Matthew Garrett <matthewgarrett@google.com>
> Cc: Jessica Yu <jeyu@kernel.org>
> ---
>  include/linux/security.h     |  1 +
>  kernel/module.c              | 39 +++++++++++++++++++++++++++++-------
>  security/lockdown/lockdown.c |  1 +
>  3 files changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index a86a7739ca24..a7612b03b42a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -82,6 +82,7 @@ enum lsm_event {
>   */
>  enum lockdown_reason {
>  	LOCKDOWN_NONE,
> +	LOCKDOWN_MODULE_SIGNATURE,
>  	LOCKDOWN_INTEGRITY_MAX,
>  	LOCKDOWN_CONFIDENTIALITY_MAX,
>  };
> diff --git a/kernel/module.c b/kernel/module.c
> index 0b9aa8ab89f0..780e9605ff88 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2763,8 +2763,9 @@ static inline void kmemleak_load_module(const struct module *mod,
>  #ifdef CONFIG_MODULE_SIG
>  static int module_sig_check(struct load_info *info, int flags)
>  {
> -	int err = -ENOKEY;
> +	int err = -ENODATA;
>  	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> +	const char *reason;
>  	const void *mod = info->hdr;
>  
>  	/*
> @@ -2779,16 +2780,40 @@ static int module_sig_check(struct load_info *info, int flags)
>  		err = mod_verify_sig(mod, info);
>  	}
>  
> -	if (!err) {
> +	switch (err) {
> +	case 0:
>  		info->sig_ok = true;
>  		return 0;
> -	}
>  
> -	/* Not having a signature is only an error if we're strict. */
> -	if (err == -ENOKEY && !is_module_sig_enforced())
> -		err = 0;
> +		/* We don't permit modules to be loaded into trusted kernels
> +		 * without a valid signature on them, but if we're not
> +		 * enforcing, certain errors are non-fatal.
> +		 */
> +	case -ENODATA:
> +		reason = "Loading of unsigned module";
> +		goto decide;
> +	case -ENOPKG:
> +		reason = "Loading of module with unsupported crypto";
> +		goto decide;
> +	case -ENOKEY:
> +		reason = "Loading of module with unavailable key";
> +	decide:
> +		if (is_module_sig_enforced()) {
> +			pr_notice("%s is rejected\n", reason);
> +			return -EKEYREJECTED;
> +		}
>  
> -	return err;
> +		if (security_is_locked_down(LOCKDOWN_MODULE_SIGNATURE))
> +			return -EPERM;

LSM hooks should return the desired error code. Here and in all the
other patches, I'd expect to see stuff like:

	ret = security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
	if (ret)
		return ret;


> +		return 0;
> +
> +		/* All other errors are fatal, including nomem, unparseable
> +		 * signatures and signature check failures - even if signatures
> +		 * aren't required.
> +		 */
> +	default:
> +		return err;
> +	}
>  }
>  #else /* !CONFIG_MODULE_SIG */
>  static int module_sig_check(struct load_info *info, int flags)
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 1ecb2eecb245..08abd7e6609b 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -18,6 +18,7 @@ static enum lockdown_reason kernel_locked_down;
>  
>  static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
>  	[LOCKDOWN_NONE] = "none",
> +	[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
>  	[LOCKDOWN_INTEGRITY_MAX] = "integrity",
>  	[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
>  };
> -- 
> 2.22.0.410.gd8fdbe21b5-goog
> 

-- 
Kees Cook

  reply	other threads:[~2019-06-21  3:46 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-21  1:19 [PATCH V33 00/30] Lockdown as an LSM Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 01/30] security: Support early LSMs Matthew Garrett
2019-06-21  3:21   ` Kees Cook
2019-06-21 19:26     ` Matthew Garrett
2019-06-21  5:23   ` Andy Lutomirski
2019-06-21 19:27     ` Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 02/30] security: Add a "locked down" LSM hook Matthew Garrett
2019-06-21  3:23   ` Kees Cook
2019-06-21 19:29     ` Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 03/30] security: Add a static lockdown policy LSM Matthew Garrett
2019-06-21  3:44   ` Kees Cook
2019-06-21 19:37     ` Matthew Garrett
2019-06-21 21:04       ` Matthew Garrett
2019-06-21 22:31   ` Mimi Zohar
2019-06-21  1:19 ` [PATCH V33 04/30] Enforce module signatures if the kernel is locked down Matthew Garrett
2019-06-21  3:46   ` Kees Cook [this message]
2019-06-21  1:19 ` [PATCH V33 05/30] Restrict /dev/{mem,kmem,port} when " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 06/30] kexec_load: Disable at runtime if " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 07/30] Copy secure_boot flag in boot params across kexec reboot Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 08/30] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 09/30] kexec_file: Restrict at runtime if the kernel is locked down Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 10/30] hibernate: Disable when " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 11/30] uswsusp: " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 12/30] PCI: Lock down BAR access " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 13/30] x86: Lock down IO port " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 14/30] x86/msr: Restrict MSR " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 15/30] ACPI: Limit access to custom_method " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 16/30] acpi: Ignore acpi_rsdp kernel param when the kernel has been " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 17/30] acpi: Disable ACPI table override if the kernel is " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 18/30] Prohibit PCMCIA CIS storage when " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 19/30] Lock down TIOCSSERIAL Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 20/30] Lock down module params that specify hardware parameters (eg. ioport) Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 21/30] x86/mmiotrace: Lock down the testmmiotrace module Matthew Garrett
2019-06-26 12:46   ` Steven Rostedt
2019-06-21  1:19 ` [PATCH V33 22/30] Lock down /proc/kcore Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 23/30] Lock down tracing and perf kprobes when in confidentiality mode Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is " Matthew Garrett
2019-06-21  5:22   ` Andy Lutomirski
2019-06-21 20:05     ` Matthew Garrett
2019-06-26 20:22     ` James Morris
2019-06-27  0:57       ` Andy Lutomirski
2019-06-27 14:35         ` Stephen Smalley
2019-06-27 18:06           ` James Morris
2019-06-27 20:16             ` Stephen Smalley
2019-06-27 23:16               ` Matthew Garrett
2019-06-27 23:23                 ` Andy Lutomirski
2019-06-27 23:27           ` Andy Lutomirski
2019-06-28 18:47             ` Matthew Garrett
2019-06-29 23:47               ` Andy Lutomirski
2019-06-21  1:19 ` [PATCH V33 25/30] Lock down perf when " Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 26/30] kexec: Allow kexec_file() with appropriate IMA policy when locked down Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 27/30] lockdown: Print current->comm in restriction messages Matthew Garrett
2019-06-21  4:09   ` Kees Cook
2019-06-21  1:19 ` [PATCH V33 28/30] debugfs: Restrict debugfs when the kernel is locked down Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 29/30] tracefs: Restrict tracefs " Matthew Garrett
2019-06-26 13:07   ` Steven Rostedt
2019-06-26 19:39     ` Matthew Garrett
2019-06-21  1:19 ` [PATCH V33 30/30] efi: Restrict efivar_ssdt_load " Matthew Garrett

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=201906202045.793323FC@keescook \
    --to=keescook@chromium.org \
    --cc=dhowells@redhat.com \
    --cc=jeyu@kernel.org \
    --cc=jmorris@namei.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security@vger.kernel.org \
    --cc=matthewgarrett@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).