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

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

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]

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

[Ben Hutchings]

On Tue, 2017-08-29 at 13:22 -0700, Matthew Garrett wrote:
> > On Tue, Aug 29, 2017 at 10:56 AM, Jessica Yu  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.



signature.asc
Description: This is a digitally signed message part

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

[Ben Hutchings]

On Tue, 2017-08-29 at 13:22 -0700, Matthew Garrett wrote:
> > On Tue, Aug 29, 2017 at 10:56 AM, Jessica Yu  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.



signature.asc
Description: This is a digitally signed message part

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

[Matthew Garrett]

On Tue, Aug 29, 2017 at 10:56 AM, Jessica Yu  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

[Jessica Yu]

+++ Matthew Garrett [14/08/17 12:50 -0400]:

On Thu, Aug 10, 2017 at 4:43 PM, Jessica Yu  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

[Jessica Yu]

+++ Matthew Garrett [14/08/17 12:50 -0400]:

On Thu, Aug 10, 2017 at 4:43 PM, Jessica Yu  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 Thu, Aug 10, 2017 at 4:43 PM, Jessica Yu  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

[Matthew Garrett]

On Thu, Aug 10, 2017 at 4:43 PM, Jessica Yu  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 [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 
---
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

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