diff mbox series

[2/2,Cosmic] UBUNTU: SAUCE: (efi-lockdown) module: remove support for deferring module signature verification to IMA

Message ID 20181026175516.21251-3-seth.forshee@canonical.com
State New
Headers show
Series [1/2,Cosmic] UBUNTU: SAUCE: (efi-lockdown) module: trust keys from secondary keyring for module signing | expand

Commit Message

Seth Forshee Oct. 26, 2018, 5:55 p.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1798863

Recent versions of the "secure boot lockdown" patches introduced
support for using IMA signatures for module signing instead of
the standard mechanism. This was causing issues and was removed,
but the code was missed which actually defers the verification to
IMA when IMA enforcement is enabled. With our config this means
that by default module signatures are not being enforced under
kernel lockdown.

Remove the remaining code to restore module signature enforcement
under lockdown.

CVE-2018-18653

Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 kernel/module.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

Comments

Thadeu Lima de Souza Cascardo Oct. 26, 2018, 6:06 p.m. UTC | #1
Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Tyler Hicks Oct. 26, 2018, 8:54 p.m. UTC | #2
On 2018-10-26 11:55:16, Seth Forshee wrote:
> BugLink: https://bugs.launchpad.net/bugs/1798863
> 
> Recent versions of the "secure boot lockdown" patches introduced
> support for using IMA signatures for module signing instead of
> the standard mechanism. This was causing issues and was removed,
> but the code was missed which actually defers the verification to
> IMA when IMA enforcement is enabled. With our config this means
> that by default module signatures are not being enforced under
> kernel lockdown.
> 
> Remove the remaining code to restore module signature enforcement
> under lockdown.
> 
> CVE-2018-18653
> 
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>

Acked-by: Tyler Hicks <tyhicks@canonical.com>

Tyler

> ---
>  kernel/module.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 9af04eebd711..a767bd326b43 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2760,8 +2760,7 @@ static inline void kmemleak_load_module(const struct module *mod,
>  #endif
>  
>  #ifdef CONFIG_MODULE_SIG
> -static int module_sig_check(struct load_info *info, int flags,
> -			    bool can_do_ima_check)
> +static int module_sig_check(struct load_info *info, int flags)
>  {
>  	int err = -ENODATA;
>  	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> @@ -2803,8 +2802,6 @@ static int module_sig_check(struct load_info *info, int flags,
>  			return -EKEYREJECTED;
>  		}
>  
> -		if (can_do_ima_check && is_ima_appraise_enabled())
> -			return 0;
>  		if (kernel_is_locked_down(reason))
>  			return -EPERM;
>  		return 0;
> @@ -2818,8 +2815,7 @@ static int module_sig_check(struct load_info *info, int flags,
>  	}
>  }
>  #else /* !CONFIG_MODULE_SIG */
> -static int module_sig_check(struct load_info *info, int flags,
> -			    bool can_do_ima_check)
> +static int module_sig_check(struct load_info *info, int flags)
>  {
>  	return 0;
>  }
> @@ -3684,13 +3680,13 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,
>  /* Allocate and load the module: note that size of section 0 is always
>     zero, and we rely on this for optional sections. */
>  static int load_module(struct load_info *info, const char __user *uargs,
> -		       int flags, bool can_do_ima_check)
> +		       int flags)
>  {
>  	struct module *mod;
>  	long err;
>  	char *after_dashes;
>  
> -	err = module_sig_check(info, flags, can_do_ima_check);
> +	err = module_sig_check(info, flags);
>  	if (err)
>  		goto free_copy;
>  
> @@ -3879,7 +3875,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
>  	if (err)
>  		return err;
>  
> -	return load_module(&info, uargs, 0, false);
> +	return load_module(&info, uargs, 0);
>  }
>  
>  SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
> @@ -3906,7 +3902,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
>  	info.hdr = hdr;
>  	info.len = size;
>  
> -	return load_module(&info, uargs, flags, true);
> +	return load_module(&info, uargs, flags);
>  }
>  
>  static inline int within(unsigned long addr, void *start, unsigned long size)
> -- 
> 2.19.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/kernel/module.c b/kernel/module.c
index 9af04eebd711..a767bd326b43 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2760,8 +2760,7 @@  static inline void kmemleak_load_module(const struct module *mod,
 #endif
 
 #ifdef CONFIG_MODULE_SIG
-static int module_sig_check(struct load_info *info, int flags,
-			    bool can_do_ima_check)
+static int module_sig_check(struct load_info *info, int flags)
 {
 	int err = -ENODATA;
 	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
@@ -2803,8 +2802,6 @@  static int module_sig_check(struct load_info *info, int flags,
 			return -EKEYREJECTED;
 		}
 
-		if (can_do_ima_check && is_ima_appraise_enabled())
-			return 0;
 		if (kernel_is_locked_down(reason))
 			return -EPERM;
 		return 0;
@@ -2818,8 +2815,7 @@  static int module_sig_check(struct load_info *info, int flags,
 	}
 }
 #else /* !CONFIG_MODULE_SIG */
-static int module_sig_check(struct load_info *info, int flags,
-			    bool can_do_ima_check)
+static int module_sig_check(struct load_info *info, int flags)
 {
 	return 0;
 }
@@ -3684,13 +3680,13 @@  static int unknown_module_param_cb(char *param, char *val, const char *modname,
 /* Allocate and load the module: note that size of section 0 is always
    zero, and we rely on this for optional sections. */
 static int load_module(struct load_info *info, const char __user *uargs,
-		       int flags, bool can_do_ima_check)
+		       int flags)
 {
 	struct module *mod;
 	long err;
 	char *after_dashes;
 
-	err = module_sig_check(info, flags, can_do_ima_check);
+	err = module_sig_check(info, flags);
 	if (err)
 		goto free_copy;
 
@@ -3879,7 +3875,7 @@  SYSCALL_DEFINE3(init_module, void __user *, umod,
 	if (err)
 		return err;
 
-	return load_module(&info, uargs, 0, false);
+	return load_module(&info, uargs, 0);
 }
 
 SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
@@ -3906,7 +3902,7 @@  SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
 	info.hdr = hdr;
 	info.len = size;
 
-	return load_module(&info, uargs, flags, true);
+	return load_module(&info, uargs, flags);
 }
 
 static inline int within(unsigned long addr, void *start, unsigned long size)