linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] module: Introduce hash-based integrity checking
@ 2025-01-20 17:44 Thomas Weißschuh
  2025-01-20 17:44 ` [PATCH v2 1/6] kbuild: add stamp file for vmlinux BTF data Thomas Weißschuh
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-01-20 17:44 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, Thomas Weißschuh

The current signature-based module integrity checking has some drawbacks
in combination with reproducible builds:
Either the module signing key is generated at build time, which makes
the build unreproducible, or a static key is used, which precludes
rebuilds by third parties and makes the whole build and packaging
process much more complicated.
Introduce a new mechanism to ensure only well-known modules are loaded
by embedding a list of hashes of all modules built as part of the full
kernel build into vmlinux.

Interest has been proclaimed by NixOS, Arch Linux, Proxmox, SUSE and the
general reproducible builds community.

To properly test the reproducibility in combination with CONFIG_INFO_BTF
another patch is needed:
"[PATCH bpf-next] kbuild, bpf: Enable reproducible BTF generation" [0]
(If you happen to test that one, please give some feedback)

Questions for current patch:
* Naming
* Can the number of built-in modules be retrieved while building
  kernel/module/hashes.o? This would remove the need for the
  preallocation step in link-vmlinux.sh.

Further improvements:
* Use a LSM/IMA/Keyring to store and validate hashes
* Use MODULE_SIG_HASH for configuration
* UAPI for discovery?

[0] https://lore.kernel.org/lkml/20241211-pahole-reproducible-v1-1-22feae19bad9@weissschuh.net/

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Changes in v2:
- Drop RFC state
- Mention interested parties in cover letter
- Expand Kconfig description
- Add compatibility with CONFIG_MODULE_SIG
- Parallelize module-hashes.sh
- Update Documentation/kbuild/reproducible-builds.rst
- Link to v1: https://lore.kernel.org/r/20241225-module-hashes-v1-0-d710ce7a3fd1@weissschuh.net

---
Thomas Weißschuh (6):
      kbuild: add stamp file for vmlinux BTF data
      module: Make module loading policy usable without MODULE_SIG
      module: Move integrity checks into dedicated function
      module: Move lockdown check into generic module loader
      lockdown: Make the relationship to MODULE_SIG a dependency
      module: Introduce hash-based integrity checking

 .gitignore                                   |  1 +
 Documentation/kbuild/reproducible-builds.rst |  5 ++-
 Makefile                                     |  8 ++++-
 include/asm-generic/vmlinux.lds.h            | 11 ++++++
 include/linux/module.h                       |  8 ++---
 include/linux/module_hashes.h                | 17 +++++++++
 kernel/module/Kconfig                        | 21 ++++++++++-
 kernel/module/Makefile                       |  1 +
 kernel/module/hashes.c                       | 52 +++++++++++++++++++++++++++
 kernel/module/internal.h                     |  8 +----
 kernel/module/main.c                         | 54 +++++++++++++++++++++++++---
 kernel/module/signing.c                      | 24 +------------
 scripts/Makefile.modfinal                    | 10 ++++--
 scripts/Makefile.vmlinux                     |  5 +++
 scripts/link-vmlinux.sh                      | 31 +++++++++++++++-
 scripts/module-hashes.sh                     | 26 ++++++++++++++
 security/lockdown/Kconfig                    |  2 +-
 17 files changed, 238 insertions(+), 46 deletions(-)
---
base-commit: 2cd5917560a84d69dd6128b640d7a68406ff019b
change-id: 20241225-module-hashes-7a50a7cc2a30

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v2 1/6] kbuild: add stamp file for vmlinux BTF data
  2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
@ 2025-01-20 17:44 ` Thomas Weißschuh
  2025-01-20 17:44 ` [PATCH v2 2/6] module: Make module loading policy usable without MODULE_SIG Thomas Weißschuh
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-01-20 17:44 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, Thomas Weißschuh

The upcoming module hashes functionality will build the modules in
between the generation of the BTF data and the final link of vmlinux.
Having a dependency from the modules on vmlinux would make this
impossible as it would mean having a cyclic dependency.
Break this cyclic dependency by introducing a new target.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 scripts/Makefile.modfinal | 4 ++--
 scripts/link-vmlinux.sh   | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 542ba462ed3ec9607e0df10e26613a4c7ac318e8..5d01b553ec9a4565c8e5a6edd05665c409003bc1 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -52,8 +52,8 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
 	printf '%s\n' 'savedcmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
 
 # Re-generate module BTFs if either module's .ko or vmlinux changed
-%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/vmlinux) FORCE
-	+$(call if_changed_except,ld_ko_o,$(objtree)/vmlinux)
+%.ko: %.o %.mod.o .module-common.o $(objtree)/scripts/module.lds $(and $(CONFIG_DEBUG_INFO_BTF_MODULES),$(KBUILD_BUILTIN),$(objtree)/.tmp_vmlinux_btf.stamp) FORCE
+	+$(call if_changed_except,ld_ko_o,$(objtree)/.tmp_vmlinux_btf.stamp)
 ifdef CONFIG_DEBUG_INFO_BTF_MODULES
 	+$(if $(newer-prereqs),$(call cmd,btf_ko))
 endif
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index d853ddb3b28c1238ec9079ebbbe77df26980a0a1..803c8d6f35a7f29fb68b29afa8546f4dde0bd4cb 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -112,6 +112,7 @@ vmlinux_link()
 gen_btf()
 {
 	local btf_data=${1}.btf.o
+	local btf_stamp=.tmp_vmlinux_btf.stamp
 
 	info BTF "${btf_data}"
 	LLVM_OBJCOPY="${OBJCOPY}" ${PAHOLE} -J ${PAHOLE_FLAGS} ${1}
@@ -132,6 +133,11 @@ gen_btf()
 	fi
 	printf "${et_rel}" | dd of="${btf_data}" conv=notrunc bs=1 seek=16 status=none
 
+	info STAMP $btf_stamp
+	if ! cmp --silent $btf_data $btf_stamp; then
+		cp $btf_data $btf_stamp
+	fi
+
 	btf_vmlinux_bin_o=${btf_data}
 }
 

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 2/6] module: Make module loading policy usable without MODULE_SIG
  2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
  2025-01-20 17:44 ` [PATCH v2 1/6] kbuild: add stamp file for vmlinux BTF data Thomas Weißschuh
@ 2025-01-20 17:44 ` Thomas Weißschuh
  2025-01-20 17:44 ` [PATCH v2 3/6] module: Move integrity checks into dedicated function Thomas Weißschuh
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-01-20 17:44 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, Thomas Weißschuh

The loading policy functionality will also be used by the hash-based
module validation. Split it out from CONFIG_MODULE_SIG so it is usable
by both.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 include/linux/module.h  |  8 ++++----
 kernel/module/Kconfig   |  6 +++++-
 kernel/module/main.c    | 26 +++++++++++++++++++++++++-
 kernel/module/signing.c | 21 ---------------------
 4 files changed, 34 insertions(+), 27 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index b3a643435357986f3f9fe852260ca07f371cf86c..ccddab25d277da84d8c2866a9b4ded7c18691c0d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -451,7 +451,7 @@ struct module {
 	const s32 *gpl_crcs;
 	bool using_gplonly_symbols;
 
-#ifdef CONFIG_MODULE_SIG
+#ifdef CONFIG_MODULE_SIG_POLICY
 	/* Signature was verified. */
 	bool sig_ok;
 #endif
@@ -928,14 +928,14 @@ static inline bool retpoline_module_ok(bool has_retpoline)
 }
 #endif
 
-#ifdef CONFIG_MODULE_SIG
+#ifdef CONFIG_MODULE_SIG_POLICY
 bool is_module_sig_enforced(void);
 
 static inline bool module_sig_ok(struct module *module)
 {
 	return module->sig_ok;
 }
-#else	/* !CONFIG_MODULE_SIG */
+#else	/* !CONFIG_MODULE_SIG_POLICY */
 static inline bool is_module_sig_enforced(void)
 {
 	return false;
@@ -945,7 +945,7 @@ static inline bool module_sig_ok(struct module *module)
 {
 	return true;
 }
-#endif	/* CONFIG_MODULE_SIG */
+#endif	/* CONFIG_MODULE_SIG_POLICY */
 
 #if defined(CONFIG_MODULES) && defined(CONFIG_KALLSYMS)
 int module_kallsyms_on_each_symbol(const char *modname,
diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index 7b329057997ad2ec310133ca84617d9bfcdb7e9f..a80de8d22efdd0f13b3eb579a8ff1e69029d0694 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -210,9 +210,13 @@ config MODULE_SIG
 	  debuginfo strip done by some packagers (such as rpmbuild) and
 	  inclusion into an initramfs that wants the module size reduced.
 
+config MODULE_SIG_POLICY
+	def_bool y
+	depends on MODULE_SIG
+
 config MODULE_SIG_FORCE
 	bool "Require modules to be validly signed"
-	depends on MODULE_SIG
+	depends on MODULE_SIG_POLICY
 	help
 	  Reject unsigned modules or signed modules for which we don't have a
 	  key.  Without this, such modules will simply taint the kernel.
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 5399c182b3cbed2dbeea0291f717f30358d8e7fc..8aa593fee22a227a482466dceda4a6b657b956e0 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2367,7 +2367,7 @@ static void module_augment_kernel_taints(struct module *mod, struct load_info *i
 				mod->name);
 		add_taint_module(mod, TAINT_TEST, LOCKDEP_STILL_OK);
 	}
-#ifdef CONFIG_MODULE_SIG
+#ifdef CONFIG_MODULE_SIG_POLICY
 	mod->sig_ok = info->sig_ok;
 	if (!mod->sig_ok) {
 		pr_notice_once("%s: module verification failed: signature "
@@ -3779,3 +3779,27 @@ static int module_debugfs_init(void)
 }
 module_init(module_debugfs_init);
 #endif
+
+#ifdef CONFIG_MODULE_SIG_POLICY
+
+#undef MODULE_PARAM_PREFIX
+#define MODULE_PARAM_PREFIX "module."
+
+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;
+}
+#endif
diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index a2ff4242e623d5d4e87d2f3d139d8620fb937579..e51920605da14771601327ea596dad2e12400518 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -16,27 +16,6 @@
 #include <uapi/linux/module.h>
 #include "internal.h"
 
-#undef MODULE_PARAM_PREFIX
-#define MODULE_PARAM_PREFIX "module."
-
-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.
  */

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 3/6] module: Move integrity checks into dedicated function
  2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
  2025-01-20 17:44 ` [PATCH v2 1/6] kbuild: add stamp file for vmlinux BTF data Thomas Weißschuh
  2025-01-20 17:44 ` [PATCH v2 2/6] module: Make module loading policy usable without MODULE_SIG Thomas Weißschuh
@ 2025-01-20 17:44 ` Thomas Weißschuh
  2025-01-20 17:44 ` [PATCH v2 4/6] module: Move lockdown check into generic module loader Thomas Weißschuh
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-01-20 17:44 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, Thomas Weißschuh

With the addition of hash-based integrity checking, the configuration
matrix is easier to represent in a dedicated function and with explicit
usage of IS_ENABLED().

Drop the now unnecessary stub for module_sig_check().

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 kernel/module/internal.h |  7 -------
 kernel/module/main.c     | 18 ++++++++++++++----
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index daef2be8390222c22220e2f168baa8d35ad531b9..c30abeefa60b884c4a69b1eb4f1123a4bbee4b47 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -333,14 +333,7 @@ int module_enable_text_rox(const struct module *mod);
 int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 				char *secstrings, struct module *mod);
 
-#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 */
 
 #ifdef CONFIG_DEBUG_KMEMLEAK
 void kmemleak_load_module(const struct module *mod, const struct load_info *info);
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 8aa593fee22a227a482466dceda4a6b657b956e0..c0ab5c37f9710a0091320c4d171275e63be9217e 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3214,6 +3214,16 @@ static int early_mod_check(struct load_info *info, int flags)
 	return err;
 }
 
+static int module_integrity_check(struct load_info *info, int flags)
+{
+	int err = 0;
+
+	if (IS_ENABLED(CONFIG_MODULE_SIG))
+		err = module_sig_check(info, flags);
+
+	return err;
+}
+
 /*
  * Allocate and load the module: note that size of section 0 is always
  * zero, and we rely on this for optional sections.
@@ -3227,18 +3237,18 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	char *after_dashes;
 
 	/*
-	 * Do the signature check (if any) first. All that
-	 * the signature check needs is info->len, it does
+	 * Do the integrity checks (if any) first. All that
+	 * they need is info->len, it does
 	 * not need any of the section info. That can be
 	 * set up later. This will minimize the chances
 	 * of a corrupt module causing problems before
-	 * we even get to the signature check.
+	 * we even get to the integrity check.
 	 *
 	 * The check will also adjust info->len by stripping
 	 * off the sig length at the end of the module, making
 	 * checks against info->len more correct.
 	 */
-	err = module_sig_check(info, flags);
+	err = module_integrity_check(info, flags);
 	if (err)
 		goto free_copy;
 

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 4/6] module: Move lockdown check into generic module loader
  2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
                   ` (2 preceding siblings ...)
  2025-01-20 17:44 ` [PATCH v2 3/6] module: Move integrity checks into dedicated function Thomas Weißschuh
@ 2025-01-20 17:44 ` Thomas Weißschuh
  2025-01-20 17:44 ` [PATCH v2 5/6] lockdown: Make the relationship to MODULE_SIG a dependency Thomas Weißschuh
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-01-20 17:44 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, Thomas Weißschuh

The lockdown check buried in module_sig_check() will not compose well
with the introduction of hash-based module validation.
Move it into module_integrity_check() which will work better.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 kernel/module/main.c    | 6 +++++-
 kernel/module/signing.c | 3 +--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index c0ab5c37f9710a0091320c4d171275e63be9217e..effe1db02973d4f60ff6cbc0d3b5241a3576fa3e 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3221,7 +3221,11 @@ static int module_integrity_check(struct load_info *info, int flags)
 	if (IS_ENABLED(CONFIG_MODULE_SIG))
 		err = module_sig_check(info, flags);
 
-	return err;
+	if (err)
+		return err;
+	if (info->sig_ok)
+		return 0;
+	return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
 }
 
 /*
diff --git a/kernel/module/signing.c b/kernel/module/signing.c
index e51920605da14771601327ea596dad2e12400518..029e1ef6f0e369fd48e8c81154b6c697ad7a6249 100644
--- a/kernel/module/signing.c
+++ b/kernel/module/signing.c
@@ -11,7 +11,6 @@
 #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"
@@ -100,5 +99,5 @@ int module_sig_check(struct load_info *info, int flags)
 		return -EKEYREJECTED;
 	}
 
-	return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
+	return 0;
 }

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 5/6] lockdown: Make the relationship to MODULE_SIG a dependency
  2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
                   ` (3 preceding siblings ...)
  2025-01-20 17:44 ` [PATCH v2 4/6] module: Move lockdown check into generic module loader Thomas Weißschuh
@ 2025-01-20 17:44 ` Thomas Weißschuh
  2025-01-20 17:44 ` [PATCH v2 6/6] module: Introduce hash-based integrity checking Thomas Weißschuh
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-01-20 17:44 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, Thomas Weißschuh

The new hash-based module integrity checking will also be able to
satisfy the requirements of lockdown.
Such an alternative is not representable with "select", so use
"depends on" instead.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 security/lockdown/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
index e84ddf48401010bcc0829a32db58e6f12bfdedcb..155959205b8eac2c85897a8c4c8b7ec471156706 100644
--- a/security/lockdown/Kconfig
+++ b/security/lockdown/Kconfig
@@ -1,7 +1,7 @@
 config SECURITY_LOCKDOWN_LSM
 	bool "Basic module for enforcing kernel lockdown"
 	depends on SECURITY
-	select MODULE_SIG if MODULES
+	depends on !MODULES || MODULE_SIG
 	help
 	  Build support for an LSM that enforces a coarse kernel lockdown
 	  behaviour.

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v2 6/6] module: Introduce hash-based integrity checking
  2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
                   ` (4 preceding siblings ...)
  2025-01-20 17:44 ` [PATCH v2 5/6] lockdown: Make the relationship to MODULE_SIG a dependency Thomas Weißschuh
@ 2025-01-20 17:44 ` Thomas Weißschuh
  2025-01-22 23:28   ` kpcyrd
  2025-02-03 14:22   ` Petr Pavlu
  2025-01-21 10:26 ` [PATCH v2 0/6] " Roberto Sassu
                   ` (2 subsequent siblings)
  8 siblings, 2 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-01-20 17:44 UTC (permalink / raw)
  To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, Thomas Weißschuh

The current signature-based module integrity checking has some drawbacks
in combination with reproducible builds:
Either the module signing key is generated at build time, which makes
the build unreproducible, or a static key is used, which precludes
rebuilds by third parties and makes the whole build and packaging
process much more complicated.
Introduce a new mechanism to ensure only well-known modules are loaded
by embedding a list of hashes of all modules built as part of the full
kernel build into vmlinux.

Non-builtin modules can be validated as before through signatures.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 .gitignore                                   |  1 +
 Documentation/kbuild/reproducible-builds.rst |  5 ++-
 Makefile                                     |  8 ++++-
 include/asm-generic/vmlinux.lds.h            | 11 ++++++
 include/linux/module_hashes.h                | 17 +++++++++
 kernel/module/Kconfig                        | 17 ++++++++-
 kernel/module/Makefile                       |  1 +
 kernel/module/hashes.c                       | 52 ++++++++++++++++++++++++++++
 kernel/module/internal.h                     |  1 +
 kernel/module/main.c                         |  6 ++++
 scripts/Makefile.modfinal                    |  6 ++++
 scripts/Makefile.vmlinux                     |  5 +++
 scripts/link-vmlinux.sh                      | 25 ++++++++++++-
 scripts/module-hashes.sh                     | 26 ++++++++++++++
 security/lockdown/Kconfig                    |  2 +-
 15 files changed, 178 insertions(+), 5 deletions(-)

diff --git a/.gitignore b/.gitignore
index 6839cf84acda0d2d3c236a2e42b0cb0fe1b14965..7c40151c3f5d0c15ac04cead5f21c291a98d779f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -28,6 +28,7 @@
 *.gz
 *.i
 *.ko
+*.ko.hash
 *.lex.c
 *.ll
 *.lst
diff --git a/Documentation/kbuild/reproducible-builds.rst b/Documentation/kbuild/reproducible-builds.rst
index f2dcc39044e66ddd165646e0b51ccb0209aca7dd..6a742ad745113a9267223b33810dbc7218c47d4c 100644
--- a/Documentation/kbuild/reproducible-builds.rst
+++ b/Documentation/kbuild/reproducible-builds.rst
@@ -79,7 +79,10 @@ generate a different temporary key for each build, resulting in the
 modules being unreproducible.  However, including a signing key with
 your source would presumably defeat the purpose of signing modules.
 
-One approach to this is to divide up the build process so that the
+Instead ``CONFIG_MODULE_HASHES`` can be used to embed a static list
+of valid modules to load.
+
+Another approach to this is to divide up the build process so that the
 unreproducible parts can be treated as sources:
 
 1. Generate a persistent signing key.  Add the certificate for the key
diff --git a/Makefile b/Makefile
index b9464c88ac7230518a756bff5e6c5c8871cc5058..fc862ffd2df843c0b68bebc8f554b88850ba1541 100644
--- a/Makefile
+++ b/Makefile
@@ -1535,8 +1535,10 @@ endif
 # is an exception.
 ifdef CONFIG_DEBUG_INFO_BTF_MODULES
 KBUILD_BUILTIN := 1
+ifndef CONFIG_MODULE_HASHES
 modules: vmlinux
 endif
+endif
 
 modules: modules_prepare
 
@@ -1916,7 +1918,11 @@ modules.order: $(build-dir)
 # KBUILD_MODPOST_NOFINAL can be set to skip the final link of modules.
 # This is solely useful to speed up test compiles.
 modules: modpost
-ifneq ($(KBUILD_MODPOST_NOFINAL),1)
+ifdef CONFIG_MODULE_HASHES
+ifeq ($(MODULE_HASHES_MODPOST_FINAL), 1)
+	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modfinal
+endif
+else ifneq ($(KBUILD_MODPOST_NOFINAL),1)
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modfinal
 endif
 
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 54504013c74915c2ed923fb3afde024a69cdae6b..aebea528aac3d7209bcee12c25f750ab0f7576a5 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -486,6 +486,8 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
 									\
 	PRINTK_INDEX							\
 									\
+	MODULE_HASHES							\
+									\
 	/* Kernel symbol table: Normal symbols */			\
 	__ksymtab         : AT(ADDR(__ksymtab) - LOAD_OFFSET) {		\
 		__start___ksymtab = .;					\
@@ -895,6 +897,15 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
 #define PRINTK_INDEX
 #endif
 
+#ifdef CONFIG_MODULE_HASHES
+#define MODULE_HASHES							\
+	.module_hashes : AT(ADDR(.module_hashes) - LOAD_OFFSET) {	\
+	BOUNDED_SECTION_BY(.module_hashes, _module_hashes)		\
+	}
+#else
+#define MODULE_HASHES
+#endif
+
 /*
  * Discard .note.GNU-stack, which is emitted as PROGBITS by the compiler.
  * Otherwise, the type of .notes section would become PROGBITS instead of NOTES.
diff --git a/include/linux/module_hashes.h b/include/linux/module_hashes.h
new file mode 100644
index 0000000000000000000000000000000000000000..5f2f0546e3875e6bc73bdd53aebaada7371b7f79
--- /dev/null
+++ b/include/linux/module_hashes.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef _LINUX_MODULE_HASHES_H
+#define _LINUX_MODULE_HASHES_H
+
+#include <linux/compiler_attributes.h>
+#include <linux/types.h>
+#include <crypto/sha2.h>
+
+#define __module_hashes_section __section(".module_hashes")
+#define MODULE_HASHES_HASH_SIZE SHA256_DIGEST_SIZE
+
+extern const u8 module_hashes[][MODULE_HASHES_HASH_SIZE];
+
+extern const typeof(module_hashes[0]) __start_module_hashes, __stop_module_hashes;
+
+#endif /* _LINUX_MODULE_HASHES_H */
diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index a80de8d22efdd0f13b3eb579a8ff1e69029d0694..cdd30b9a08d8cdf3ec0595b5e414265b869d343e 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -212,7 +212,7 @@ config MODULE_SIG
 
 config MODULE_SIG_POLICY
 	def_bool y
-	depends on MODULE_SIG
+	depends on MODULE_SIG || MODULE_HASHES
 
 config MODULE_SIG_FORCE
 	bool "Require modules to be validly signed"
@@ -348,6 +348,21 @@ config MODULE_DECOMPRESS
 
 	  If unsure, say N.
 
+config MODULE_HASHES
+	bool "Module hash validation"
+	depends on $(success,cksum --algorithm sha256 --raw /dev/null)
+	select CRYPTO_LIB_SHA256
+	help
+	  Validate modules by their hashes.
+	  Only modules built together with the main kernel image can be
+	  validated that way.
+
+	  This is a reproducible-build compatible alternative to a build-time
+	  generated module keyring, as enabled by
+	  CONFIG_MODULE_SIG_KEY=certs/signing_key.pem.
+
+	  Also see the warning in MODULE_SIG about stripping modules.
+
 config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
 	bool "Allow loading of modules with missing namespace imports"
 	help
diff --git a/kernel/module/Makefile b/kernel/module/Makefile
index 50ffcc413b54504db946af4dce3b41dc4aece1a5..6fe0c14ca5a05b49c1161fcfa8aaa130f89b70e1 100644
--- a/kernel/module/Makefile
+++ b/kernel/module/Makefile
@@ -23,3 +23,4 @@ obj-$(CONFIG_KGDB_KDB) += kdb.o
 obj-$(CONFIG_MODVERSIONS) += version.o
 obj-$(CONFIG_MODULE_UNLOAD_TAINT_TRACKING) += tracking.o
 obj-$(CONFIG_MODULE_STATS) += stats.o
+obj-$(CONFIG_MODULE_HASHES) += hashes.o
diff --git a/kernel/module/hashes.c b/kernel/module/hashes.c
new file mode 100644
index 0000000000000000000000000000000000000000..1aa49767a39b4e0c495b17d3f2edcb5a6ceb839e
--- /dev/null
+++ b/kernel/module/hashes.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#define pr_fmt(fmt) "module/hash: " fmt
+
+#include <linux/int_log.h>
+#include <linux/module_hashes.h>
+#include <linux/module.h>
+#include <crypto/sha2.h>
+#include "internal.h"
+
+static inline size_t module_hashes_count(void)
+{
+	return (__stop_module_hashes - __start_module_hashes) / MODULE_HASHES_HASH_SIZE;
+}
+
+static __init __maybe_unused int module_hashes_init(void)
+{
+	size_t num_hashes = module_hashes_count();
+	int num_width = (intlog10(num_hashes) >> 24) + 1;
+	size_t i;
+
+	pr_debug("Known hashes (%zu):\n", num_hashes);
+
+	for (i = 0; i < num_hashes; i++)
+		pr_debug("%*zu %*phN\n", num_width, i,
+			 (int)sizeof(module_hashes[i]), module_hashes[i]);
+
+	return 0;
+}
+
+#if IS_ENABLED(CONFIG_MODULE_DEBUG)
+early_initcall(module_hashes_init);
+#endif
+
+int module_hash_check(struct load_info *info, int flags)
+{
+	u8 digest[MODULE_HASHES_HASH_SIZE];
+	size_t i;
+
+	sha256((const u8 *)info->hdr, info->len, digest);
+
+	for (i = 0; i < module_hashes_count(); i++) {
+		if (memcmp(module_hashes[i], digest, MODULE_HASHES_HASH_SIZE) == 0) {
+			pr_debug("allow %*phN\n", (int)sizeof(digest), digest);
+			info->sig_ok = true;
+			return 0;
+		}
+	}
+
+	pr_debug("block %*phN\n", (int)sizeof(digest), digest);
+	return -ENOKEY;
+}
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index c30abeefa60b884c4a69b1eb4f1123a4bbee4b47..9c927c212f862fff7000f1cfac3c7e391a2390ac 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -334,6 +334,7 @@ int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
 				char *secstrings, struct module *mod);
 
 int module_sig_check(struct load_info *info, int flags);
+int module_hash_check(struct load_info *info, int flags);
 
 #ifdef CONFIG_DEBUG_KMEMLEAK
 void kmemleak_load_module(const struct module *mod, const struct load_info *info);
diff --git a/kernel/module/main.c b/kernel/module/main.c
index effe1db02973d4f60ff6cbc0d3b5241a3576fa3e..094ace81d795711b56d12a2abc75ea35449c8300 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3218,6 +3218,12 @@ static int module_integrity_check(struct load_info *info, int flags)
 {
 	int err = 0;
 
+	if (IS_ENABLED(CONFIG_MODULE_HASHES)) {
+		err = module_hash_check(info, flags);
+		if (!err)
+			return 0;
+	}
+
 	if (IS_ENABLED(CONFIG_MODULE_SIG))
 		err = module_sig_check(info, flags);
 
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 5d01b553ec9a4565c8e5a6edd05665c409003bc1..080b4fc3a9ba5036b45f04a7e79f2fc02364f93a 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -43,6 +43,9 @@ quiet_cmd_btf_ko = BTF [M] $@
 		$(RESOLVE_BTFIDS) -b $(objtree)/vmlinux $@;		\
 	fi;
 
+quiet_cmd_cksum_ko =
+      cmd_cksum_ko = cksum --algorithm sha256 --raw $@ > $@.hash
+
 # Same as newer-prereqs, but allows to exclude specified extra dependencies
 newer_prereqs_except = $(filter-out $(PHONY) $(1),$?)
 
@@ -57,6 +60,9 @@ if_changed_except = $(if $(call newer_prereqs_except,$(2))$(cmd-check),      \
 ifdef CONFIG_DEBUG_INFO_BTF_MODULES
 	+$(if $(newer-prereqs),$(call cmd,btf_ko))
 endif
+ifdef CONFIG_MODULE_HASHES
+	$(call cmd,cksum_ko)
+endif
 
 targets += $(modules:%.o=%.ko) $(modules:%.o=%.mod.o) .module-common.o
 
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index 873caaa553134e09d034e0c4e0ac7f07c9e3f31b..4b6ba03cdd5e4faad30a0b533407955c542c7a20 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -79,6 +79,11 @@ ifdef CONFIG_DEBUG_INFO_BTF
 vmlinux: $(RESOLVE_BTFIDS)
 endif
 
+ifdef CONFIG_MODULE_HASHES
+vmlinux: $(srctree)/scripts/module-hashes.sh
+vmlinux: modules.order
+endif
+
 # module.builtin.ranges
 # ---------------------------------------------------------------------------
 ifdef CONFIG_BUILTIN_MODULE_RANGES
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 803c8d6f35a7f29fb68b29afa8546f4dde0bd4cb..db072e4e5d6581453a009a9e837042ba28a138ce 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -104,7 +104,7 @@ vmlinux_link()
 	${ld} ${ldflags} -o ${output}					\
 		${wl}--whole-archive ${objs} ${wl}--no-whole-archive	\
 		${wl}--start-group ${libs} ${wl}--end-group		\
-		${kallsymso} ${btf_vmlinux_bin_o} ${arch_vmlinux_o} ${ldlibs}
+		${kallsymso} ${btf_vmlinux_bin_o} ${module_hashes_o} ${arch_vmlinux_o} ${ldlibs}
 }
 
 # generate .BTF typeinfo from DWARF debuginfo
@@ -215,6 +215,7 @@ fi
 
 btf_vmlinux_bin_o=
 kallsymso=
+module_hashes_o=
 strip_debug=
 
 if is_enabled CONFIG_KALLSYMS; then
@@ -222,6 +223,17 @@ if is_enabled CONFIG_KALLSYMS; then
 	kallsyms .tmp_vmlinux0.syms .tmp_vmlinux0.kallsyms
 fi
 
+if is_enabled CONFIG_MODULE_HASHES; then
+	# At this point the hashes are still wrong.
+	# This step reserves the exact amount of space for the objcopy step
+	# after BTF generation.
+	${srctree}/scripts/module-hashes.sh prealloc > .tmp_module_hashes.c
+	module_hashes_o=.tmp_module_hashes.o
+	info CC ${module_hashes_o}
+	${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} ${KBUILD_CFLAGS} \
+		${KBUILD_CFLAGS_KERNEL} -c -o "${module_hashes_o}" ".tmp_module_hashes.c"
+fi
+
 if is_enabled CONFIG_KALLSYMS || is_enabled CONFIG_DEBUG_INFO_BTF; then
 
 	# The kallsyms linking does not need debug symbols, but the BTF does.
@@ -302,6 +314,17 @@ if is_enabled CONFIG_BUILDTIME_TABLE_SORT; then
 	fi
 fi
 
+if is_enabled CONFIG_MODULE_HASHES; then
+	info MAKE modules
+	${MAKE} -f Makefile MODULE_HASHES_MODPOST_FINAL=1 modules
+	${srctree}/scripts/module-hashes.sh > .tmp_module_hashes.c
+	info CC ${module_hashes_o}
+	${CC} ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS} ${KBUILD_CFLAGS} \
+		${KBUILD_CFLAGS_KERNEL} -c -o "${module_hashes_o}" ".tmp_module_hashes.c"
+	${OBJCOPY} --dump-section .module_hashes=.tmp_module_hashes.bin ${module_hashes_o}
+	${OBJCOPY} --update-section .module_hashes=.tmp_module_hashes.bin vmlinux
+fi
+
 # step a (see comment above)
 if is_enabled CONFIG_KALLSYMS; then
 	if ! cmp -s System.map "${kallsyms_sysmap}"; then
diff --git a/scripts/module-hashes.sh b/scripts/module-hashes.sh
new file mode 100755
index 0000000000000000000000000000000000000000..120ce924105c51cdd7a704cbec7e5fa356f9ce1a
--- /dev/null
+++ b/scripts/module-hashes.sh
@@ -0,0 +1,26 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+set -e
+set -u
+set -o pipefail
+
+prealloc="${1:-}"
+
+echo "#include <linux/module_hashes.h>"
+echo
+echo "const u8 module_hashes[][MODULE_HASHES_HASH_SIZE] __module_hashes_section = {"
+
+for mod in $(< modules.order); do
+	mod="${mod/%.o/.ko}"
+	if [ "$prealloc" = "prealloc" ]; then
+		modhash=""
+	else
+		modhash="$(cat "$mod".hash | hexdump -v -e '"0x" 1/1 "%02x, "')"
+	fi
+	echo -e "\t/* $mod */"
+	echo -e "\t{ $modhash},"
+	echo
+done
+
+echo "};"
diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
index 155959205b8eac2c85897a8c4c8b7ec471156706..60b240e3ef1f9609e3f3241befc0bbc7e4a3db74 100644
--- a/security/lockdown/Kconfig
+++ b/security/lockdown/Kconfig
@@ -1,7 +1,7 @@
 config SECURITY_LOCKDOWN_LSM
 	bool "Basic module for enforcing kernel lockdown"
 	depends on SECURITY
-	depends on !MODULES || MODULE_SIG
+	depends on !MODULES || MODULE_SIG || MODULE_HASHES
 	help
 	  Build support for an LSM that enforces a coarse kernel lockdown
 	  behaviour.

-- 
2.48.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/6] module: Introduce hash-based integrity checking
  2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
                   ` (5 preceding siblings ...)
  2025-01-20 17:44 ` [PATCH v2 6/6] module: Introduce hash-based integrity checking Thomas Weißschuh
@ 2025-01-21 10:26 ` Roberto Sassu
  2025-01-21 12:58   ` Thomas Weißschuh
  2025-01-25 21:16 ` Câju Mihai-Drosi
  2025-02-03 13:14 ` Christian Heusel
  8 siblings, 1 reply; 17+ messages in thread
From: Roberto Sassu @ 2025-01-21 10:26 UTC (permalink / raw)
  To: Thomas Weißschuh, Masahiro Yamada, Nathan Chancellor,
	Nicolas Schier, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
	Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, linux-integrity, zohar

On Mon, 2025-01-20 at 18:44 +0100, Thomas Weißschuh wrote:
> The current signature-based module integrity checking has some drawbacks
> in combination with reproducible builds:
> Either the module signing key is generated at build time, which makes
> the build unreproducible, or a static key is used, which precludes
> rebuilds by third parties and makes the whole build and packaging
> process much more complicated.
> Introduce a new mechanism to ensure only well-known modules are loaded
> by embedding a list of hashes of all modules built as part of the full
> kernel build into vmlinux.
> 
> Interest has been proclaimed by NixOS, Arch Linux, Proxmox, SUSE and the
> general reproducible builds community.
> 
> To properly test the reproducibility in combination with CONFIG_INFO_BTF
> another patch is needed:
> "[PATCH bpf-next] kbuild, bpf: Enable reproducible BTF generation" [0]
> (If you happen to test that one, please give some feedback)
> 
> Questions for current patch:
> * Naming
> * Can the number of built-in modules be retrieved while building
>   kernel/module/hashes.o? This would remove the need for the
>   preallocation step in link-vmlinux.sh.
> 
> Further improvements:
> * Use a LSM/IMA/Keyring to store and validate hashes

+ linux-integrity, Mimi

Hi Thomas

I developed something related to it, it is called Integrity Digest
Cache [1].

It has the ability to store in the kernel memory a cache of digests
extracted from a file (or if desired in the future, from a reserved
area in the kernel image).

It exposes an API to query a digest (get/lookup/put) from a digest
cache and to verify whether or not the integrity of the file digests
were extracted from was verified by IMA or another LSM
(verif_set/verif_get). 

Roberto


[1]: https://lore.kernel.org/linux-integrity/20241119104922.2772571-1-roberto.sassu@huaweicloud.com/

> * Use MODULE_SIG_HASH for configuration
> * UAPI for discovery?
> 
> [0] https://lore.kernel.org/lkml/20241211-pahole-reproducible-v1-1-22feae19bad9@weissschuh.net/
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Changes in v2:
> - Drop RFC state
> - Mention interested parties in cover letter
> - Expand Kconfig description
> - Add compatibility with CONFIG_MODULE_SIG
> - Parallelize module-hashes.sh
> - Update Documentation/kbuild/reproducible-builds.rst
> - Link to v1: https://lore.kernel.org/r/20241225-module-hashes-v1-0-d710ce7a3fd1@weissschuh.net
> 
> ---
> Thomas Weißschuh (6):
>       kbuild: add stamp file for vmlinux BTF data
>       module: Make module loading policy usable without MODULE_SIG
>       module: Move integrity checks into dedicated function
>       module: Move lockdown check into generic module loader
>       lockdown: Make the relationship to MODULE_SIG a dependency
>       module: Introduce hash-based integrity checking
> 
>  .gitignore                                   |  1 +
>  Documentation/kbuild/reproducible-builds.rst |  5 ++-
>  Makefile                                     |  8 ++++-
>  include/asm-generic/vmlinux.lds.h            | 11 ++++++
>  include/linux/module.h                       |  8 ++---
>  include/linux/module_hashes.h                | 17 +++++++++
>  kernel/module/Kconfig                        | 21 ++++++++++-
>  kernel/module/Makefile                       |  1 +
>  kernel/module/hashes.c                       | 52 +++++++++++++++++++++++++++
>  kernel/module/internal.h                     |  8 +----
>  kernel/module/main.c                         | 54 +++++++++++++++++++++++++---
>  kernel/module/signing.c                      | 24 +------------
>  scripts/Makefile.modfinal                    | 10 ++++--
>  scripts/Makefile.vmlinux                     |  5 +++
>  scripts/link-vmlinux.sh                      | 31 +++++++++++++++-
>  scripts/module-hashes.sh                     | 26 ++++++++++++++
>  security/lockdown/Kconfig                    |  2 +-
>  17 files changed, 238 insertions(+), 46 deletions(-)
> ---
> base-commit: 2cd5917560a84d69dd6128b640d7a68406ff019b
> change-id: 20241225-module-hashes-7a50a7cc2a30
> 
> Best regards,


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/6] module: Introduce hash-based integrity checking
  2025-01-21 10:26 ` [PATCH v2 0/6] " Roberto Sassu
@ 2025-01-21 12:58   ` Thomas Weißschuh
  2025-01-21 13:11     ` Roberto Sassu
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Weißschuh @ 2025-01-21 12:58 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet,
	Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, linux-integrity, zohar

Hi Roberto,

On 2025-01-21 11:26:29+0100, Roberto Sassu wrote:
> On Mon, 2025-01-20 at 18:44 +0100, Thomas Weißschuh wrote:
> > The current signature-based module integrity checking has some drawbacks
> > in combination with reproducible builds:
> > Either the module signing key is generated at build time, which makes
> > the build unreproducible, or a static key is used, which precludes
> > rebuilds by third parties and makes the whole build and packaging
> > process much more complicated.
> > Introduce a new mechanism to ensure only well-known modules are loaded
> > by embedding a list of hashes of all modules built as part of the full
> > kernel build into vmlinux.
> > 
> > Interest has been proclaimed by NixOS, Arch Linux, Proxmox, SUSE and the
> > general reproducible builds community.
> > 
> > To properly test the reproducibility in combination with CONFIG_INFO_BTF
> > another patch is needed:
> > "[PATCH bpf-next] kbuild, bpf: Enable reproducible BTF generation" [0]
> > (If you happen to test that one, please give some feedback)
> > 
> > Questions for current patch:
> > * Naming
> > * Can the number of built-in modules be retrieved while building
> >   kernel/module/hashes.o? This would remove the need for the
> >   preallocation step in link-vmlinux.sh.
> > 
> > Further improvements:
> > * Use a LSM/IMA/Keyring to store and validate hashes
> 
> + linux-integrity, Mimi
> 
> Hi Thomas
> 
> I developed something related to it, it is called Integrity Digest
> Cache [1].

Thanks for the pointer.

> It has the ability to store in the kernel memory a cache of digests
> extracted from a file (or if desired in the future, from a reserved
> area in the kernel image).
> 
> It exposes an API to query a digest (get/lookup/put) from a digest
> cache and to verify whether or not the integrity of the file digests
> were extracted from was verified by IMA or another LSM
> (verif_set/verif_get). 

I see how this could be used together with the module hashes.
For now I think both features should be developed independently.
Integrating them will require some extra code and coordination.

While the current linear, unsorted list of hashes used by my code may be
slightly inefficient, it shouldn't matter in practize as the hash
validation is only a bunch of memcmp()s over a contiguous chunk of
memory, which is very cheap.

When both features are well established we can look at integrating them.
At least a build-time userspace generator of a digest cache would be
necessary.
And due to the current implementation details it would be necessary to
estimate the size of a static digest cache more or less exactly by its
number of elements alone.


Thomas

> [1]: https://lore.kernel.org/linux-integrity/20241119104922.2772571-1-roberto.sassu@huaweicloud.com/
> 
> > * Use MODULE_SIG_HASH for configuration
> > * UAPI for discovery?
> > 
> > [0] https://lore.kernel.org/lkml/20241211-pahole-reproducible-v1-1-22feae19bad9@weissschuh.net/
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> > Changes in v2:
> > - Drop RFC state
> > - Mention interested parties in cover letter
> > - Expand Kconfig description
> > - Add compatibility with CONFIG_MODULE_SIG
> > - Parallelize module-hashes.sh
> > - Update Documentation/kbuild/reproducible-builds.rst
> > - Link to v1: https://lore.kernel.org/r/20241225-module-hashes-v1-0-d710ce7a3fd1@weissschuh.net
> > 
> > ---
> > Thomas Weißschuh (6):
> >       kbuild: add stamp file for vmlinux BTF data
> >       module: Make module loading policy usable without MODULE_SIG
> >       module: Move integrity checks into dedicated function
> >       module: Move lockdown check into generic module loader
> >       lockdown: Make the relationship to MODULE_SIG a dependency
> >       module: Introduce hash-based integrity checking
> > 
> >  .gitignore                                   |  1 +
> >  Documentation/kbuild/reproducible-builds.rst |  5 ++-
> >  Makefile                                     |  8 ++++-
> >  include/asm-generic/vmlinux.lds.h            | 11 ++++++
> >  include/linux/module.h                       |  8 ++---
> >  include/linux/module_hashes.h                | 17 +++++++++
> >  kernel/module/Kconfig                        | 21 ++++++++++-
> >  kernel/module/Makefile                       |  1 +
> >  kernel/module/hashes.c                       | 52 +++++++++++++++++++++++++++
> >  kernel/module/internal.h                     |  8 +----
> >  kernel/module/main.c                         | 54 +++++++++++++++++++++++++---
> >  kernel/module/signing.c                      | 24 +------------
> >  scripts/Makefile.modfinal                    | 10 ++++--
> >  scripts/Makefile.vmlinux                     |  5 +++
> >  scripts/link-vmlinux.sh                      | 31 +++++++++++++++-
> >  scripts/module-hashes.sh                     | 26 ++++++++++++++
> >  security/lockdown/Kconfig                    |  2 +-
> >  17 files changed, 238 insertions(+), 46 deletions(-)
> > ---
> > base-commit: 2cd5917560a84d69dd6128b640d7a68406ff019b
> > change-id: 20241225-module-hashes-7a50a7cc2a30
> > 
> > Best regards,
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/6] module: Introduce hash-based integrity checking
  2025-01-21 12:58   ` Thomas Weißschuh
@ 2025-01-21 13:11     ` Roberto Sassu
  0 siblings, 0 replies; 17+ messages in thread
From: Roberto Sassu @ 2025-01-21 13:11 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet,
	Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, linux-integrity, zohar

On Tue, 2025-01-21 at 13:58 +0100, Thomas Weißschuh wrote:
> Hi Roberto,
> 
> On 2025-01-21 11:26:29+0100, Roberto Sassu wrote:
> > On Mon, 2025-01-20 at 18:44 +0100, Thomas Weißschuh wrote:
> > > The current signature-based module integrity checking has some drawbacks
> > > in combination with reproducible builds:
> > > Either the module signing key is generated at build time, which makes
> > > the build unreproducible, or a static key is used, which precludes
> > > rebuilds by third parties and makes the whole build and packaging
> > > process much more complicated.
> > > Introduce a new mechanism to ensure only well-known modules are loaded
> > > by embedding a list of hashes of all modules built as part of the full
> > > kernel build into vmlinux.
> > > 
> > > Interest has been proclaimed by NixOS, Arch Linux, Proxmox, SUSE and the
> > > general reproducible builds community.
> > > 
> > > To properly test the reproducibility in combination with CONFIG_INFO_BTF
> > > another patch is needed:
> > > "[PATCH bpf-next] kbuild, bpf: Enable reproducible BTF generation" [0]
> > > (If you happen to test that one, please give some feedback)
> > > 
> > > Questions for current patch:
> > > * Naming
> > > * Can the number of built-in modules be retrieved while building
> > >   kernel/module/hashes.o? This would remove the need for the
> > >   preallocation step in link-vmlinux.sh.
> > > 
> > > Further improvements:
> > > * Use a LSM/IMA/Keyring to store and validate hashes
> > 
> > + linux-integrity, Mimi
> > 
> > Hi Thomas
> > 
> > I developed something related to it, it is called Integrity Digest
> > Cache [1].
> 
> Thanks for the pointer.
> 
> > It has the ability to store in the kernel memory a cache of digests
> > extracted from a file (or if desired in the future, from a reserved
> > area in the kernel image).
> > 
> > It exposes an API to query a digest (get/lookup/put) from a digest
> > cache and to verify whether or not the integrity of the file digests
> > were extracted from was verified by IMA or another LSM
> > (verif_set/verif_get). 
> 
> I see how this could be used together with the module hashes.
> For now I think both features should be developed independently.
> Integrating them will require some extra code and coordination.

Yes, I agree.

> While the current linear, unsorted list of hashes used by my code may be
> slightly inefficient, it shouldn't matter in practize as the hash
> validation is only a bunch of memcmp()s over a contiguous chunk of
> memory, which is very cheap.

Ok, I guess so, should not be too slow for this use case.

> When both features are well established we can look at integrating them.
> At least a build-time userspace generator of a digest cache would be
> necessary.
> And due to the current implementation details it would be necessary to
> estimate the size of a static digest cache more or less exactly by its
> number of elements alone.

This information is included in the digest list, since it is also used
by the Integrity Digest Cache itself to determine the correct size of
the hash table.

Thanks

Roberto

> Thomas
> 
> > [1]: https://lore.kernel.org/linux-integrity/20241119104922.2772571-1-roberto.sassu@huaweicloud.com/
> > 
> > > * Use MODULE_SIG_HASH for configuration
> > > * UAPI for discovery?
> > > 
> > > [0] https://lore.kernel.org/lkml/20241211-pahole-reproducible-v1-1-22feae19bad9@weissschuh.net/
> > > 
> > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > > ---
> > > Changes in v2:
> > > - Drop RFC state
> > > - Mention interested parties in cover letter
> > > - Expand Kconfig description
> > > - Add compatibility with CONFIG_MODULE_SIG
> > > - Parallelize module-hashes.sh
> > > - Update Documentation/kbuild/reproducible-builds.rst
> > > - Link to v1: https://lore.kernel.org/r/20241225-module-hashes-v1-0-d710ce7a3fd1@weissschuh.net
> > > 
> > > ---
> > > Thomas Weißschuh (6):
> > >       kbuild: add stamp file for vmlinux BTF data
> > >       module: Make module loading policy usable without MODULE_SIG
> > >       module: Move integrity checks into dedicated function
> > >       module: Move lockdown check into generic module loader
> > >       lockdown: Make the relationship to MODULE_SIG a dependency
> > >       module: Introduce hash-based integrity checking
> > > 
> > >  .gitignore                                   |  1 +
> > >  Documentation/kbuild/reproducible-builds.rst |  5 ++-
> > >  Makefile                                     |  8 ++++-
> > >  include/asm-generic/vmlinux.lds.h            | 11 ++++++
> > >  include/linux/module.h                       |  8 ++---
> > >  include/linux/module_hashes.h                | 17 +++++++++
> > >  kernel/module/Kconfig                        | 21 ++++++++++-
> > >  kernel/module/Makefile                       |  1 +
> > >  kernel/module/hashes.c                       | 52 +++++++++++++++++++++++++++
> > >  kernel/module/internal.h                     |  8 +----
> > >  kernel/module/main.c                         | 54 +++++++++++++++++++++++++---
> > >  kernel/module/signing.c                      | 24 +------------
> > >  scripts/Makefile.modfinal                    | 10 ++++--
> > >  scripts/Makefile.vmlinux                     |  5 +++
> > >  scripts/link-vmlinux.sh                      | 31 +++++++++++++++-
> > >  scripts/module-hashes.sh                     | 26 ++++++++++++++
> > >  security/lockdown/Kconfig                    |  2 +-
> > >  17 files changed, 238 insertions(+), 46 deletions(-)
> > > ---
> > > base-commit: 2cd5917560a84d69dd6128b640d7a68406ff019b
> > > change-id: 20241225-module-hashes-7a50a7cc2a30
> > > 
> > > Best regards,
> > 


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 6/6] module: Introduce hash-based integrity checking
  2025-01-20 17:44 ` [PATCH v2 6/6] module: Introduce hash-based integrity checking Thomas Weißschuh
@ 2025-01-22 23:28   ` kpcyrd
  2025-03-06  8:10     ` Thomas Weißschuh
  2025-02-03 14:22   ` Petr Pavlu
  1 sibling, 1 reply; 17+ messages in thread
From: kpcyrd @ 2025-01-22 23:28 UTC (permalink / raw)
  To: Thomas Weißschuh, Masahiro Yamada, Nathan Chancellor,
	Nicolas Schier, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
	Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc

Hi!

Thanks for reaching out, also your work on this is much appreciated and 
followed with great interest. <3

On 1/20/25 6:44 PM, Thomas Weißschuh wrote:
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index effe1db02973d4f60ff6cbc0d3b5241a3576fa3e..094ace81d795711b56d12a2abc75ea35449c8300 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3218,6 +3218,12 @@ static int module_integrity_check(struct load_info *info, int flags)
>   {
>   	int err = 0;
>   
> +	if (IS_ENABLED(CONFIG_MODULE_HASHES)) {
> +		err = module_hash_check(info, flags);
> +		if (!err)
> +			return 0;
> +	}
> +
>   	if (IS_ENABLED(CONFIG_MODULE_SIG))
>   		err = module_sig_check(info, flags);
>   

 From how I'm reading this (please let me know if I'm wrong):

## !CONFIG_MODULE_HASHES && !CONFIG_MODULE_SIG

No special checks, CAP_SYS_MODULE only.

## !CONFIG_MODULE_HASHES && CONFIG_MODULE_SIG

No change from how things work today:

- if the module signature verifies the module is permitted
- else, if sig_enforce=1, the module is rejected
- else, if lockdown mode is enabled, the module is rejected
- else, the module is permitted

## CONFIG_MODULE_HASHES && CONFIG_MODULE_SIG

This configuration is the one relevant for Arch Linux:

- if the module is in the set of allowed module_hashes it is permitted
- else, if the module signature verifies, the module is permitted
- else, if sig_enforce=1, the module is rejected
- else, if lockdown mode is enabled, the module is rejected
- else, the module is permitted

## CONFIG_MODULE_HASHES && !CONFIG_MODULE_SIG

This one is new:

- if the module is in the set of allowed module_hashes it is permitted
- else, if lockdown mode is enabled, the module is rejected
- else, the module is permitted

---

This all seems reasonable to me, maybe the check for 
is_module_sig_enforced() could be moved from kernel/module/signing.c to 
kernel/module/main.c, otherwise `sig_enforce=1` would not have any 
effect for a `CONFIG_MODULE_HASHES && !CONFIG_MODULE_SIG` kernel.

cheers,
kpcyrd

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/6] module: Introduce hash-based integrity checking
  2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
                   ` (6 preceding siblings ...)
  2025-01-21 10:26 ` [PATCH v2 0/6] " Roberto Sassu
@ 2025-01-25 21:16 ` Câju Mihai-Drosi
  2025-02-03 13:14 ` Christian Heusel
  8 siblings, 0 replies; 17+ messages in thread
From: Câju Mihai-Drosi @ 2025-01-25 21:16 UTC (permalink / raw)
  To: Thomas Weißschuh, Masahiro Yamada, Nathan Chancellor,
	Nicolas Schier, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
	Serge E. Hallyn, Jonathan Corbet
  Cc: Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc

On 1/20/25 19:44, Thomas Weißschuh wrote:
> The current signature-based module integrity checking has some drawbacks
> in combination with reproducible builds:
> Either the module signing key is generated at build time, which makes
> the build unreproducible, or a static key is used, which precludes
> rebuilds by third parties and makes the whole build and packaging
> process much more complicated.
> Introduce a new mechanism to ensure only well-known modules are loaded
> by embedding a list of hashes of all modules built as part of the full
> kernel build into vmlinux.
> 
> Interest has been proclaimed by NixOS, Arch Linux, Proxmox, SUSE and the
> general reproducible builds community.
> 
> To properly test the reproducibility in combination with CONFIG_INFO_BTF
> another patch is needed:
> "[PATCH bpf-next] kbuild, bpf: Enable reproducible BTF generation" [0]
> (If you happen to test that one, please give some feedback)
> 
> Questions for current patch:
> * Naming
> * Can the number of built-in modules be retrieved while building
>    kernel/module/hashes.o? This would remove the need for the
>    preallocation step in link-vmlinux.sh.
> 
> Further improvements:
> * Use a LSM/IMA/Keyring to store and validate hashes
> * Use MODULE_SIG_HASH for configuration
> * UAPI for discovery?
> 
> [0] https://lore.kernel.org/lkml/20241211-pahole-reproducible-v1-1-22feae19bad9@weissschuh.net/

Hello,

Thank you for your work on helping to enable kernel lockdown coupled 
with reproducible builds.

This may be out scope for this patch series, however I think it is worth 
considering: How does one include hashes of modules that have not been 
built as part of the kernel into the array? For example a DKMS module or 
NVIDIA driver?

A solution that may be worth considering would be to include a list of 
modules hashes into the kernel command-line. It may be even worth 
considering keeping a dynamic array of hashes that can be locked at a 
given point in time?

All the best,
Mihai

> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Changes in v2:
> - Drop RFC state
> - Mention interested parties in cover letter
> - Expand Kconfig description
> - Add compatibility with CONFIG_MODULE_SIG
> - Parallelize module-hashes.sh
> - Update Documentation/kbuild/reproducible-builds.rst
> - Link to v1: https://lore.kernel.org/r/20241225-module-hashes-v1-0-d710ce7a3fd1@weissschuh.net
> 
> ---
> Thomas Weißschuh (6):
>        kbuild: add stamp file for vmlinux BTF data
>        module: Make module loading policy usable without MODULE_SIG
>        module: Move integrity checks into dedicated function
>        module: Move lockdown check into generic module loader
>        lockdown: Make the relationship to MODULE_SIG a dependency
>        module: Introduce hash-based integrity checking
> 
>   .gitignore                                   |  1 +
>   Documentation/kbuild/reproducible-builds.rst |  5 ++-
>   Makefile                                     |  8 ++++-
>   include/asm-generic/vmlinux.lds.h            | 11 ++++++
>   include/linux/module.h                       |  8 ++---
>   include/linux/module_hashes.h                | 17 +++++++++
>   kernel/module/Kconfig                        | 21 ++++++++++-
>   kernel/module/Makefile                       |  1 +
>   kernel/module/hashes.c                       | 52 +++++++++++++++++++++++++++
>   kernel/module/internal.h                     |  8 +----
>   kernel/module/main.c                         | 54 +++++++++++++++++++++++++---
>   kernel/module/signing.c                      | 24 +------------
>   scripts/Makefile.modfinal                    | 10 ++++--
>   scripts/Makefile.vmlinux                     |  5 +++
>   scripts/link-vmlinux.sh                      | 31 +++++++++++++++-
>   scripts/module-hashes.sh                     | 26 ++++++++++++++
>   security/lockdown/Kconfig                    |  2 +-
>   17 files changed, 238 insertions(+), 46 deletions(-)
> ---
> base-commit: 2cd5917560a84d69dd6128b640d7a68406ff019b
> change-id: 20241225-module-hashes-7a50a7cc2a30
> 
> Best regards,


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/6] module: Introduce hash-based integrity checking
  2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
                   ` (7 preceding siblings ...)
  2025-01-25 21:16 ` Câju Mihai-Drosi
@ 2025-02-03 13:14 ` Christian Heusel
  2025-02-04 21:08   ` Thomas Weißschuh
  8 siblings, 1 reply; 17+ messages in thread
From: Christian Heusel @ 2025-02-03 13:14 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet,
	Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, linux-integrity

[-- Attachment #1: Type: text/plain, Size: 1677 bytes --]

Hey Thomas,

On 25/01/20 06:44PM, Thomas Weißschuh wrote:
> Thomas Weißschuh (6):
>       kbuild: add stamp file for vmlinux BTF data
>       module: Make module loading policy usable without MODULE_SIG
>       module: Move integrity checks into dedicated function
>       module: Move lockdown check into generic module loader
>       lockdown: Make the relationship to MODULE_SIG a dependency
>       module: Introduce hash-based integrity checking

thanks for working on this!

I had a look at this patch series together with kpcyrd over the weekend
and we were able to verify that this indeed allows one to get a
reproducible kernel image with the toolchain on Arch Linux (if the patch
you mentioned in your cover letter is also applied), which is of course
great news! :)

We also found a major issues with it, as adding it on top of the v6.13
kernel and setting the needed config options while removing modules
signatures made the kernel unable to load any module while also not
printing any error for the failure, therefore resulting in an early boot
failure on my machine.

Do you have any clue what could be going wrong here or what we could
investigate? I have pushed my build config into [this repository][0] and
also uploaded a prebuilt version (signed with my packager key)
[here][1] (you can therefore just install it via "sudo pacman -U
<link>").

Happy to test more stuff, feel free to CC me on any further revision /
thread on this!

Cheers,
Christian

[0]: https://gitlab.archlinux.org/gromit/linux-mainline-repro-test
[1]: https://pkgbuild.com/~gromit/linux-bisection-kernels/linux-mainline-6.13-1.2-x86_64.pkg.tar.zst

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 6/6] module: Introduce hash-based integrity checking
  2025-01-20 17:44 ` [PATCH v2 6/6] module: Introduce hash-based integrity checking Thomas Weißschuh
  2025-01-22 23:28   ` kpcyrd
@ 2025-02-03 14:22   ` Petr Pavlu
  2025-02-04 21:22     ` Thomas Weißschuh
  1 sibling, 1 reply; 17+ messages in thread
From: Petr Pavlu @ 2025-02-03 14:22 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Sami Tolvanen, Daniel Gomez, Paul Moore,
	James Morris, Serge E. Hallyn, Jonathan Corbet,
	Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc

On 1/20/25 18:44, Thomas Weißschuh wrote:
> The current signature-based module integrity checking has some drawbacks
> in combination with reproducible builds:
> Either the module signing key is generated at build time, which makes
> the build unreproducible, or a static key is used, which precludes
> rebuilds by third parties and makes the whole build and packaging
> process much more complicated.
> Introduce a new mechanism to ensure only well-known modules are loaded
> by embedding a list of hashes of all modules built as part of the full
> kernel build into vmlinux.
> 
> Non-builtin modules can be validated as before through signatures.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  .gitignore                                   |  1 +
>  Documentation/kbuild/reproducible-builds.rst |  5 ++-
>  Makefile                                     |  8 ++++-
>  include/asm-generic/vmlinux.lds.h            | 11 ++++++
>  include/linux/module_hashes.h                | 17 +++++++++
>  kernel/module/Kconfig                        | 17 ++++++++-
>  kernel/module/Makefile                       |  1 +
>  kernel/module/hashes.c                       | 52 ++++++++++++++++++++++++++++
>  kernel/module/internal.h                     |  1 +
>  kernel/module/main.c                         |  6 ++++
>  scripts/Makefile.modfinal                    |  6 ++++
>  scripts/Makefile.vmlinux                     |  5 +++
>  scripts/link-vmlinux.sh                      | 25 ++++++++++++-
>  scripts/module-hashes.sh                     | 26 ++++++++++++++
>  security/lockdown/Kconfig                    |  2 +-
>  15 files changed, 178 insertions(+), 5 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 6839cf84acda0d2d3c236a2e42b0cb0fe1b14965..7c40151c3f5d0c15ac04cead5f21c291a98d779f 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -28,6 +28,7 @@
>  *.gz
>  *.i
>  *.ko
> +*.ko.hash
>  *.lex.c
>  *.ll
>  *.lst
> diff --git a/Documentation/kbuild/reproducible-builds.rst b/Documentation/kbuild/reproducible-builds.rst
> index f2dcc39044e66ddd165646e0b51ccb0209aca7dd..6a742ad745113a9267223b33810dbc7218c47d4c 100644
> --- a/Documentation/kbuild/reproducible-builds.rst
> +++ b/Documentation/kbuild/reproducible-builds.rst
> @@ -79,7 +79,10 @@ generate a different temporary key for each build, resulting in the
>  modules being unreproducible.  However, including a signing key with
>  your source would presumably defeat the purpose of signing modules.
>  
> -One approach to this is to divide up the build process so that the
> +Instead ``CONFIG_MODULE_HASHES`` can be used to embed a static list
> +of valid modules to load.
> +
> +Another approach to this is to divide up the build process so that the
>  unreproducible parts can be treated as sources:
>  
>  1. Generate a persistent signing key.  Add the certificate for the key
> diff --git a/Makefile b/Makefile
> index b9464c88ac7230518a756bff5e6c5c8871cc5058..fc862ffd2df843c0b68bebc8f554b88850ba1541 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1535,8 +1535,10 @@ endif
>  # is an exception.
>  ifdef CONFIG_DEBUG_INFO_BTF_MODULES
>  KBUILD_BUILTIN := 1
> +ifndef CONFIG_MODULE_HASHES
>  modules: vmlinux
>  endif
> +endif
>  
>  modules: modules_prepare
>  

I think that the way the feature is integrated into the build is
currently suboptimal. We should look how to make it properly orthogonal
with the BTF stuff and also try to obtain the number of modules early.
Sorry, I can't immediately provide any advice here as I need to
understand the relevant logic more myself.

> @@ -1916,7 +1918,11 @@ modules.order: $(build-dir)
>  # KBUILD_MODPOST_NOFINAL can be set to skip the final link of modules.
>  # This is solely useful to speed up test compiles.
>  modules: modpost
> -ifneq ($(KBUILD_MODPOST_NOFINAL),1)
> +ifdef CONFIG_MODULE_HASHES
> +ifeq ($(MODULE_HASHES_MODPOST_FINAL), 1)
> +	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modfinal
> +endif
> +else ifneq ($(KBUILD_MODPOST_NOFINAL),1)
>  	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modfinal
>  endif
>  
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 54504013c74915c2ed923fb3afde024a69cdae6b..aebea528aac3d7209bcee12c25f750ab0f7576a5 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -486,6 +486,8 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
>  									\
>  	PRINTK_INDEX							\
>  									\
> +	MODULE_HASHES							\
> +									\
>  	/* Kernel symbol table: Normal symbols */			\
>  	__ksymtab         : AT(ADDR(__ksymtab) - LOAD_OFFSET) {		\
>  		__start___ksymtab = .;					\
> @@ -895,6 +897,15 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
>  #define PRINTK_INDEX
>  #endif
>  
> +#ifdef CONFIG_MODULE_HASHES
> +#define MODULE_HASHES							\
> +	.module_hashes : AT(ADDR(.module_hashes) - LOAD_OFFSET) {	\
> +	BOUNDED_SECTION_BY(.module_hashes, _module_hashes)		\
> +	}
> +#else
> +#define MODULE_HASHES
> +#endif
> +
>  /*
>   * Discard .note.GNU-stack, which is emitted as PROGBITS by the compiler.
>   * Otherwise, the type of .notes section would become PROGBITS instead of NOTES.
> diff --git a/include/linux/module_hashes.h b/include/linux/module_hashes.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..5f2f0546e3875e6bc73bdd53aebaada7371b7f79
> --- /dev/null
> +++ b/include/linux/module_hashes.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +
> +#ifndef _LINUX_MODULE_HASHES_H
> +#define _LINUX_MODULE_HASHES_H
> +
> +#include <linux/compiler_attributes.h>
> +#include <linux/types.h>
> +#include <crypto/sha2.h>
> +
> +#define __module_hashes_section __section(".module_hashes")
> +#define MODULE_HASHES_HASH_SIZE SHA256_DIGEST_SIZE
> +
> +extern const u8 module_hashes[][MODULE_HASHES_HASH_SIZE];
> +
> +extern const typeof(module_hashes[0]) __start_module_hashes, __stop_module_hashes;
> +
> +#endif /* _LINUX_MODULE_HASHES_H */
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index a80de8d22efdd0f13b3eb579a8ff1e69029d0694..cdd30b9a08d8cdf3ec0595b5e414265b869d343e 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -212,7 +212,7 @@ config MODULE_SIG
>  
>  config MODULE_SIG_POLICY
>  	def_bool y
> -	depends on MODULE_SIG
> +	depends on MODULE_SIG || MODULE_HASHES
>  
>  config MODULE_SIG_FORCE
>  	bool "Require modules to be validly signed"
> @@ -348,6 +348,21 @@ config MODULE_DECOMPRESS
>  
>  	  If unsure, say N.
>  
> +config MODULE_HASHES
> +	bool "Module hash validation"
> +	depends on $(success,cksum --algorithm sha256 --raw /dev/null)

The cksum utility from GNU coreutils gained the --algorithm option in
2021 [1] and the --raw option in 2023 [2], which looks quite recent.
The document process/changes.rst doesn't mention a minimum version for
coreutils. However, I'd consider using sha256sum or
'openssl dgst -sha256 -binary' as that should cover more systems.

[1] https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=ad6c8e1181a3966e35d68c1c354deb1c73f3e974
[2] https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=ead07bb3d461389bb52336109be7858458e49c38

> +	select CRYPTO_LIB_SHA256
> +	help
> +	  Validate modules by their hashes.
> +	  Only modules built together with the main kernel image can be
> +	  validated that way.
> +
> +	  This is a reproducible-build compatible alternative to a build-time
> +	  generated module keyring, as enabled by
> +	  CONFIG_MODULE_SIG_KEY=certs/signing_key.pem.
> +
> +	  Also see the warning in MODULE_SIG about stripping modules.
> +
>  config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
>  	bool "Allow loading of modules with missing namespace imports"
>  	help
> diff --git a/kernel/module/Makefile b/kernel/module/Makefile
> index 50ffcc413b54504db946af4dce3b41dc4aece1a5..6fe0c14ca5a05b49c1161fcfa8aaa130f89b70e1 100644
> --- a/kernel/module/Makefile
> +++ b/kernel/module/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_KGDB_KDB) += kdb.o
>  obj-$(CONFIG_MODVERSIONS) += version.o
>  obj-$(CONFIG_MODULE_UNLOAD_TAINT_TRACKING) += tracking.o
>  obj-$(CONFIG_MODULE_STATS) += stats.o
> +obj-$(CONFIG_MODULE_HASHES) += hashes.o
> diff --git a/kernel/module/hashes.c b/kernel/module/hashes.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..1aa49767a39b4e0c495b17d3f2edcb5a6ceb839e
> --- /dev/null
> +++ b/kernel/module/hashes.c
> @@ -0,0 +1,52 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#define pr_fmt(fmt) "module/hash: " fmt
> +
> +#include <linux/int_log.h>
> +#include <linux/module_hashes.h>
> +#include <linux/module.h>
> +#include <crypto/sha2.h>
> +#include "internal.h"
> +
> +static inline size_t module_hashes_count(void)
> +{
> +	return (__stop_module_hashes - __start_module_hashes) / MODULE_HASHES_HASH_SIZE;
> +}
> +
> +static __init __maybe_unused int module_hashes_init(void)
> +{
> +	size_t num_hashes = module_hashes_count();
> +	int num_width = (intlog10(num_hashes) >> 24) + 1;

It is unlikely, but I suppose one could configure the kernel with
CONFIG_MODULE_DEBUG and CONFIG_MODULE_HASHES but without any actual
modules. In this corner case, intlog10() will be called with 0 and
produces a warning.

> +	size_t i;
> +
> +	pr_debug("Known hashes (%zu):\n", num_hashes);
> +
> +	for (i = 0; i < num_hashes; i++)
> +		pr_debug("%*zu %*phN\n", num_width, i,
> +			 (int)sizeof(module_hashes[i]), module_hashes[i]);
> +
> +	return 0;
> +}
> +
> +#if IS_ENABLED(CONFIG_MODULE_DEBUG)
> +early_initcall(module_hashes_init);
> +#endif
> +
> +int module_hash_check(struct load_info *info, int flags)
> +{
> +	u8 digest[MODULE_HASHES_HASH_SIZE];
> +	size_t i;
> +
> +	sha256((const u8 *)info->hdr, info->len, digest);
> +
> +	for (i = 0; i < module_hashes_count(); i++) {
> +		if (memcmp(module_hashes[i], digest, MODULE_HASHES_HASH_SIZE) == 0) {
> +			pr_debug("allow %*phN\n", (int)sizeof(digest), digest);
> +			info->sig_ok = true;
> +			return 0;
> +		}
> +	}
> +
> +	pr_debug("block %*phN\n", (int)sizeof(digest), digest);
> +	return -ENOKEY;
> +}
> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> index c30abeefa60b884c4a69b1eb4f1123a4bbee4b47..9c927c212f862fff7000f1cfac3c7e391a2390ac 100644
> --- a/kernel/module/internal.h
> +++ b/kernel/module/internal.h
> @@ -334,6 +334,7 @@ int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
>  				char *secstrings, struct module *mod);
>  
>  int module_sig_check(struct load_info *info, int flags);
> +int module_hash_check(struct load_info *info, int flags);
>  
>  #ifdef CONFIG_DEBUG_KMEMLEAK
>  void kmemleak_load_module(const struct module *mod, const struct load_info *info);
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index effe1db02973d4f60ff6cbc0d3b5241a3576fa3e..094ace81d795711b56d12a2abc75ea35449c8300 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3218,6 +3218,12 @@ static int module_integrity_check(struct load_info *info, int flags)
>  {
>  	int err = 0;
>  
> +	if (IS_ENABLED(CONFIG_MODULE_HASHES)) {
> +		err = module_hash_check(info, flags);
> +		if (!err)
> +			return 0;
> +	}
> +
>  	if (IS_ENABLED(CONFIG_MODULE_SIG))
>  		err = module_sig_check(info, flags);
>  

Nit: I'd write this function as follows to make the logic shorter and to
express that the code looks for the first handler that successfully
verifies the module by setting info->sig_ok.

static int module_integrity_check(struct load_info *info, int flags)
{
	int err = 0;

	if (IS_ENABLED(CONFIG_MODULE_HASHES))
		err = module_hash_check(info, flags);

	if (!info->sig_ok && IS_ENABLED(CONFIG_MODULE_SIG))
		err = module_sig_check(info, flags);

	if (err)
		return err;
	if (info->sig_ok)
		return 0;
	return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
}

[...]

-- 
Thanks,
Petr

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 0/6] module: Introduce hash-based integrity checking
  2025-02-03 13:14 ` Christian Heusel
@ 2025-02-04 21:08   ` Thomas Weißschuh
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-02-04 21:08 UTC (permalink / raw)
  To: Christian Heusel
  Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet,
	Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc, linux-integrity

On 2025-02-03 14:14:41+0100, Christian Heusel wrote:
> Hey Thomas,
> 
> On 25/01/20 06:44PM, Thomas Weißschuh wrote:
> > Thomas Weißschuh (6):
> >       kbuild: add stamp file for vmlinux BTF data
> >       module: Make module loading policy usable without MODULE_SIG
> >       module: Move integrity checks into dedicated function
> >       module: Move lockdown check into generic module loader
> >       lockdown: Make the relationship to MODULE_SIG a dependency
> >       module: Introduce hash-based integrity checking
> 
> thanks for working on this!
> 
> I had a look at this patch series together with kpcyrd over the weekend
> and we were able to verify that this indeed allows one to get a
> reproducible kernel image with the toolchain on Arch Linux (if the patch
> you mentioned in your cover letter is also applied), which is of course
> great news! :)

Great!
FYI the BTF patch shouldn't be necessary anymore with pahole 1.29.

> We also found a major issues with it, as adding it on top of the v6.13
> kernel and setting the needed config options while removing modules
> signatures made the kernel unable to load any module while also not
> printing any error for the failure, therefore resulting in an early boot
> failure on my machine.
> 
> Do you have any clue what could be going wrong here or what we could
> investigate? I have pushed my build config into [this repository][0] and
> also uploaded a prebuilt version (signed with my packager key)
> [here][1] (you can therefore just install it via "sudo pacman -U
> <link>").

I would guess the issue is the usage of INSTALL_MOD_STRIP.

What are the contents of .tmp_module_hashes.c ?
Do they match the hashes from the build directory and package?
You can also enable CONFIG_MODULE_DEBUG and '#define DEBUG' in
kernel/module/hashes.c

> Happy to test more stuff, feel free to CC me on any further revision /
> thread on this!

Will do!

> Cheers,
> Christian
> 
> [0]: https://gitlab.archlinux.org/gromit/linux-mainline-repro-test
> [1]: https://pkgbuild.com/~gromit/linux-bisection-kernels/linux-mainline-6.13-1.2-x86_64.pkg.tar.zst



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 6/6] module: Introduce hash-based integrity checking
  2025-02-03 14:22   ` Petr Pavlu
@ 2025-02-04 21:22     ` Thomas Weißschuh
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-02-04 21:22 UTC (permalink / raw)
  To: Petr Pavlu
  Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Sami Tolvanen, Daniel Gomez, Paul Moore,
	James Morris, Serge E. Hallyn, Jonathan Corbet,
	Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc

Hi Petr,

On 2025-02-03 15:22:54+0100, Petr Pavlu wrote:
> On 1/20/25 18:44, Thomas Weißschuh wrote:
> > The current signature-based module integrity checking has some drawbacks
> > in combination with reproducible builds:
> > Either the module signing key is generated at build time, which makes
> > the build unreproducible, or a static key is used, which precludes
> > rebuilds by third parties and makes the whole build and packaging
> > process much more complicated.
> > Introduce a new mechanism to ensure only well-known modules are loaded
> > by embedding a list of hashes of all modules built as part of the full
> > kernel build into vmlinux.
> > 
> > Non-builtin modules can be validated as before through signatures.
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> >  .gitignore                                   |  1 +
> >  Documentation/kbuild/reproducible-builds.rst |  5 ++-
> >  Makefile                                     |  8 ++++-
> >  include/asm-generic/vmlinux.lds.h            | 11 ++++++
> >  include/linux/module_hashes.h                | 17 +++++++++
> >  kernel/module/Kconfig                        | 17 ++++++++-
> >  kernel/module/Makefile                       |  1 +
> >  kernel/module/hashes.c                       | 52 ++++++++++++++++++++++++++++
> >  kernel/module/internal.h                     |  1 +
> >  kernel/module/main.c                         |  6 ++++
> >  scripts/Makefile.modfinal                    |  6 ++++
> >  scripts/Makefile.vmlinux                     |  5 +++
> >  scripts/link-vmlinux.sh                      | 25 ++++++++++++-
> >  scripts/module-hashes.sh                     | 26 ++++++++++++++
> >  security/lockdown/Kconfig                    |  2 +-
> >  15 files changed, 178 insertions(+), 5 deletions(-)
> > 
> > diff --git a/.gitignore b/.gitignore
> > index 6839cf84acda0d2d3c236a2e42b0cb0fe1b14965..7c40151c3f5d0c15ac04cead5f21c291a98d779f 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -28,6 +28,7 @@
> >  *.gz
> >  *.i
> >  *.ko
> > +*.ko.hash
> >  *.lex.c
> >  *.ll
> >  *.lst
> > diff --git a/Documentation/kbuild/reproducible-builds.rst b/Documentation/kbuild/reproducible-builds.rst
> > index f2dcc39044e66ddd165646e0b51ccb0209aca7dd..6a742ad745113a9267223b33810dbc7218c47d4c 100644
> > --- a/Documentation/kbuild/reproducible-builds.rst
> > +++ b/Documentation/kbuild/reproducible-builds.rst
> > @@ -79,7 +79,10 @@ generate a different temporary key for each build, resulting in the
> >  modules being unreproducible.  However, including a signing key with
> >  your source would presumably defeat the purpose of signing modules.
> >  
> > -One approach to this is to divide up the build process so that the
> > +Instead ``CONFIG_MODULE_HASHES`` can be used to embed a static list
> > +of valid modules to load.
> > +
> > +Another approach to this is to divide up the build process so that the
> >  unreproducible parts can be treated as sources:
> >  
> >  1. Generate a persistent signing key.  Add the certificate for the key
> > diff --git a/Makefile b/Makefile
> > index b9464c88ac7230518a756bff5e6c5c8871cc5058..fc862ffd2df843c0b68bebc8f554b88850ba1541 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1535,8 +1535,10 @@ endif
> >  # is an exception.
> >  ifdef CONFIG_DEBUG_INFO_BTF_MODULES
> >  KBUILD_BUILTIN := 1
> > +ifndef CONFIG_MODULE_HASHES
> >  modules: vmlinux
> >  endif
> > +endif
> >  
> >  modules: modules_prepare
> >  
> 
> I think that the way the feature is integrated into the build is
> currently suboptimal. We should look how to make it properly orthogonal
> with the BTF stuff and also try to obtain the number of modules early.
> Sorry, I can't immediately provide any advice here as I need to
> understand the relevant logic more myself.

The feature should be usable without BTF. The ordering requirement does
not only exist for BTF generation but also modpost.
As for counting the modules early I'm hoping for some feedback from
the kbuild experts.

> > @@ -1916,7 +1918,11 @@ modules.order: $(build-dir)
> >  # KBUILD_MODPOST_NOFINAL can be set to skip the final link of modules.
> >  # This is solely useful to speed up test compiles.
> >  modules: modpost
> > -ifneq ($(KBUILD_MODPOST_NOFINAL),1)
> > +ifdef CONFIG_MODULE_HASHES
> > +ifeq ($(MODULE_HASHES_MODPOST_FINAL), 1)
> > +	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modfinal
> > +endif
> > +else ifneq ($(KBUILD_MODPOST_NOFINAL),1)
> >  	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modfinal
> >  endif
> >  
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index 54504013c74915c2ed923fb3afde024a69cdae6b..aebea528aac3d7209bcee12c25f750ab0f7576a5 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -486,6 +486,8 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
> >  									\
> >  	PRINTK_INDEX							\
> >  									\
> > +	MODULE_HASHES							\
> > +									\
> >  	/* Kernel symbol table: Normal symbols */			\
> >  	__ksymtab         : AT(ADDR(__ksymtab) - LOAD_OFFSET) {		\
> >  		__start___ksymtab = .;					\
> > @@ -895,6 +897,15 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
> >  #define PRINTK_INDEX
> >  #endif
> >  
> > +#ifdef CONFIG_MODULE_HASHES
> > +#define MODULE_HASHES							\
> > +	.module_hashes : AT(ADDR(.module_hashes) - LOAD_OFFSET) {	\
> > +	BOUNDED_SECTION_BY(.module_hashes, _module_hashes)		\
> > +	}
> > +#else
> > +#define MODULE_HASHES
> > +#endif
> > +
> >  /*
> >   * Discard .note.GNU-stack, which is emitted as PROGBITS by the compiler.
> >   * Otherwise, the type of .notes section would become PROGBITS instead of NOTES.
> > diff --git a/include/linux/module_hashes.h b/include/linux/module_hashes.h
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..5f2f0546e3875e6bc73bdd53aebaada7371b7f79
> > --- /dev/null
> > +++ b/include/linux/module_hashes.h
> > @@ -0,0 +1,17 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +
> > +#ifndef _LINUX_MODULE_HASHES_H
> > +#define _LINUX_MODULE_HASHES_H
> > +
> > +#include <linux/compiler_attributes.h>
> > +#include <linux/types.h>
> > +#include <crypto/sha2.h>
> > +
> > +#define __module_hashes_section __section(".module_hashes")
> > +#define MODULE_HASHES_HASH_SIZE SHA256_DIGEST_SIZE
> > +
> > +extern const u8 module_hashes[][MODULE_HASHES_HASH_SIZE];
> > +
> > +extern const typeof(module_hashes[0]) __start_module_hashes, __stop_module_hashes;
> > +
> > +#endif /* _LINUX_MODULE_HASHES_H */
> > diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> > index a80de8d22efdd0f13b3eb579a8ff1e69029d0694..cdd30b9a08d8cdf3ec0595b5e414265b869d343e 100644
> > --- a/kernel/module/Kconfig
> > +++ b/kernel/module/Kconfig
> > @@ -212,7 +212,7 @@ config MODULE_SIG
> >  
> >  config MODULE_SIG_POLICY
> >  	def_bool y
> > -	depends on MODULE_SIG
> > +	depends on MODULE_SIG || MODULE_HASHES
> >  
> >  config MODULE_SIG_FORCE
> >  	bool "Require modules to be validly signed"
> > @@ -348,6 +348,21 @@ config MODULE_DECOMPRESS
> >  
> >  	  If unsure, say N.
> >  
> > +config MODULE_HASHES
> > +	bool "Module hash validation"
> > +	depends on $(success,cksum --algorithm sha256 --raw /dev/null)
> 
> The cksum utility from GNU coreutils gained the --algorithm option in
> 2021 [1] and the --raw option in 2023 [2], which looks quite recent.
> The document process/changes.rst doesn't mention a minimum version for
> coreutils. However, I'd consider using sha256sum or
> 'openssl dgst -sha256 -binary' as that should cover more systems.

Then I'd prefer 'openssl'. Or we reuse the 150 lines of sha256
implementation from lib/crypto/sha256.c and have no external
dependencies.

> [1] https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=ad6c8e1181a3966e35d68c1c354deb1c73f3e974
> [2] https://git.savannah.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=ead07bb3d461389bb52336109be7858458e49c38
> 
> > +	select CRYPTO_LIB_SHA256
> > +	help
> > +	  Validate modules by their hashes.
> > +	  Only modules built together with the main kernel image can be
> > +	  validated that way.
> > +
> > +	  This is a reproducible-build compatible alternative to a build-time
> > +	  generated module keyring, as enabled by
> > +	  CONFIG_MODULE_SIG_KEY=certs/signing_key.pem.
> > +
> > +	  Also see the warning in MODULE_SIG about stripping modules.
> > +
> >  config MODULE_ALLOW_MISSING_NAMESPACE_IMPORTS
> >  	bool "Allow loading of modules with missing namespace imports"
> >  	help
> > diff --git a/kernel/module/Makefile b/kernel/module/Makefile
> > index 50ffcc413b54504db946af4dce3b41dc4aece1a5..6fe0c14ca5a05b49c1161fcfa8aaa130f89b70e1 100644
> > --- a/kernel/module/Makefile
> > +++ b/kernel/module/Makefile
> > @@ -23,3 +23,4 @@ obj-$(CONFIG_KGDB_KDB) += kdb.o
> >  obj-$(CONFIG_MODVERSIONS) += version.o
> >  obj-$(CONFIG_MODULE_UNLOAD_TAINT_TRACKING) += tracking.o
> >  obj-$(CONFIG_MODULE_STATS) += stats.o
> > +obj-$(CONFIG_MODULE_HASHES) += hashes.o
> > diff --git a/kernel/module/hashes.c b/kernel/module/hashes.c
> > new file mode 100644
> > index 0000000000000000000000000000000000000000..1aa49767a39b4e0c495b17d3f2edcb5a6ceb839e
> > --- /dev/null
> > +++ b/kernel/module/hashes.c
> > @@ -0,0 +1,52 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +
> > +#define pr_fmt(fmt) "module/hash: " fmt
> > +
> > +#include <linux/int_log.h>
> > +#include <linux/module_hashes.h>
> > +#include <linux/module.h>
> > +#include <crypto/sha2.h>
> > +#include "internal.h"
> > +
> > +static inline size_t module_hashes_count(void)
> > +{
> > +	return (__stop_module_hashes - __start_module_hashes) / MODULE_HASHES_HASH_SIZE;
> > +}
> > +
> > +static __init __maybe_unused int module_hashes_init(void)
> > +{
> > +	size_t num_hashes = module_hashes_count();
> > +	int num_width = (intlog10(num_hashes) >> 24) + 1;
> 
> It is unlikely, but I suppose one could configure the kernel with
> CONFIG_MODULE_DEBUG and CONFIG_MODULE_HASHES but without any actual
> modules. In this corner case, intlog10() will be called with 0 and
> produces a warning.

Ack.

> > +	size_t i;
> > +
> > +	pr_debug("Known hashes (%zu):\n", num_hashes);
> > +
> > +	for (i = 0; i < num_hashes; i++)
> > +		pr_debug("%*zu %*phN\n", num_width, i,
> > +			 (int)sizeof(module_hashes[i]), module_hashes[i]);
> > +
> > +	return 0;
> > +}
> > +
> > +#if IS_ENABLED(CONFIG_MODULE_DEBUG)
> > +early_initcall(module_hashes_init);
> > +#endif
> > +
> > +int module_hash_check(struct load_info *info, int flags)
> > +{
> > +	u8 digest[MODULE_HASHES_HASH_SIZE];
> > +	size_t i;
> > +
> > +	sha256((const u8 *)info->hdr, info->len, digest);
> > +
> > +	for (i = 0; i < module_hashes_count(); i++) {
> > +		if (memcmp(module_hashes[i], digest, MODULE_HASHES_HASH_SIZE) == 0) {
> > +			pr_debug("allow %*phN\n", (int)sizeof(digest), digest);
> > +			info->sig_ok = true;
> > +			return 0;
> > +		}
> > +	}
> > +
> > +	pr_debug("block %*phN\n", (int)sizeof(digest), digest);
> > +	return -ENOKEY;
> > +}
> > diff --git a/kernel/module/internal.h b/kernel/module/internal.h
> > index c30abeefa60b884c4a69b1eb4f1123a4bbee4b47..9c927c212f862fff7000f1cfac3c7e391a2390ac 100644
> > --- a/kernel/module/internal.h
> > +++ b/kernel/module/internal.h
> > @@ -334,6 +334,7 @@ int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
> >  				char *secstrings, struct module *mod);
> >  
> >  int module_sig_check(struct load_info *info, int flags);
> > +int module_hash_check(struct load_info *info, int flags);
> >  
> >  #ifdef CONFIG_DEBUG_KMEMLEAK
> >  void kmemleak_load_module(const struct module *mod, const struct load_info *info);
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index effe1db02973d4f60ff6cbc0d3b5241a3576fa3e..094ace81d795711b56d12a2abc75ea35449c8300 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -3218,6 +3218,12 @@ static int module_integrity_check(struct load_info *info, int flags)
> >  {
> >  	int err = 0;
> >  
> > +	if (IS_ENABLED(CONFIG_MODULE_HASHES)) {
> > +		err = module_hash_check(info, flags);
> > +		if (!err)
> > +			return 0;
> > +	}
> > +
> >  	if (IS_ENABLED(CONFIG_MODULE_SIG))
> >  		err = module_sig_check(info, flags);
> >  
> 
> Nit: I'd write this function as follows to make the logic shorter and to
> express that the code looks for the first handler that successfully
> verifies the module by setting info->sig_ok.
> 
> static int module_integrity_check(struct load_info *info, int flags)
> {
> 	int err = 0;
> 
> 	if (IS_ENABLED(CONFIG_MODULE_HASHES))
> 		err = module_hash_check(info, flags);
> 
> 	if (!info->sig_ok && IS_ENABLED(CONFIG_MODULE_SIG))
> 		err = module_sig_check(info, flags);
> 
> 	if (err)
> 		return err;
> 	if (info->sig_ok)
> 		return 0;
> 	return security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
> }

Ack.


Thanks!
Thomas

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v2 6/6] module: Introduce hash-based integrity checking
  2025-01-22 23:28   ` kpcyrd
@ 2025-03-06  8:10     ` Thomas Weißschuh
  0 siblings, 0 replies; 17+ messages in thread
From: Thomas Weißschuh @ 2025-03-06  8:10 UTC (permalink / raw)
  To: kpcyrd
  Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Arnd Bergmann,
	Luis Chamberlain, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Paul Moore, James Morris, Serge E. Hallyn, Jonathan Corbet,
	Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo,
	linux-kbuild, linux-kernel, linux-arch, linux-modules,
	linux-security-module, linux-doc

On 2025-01-23 00:28:40+0100, kpcyrd wrote:
> Thanks for reaching out, also your work on this is much appreciated and
> followed with great interest. <3
> 
> On 1/20/25 6:44 PM, Thomas Weißschuh wrote:
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index effe1db02973d4f60ff6cbc0d3b5241a3576fa3e..094ace81d795711b56d12a2abc75ea35449c8300 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -3218,6 +3218,12 @@ static int module_integrity_check(struct load_info *info, int flags)
> >   {
> >   	int err = 0;
> > +	if (IS_ENABLED(CONFIG_MODULE_HASHES)) {
> > +		err = module_hash_check(info, flags);
> > +		if (!err)
> > +			return 0;
> > +	}
> > +
> >   	if (IS_ENABLED(CONFIG_MODULE_SIG))
> >   		err = module_sig_check(info, flags);
> 
> From how I'm reading this (please let me know if I'm wrong):

<snip>

This is how it is intended, thanks for checking.

> This all seems reasonable to me, maybe the check for
> is_module_sig_enforced() could be moved from kernel/module/signing.c to
> kernel/module/main.c, otherwise `sig_enforce=1` would not have any effect
> for a `CONFIG_MODULE_HASHES && !CONFIG_MODULE_SIG` kernel.

Moving the check would complicate the logic and shouldn't make a
difference. In signing.c it ensures that a validation failure is
propagated. However that is the default behaviour in hashes.c.


Thomas

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2025-03-06  8:10 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-20 17:44 [PATCH v2 0/6] module: Introduce hash-based integrity checking Thomas Weißschuh
2025-01-20 17:44 ` [PATCH v2 1/6] kbuild: add stamp file for vmlinux BTF data Thomas Weißschuh
2025-01-20 17:44 ` [PATCH v2 2/6] module: Make module loading policy usable without MODULE_SIG Thomas Weißschuh
2025-01-20 17:44 ` [PATCH v2 3/6] module: Move integrity checks into dedicated function Thomas Weißschuh
2025-01-20 17:44 ` [PATCH v2 4/6] module: Move lockdown check into generic module loader Thomas Weißschuh
2025-01-20 17:44 ` [PATCH v2 5/6] lockdown: Make the relationship to MODULE_SIG a dependency Thomas Weißschuh
2025-01-20 17:44 ` [PATCH v2 6/6] module: Introduce hash-based integrity checking Thomas Weißschuh
2025-01-22 23:28   ` kpcyrd
2025-03-06  8:10     ` Thomas Weißschuh
2025-02-03 14:22   ` Petr Pavlu
2025-02-04 21:22     ` Thomas Weißschuh
2025-01-21 10:26 ` [PATCH v2 0/6] " Roberto Sassu
2025-01-21 12:58   ` Thomas Weißschuh
2025-01-21 13:11     ` Roberto Sassu
2025-01-25 21:16 ` Câju Mihai-Drosi
2025-02-03 13:14 ` Christian Heusel
2025-02-04 21:08   ` Thomas Weißschuh

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).