lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Jun 2018 20:27:42 +0000 From: "Warden, Brett T" <brett.t.warden@...el.com> To: Jessica Yu <jeyu@...nel.org> CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: RE: [PATCH v2] module: Implement sig_unenforce parameter > -----Original Message----- > From: Jessica Yu [mailto:jeyu@...nel.org] > Sent: Wednesday, June 13, 2018 6:15 AM > To: Warden, Brett T <brett.t.warden@...el.com> > Cc: linux-kernel@...r.kernel.org > Subject: Re: [PATCH v2] module: Implement sig_unenforce parameter > > +++ Brett T. Warden [06/06/18 12:44 -0700]: > >When CONFIG_MODULE_SIG_FORCE is enabled, also provide a boot-time-only > >parameter, module.sig_unenforce, to disable signature enforcement. This > >allows distributions to ship with signature verification enforcement > >enabled by default, but for users to elect to disable it without > >recompiling, to support building and loading out-of-tree modules. > > > >Signed-off-by: Brett T. Warden <brett.t.warden@...el.com> > >--- > > > >Added CONFIG_X86 guards around use of boot_params since this structure > >is arch-specific. > > > > .../admin-guide/kernel-parameters.txt | 4 +++ > > kernel/module.c | 31 +++++++++++++++++-- > > 2 files changed, 33 insertions(+), 2 deletions(-) > > > >diff --git a/Documentation/admin-guide/kernel-parameters.txt > b/Documentation/admin-guide/kernel-parameters.txt > >index 1beb30d8d7fc..87909e021558 100644 > >--- a/Documentation/admin-guide/kernel-parameters.txt > >+++ b/Documentation/admin-guide/kernel-parameters.txt > >@@ -2380,6 +2380,10 @@ > > Note that if CONFIG_MODULE_SIG_FORCE is set, that > > is always true, so this option does nothing. > > > >+ module.sig_unenforce > >+ [KNL] This parameter allows modules without signatures > >+ to be loaded, overriding CONFIG_MODULE_SIG_FORCE. > >+ > > module_blacklist= [KNL] Do not load a comma-separated list of > > modules. Useful for debugging problem modules. > > > >diff --git a/kernel/module.c b/kernel/module.c > >index c9bea7f2b43e..27f23d85e1ba 100644 > >--- a/kernel/module.c > >+++ b/kernel/module.c > >@@ -64,6 +64,7 @@ > > #include <linux/bsearch.h> > > #include <linux/dynamic_debug.h> > > #include <linux/audit.h> > >+#include <linux/efi.h> > > #include <uapi/linux/module.h> > > #include "module-internal.h" > > > >@@ -274,9 +275,13 @@ static void module_assert_mutex_or_preempt(void) > > } > > > > static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE); > >-#ifndef CONFIG_MODULE_SIG_FORCE > >+#ifdef CONFIG_MODULE_SIG_FORCE > >+/* Allow disabling module signature requirement by adding boot param */ > >+static bool sig_unenforce; > >+module_param(sig_unenforce, bool_enable_only, 0444); > >+#else /* !CONFIG_MODULE_SIG_FORCE */ > > module_param(sig_enforce, bool_enable_only, 0644); > >-#endif /* !CONFIG_MODULE_SIG_FORCE */ > >+#endif /* CONFIG_MODULE_SIG_FORCE */ > > > > /* > > * Export sig_enforce kernel cmdline parameter to allow other subsystems > rely > >@@ -415,6 +420,10 @@ extern const s32 __start___kcrctab_unused[]; > > extern const s32 __start___kcrctab_unused_gpl[]; > > #endif > > > >+#ifdef CONFIG_X86 > >+extern struct boot_params boot_params; > >+#endif > >+ > > #ifndef CONFIG_MODVERSIONS > > #define symversion(base, idx) NULL > > #else > >@@ -4243,6 +4252,24 @@ static const struct file_operations > proc_modules_operations = { > > static int __init proc_modules_init(void) > > { > > proc_create("modules", 0, NULL, &proc_modules_operations); > >+ > >+#ifdef CONFIG_MODULE_SIG_FORCE > >+#ifdef CONFIG_X86 > >+ switch (boot_params.secure_boot) { > >+ case efi_secureboot_mode_unset: > >+ case efi_secureboot_mode_unknown: > >+ case efi_secureboot_mode_disabled: > >+ /* > >+ * sig_unenforce is only applied if SecureBoot is not > >+ * enabled. > >+ */ > >+ sig_enforce = !sig_unenforce; > >+ } > >+#else > >+ sig_enforce = !sig_unenforce; > >+#endif > >+#endif > > I don't want to rope in Secure Boot specifics and arch-specific code > (other than what's in arch/) into the module loader. I also don't want > to change the current semantics of CONFIG_MODULE_SIG_FORCE, for any > existing users who just want to set a build-time policy and already > assume that unsigned modules can't be loaded, period. Lastly, having > both sig_enforce and sig_unenforce parameters is really confusing. > Might as well just make it one parameter - sig_enforce - and make that > toggleable under a certain configuration. > > It sounds like you want to ship with signature enforcement enabled by > default, but stil want to make this toggleable for users. So maybe > something in between CONFIG_MODULE_SIG and CONFIG_MODULE_SIG_FORCE. > Half baked suggestion: Would a new config option that enforces module > signatures by default but makes sig_enforce toggleable for users suit > your use case? This would be less strict than CONFIG_MODULE_SIG_FORCE. > > Thanks, > > Jessica What if I make a sub-option (default n) of CONFIG_MODULE_SIG_FORCE that would allow toggling sig_enforce (eliminating sig_unenforce)? CONFIG_MODULE_SIG_FORCE_ALLOW_OVERRIDE? Our internal security review produced the requirement to check for SecureBoot before disabling sig_enforce. I'll have to do some research to figure out a cleaner way to accomplish that without X86 code in module.c. Thanks for the feedback, -- Brett
Powered by blists - more mailing lists