* [PATCH] module: Implement sig_unenforce parameter @ 2018-06-06 0:43 Brett T. Warden 2018-06-06 13:37 ` kbuild test robot 0 siblings, 1 reply; 5+ messages in thread From: Brett T. Warden @ 2018-06-06 0:43 UTC (permalink / raw) To: linux-kernel, jeyu 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> --- .../admin-guide/kernel-parameters.txt | 4 +++ kernel/module.c | 25 +++++++++++++++++-- 2 files changed, 27 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..53cd6cd52dc6 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,8 @@ extern const s32 __start___kcrctab_unused[]; extern const s32 __start___kcrctab_unused_gpl[]; #endif +extern struct boot_params boot_params; + #ifndef CONFIG_MODVERSIONS #define symversion(base, idx) NULL #else @@ -4243,6 +4250,20 @@ 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 + 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; + } +#endif + return 0; } module_init(proc_modules_init); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] module: Implement sig_unenforce parameter 2018-06-06 0:43 [PATCH] module: Implement sig_unenforce parameter Brett T. Warden @ 2018-06-06 13:37 ` kbuild test robot 2018-06-06 19:44 ` [PATCH v2] " Brett T. Warden 0 siblings, 1 reply; 5+ messages in thread From: kbuild test robot @ 2018-06-06 13:37 UTC (permalink / raw) To: Brett T. Warden; +Cc: kbuild-all, linux-kernel, jeyu [-- Attachment #1: Type: text/plain, Size: 6442 bytes --] Hi Brett, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v4.17] [cannot apply to next-20180605] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Brett-T-Warden/module-Implement-sig_unenforce-parameter/20180606-200056 config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 8.1.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=xtensa All errors (new ones prefixed by >>): kernel/module.c: In function 'proc_modules_init': >> kernel/module.c:4255:21: error: invalid use of undefined type 'struct boot_params' switch (boot_params.secure_boot) { ^ In file included from kernel/module.c:35: kernel/module.c: At top level: include/linux/syscalls.h:233:18: warning: 'sys_delete_module' alias between functions of incompatible types 'long int(const char *, unsigned int)' and 'long int(long int, long int)' [-Wattribute-alias] asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ ^~~ include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx' __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) ^~~~~~~~~~~~~~~~~ include/linux/syscalls.h:212:36: note: in expansion of macro 'SYSCALL_DEFINEx' #define SYSCALL_DEFINE2(name, ...) SYSCALL_DEFINEx(2, _##name, __VA_ARGS__) ^~~~~~~~~~~~~~~ kernel/module.c:969:1: note: in expansion of macro 'SYSCALL_DEFINE2' SYSCALL_DEFINE2(delete_module, const char __user *, name_user, ^~~~~~~~~~~~~~~ include/linux/syscalls.h:238:18: note: aliased declaration here asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \ ^~~~~~~~ include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx' __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) ^~~~~~~~~~~~~~~~~ include/linux/syscalls.h:212:36: note: in expansion of macro 'SYSCALL_DEFINEx' #define SYSCALL_DEFINE2(name, ...) SYSCALL_DEFINEx(2, _##name, __VA_ARGS__) ^~~~~~~~~~~~~~~ kernel/module.c:969:1: note: in expansion of macro 'SYSCALL_DEFINE2' SYSCALL_DEFINE2(delete_module, const char __user *, name_user, ^~~~~~~~~~~~~~~ include/linux/syscalls.h:233:18: warning: 'sys_finit_module' alias between functions of incompatible types 'long int(int, const char *, int)' and 'long int(long int, long int, long int)' [-Wattribute-alias] asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ ^~~ include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx' __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) ^~~~~~~~~~~~~~~~~ include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx' #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__) ^~~~~~~~~~~~~~~ kernel/module.c:3865:1: note: in expansion of macro 'SYSCALL_DEFINE3' SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags) ^~~~~~~~~~~~~~~ include/linux/syscalls.h:238:18: note: aliased declaration here asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \ ^~~~~~~~ include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx' __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) ^~~~~~~~~~~~~~~~~ include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx' #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__) ^~~~~~~~~~~~~~~ kernel/module.c:3865:1: note: in expansion of macro 'SYSCALL_DEFINE3' SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags) ^~~~~~~~~~~~~~~ include/linux/syscalls.h:233:18: warning: 'sys_init_module' alias between functions of incompatible types 'long int(void *, long unsigned int, const char *)' and 'long int(long int, long int, long int)' [-Wattribute-alias] asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ ^~~ include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx' __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) ^~~~~~~~~~~~~~~~~ include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx' #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__) ^~~~~~~~~~~~~~~ kernel/module.c:3845:1: note: in expansion of macro 'SYSCALL_DEFINE3' SYSCALL_DEFINE3(init_module, void __user *, umod, ^~~~~~~~~~~~~~~ include/linux/syscalls.h:238:18: note: aliased declaration here asmlinkage long __se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \ ^~~~~~~~ include/linux/syscalls.h:222:2: note: in expansion of macro '__SYSCALL_DEFINEx' __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) ^~~~~~~~~~~~~~~~~ include/linux/syscalls.h:213:36: note: in expansion of macro 'SYSCALL_DEFINEx' #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__) ^~~~~~~~~~~~~~~ kernel/module.c:3845:1: note: in expansion of macro 'SYSCALL_DEFINE3' SYSCALL_DEFINE3(init_module, void __user *, umod, ^~~~~~~~~~~~~~~ vim +4255 kernel/module.c 4249 4250 static int __init proc_modules_init(void) 4251 { 4252 proc_create("modules", 0, NULL, &proc_modules_operations); 4253 4254 #ifdef CONFIG_MODULE_SIG_FORCE > 4255 switch (boot_params.secure_boot) { 4256 case efi_secureboot_mode_unset: 4257 case efi_secureboot_mode_unknown: 4258 case efi_secureboot_mode_disabled: 4259 /* 4260 * sig_unenforce is only applied if SecureBoot is not 4261 * enabled. 4262 */ 4263 sig_enforce = !sig_unenforce; 4264 } 4265 #endif 4266 4267 return 0; 4268 } 4269 module_init(proc_modules_init); 4270 #endif 4271 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 52954 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] module: Implement sig_unenforce parameter 2018-06-06 13:37 ` kbuild test robot @ 2018-06-06 19:44 ` Brett T. Warden 2018-06-13 13:15 ` Jessica Yu 0 siblings, 1 reply; 5+ messages in thread From: Brett T. Warden @ 2018-06-06 19:44 UTC (permalink / raw) To: linux-kernel, jeyu 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 + return 0; } module_init(proc_modules_init); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] module: Implement sig_unenforce parameter 2018-06-06 19:44 ` [PATCH v2] " Brett T. Warden @ 2018-06-13 13:15 ` Jessica Yu 2018-06-13 20:27 ` Warden, Brett T 0 siblings, 1 reply; 5+ messages in thread From: Jessica Yu @ 2018-06-13 13:15 UTC (permalink / raw) To: Brett T. Warden; +Cc: linux-kernel +++ 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2] module: Implement sig_unenforce parameter 2018-06-13 13:15 ` Jessica Yu @ 2018-06-13 20:27 ` Warden, Brett T 0 siblings, 0 replies; 5+ messages in thread From: Warden, Brett T @ 2018-06-13 20:27 UTC (permalink / raw) To: Jessica Yu; +Cc: linux-kernel > -----Original Message----- > From: Jessica Yu [mailto:jeyu@kernel.org] > Sent: Wednesday, June 13, 2018 6:15 AM > To: Warden, Brett T <brett.t.warden@intel.com> > Cc: linux-kernel@vger.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@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 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-13 20:27 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-06-06 0:43 [PATCH] module: Implement sig_unenforce parameter Brett T. Warden 2018-06-06 13:37 ` kbuild test robot 2018-06-06 19:44 ` [PATCH v2] " Brett T. Warden 2018-06-13 13:15 ` Jessica Yu 2018-06-13 20:27 ` Warden, Brett T
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).