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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <804fb52d-f57a-445c-f7d0-ed04dee061a4@csgroup.eu>
Date:   Tue, 22 Feb 2022 17:59:15 +0000
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     Aaron Tomlin <atomlin@...hat.com>,
        "mcgrof@...nel.org" <mcgrof@...nel.org>
CC:     "cl@...ux.com" <cl@...ux.com>,
        "pmladek@...e.com" <pmladek@...e.com>,
        "mbenes@...e.cz" <mbenes@...e.cz>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "jeyu@...nel.org" <jeyu@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-modules@...r.kernel.org" <linux-modules@...r.kernel.org>,
        "void@...ifault.com" <void@...ifault.com>,
        "atomlin@...mlin.com" <atomlin@...mlin.com>,
        "allen.lkml@...il.com" <allen.lkml@...il.com>,
        "joe@...ches.com" <joe@...ches.com>,
        "msuchanek@...e.de" <msuchanek@...e.de>,
        "oleksandr@...alenko.name" <oleksandr@...alenko.name>
Subject: Re: [PATCH v8 07/13] module: Move extra signature support out of core
 code



Le 22/02/2022 à 15:12, Aaron Tomlin a écrit :
> No functional change.
> 
> This patch migrates additional module signature check
> code from core module code into kernel/module/signing.c.
> 
> Signed-off-by: Aaron Tomlin <atomlin@...hat.com>

Reviewed-by: Christophe Leroy <christophe.leroy@...roup.eu>

> ---
>   kernel/module/internal.h |  9 +++++
>   kernel/module/main.c     | 87 ----------------------------------------
>   kernel/module/signing.c  | 77 +++++++++++++++++++++++++++++++++++
>   3 files changed, 86 insertions(+), 87 deletions(-)
> 
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index a6895bb5598a..d6f646a5da41 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -158,3 +158,12 @@ static inline int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>   	return 0;
>   }
>   #endif /* CONFIG_STRICT_MODULE_RWX */
> +
> +#ifdef CONFIG_MODULE_SIG
> +int module_sig_check(struct load_info *info, int flags);
> +#else /* !CONFIG_MODULE_SIG */
> +static inline int module_sig_check(struct load_info *info, int flags)
> +{
> +	return 0;
> +}
> +#endif /* !CONFIG_MODULE_SIG */
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 5cd63f14b1ef..c63e10c61694 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -23,7 +23,6 @@
>   #include <linux/vmalloc.h>
>   #include <linux/elf.h>
>   #include <linux/proc_fs.h>
> -#include <linux/security.h>
>   #include <linux/seq_file.h>
>   #include <linux/syscalls.h>
>   #include <linux/fcntl.h>
> @@ -127,28 +126,6 @@ static void module_assert_mutex_or_preempt(void)
>   #endif
>   }
>   
> -#ifdef CONFIG_MODULE_SIG
> -static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
> -module_param(sig_enforce, bool_enable_only, 0644);
> -
> -void set_module_sig_enforced(void)
> -{
> -	sig_enforce = true;
> -}
> -#else
> -#define sig_enforce false
> -#endif
> -
> -/*
> - * Export sig_enforce kernel cmdline parameter to allow other subsystems rely
> - * on that instead of directly to CONFIG_MODULE_SIG_FORCE config.
> - */
> -bool is_module_sig_enforced(void)
> -{
> -	return sig_enforce;
> -}
> -EXPORT_SYMBOL(is_module_sig_enforced);
> -
>   /* Block module loading/unloading? */
>   int modules_disabled = 0;
>   core_param(nomodule, modules_disabled, bint, 0);
> @@ -2569,70 +2546,6 @@ 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)
> -{
> -	int err = -ENODATA;
> -	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> -	const char *reason;
> -	const void *mod = info->hdr;
> -	bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> -				       MODULE_INIT_IGNORE_VERMAGIC);
> -	/*
> -	 * Do not allow mangled modules as a module with version information
> -	 * removed is no longer the module that was signed.
> -	 */
> -	if (!mangled_module &&
> -	    info->len > markerlen &&
> -	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> -		/* We truncate the module to discard the signature */
> -		info->len -= markerlen;
> -		err = mod_verify_sig(mod, info);
> -		if (!err) {
> -			info->sig_ok = true;
> -			return 0;
> -		}
> -	}
> -
> -	/*
> -	 * We don't permit modules to be loaded into the trusted kernels
> -	 * without a valid signature on them, but if we're not enforcing,
> -	 * certain errors are non-fatal.
> -	 */
> -	switch (err) {
> -	case -ENODATA:
> -		reason = "unsigned module";
> -		break;
> -	case -ENOPKG:
> -		reason = "module with unsupported crypto";
> -		break;
> -	case -ENOKEY:
> -		reason = "module with unavailable key";
> -		break;
> -
> -	default:
> -		/*
> -		 * All other errors are fatal, including lack of memory,
> -		 * unparseable signatures, and signature check failures --
> -		 * even if signatures aren't required.
> -		 */
> -		return err;
> -	}
> -
> -	if (is_module_sig_enforced()) {
> -		pr_notice("Loading of %s is rejected\n", reason);
> -		return -EKEYREJECTED;
> -	}
> -
> -	return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
> -}
> -#else /* !CONFIG_MODULE_SIG */
> -static int module_sig_check(struct load_info *info, int flags)
> -{
> -	return 0;
> -}
> -#endif /* !CONFIG_MODULE_SIG */
> -
>   static int validate_section_offset(struct load_info *info, Elf_Shdr *shdr)
>   {
>   #if defined(CONFIG_64BIT)
> diff --git a/kernel/module/signing.c b/kernel/module/signing.c
> index 8aeb6d2ee94b..85c8999dfecf 100644
> --- a/kernel/module/signing.c
> +++ b/kernel/module/signing.c
> @@ -11,9 +11,29 @@
>   #include <linux/module_signature.h>
>   #include <linux/string.h>
>   #include <linux/verification.h>
> +#include <linux/security.h>
>   #include <crypto/public_key.h>
> +#include <uapi/linux/module.h>
>   #include "internal.h"
>   
> +static bool sig_enforce = IS_ENABLED(CONFIG_MODULE_SIG_FORCE);
> +module_param(sig_enforce, bool_enable_only, 0644);
> +
> +/*
> + * Export sig_enforce kernel cmdline parameter to allow other subsystems rely
> + * on that instead of directly to CONFIG_MODULE_SIG_FORCE config.
> + */
> +bool is_module_sig_enforced(void)
> +{
> +	return sig_enforce;
> +}
> +EXPORT_SYMBOL(is_module_sig_enforced);
> +
> +void set_module_sig_enforced(void)
> +{
> +	sig_enforce = true;
> +}
> +
>   /*
>    * Verify the signature on a module.
>    */
> @@ -43,3 +63,60 @@ int mod_verify_sig(const void *mod, struct load_info *info)
>   				      VERIFYING_MODULE_SIGNATURE,
>   				      NULL, NULL);
>   }
> +
> +int module_sig_check(struct load_info *info, int flags)
> +{
> +	int err = -ENODATA;
> +	const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> +	const char *reason;
> +	const void *mod = info->hdr;
> +	bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> +				       MODULE_INIT_IGNORE_VERMAGIC);
> +	/*
> +	 * Do not allow mangled modules as a module with version information
> +	 * removed is no longer the module that was signed.
> +	 */
> +	if (!mangled_module &&
> +	    info->len > markerlen &&
> +	    memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) == 0) {
> +		/* We truncate the module to discard the signature */
> +		info->len -= markerlen;
> +		err = mod_verify_sig(mod, info);
> +		if (!err) {
> +			info->sig_ok = true;
> +			return 0;
> +		}
> +	}
> +
> +	/*
> +	 * We don't permit modules to be loaded into the trusted kernels
> +	 * without a valid signature on them, but if we're not enforcing,
> +	 * certain errors are non-fatal.
> +	 */
> +	switch (err) {
> +	case -ENODATA:
> +		reason = "unsigned module";
> +		break;
> +	case -ENOPKG:
> +		reason = "module with unsupported crypto";
> +		break;
> +	case -ENOKEY:
> +		reason = "module with unavailable key";
> +		break;
> +
> +	default:
> +		/*
> +		 * All other errors are fatal, including lack of memory,
> +		 * unparseable signatures, and signature check failures --
> +		 * even if signatures aren't required.
> +		 */
> +		return err;
> +	}
> +
> +	if (is_module_sig_enforced()) {
> +		pr_notice("Loading of %s is rejected\n", reason);
> +		return -EKEYREJECTED;
> +	}
> +
> +	return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
> +}

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ