[PATCH] Make kernel taint on invalid module signatures configurable

[PATCH] Make kernel taint on invalid module signatures configurable

[Matthew Garrett]

The default kernel behaviour is for unsigned or invalidly signed modules
to load without warning. Right now, If CONFIG_MODULE_SIG is enabled the
kernel will be tainted in this case. Distributions may wish to enable
CONFIG_MODULE_SIG in order to permit signature enforcement, but may not
wish to alter the behaviour of the kernel in the situation where
signature enforcement is not required. Add a new kernel configuration
option to allow the default behaviour to be toggled, and add a kernel
parameter in order to allow tainting to be enabled at runtime.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---

Rusty, with luck this makes it clearer what I'm trying to achieve here.

 Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
 Documentation/admin-guide/module-signing.rst    | 13 +++++++++----
 init/Kconfig                                    | 13 ++++++++++++-
 kernel/module.c                                 |  7 ++++++-
 4 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index d9c171ce4190..7ea1e9aeb7d8 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2291,6 +2291,12 @@
 			Note that if CONFIG_MODULE_SIG_FORCE is set, that
 			is always true, so this option does nothing.
 
+	module.sig_taint
+			[KNL] When CONFIG_MODULE_SIG is set, this means
+			that modules without (valid) signatures will taint the
+			kernel on load. Note that if CONFIG_MODULE_SIG_TAINT is
+			set, that is always true, so this option does nothing.
+
 	module_blacklist=  [KNL] Do not load a comma-separated list of
 			modules.  Useful for debugging problem modules.
 
diff --git a/Documentation/admin-guide/module-signing.rst b/Documentation/admin-guide/module-signing.rst
index 27e59498b487..96709b93c606 100644
--- a/Documentation/admin-guide/module-signing.rst
+++ b/Documentation/admin-guide/module-signing.rst
@@ -52,10 +52,11 @@ This has a number of options available:
      This specifies how the kernel should deal with a module that has a
      signature for which the key is not known or a module that is unsigned.
 
-     If this is off (ie. "permissive"), then modules for which the key is not
-     available and modules that are unsigned are permitted, but the kernel will
-     be marked as being tainted, and the concerned modules will be marked as
-     tainted, shown with the character 'E'.
+     If this is off (ie. "permissive"), then modules for which the key
+     is not available and modules that are unsigned are permitted. If
+     ``CONFIG_MODULE_SIG_TAINT`` is enabled or the sig_taint parameter is
+     set the kernel will be marked as being tainted, and the concerned
+     modules will be marked as tainted, shown with the character 'E'.
 
      If this is on (ie. "restrictive"), only modules that have a valid
      signature that can be verified by a public key in the kernel's possession
@@ -266,6 +267,10 @@ for which it has a public key.   Otherwise, it will also load modules that are
 unsigned.   Any module for which the kernel has a key, but which proves to have
 a signature mismatch will not be permitted to load.
 
+If ``CONFIG_MODULE_SIG_TAINT`` is enabled or module.sig_taint=1 is
+supplied on the kernel command line, the kernel will be tainted if an
+invalidly signed or unsigned module is loaded.
+
 Any module that has an unparseable signature will be rejected.
 
 
diff --git a/init/Kconfig b/init/Kconfig
index 8514b25db21c..ffad18a3e890 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1749,12 +1749,23 @@ config MODULE_SIG
 	  debuginfo strip done by some packagers (such as rpmbuild) and
 	  inclusion into an initramfs that wants the module size reduced.
 
+config MODULE_SIG_TAINT
+	bool "Taint the kernel on unsigned or incorrectly signed module load"
+	default y
+	depends on MODULE_SIG
+	help
+	  Taint the kernel if an unsigned or untrusted kernel module is loaded.
+	  If this option is enabled, the kernel will be tainted on an attempt
+	  to load an unsigned module or signed modules for which we don't have
+	  a key.
+
 config MODULE_SIG_FORCE
 	bool "Require modules to be validly signed"
 	depends on MODULE_SIG
 	help
 	  Reject unsigned modules or signed modules for which we don't have a
-	  key.  Without this, such modules will simply taint the kernel.
+	  key. Without this, such modules will be loaded successfully but will
+	  (if MODULE_SIG_TAINT is set) taint the kernel.
 
 config MODULE_SIG_ALL
 	bool "Automatically sign all modules"
diff --git a/kernel/module.c b/kernel/module.c
index 40f983cbea81..70c2edc5693c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -278,6 +278,11 @@ static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
 module_param(sig_enforce, bool_enable_only, 0644);
 #endif /* !CONFIG_MODULE_SIG_FORCE */
 
+static bool sig_taint = IS_ENABLED(CONFIG_MODULE_SIG_TAINT);
+#ifndef CONFIG_MODULE_SIG_TAINT
+module_param(sig_taint, bool_enable_only, 0644);
+#endif /* !CONFIG_MODULE_SIG_TAINT */
+
 /* Block module loading/unloading? */
 int modules_disabled = 0;
 core_param(nomodule, modules_disabled, bint, 0);
@@ -3660,7 +3665,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
 
 #ifdef CONFIG_MODULE_SIG
 	mod->sig_ok = info->sig_ok;
-	if (!mod->sig_ok) {
+	if (!mod->sig_ok && sig_taint) {
 		pr_notice_once("%s: module verification failed: signature "
 			       "and/or required key missing - tainting "
 			       "kernel\n", mod->name);
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Matthew Garrett]

Hi Jessica,

Any objections to this patch?

Thanks!

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Jessica Yu]

+++ Matthew Garrett [14/02/18 18:21 +0000]:
>Hi Jessica,
>
>Any objections to this patch?
>
>Thanks!

Hi Matthew!

My questions and comments from last year still apply here -

  http://lkml.kernel.org/r/20170829175647.ej5fqszss2mbpc5i@redbean

I'm still unclear on why a distro would enable CONFIG_MODULE_SIG and
then _not_ want to know about unsigned modules.

>From what I understand from Ben's post from last year
(http://lkml.kernel.org/r/1504044122.4448.24.camel@decadent.org.uk),
it sounds like the main issue is that Debian doesn't support their own
centralised module signing yet, causing all of their modules to be
automatically tainted if they enable CONFIG_MODULE_SIG, and that a new
option like this would likely be used as a temporary "fix". Am I
understanding correctly?

I understand this predicament, but it seems like adding a new set of
options/parameters like this is just hiding the symptoms of the
problem (modules distributed by Debian getting tainted by default)
instead of fixing what seems to be the heart of the issue (Debian
doesn't support their own module signing yet), if that makes sense.
I am hesitant about merging something that would only serve as a
temporary solution until Debian supports their own module signing. In
that case, I would prefer the Debian folks to maintain their own patch
removing the taint until they support module signing for their own
modules, especially if - and please correct me if I'm wrong - the
new option is not going to see long-term usage.

Thanks,

Jessica

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Matthew Garrett]

On Thu, Feb 15, 2018 at 7:25 AM Jessica Yu <jeyu@kernel.org> wrote:
> I'm still unclear on why a distro would enable CONFIG_MODULE_SIG and
> then _not_ want to know about unsigned modules.

The same kernel image may be used in situations where the use case benefits
from enforcement of module signatures and cases where it doesn't -
distributions don't generally have the resources to ship multiple kernel
packages with lightly modified configurations.

>  From what I understand from Ben's post from last year
> (http://lkml.kernel.org/r/1504044122.4448.24.camel@decadent.org.uk),
> it sounds like the main issue is that Debian doesn't support their own
> centralised module signing yet, causing all of their modules to be
> automatically tainted if they enable CONFIG_MODULE_SIG, and that a new
> option like this would likely be used as a temporary "fix". Am I
> understanding correctly?

Not entirely. There's two cases where the current situation causes problems:

1) Distributions that build out of tree kernel modules and don't have
infrastructure to sign them will end up with kernel taint. That's something
that can be resolved by implementing that infrastructure.
2) End-users who build out of tree kernel modules will end up with kernel
taint and will file bugs. This cannot be fixed but will increase
distribution load anyway.

> I understand this predicament, but it seems like adding a new set of
> options/parameters like this is just hiding the symptoms of the
> problem (modules distributed by Debian getting tainted by default)
> instead of fixing what seems to be the heart of the issue (Debian
> doesn't support their own module signing yet), if that makes sense.
> I am hesitant about merging something that would only serve as a
> temporary solution until Debian supports their own module signing. In
> that case, I would prefer the Debian folks to maintain their own patch
> removing the taint until they support module signing for their own
> modules, especially if - and please correct me if I'm wrong - the
> new option is not going to see long-term usage.

I'd expect this option to be used by distributions for as long as
distributions need to support use-cases that don't need signed modules, so
probably forever.

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Philipp Hahn]

[-- Attachment #1: Type: text/plain, Size: 1794 bytes --]

Hello,

Am 15.02.2018 um 20:36 schrieb Matthew Garrett:
> On Thu, Feb 15, 2018 at 7:25 AM Jessica Yu <jeyu@kernel.org> wrote:
>>  From what I understand from Ben's post from last year
>> (http://lkml.kernel.org/r/1504044122.4448.24.camel@decadent.org.uk),
>> it sounds like the main issue is that Debian doesn't support their own
>> centralised module signing yet, causing all of their modules to be
>> automatically tainted if they enable CONFIG_MODULE_SIG, and that a new
>> option like this would likely be used as a temporary "fix". Am I
>> understanding correctly?
> 
> Not entirely. There's two cases where the current situation causes problems:
> 
> 1) Distributions that build out of tree kernel modules and don't have
> infrastructure to sign them will end up with kernel taint. That's something
> that can be resolved by implementing that infrastructure.
> 2) End-users who build out of tree kernel modules will end up with kernel
> taint and will file bugs. This cannot be fixed but will increase
> distribution load anyway.

Just yesterday I sent the attached email to the crypto/-maintainers as I
have read some Fedora documentation about adding the UEFI SecureBoot
keys to the kernel secondary trusted keyring:
<https://docs-old.fedoraproject.org/en-US/Fedora/23/html/System_Administrators_Guide/sect-kernel-module-authentication.html>

Sadly didn't work for me :-(
If my understanding is correct and iff that would work, Debian (and
others) could load their public key into Shim and then use the
associated private key for singing their modules.

Debian currently plans to have a Sprint for their SecureBoot process in
April, which I will attend. Hopefully we will find a solution their:
<https://wiki.debian.org/Sprints/2018/SecureBootSprint>

Philipp (also a Debian developer)

[-- Attachment #2: Nachricht als Anhang --]
[-- Type: message/rfc822, Size: 2691 bytes --]

From: Philipp Hahn <hahn@univention.de>
To: David Howells <dhowells@redhat.com>, David Woodhouse <dwmw2@infradead.org>
Cc: keyrings@vger.kernel.org
Subject: [linux] .system_keyring ?
Date: Thu, 15 Feb 2018 12:51:50 +0100
Message-ID: <49ef1eb6-802b-ae86-a5e4-eb29a8ef4c4c@univention.de>

Hello,

reading "Documentation/admin-guide/module-signing.rst":
> The kernel contains a ring of public keys that can be viewed by root.  They're
> in a keyring called ".system_keyring" that can be seen by::
> 
>         [root@deneb ~]# cat /proc/keys
>         ...
>         223c7853 I------     1 perm 1f030000     0     0 keyring   .system_keyring: 1

I don't have that ".system_keyring":
> cat /proc/keys
> 00a8459a I------     1 perm 1f0f0000     0     0 keyring   .secondary_trusted_keys: 1
> 02b66804 I--Q---     8 perm 3f030000     0     0 keyring   _ses: 1
> 0639503a I--Q---     3 perm 1f3f0000     0 65534 keyring   _uid.0: empty
> 1afb3552 I------     2 perm 1f0b0000     0     0 keyring   .builtin_trusted_keys: 1
> 3167cca3 I--Q---     1 perm 1f3f0000     0 65534 keyring   _uid_ses.0: 1
> 37b744d9 I------     1 perm 1f030000     0     0 asymmetri Build time autogenerated kernel key: 8943e26cd249e2fcdafea805149fcf9ed5912e10: X509.rsa d5912e10 []

Grepping the Linux kernel source tree git also find no '.system_keyring'
in any source file - only the name of the header file and in Documentation/.
Am I missing something? If that documentation out-dated?

My .config is this:
> $ sed -ne 's/^config /CONFIG_/p' certs/Kconfig | ssh uefi 'grep -F -f - /boot/config-`uname -r`'
> CONFIG_MODULE_SIG_KEY="certs/signing_key.pem"
> CONFIG_SYSTEM_TRUSTED_KEYRING=y
> CONFIG_SYSTEM_TRUSTED_KEYS=""
> # CONFIG_SYSTEM_EXTRA_CERTIFICATE is not set
> CONFIG_SECONDARY_TRUSTED_KEYRING=y


I was looking at
<https://docs-old.fedoraproject.org/en-US/Fedora/23/html/System_Administrators_Guide/sect-kernel-module-authentication.html>
and I'm trying to get my UEFI keys added to the Linux keyring. I want to
sign my modules with that "external" key instead of embedding the key
into the Linux kernel itself.

Thanks in advance.

Philipp

PS: I'm not subscribed to 'keyring, but LKML.
-- 
Philipp Hahn
Open Source Software Engineer

Univention GmbH
be open.
Mary-Somerville-Str. 1
D-28359 Bremen
Tel.: +49 421 22232-0
Fax : +49 421 22232-99
hahn@univention.de

http://www.univention.de/
Geschäftsführer: Peter H. Ganten
HRB 20755 Amtsgericht Bremen
Steuer-Nr.: 71-597-02876

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Matthew Garrett]

On Fri, Feb 16, 2018 at 12:25 AM Philipp Hahn <pmhahn@pmhahn.de> wrote:
> Sadly didn't work for me :-(
> If my understanding is correct and iff that would work, Debian (and
> others) could load their public key into Shim and then use the
> associated private key for singing their modules.

This works for UEFI systems, but distributions have to support non-UEFI as
well.

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Jessica Yu]

+++ Matthew Garrett [15/02/18 19:36 +0000]:
>On Thu, Feb 15, 2018 at 7:25 AM Jessica Yu <jeyu@kernel.org> wrote:
>> I'm still unclear on why a distro would enable CONFIG_MODULE_SIG and
>> then _not_ want to know about unsigned modules.
>
>The same kernel image may be used in situations where the use case benefits
>from enforcement of module signatures and cases where it doesn't -
>distributions don't generally have the resources to ship multiple kernel
>packages with lightly modified configurations.

Ah, OK. So if I'm understanding correctly, you want to use the same kernel
image/configuration but for two different use cases, one where the module
signatures do not matter, and one where they do matter. But the config you
want to use in both use cases would have CONFIG_MODULE_SIG=y and
CONFIG_MODULE_SIG_TAINT=n (to avoid tainting of unsigned/invalidly signed
modules).

This seems convoluted, but only because you're trying to get two
different behaviors for the price of one (config). I.e., trying to get
non-CONFIG_MODULE_SIG behavior despite having it enabled. Normally, if
you have a setup where module signatures don't matter at all, and you
want to avoid the unsigned module taint, then I'd just suggest turning
CONFIG_MODULE_SIG off, because that'd be the most obvious solution,
wouldn't it?  However, in your case you want to keep CONFIG_MODULE_SIG on,
due to distro contraints. But without knowing that, the problem
statement seems contradictory, because if you don't care about
signatures, then you probably wouldn't have CONFIG_MODULE_SIG enabled
in the first place.

In any case, I think I'd be willing to merge it as a module_param made
available under CONFIG_MODULE_SIG=y (rather than as a new separate config
option), while preserving the default behavior of tainting on
unsigned/invalidly signed module loads (so let's keep the param parts of
your patch). I think it makes sense to consider the turning-off-the-taint
param as a behavioral tweak under CONFIG_MODULE_SIG. Then you could turn
off the tainting behavior on the kernel command line, would this sufficient
enough for your use cases?

>>  From what I understand from Ben's post from last year
>> (http://lkml.kernel.org/r/1504044122.4448.24.camel@decadent.org.uk),
>> it sounds like the main issue is that Debian doesn't support their own
>> centralised module signing yet, causing all of their modules to be
>> automatically tainted if they enable CONFIG_MODULE_SIG, and that a new
>> option like this would likely be used as a temporary "fix". Am I
>> understanding correctly?
>
>Not entirely. There's two cases where the current situation causes problems:
>
>1) Distributions that build out of tree kernel modules and don't have
>infrastructure to sign them will end up with kernel taint. That's something
>that can be resolved by implementing that infrastructure.
>2) End-users who build out of tree kernel modules will end up with kernel
>taint and will file bugs. This cannot be fixed but will increase
>distribution load anyway.

I thought these two cases (particularly #2) were the very situations
where distros might find the unsigned module taint useful (especially
in the use case where you do benefit from module signatures). From my
understanding, the unsigned module taint is intended to be useful when
looking at crashes/OOPses, to provide a clear indication of whether or
not a developer could reliably debug the crash, or choose to tread
carefully, because the end-user has loaded an unsigned/out-of-tree
module that wasn't signed/shipped by the distribution. Is the taint
just not useful to distros in this manner anymore?

Thanks!

Jessica

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Matthew Garrett]

On Tue, Feb 20, 2018 at 11:21 AM Jessica Yu <jeyu@kernel.org> wrote:

> Ah, OK. So if I'm understanding correctly, you want to use the same kernel
> image/configuration but for two different use cases, one where the module
> signatures do not matter, and one where they do matter. But the config you
> want to use in both use cases would have CONFIG_MODULE_SIG=y and
> CONFIG_MODULE_SIG_TAINT=n (to avoid tainting of unsigned/invalidly signed
> modules).

Right. Distributions generally have to try to satisfy multiple use cases
with as few kernel images as possible.

> In any case, I think I'd be willing to merge it as a module_param made
> available under CONFIG_MODULE_SIG=y (rather than as a new separate config
> option), while preserving the default behavior of tainting on
> unsigned/invalidly signed module loads (so let's keep the param parts of
> your patch). I think it makes sense to consider the turning-off-the-taint
> param as a behavioral tweak under CONFIG_MODULE_SIG. Then you could turn
> off the tainting behavior on the kernel command line, would this
sufficient
> enough for your use cases?

I think that's probably not practical - distributions often aren't in
control of the kernel command line after initial installation, so they'd
end up with different behaviour depending on whether a machine was a clean
install or not (which is why several things that are module_params have
defaults controlled by additional kernel config options)

> >Not entirely. There's two cases where the current situation causes
problems:
> >
> >1) Distributions that build out of tree kernel modules and don't have
> >infrastructure to sign them will end up with kernel taint. That's
something
> >that can be resolved by implementing that infrastructure.
> >2) End-users who build out of tree kernel modules will end up with kernel
> >taint and will file bugs. This cannot be fixed but will increase
> >distribution load anyway.

> I thought these two cases (particularly #2) were the very situations
> where distros might find the unsigned module taint useful (especially
> in the use case where you do benefit from module signatures). From my
> understanding, the unsigned module taint is intended to be useful when
> looking at crashes/OOPses, to provide a clear indication of whether or
> not a developer could reliably debug the crash, or choose to tread
> carefully, because the end-user has loaded an unsigned/out-of-tree
> module that wasn't signed/shipped by the distribution. Is the taint
> just not useful to distros in this manner anymore?

The module list is usually sufficient for that - users tend not to replace
individual distribution modules without rebuilding their entire kernel.

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Jessica Yu]

+++ Matthew Garrett [20/02/18 20:37 +0000]:
>On Tue, Feb 20, 2018 at 11:21 AM Jessica Yu <jeyu@kernel.org> wrote:
>
>> Ah, OK. So if I'm understanding correctly, you want to use the same kernel
>> image/configuration but for two different use cases, one where the module
>> signatures do not matter, and one where they do matter. But the config you
>> want to use in both use cases would have CONFIG_MODULE_SIG=y and
>> CONFIG_MODULE_SIG_TAINT=n (to avoid tainting of unsigned/invalidly signed
>> modules).
>
>Right. Distributions generally have to try to satisfy multiple use cases
>with as few kernel images as possible.
>
>> In any case, I think I'd be willing to merge it as a module_param made
>> available under CONFIG_MODULE_SIG=y (rather than as a new separate config
>> option), while preserving the default behavior of tainting on
>> unsigned/invalidly signed module loads (so let's keep the param parts of
>> your patch). I think it makes sense to consider the turning-off-the-taint
>> param as a behavioral tweak under CONFIG_MODULE_SIG. Then you could turn
>> off the tainting behavior on the kernel command line, would this
>sufficient
>> enough for your use cases?
>
>I think that's probably not practical - distributions often aren't in
>control of the kernel command line after initial installation, so they'd
>end up with different behaviour depending on whether a machine was a clean
>install or not (which is why several things that are module_params have
>defaults controlled by additional kernel config options)

Ah I see.. Fair enough!

>> >Not entirely. There's two cases where the current situation causes
>problems:
>> >
>> >1) Distributions that build out of tree kernel modules and don't have
>> >infrastructure to sign them will end up with kernel taint. That's
>something
>> >that can be resolved by implementing that infrastructure.
>> >2) End-users who build out of tree kernel modules will end up with kernel
>> >taint and will file bugs. This cannot be fixed but will increase
>> >distribution load anyway.
>
>> I thought these two cases (particularly #2) were the very situations
>> where distros might find the unsigned module taint useful (especially
>> in the use case where you do benefit from module signatures). From my
>> understanding, the unsigned module taint is intended to be useful when
>> looking at crashes/OOPses, to provide a clear indication of whether or
>> not a developer could reliably debug the crash, or choose to tread
>> carefully, because the end-user has loaded an unsigned/out-of-tree
>> module that wasn't signed/shipped by the distribution. Is the taint
>> just not useful to distros in this manner anymore?
>
>The module list is usually sufficient for that - users tend not to replace
>individual distribution modules without rebuilding their entire kernel.

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Ben Hutchings]

[-- Attachment #1: Type: text/plain, Size: 2777 bytes --]

On Tue, 2018-02-20 at 20:37 +0000, Matthew Garrett wrote:
> On Tue, Feb 20, 2018 at 11:21 AM Jessica Yu <jeyu@kernel.org> wrote:
[...]
> > In any case, I think I'd be willing to merge it as a module_param made
> > available under CONFIG_MODULE_SIG=y (rather than as a new separate config
> > option), while preserving the default behavior of tainting on
> > unsigned/invalidly signed module loads (so let's keep the param parts of
> > your patch). I think it makes sense to consider the turning-off-the-taint
> > param as a behavioral tweak under CONFIG_MODULE_SIG. Then you could turn
> > off the tainting behavior on the kernel command line, would this sufficient
> > enough for your use cases?
>
> I think that's probably not practical - distributions often aren't in
> control of the kernel command line after initial installation, so they'd
> end up with different behaviour depending on whether a machine was a clean
> install or not (which is why several things that are module_params have
> defaults controlled by additional kernel config options)

Indeed.  So long as Debian doesn't do module signing, the default
behaviour in our kernel images will need to be that they don't complain
about lack of signatures.

[...]
> > > 1) Distributions that build out of tree kernel modules and don't have
> > > infrastructure to sign them will end up with kernel taint. That's something
> > > that can be resolved by implementing that infrastructure.
> > > 2) End-users who build out of tree kernel modules will end up with kernel
> > > taint and will file bugs. This cannot be fixed but will increase
> > > distribution load anyway.
> > I thought these two cases (particularly #2) were the very situations
> > where distros might find the unsigned module taint useful (especially
> > in the use case where you do benefit from module signatures). From my
> > understanding, the unsigned module taint is intended to be useful when
> > looking at crashes/OOPses, to provide a clear indication of whether or
> > not a developer could reliably debug the crash, or choose to tread
> > carefully, because the end-user has loaded an unsigned/out-of-tree
> > module that wasn't signed/shipped by the distribution. Is the taint
> > just not useful to distros in this manner anymore?
> 
> The module list is usually sufficient for that - users tend not to replace
> individual distribution modules without rebuilding their entire kernel.

And we already have an O (out-of-tree) taint flag.

Ben.

-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had
thought. ... I realized that a large part of my life from then on was
going to be spent in finding mistakes in my own programs. - Maurice
Wilkes, 1949


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] Make kernel taint on invalid module signatures configurable

[Jessica Yu]

+++ Matthew Garrett [07/08/17 12:50 -0700]:
>The default kernel behaviour is for unsigned or invalidly signed modules
>to load without warning. Right now, If CONFIG_MODULE_SIG is enabled the
>kernel will be tainted in this case. Distributions may wish to enable
>CONFIG_MODULE_SIG in order to permit signature enforcement, but may not
>wish to alter the behaviour of the kernel in the situation where
>signature enforcement is not required. Add a new kernel configuration
>option to allow the default behaviour to be toggled, and add a kernel
>parameter in order to allow tainting to be enabled at runtime.

It would be great if you could you expand on or add excerpts from our
discussion to the commit message, explaining why distros don't want
the unsigned taint in certain situations (what issues is it causing?),
and especially why CONFIG_MODULE_SIG remains enabled in both use cases
(reusing configs, distros not having the bandwidth to repackage
kernels for slightly modified configs).

>Signed-off-by: Matthew Garrett <mjg59@google.com>
>---
>
>Rusty, with luck this makes it clearer what I'm trying to achieve here.
>
> Documentation/admin-guide/kernel-parameters.txt |  6 ++++++
> Documentation/admin-guide/module-signing.rst    | 13 +++++++++----
> init/Kconfig                                    | 13 ++++++++++++-
> kernel/module.c                                 |  7 ++++++-
> 4 files changed, 33 insertions(+), 6 deletions(-)
>
>diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>index d9c171ce4190..7ea1e9aeb7d8 100644
>--- a/Documentation/admin-guide/kernel-parameters.txt
>+++ b/Documentation/admin-guide/kernel-parameters.txt
>@@ -2291,6 +2291,12 @@
> 			Note that if CONFIG_MODULE_SIG_FORCE is set, that
> 			is always true, so this option does nothing.
>
>+	module.sig_taint
>+			[KNL] When CONFIG_MODULE_SIG is set, this means
>+			that modules without (valid) signatures will taint the
>+			kernel on load. Note that if CONFIG_MODULE_SIG_TAINT is
>+			set, that is always true, so this option does nothing.
>+

I'm wondering now if we should call the param unsigned_taint? Or
taint_on_unsigned? With sig_taint I'm initially not sure what the
parameter is trying to convey.

> 	module_blacklist=  [KNL] Do not load a comma-separated list of
> 			modules.  Useful for debugging problem modules.
>
>diff --git a/Documentation/admin-guide/module-signing.rst b/Documentation/admin-guide/module-signing.rst
>index 27e59498b487..96709b93c606 100644
>--- a/Documentation/admin-guide/module-signing.rst
>+++ b/Documentation/admin-guide/module-signing.rst
>@@ -52,10 +52,11 @@ This has a number of options available:
>      This specifies how the kernel should deal with a module that has a
>      signature for which the key is not known or a module that is unsigned.
>
>-     If this is off (ie. "permissive"), then modules for which the key is not
>-     available and modules that are unsigned are permitted, but the kernel will
>-     be marked as being tainted, and the concerned modules will be marked as
>-     tainted, shown with the character 'E'.
>+     If this is off (ie. "permissive"), then modules for which the key
>+     is not available and modules that are unsigned are permitted. If
>+     ``CONFIG_MODULE_SIG_TAINT`` is enabled or the sig_taint parameter is
>+     set the kernel will be marked as being tainted, and the concerned
>+     modules will be marked as tainted, shown with the character 'E'.
>
>      If this is on (ie. "restrictive"), only modules that have a valid
>      signature that can be verified by a public key in the kernel's possession
>@@ -266,6 +267,10 @@ for which it has a public key.   Otherwise, it will also load modules that are
> unsigned.   Any module for which the kernel has a key, but which proves to have
> a signature mismatch will not be permitted to load.
>
>+If ``CONFIG_MODULE_SIG_TAINT`` is enabled or module.sig_taint=1 is
>+supplied on the kernel command line, the kernel will be tainted if an
>+invalidly signed or unsigned module is loaded.
>+
> Any module that has an unparseable signature will be rejected.
>
>
>diff --git a/init/Kconfig b/init/Kconfig
>index 8514b25db21c..ffad18a3e890 100644
>--- a/init/Kconfig
>+++ b/init/Kconfig
>@@ -1749,12 +1749,23 @@ config MODULE_SIG
> 	  debuginfo strip done by some packagers (such as rpmbuild) and
> 	  inclusion into an initramfs that wants the module size reduced.
>
>+config MODULE_SIG_TAINT
>+	bool "Taint the kernel on unsigned or incorrectly signed module load"
>+	default y
>+	depends on MODULE_SIG
>+	help
>+	  Taint the kernel if an unsigned or untrusted kernel module is loaded.
>+	  If this option is enabled, the kernel will be tainted on an attempt
>+	  to load an unsigned module or signed modules for which we don't have
>+	  a key.
>+

Same for here, although I understand that you were trying to keep the
naming convention consistent. Maybe MODULE_UNSIGNED_TAINT is a bit
more descriptive? I think MODULE_SIG_TAINT and sig_taint sound a bit
ambiguous (are you tainting on signed modules? unsigned modules?)
without looking at the description, but this is purely a matter of
taste/preference.

> config MODULE_SIG_FORCE
> 	bool "Require modules to be validly signed"
> 	depends on MODULE_SIG
> 	help
> 	  Reject unsigned modules or signed modules for which we don't have a
>-	  key.  Without this, such modules will simply taint the kernel.
>+	  key. Without this, such modules will be loaded successfully but will
>+	  (if MODULE_SIG_TAINT is set) taint the kernel.
>
> config MODULE_SIG_ALL
> 	bool "Automatically sign all modules"
>diff --git a/kernel/module.c b/kernel/module.c
>index 40f983cbea81..70c2edc5693c 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -278,6 +278,11 @@ static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
> module_param(sig_enforce, bool_enable_only, 0644);
> #endif /* !CONFIG_MODULE_SIG_FORCE */
>
>+static bool sig_taint = IS_ENABLED(CONFIG_MODULE_SIG_TAINT);
>+#ifndef CONFIG_MODULE_SIG_TAINT
>+module_param(sig_taint, bool_enable_only, 0644);
>+#endif /* !CONFIG_MODULE_SIG_TAINT */
>+
> /* Block module loading/unloading? */
> int modules_disabled = 0;
> core_param(nomodule, modules_disabled, bint, 0);
>@@ -3660,7 +3665,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>
> #ifdef CONFIG_MODULE_SIG
> 	mod->sig_ok = info->sig_ok;
>-	if (!mod->sig_ok) {
>+	if (!mod->sig_ok && sig_taint) {
> 		pr_notice_once("%s: module verification failed: signature "
> 			       "and/or required key missing - tainting "
> 			       "kernel\n", mod->name);
>-- 
>2.14.0.rc1.383.gd1ce394fe2-goog
>