[PATCH] Allow automatic kernel taint on unsigned module load to be disabled

[PATCH] Allow automatic kernel taint on unsigned module load to be disabled

[Matthew Garrett]

Distributions may wish to provide kernels that permit loading of
unsigned modules based on certain policy decisions. Right now that
results in the kernel being tainted whenever an unsigned module is
loaded, which may not be desirable. Add a config option to disable that.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 init/Kconfig    | 13 ++++++++++++-
 kernel/module.c |  2 ++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/init/Kconfig b/init/Kconfig
index 8514b25db21c..196860c5d1e5 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_UNSIGNED_TAINT
+	bool "Taint the kernel if unsigned modules are loaded"
+	default y
+	depends on MODULE_SIG
+	help
+	  Taint the kernel if an unsigned 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
+	  even if signature enforcement is disabled.
+
 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_UNSIGNED_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..71f80c8816f2 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3660,12 +3660,14 @@ static int load_module(struct load_info *info, const char __user *uargs,
 
 #ifdef CONFIG_MODULE_SIG
 	mod->sig_ok = info->sig_ok;
+#ifdef CONFIG_MODULE_UNSIGNED_TAINT
 	if (!mod->sig_ok) {
 		pr_notice_once("%s: module verification failed: signature "
 			       "and/or required key missing - tainting "
 			       "kernel\n", mod->name);
 		add_taint_module(mod, TAINT_UNSIGNED_MODULE, LOCKDEP_STILL_OK);
 	}
+#endif
 #endif
 
 	/* To avoid stressing percpu allocator, do this once we're unique. */
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

Re: [PATCH] Allow automatic kernel taint on unsigned module load to be disabled

[Rusty Russell]

Matthew Garrett <mjg59@google.com> writes:
> Distributions may wish to provide kernels that permit loading of
> unsigned modules based on certain policy decisions.

Sorry, that's way too vague to accept this patch.

So I'm guessing a binary module is behind this vagueness.  If you want
some other method than signing to vet modules, please do it in
userspace.  You can do arbitrary things that way...

Cheers,
Rusty.

> Right now that
> results in the kernel being tainted whenever an unsigned module is
> loaded, which may not be desirable. Add a config option to disable that.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
>  init/Kconfig    | 13 ++++++++++++-
>  kernel/module.c |  2 ++
>  2 files changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 8514b25db21c..196860c5d1e5 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_UNSIGNED_TAINT
> +	bool "Taint the kernel if unsigned modules are loaded"
> +	default y
> +	depends on MODULE_SIG
> +	help
> +	  Taint the kernel if an unsigned 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
> +	  even if signature enforcement is disabled.
> +
>  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_UNSIGNED_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..71f80c8816f2 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3660,12 +3660,14 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  
>  #ifdef CONFIG_MODULE_SIG
>  	mod->sig_ok = info->sig_ok;
> +#ifdef CONFIG_MODULE_UNSIGNED_TAINT
>  	if (!mod->sig_ok) {
>  		pr_notice_once("%s: module verification failed: signature "
>  			       "and/or required key missing - tainting "
>  			       "kernel\n", mod->name);
>  		add_taint_module(mod, TAINT_UNSIGNED_MODULE, LOCKDEP_STILL_OK);
>  	}
> +#endif
>  #endif
>  
>  	/* To avoid stressing percpu allocator, do this once we're unique. */
> -- 
> 2.14.0.rc1.383.gd1ce394fe2-goog

Re: [PATCH] Allow automatic kernel taint on unsigned module load to be disabled

[Matthew Garrett]

On Sat, Aug 5, 2017 at 11:54 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Matthew Garrett <mjg59@google.com> writes:
>> Distributions may wish to provide kernels that permit loading of
>> unsigned modules based on certain policy decisions.
>
> Sorry, that's way too vague to accept this patch.
>
> So I'm guessing a binary module is behind this vagueness.  If you want
> some other method than signing to vet modules, please do it in
> userspace.  You can do arbitrary things that way...

Binary modules will still be tainted by the license checker. The issue
is that if you want to enforce module signatures under *some*
circumstances, you need to build with CONFIG_MODULE_SIG, but that
changes the behaviour of the kernel even when you're not enforcing
module signatures. The same kernel may be used in environments where
you can verify the kernel and environments where you can't, and in the
latter you may not care that modules are unsigned. In that scenario,
tainting doesn't buy you anything.

Re: [PATCH] Allow automatic kernel taint on unsigned module load to be disabled

[Rusty Russell]

Matthew Garrett <mjg59@google.com> writes:
> On Sat, Aug 5, 2017 at 11:54 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Matthew Garrett <mjg59@google.com> writes:
>>> Distributions may wish to provide kernels that permit loading of
>>> unsigned modules based on certain policy decisions.
>>
>> Sorry, that's way too vague to accept this patch.
>>
>> So I'm guessing a binary module is behind this vagueness.  If you want
>> some other method than signing to vet modules, please do it in
>> userspace.  You can do arbitrary things that way...
>
> Binary modules will still be tainted by the license checker. The issue
> is that if you want to enforce module signatures under *some*
> circumstances, you need to build with CONFIG_MODULE_SIG

Not at all!  You can validate them in userspace.

> but that
> changes the behaviour of the kernel even when you're not enforcing
> module signatures. The same kernel may be used in environments where
> you can verify the kernel and environments where you can't, and in the
> latter you may not care that modules are unsigned. In that scenario,
> tainting doesn't buy you anything.

With your patch, you don't get tainting in the environment where you can
verify.

You'd be better adding a sysctl or equiv. to turn off force loading, and
use that in your can-verify system.

Cheers,
Rusty.

Re: [PATCH] Allow automatic kernel taint on unsigned module load to be disabled

[Matthew Garrett]

On Sun, Aug 6, 2017 at 7:49 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Matthew Garrett <mjg59@google.com> writes:
>> Binary modules will still be tainted by the license checker. The issue
>> is that if you want to enforce module signatures under *some*
>> circumstances, you need to build with CONFIG_MODULE_SIG
>
> Not at all!  You can validate them in userspace.

And then you need an entire trusted userland, at which point you can
assert that the modules are trustworthy without having to validate
them so you don't need CONFIG_MODULE_SIG anyway.

>> but that
>> changes the behaviour of the kernel even when you're not enforcing
>> module signatures. The same kernel may be used in environments where
>> you can verify the kernel and environments where you can't, and in the
>> latter you may not care that modules are unsigned. In that scenario,
>> tainting doesn't buy you anything.
>
> With your patch, you don't get tainting in the environment where you can
> verify.

You don't anyway, do you? Loading has already failed before this point
if sig_enforce is set.

> You'd be better adding a sysctl or equiv. to turn off force loading, and
> use that in your can-verify system.

I'm not sure what you mean by "force loading" here - if sig_enforce is
set, you can't force load an unsigned module. If sig_enforce isn't
set, you'll taint regardless of whether or not you force.

Wait. Hang on - are you confusing CONFIG_MODULE_SIG with CONFIG_MODVERSIONS?

Re: [PATCH] Allow automatic kernel taint on unsigned module load to be disabled

[Rusty Russell]

Matthew Garrett <mjg59@google.com> writes:
> On Sun, Aug 6, 2017 at 7:49 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Matthew Garrett <mjg59@google.com> writes:
>>> Binary modules will still be tainted by the license checker. The issue
>>> is that if you want to enforce module signatures under *some*
>>> circumstances, you need to build with CONFIG_MODULE_SIG
>>
>> Not at all!  You can validate them in userspace.
>
> And then you need an entire trusted userland, at which point you can
> assert that the modules are trustworthy without having to validate
> them so you don't need CONFIG_MODULE_SIG anyway.

Yep.  But your patch already gives userland that power, to silently load
unsigned modules.

>>> but that
>>> changes the behaviour of the kernel even when you're not enforcing
>>> module signatures. The same kernel may be used in environments where
>>> you can verify the kernel and environments where you can't, and in the
>>> latter you may not care that modules are unsigned. In that scenario,
>>> tainting doesn't buy you anything.
>>
>> With your patch, you don't get tainting in the environment where you can
>> verify.
>
> You don't anyway, do you? Loading has already failed before this point
> if sig_enforce is set.

No.  You used to get a warning and a taint when you had a kernel
configured to expect signatures and it didn't get one.  You want to
remove that warning, to silently accept unsigned modules.

>> You'd be better adding a sysctl or equiv. to turn off force loading, and
>> use that in your can-verify system.
>
> I'm not sure what you mean by "force loading" here - if sig_enforce is
> set, you can't force load an unsigned module. If sig_enforce isn't
> set, you'll taint regardless of whether or not you force.
>
> Wait. Hang on - are you confusing CONFIG_MODULE_SIG with CONFIG_MODVERSIONS?

No, I mean stripping the signatures.  (I thought modprobe could do this
these days, but apparently not!)

So, you're actually building the same kernel, but building two sets of
modules: one without signatures, one with?

And when deploying the one with signatures, you're setting sig_enforce.
On the other, you don't want signatures because um, reasons?  And you
want to suppress the message?

This seems so convoluted already, I can see how you considered an
upstream patch your most productive path forward.

But it's possible that this scenario makes sense to Jeyu and I'm just
incapable of seeing its beauty?

Cheers,
Rusty.

Re: [PATCH] Allow automatic kernel taint on unsigned module load to be disabled

[Matthew Garrett]

On Sun, Aug 6, 2017 at 9:47 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Matthew Garrett <mjg59@google.com> writes:
>> And then you need an entire trusted userland, at which point you can
>> assert that the modules are trustworthy without having to validate
>> them so you don't need CONFIG_MODULE_SIG anyway.
>
> Yep.  But your patch already gives userland that power, to silently load
> unsigned modules.

Only if sig_enforce isn't set.

>>> With your patch, you don't get tainting in the environment where you can
>>> verify.
>>
>> You don't anyway, do you? Loading has already failed before this point
>> if sig_enforce is set.
>
> No.  You used to get a warning and a taint when you had a kernel
> configured to expect signatures and it didn't get one.  You want to
> remove that warning, to silently accept unsigned modules.

I'm very confused here. If sig_enforce is set, the kernel will refuse
to load an unsigned module - it won't be tainted, modprobe will just
return an error. If sig_enforce is not set, any attacker in a position
to provide unsigned modules is also in a position to just subvert
modprobe, so you aren't in an environment where you can verify
anything. The taint is informational, not any form of security. You're
only able to securely verify module signatures in userland in very
constrainted setups.

>>> You'd be better adding a sysctl or equiv. to turn off force loading, and
>>> use that in your can-verify system.
>>
>> I'm not sure what you mean by "force loading" here - if sig_enforce is
>> set, you can't force load an unsigned module. If sig_enforce isn't
>> set, you'll taint regardless of whether or not you force.
>>
>> Wait. Hang on - are you confusing CONFIG_MODULE_SIG with CONFIG_MODVERSIONS?
>
> No, I mean stripping the signatures.  (I thought modprobe could do this
> these days, but apparently not!)
>
> So, you're actually building the same kernel, but building two sets of
> modules: one without signatures, one with?
>
> And when deploying the one with signatures, you're setting sig_enforce.
> On the other, you don't want signatures because um, reasons?  And you
> want to suppress the message?

No. A distribution may ship a kernel with signed modules. In some
configurations, the signatures are irrelevant - there's no mechanism
to verify that the correct kernel was loaded in the first place, so
for all you know the signature validation code has already been
removed at runtime. In that scenario you're fine with users loading
unsigned kernel modules and there's no benefit in tainting the kernel.
But the same kernel may be booted under circumstances where it *is*
possible to validate the kernel, and in those circumstances you want
to enforce module signatures and so sig_enforce is set.

Right now you have two choices:

1) unsigned modules taint the kernel if sig_enforce is false, unsigned
modules can't be loaded if sig_enforce is true (ie, CONFIG_MODULE_SIG
is set)
2) unsigned modules do not taint the kernel, unsigned modules can
always be loaded (ie, CONFIG_MODULE_SIG is unset)

What I want is:

3) unsigned modules do not taint the kernel if sig_enforce is false,
unsigned modules can't be loaded if sig_enforce is true

This is currently impossible to express, and as a result some
distributions ship with CONFIG_MODULE_SIG disabled in order to avoid
dealing with user questions about why loading locally built modules
now taints the kernel. Being able to build a single kernel that
satisfies more use cases seems like a win.

But maybe there's a cleaner way. How about adding a paramter like
sig_enforce (say taint_on_unsigned) and then adding a config parameter
equivalent to CONFIG_MODULE_SIG_FORCE? That way the default policy can
be set at build time, but can also be overridden by end users who
still want to be able to taint on unsigned module load.

Re: Allow automatic kernel taint on unsigned module load to be disabled

[Jessica Yu]

+++ Matthew Garrett [04/08/17 11:07 -0700]:
>Distributions may wish to provide kernels that permit loading of
>unsigned modules based on certain policy decisions. Right now that
>results in the kernel being tainted whenever an unsigned module is
>loaded, which may not be desirable. Add a config option to disable that.

Hi Matthew!

I think I'm missing some context here. Could you provide some more
background and help me understand why we want to go into all this
trouble just to avoid a taint?  Was there a recent bug report, mailing
list discussion, etc. that spurred you to write this patch? I'm not
understanding why this particular taint is undesirable.

I still think there is informational value in providing the unsigned
module taint on a kernel that supports module signatures (CONFIG_MODULE_SIG).
When debugging or trawling through crash dumps, module taints are
useful for developers to immediately identify which modules were
out-of-tree, which were unsigned and therefore not originally shipped
by the distro etc, which often applies to e.g. 3rd party/dkms modules.
And if a user for example locally compiles a module without signing it
why would the unsigned module taint bother them more than the
out-of-tree one (because that module would get both taints)?

If it is the "module verification failed" message that is actually
scaring users, we could perhaps "soften" it to say something like
"loading unsigned module X".

Jessica

>Signed-off-by: Matthew Garrett <mjg59@google.com>
>---
> init/Kconfig    | 13 ++++++++++++-
> kernel/module.c |  2 ++
> 2 files changed, 14 insertions(+), 1 deletion(-)
>
>diff --git a/init/Kconfig b/init/Kconfig
>index 8514b25db21c..196860c5d1e5 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_UNSIGNED_TAINT
>+	bool "Taint the kernel if unsigned modules are loaded"
>+	default y
>+	depends on MODULE_SIG
>+	help
>+	  Taint the kernel if an unsigned 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
>+	  even if signature enforcement is disabled.
>+
> 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_UNSIGNED_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..71f80c8816f2 100644
>--- a/kernel/module.c
>+++ b/kernel/module.c
>@@ -3660,12 +3660,14 @@ static int load_module(struct load_info *info, const char __user *uargs,
>
> #ifdef CONFIG_MODULE_SIG
> 	mod->sig_ok = info->sig_ok;
>+#ifdef CONFIG_MODULE_UNSIGNED_TAINT
> 	if (!mod->sig_ok) {
> 		pr_notice_once("%s: module verification failed: signature "
> 			       "and/or required key missing - tainting "
> 			       "kernel\n", mod->name);
> 		add_taint_module(mod, TAINT_UNSIGNED_MODULE, LOCKDEP_STILL_OK);
> 	}
>+#endif
> #endif
>
> 	/* To avoid stressing percpu allocator, do this once we're unique. */
>-- 
>2.14.0.rc1.383.gd1ce394fe2-goog
>

Re: Allow automatic kernel taint on unsigned module load to be disabled

[Matthew Garrett]

On Thu, Aug 10, 2017 at 4:43 PM, Jessica Yu <jeyu@kernel.org> wrote:
> I think I'm missing some context here. Could you provide some more
> background and help me understand why we want to go into all this
> trouble just to avoid a taint?  Was there a recent bug report, mailing
> list discussion, etc. that spurred you to write this patch? I'm not
> understanding why this particular taint is undesirable.

Hi Jessica,

Does the version in https://lkml.org/lkml/2017/8/7/764 make this clearer?

Re: Allow automatic kernel taint on unsigned module load to be disabled

[Jessica Yu]

+++ Matthew Garrett [14/08/17 12:50 -0400]:
>On Thu, Aug 10, 2017 at 4:43 PM, Jessica Yu <jeyu@kernel.org> wrote:
>> I think I'm missing some context here. Could you provide some more
>> background and help me understand why we want to go into all this
>> trouble just to avoid a taint?  Was there a recent bug report, mailing
>> list discussion, etc. that spurred you to write this patch? I'm not
>> understanding why this particular taint is undesirable.
>
>Hi Jessica,
>
>Does the version in https://lkml.org/lkml/2017/8/7/764 make this clearer?

Hi Matthew,

Sorry for the delay, I'm currently on leave traveling.

I understand what the patch is doing, what I don't yet understand is
_why_ you would want to remove the unsigned module taint when
CONFIG_MODULE_SIG is enabled. Which distributions are asking for this
exactly, and for what use cases? I find it a bit contradictory to have
CONFIG_MODULE_SIG enabled and at the same time expect the kernel to
behave as if the option wasn't enabled.

I would really prefer not to add extra code to remove what is cosmetic
and still has informational/debug value. If the unsigned module taint
is for whatever reason that bothersome, why can't distro(s) carry a
2-line patch removing the message and taint for those particular
setups where signatures are considered "irrelevant" even with
CONFIG_MODULE_SIG=y?

Thanks,

Jessica

Re: Allow automatic kernel taint on unsigned module load to be disabled

[Matthew Garrett]

On Tue, Aug 29, 2017 at 10:56 AM, Jessica Yu <jeyu@kernel.org> wrote:
> I understand what the patch is doing, what I don't yet understand is
> _why_ you would want to remove the unsigned module taint when
> CONFIG_MODULE_SIG is enabled. Which distributions are asking for this
> exactly, and for what use cases? I find it a bit contradictory to have
> CONFIG_MODULE_SIG enabled and at the same time expect the kernel to
> behave as if the option wasn't enabled.

Debian disable CONFIG_MODULE_SIG because of this additional taint
(I've Cc:ed Ben who made this change).

> I would really prefer not to add extra code to remove what is cosmetic
> and still has informational/debug value. If the unsigned module taint
> is for whatever reason that bothersome, why can't distro(s) carry a
> 2-line patch removing the message and taint for those particular
> setups where signatures are considered "irrelevant" even with
> CONFIG_MODULE_SIG=y?

If it's functionality that distributions want to patch out, it makes
sense to provide them with a config option rather than forcing them to
maintain a patch separately.

Re: Allow automatic kernel taint on unsigned module load to be disabled

[Ben Hutchings]

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

On Tue, 2017-08-29 at 13:22 -0700, Matthew Garrett wrote:
> > On Tue, Aug 29, 2017 at 10:56 AM, Jessica Yu <jeyu@kernel.org> wrote:
> > I understand what the patch is doing, what I don't yet understand is
> > _why_ you would want to remove the unsigned module taint when
> > CONFIG_MODULE_SIG is enabled. Which distributions are asking for this
> > exactly, and for what use cases? I find it a bit contradictory to have
> > CONFIG_MODULE_SIG enabled and at the same time expect the kernel to
> > behave as if the option wasn't enabled.
> 
> Debian disable CONFIG_MODULE_SIG because of this additional taint
> (I've Cc:ed Ben who made this change).

The current state of affairs is that Debian doesn't have the mechanism
in place to sign modules with a trusted key.  If we were to allow third
parties to add signatures in some way (I think that's what Matthew's
interested in doing) we would have to enabled CONFIG_MODULE_SIG, but
that would cause modules to be tainted by default.

> > I would really prefer not to add extra code to remove what is cosmetic
> > and still has informational/debug value. If the unsigned module taint
> > is for whatever reason that bothersome, why can't distro(s) carry a
> > 2-line patch removing the message and taint for those particular
> > setups where signatures are considered "irrelevant" even with
> > CONFIG_MODULE_SIG=y?
> 
> If it's functionality that distributions want to patch out, it makes
> sense to provide them with a config option rather than forcing them to
> maintain a patch separately.

We could use this in Debian.  It would likely be a temporary stage
until we do our own centralised module signing (or someone implements a
Merkle tree for in-tree modules).

Ben.

-- 
Ben Hutchings
Teamwork is essential - it allows you to blame someone else.


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

Re: Allow automatic kernel taint on unsigned module load to be disabled

[Matthew Garrett]

Hi Jessica,

It seems that there's distribution interest in this - any feedback?

Re: Allow automatic kernel taint on unsigned module load to be disabled

[Matthew Garrett]

On Wed, Oct 18, 2017 at 11:27 AM, Matthew Garrett <mjg59@google.com> wrote:
> Hi Jessica,
>
> It seems that there's distribution interest in this - any feedback?

Hi,

I think we'd still benefit from this being available, so just checking
whether you had any further feelings on it.