Re: [PATCH] Allow automatic kernel taint on unsigned module load to be disabled
On Sun, Aug 6, 2017 at 9:47 PM, Rusty Russell wrote: > Matthew Garrett 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: [PATCH] Allow automatic kernel taint on unsigned module load to be disabled
Matthew Garrett writes: > On Sun, Aug 6, 2017 at 7:49 PM, Rusty Russell wrote: >> Matthew Garrett 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
On Sun, Aug 6, 2017 at 7:49 PM, Rusty Russell wrote: > Matthew Garrett 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
Matthew Garrett writes: > On Sat, Aug 5, 2017 at 11:54 PM, Rusty Russell wrote: >> Matthew Garrett 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
On Sat, Aug 5, 2017 at 11:54 PM, Rusty Russell wrote: > Matthew Garrett 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
Matthew Garrett 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
> ---
> 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
[PATCH] Allow automatic kernel taint on unsigned module load to be disabled
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
---
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
