On 5/5/26 11:05 AM, Thomas Weißschuh wrote:
> The module authentication functionality will also be used by the
> hash-based module authentication. To make it usable even if
> CONFIG_MODULE_SIG is disabled, move it to a new file.
>
> Signed-off-by: Thomas Weißschuh <[email protected]>
> ---
> [...]
> diff --git a/kernel/module/auth.c b/kernel/module/auth.c
> index 956ac63d9d33..831a13eb0c9b 100644
> --- a/kernel/module/auth.c
> +++ b/kernel/module/auth.c
> @@ -5,10 +5,16 @@
> * Written by David Howells ([email protected])
> */
>
> +#include <linux/errno.h>
> #include <linux/export.h>
> #include <linux/module.h>
> +#include <linux/module_signature.h>
> #include <linux/moduleparam.h>
> +#include <linux/security.h>
> +#include <linux/string.h>
> #include <linux/types.h>
> +#include <uapi/linux/module.h>
> +#include "internal.h"
>
> #undef MODULE_PARAM_PREFIX
> #define MODULE_PARAM_PREFIX "module."
> @@ -30,3 +36,82 @@ void set_module_sig_enforced(void)
> {
> sig_enforce = true;
> }
> +
> +static int mod_verify_sig(const void *mod, struct load_info *info)
> +{
> + struct module_signature ms;
> + size_t sig_len, modlen = info->len;
> + int ret;
> +
> + if (modlen <= sizeof(ms))
> + return -EBADMSG;
> +
> + memcpy(&ms, mod + (modlen - sizeof(ms)), sizeof(ms));
> +
> + ret = mod_check_sig(&ms, modlen, "module");
> + if (ret)
> + return ret;
> +
> + sig_len = be32_to_cpu(ms.sig_len);
> + modlen -= sig_len + sizeof(ms);
> + info->len = modlen;
> +
> + return module_sig_check(mod, modlen, mod + modlen, sig_len);
> +}
> +
> +int module_auth_check(struct load_info *info, int flags)
> +{
> + int err = -ENODATA;
> + const unsigned long markerlen = sizeof(MODULE_SIGNATURE_MARKER) - 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_SIGNATURE_MARKER,
> markerlen) == 0) {
> + /* We truncate the module to discard the signature */
> + info->len -= markerlen;
> + err = mod_verify_sig(mod, info);
> + if (!err) {
> + info->auth_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);
> +}
The resulting call chain of the module authentication/signature
functions is as follows:
ima_read_modsig() -----------------------------,
v
module_auth_check() -> mod_verify_sig() -> mod_check_sig()
|
|-> module_sig_check()
'-> module_hash_check()
I think this logic is quite hard to follow because mod_verify_sig(),
mod_check_sig() and module_sig_check() have very similar names.
The naming of module_auth_check(), module_sig_check() and
module_hash_check() looks good to me, but I would prefer to rename
mod_check_sig() and mod_verify_sig(). Perhaps mod_check_sig() could be
renamed to mod_check_sig_header(), and mod_verify_sig() to
mod_dispatch_auth_check()?
Otherwise, the patch looks ok to me. Feel free to add:
Reviewed-by: Petr Pavlu <[email protected]>
--
Thanks,
Petr
