Re: [PATCH v8 07/13] module: Move extra signature support out of core code

Christophe Leroy
2022-02-22T17:59:25Z

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@redhat.com>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.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);
> +}