linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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).