Messages in this thread Patch in this message | | | Date | Wed, 13 Jun 2018 15:15:20 +0200 | From | Jessica Yu <> | 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@intel.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
| |