[PATCH RFC 0/2] module: Introduce hash-based integrity checking

[PATCH RFC 0/2] module: Introduce hash-based integrity checking

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

To properly test the reproducibility in combination with BTF the patch
"[PATCH bpf-next] kbuild, bpf: Enable reproducible BTF generation" [0]
is also needed.

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
* Make compatible with lockdown
* Use MODULE_SIG_HASH for configuration
* Enable coexistence with MODULE_SIG
* Set mod->sig_ok()
* 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>
---
Thomas Weißschuh (2):
      kbuild: add stamp file for vmlinux BTF data
      module: Introduce hash-based integrity checking

 Makefile                          |  8 +++++-
 include/asm-generic/vmlinux.lds.h | 11 +++++++++
 include/linux/module_hashes.h     | 17 +++++++++++++
 kernel/module/Kconfig             | 11 +++++++++
 kernel/module/Makefile            |  1 +
 kernel/module/hashes.c            | 51 +++++++++++++++++++++++++++++++++++++++
 kernel/module/internal.h          |  9 +++++++
 kernel/module/main.c              |  4 +++
 scripts/Makefile.modfinal         |  4 +--
 scripts/Makefile.vmlinux          |  5 ++++
 scripts/link-vmlinux.sh           | 31 +++++++++++++++++++++++-
 scripts/module-hashes.sh          | 26 ++++++++++++++++++++
 12 files changed, 174 insertions(+), 4 deletions(-)
---
base-commit: f722972b5df307d8c93c706c62d2e27e963c8f66
change-id: 20241225-module-hashes-7a50a7cc2a30

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

[PATCH RFC 1/2] kbuild: add stamp file for vmlinux BTF data

[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.47.1

[PATCH RFC 2/2] module: Introduce hash-based integrity checking

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

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 Makefile                          |  8 +++++-
 include/asm-generic/vmlinux.lds.h | 11 +++++++++
 include/linux/module_hashes.h     | 17 +++++++++++++
 kernel/module/Kconfig             | 11 +++++++++
 kernel/module/Makefile            |  1 +
 kernel/module/hashes.c            | 51 +++++++++++++++++++++++++++++++++++++++
 kernel/module/internal.h          |  9 +++++++
 kernel/module/main.c              |  4 +++
 scripts/Makefile.vmlinux          |  5 ++++
 scripts/link-vmlinux.sh           | 25 ++++++++++++++++++-
 scripts/module-hashes.sh          | 26 ++++++++++++++++++++
 11 files changed, 166 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 5c9b1d2d59b4dc3d78ce01d917f9f47ab0c0adb6..359cd459440cd1037ecebe660e09f0058a62f49d 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 7b329057997ad2ec310133ca84617d9bfcdb7e9f..57d317a6fa444195d0806e6bd7a2af6e338a7f01 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -344,6 +344,17 @@ config MODULE_DECOMPRESS
 
 	  If unsure, say N.
 
+config MODULE_HASHES
+	bool "Module hash validation"
+	depends on !MODULE_SIG
+	select CRYPTO_LIB_SHA256
+	help
+	  Validate modules by their hashes.
+	  Only modules built together with the main kernel image can be
+	  validated that way.
+
+	  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..f19eccb0e3754e3edbf5cdea6d418da5c6ae6c65
--- /dev/null
+++ b/kernel/module/hashes.c
@@ -0,0 +1,51 @@
+// 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("Builtin 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;
+}
+
+#ifdef 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);
+			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 daef2be8390222c22220e2f168baa8d35ad531b9..418fc4eaadd7070915ed20c3213a78937841b352 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -342,6 +342,15 @@ static inline int module_sig_check(struct load_info *info, int flags)
 }
 #endif /* !CONFIG_MODULE_SIG */
 
+#ifdef CONFIG_MODULE_HASHES
+int module_hash_check(struct load_info *info, int flags);
+#else /* !CONFIG_MODULE_HASHES */
+static inline int module_hash_check(struct load_info *info, int flags)
+{
+	return 0;
+}
+#endif /* !CONFIG_MODULE_HASHES */
+
 #ifdef CONFIG_DEBUG_KMEMLEAK
 void kmemleak_load_module(const struct module *mod, const struct load_info *info);
 #else /* !CONFIG_DEBUG_KMEMLEAK */
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 5399c182b3cbed2dbeea0291f717f30358d8e7fc..008415c21c72dfa45953a9ac6dfc0507650e18ca 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3242,6 +3242,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	if (err)
 		goto free_copy;
 
+	err = module_hash_check(info, flags);
+	if (err)
+		goto free_copy;
+
 	/*
 	 * Do basic sanity checks against the ELF header and
 	 * sections. Cache useful sections and set the
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..7ca4e84f4c74266b9902d9f377aa2c901a06f995
--- /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="$(cksum -a sha256 --raw "$mod" | hexdump -v -e '"0x" 1/1 "%02x, "')"
+	fi
+	echo "	/* $mod */"
+	echo "	{ $modhash },"
+	echo
+done
+
+echo "};"

-- 
2.47.1

Re: [PATCH RFC 2/2] module: Introduce hash-based integrity checking

[Luis Chamberlain]

On Wed, Dec 25, 2024 at 11:52:00PM +0100, Thomas Weißschuh wrote:
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index 7b329057997ad2ec310133ca84617d9bfcdb7e9f..57d317a6fa444195d0806e6bd7a2af6e338a7f01 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -344,6 +344,17 @@ config MODULE_DECOMPRESS
>  
>  	  If unsure, say N.
>  
> +config MODULE_HASHES
> +	bool "Module hash validation"
> +	depends on !MODULE_SIG

Why are these mutually exclusive? Can't you want module signatures *and*
this as well? What distro which is using module signatures would switch
to this as an alternative instead? The help menu does not clarify any of
this at all, and neither does the patch.

> +	select CRYPTO_LIB_SHA256
> +	help
> +	  Validate modules by their hashes.
> +	  Only modules built together with the main kernel image can be
> +	  validated that way.
> +
> +	  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..f19eccb0e3754e3edbf5cdea6d418da5c6ae6c65
> --- /dev/null
> +++ b/kernel/module/hashes.c
> @@ -0,0 +1,51 @@
> +// 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("Builtin 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;
> +}
> +
> +#ifdef DEBUG

We have MODULE_DEBUG so just add depend on that and leverage that for
this instead.


> diff --git a/scripts/module-hashes.sh b/scripts/module-hashes.sh
> new file mode 100755
> index 0000000000000000000000000000000000000000..7ca4e84f4c74266b9902d9f377aa2c901a06f995
> --- /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="$(cksum -a sha256 --raw "$mod" | hexdump -v -e '"0x" 1/1 "%02x, "')"
> +	fi
> +	echo "	/* $mod */"
> +	echo "	{ $modhash },"
> +	echo
> +done
> +
> +echo "};"

Parallelize this.

  Luis

Re: [PATCH RFC 2/2] module: Introduce hash-based integrity checking

[Thomas Weißschuh]

Hi Luis,

On 2025-01-03 17:37:52-0800, Luis Chamberlain wrote:
> On Wed, Dec 25, 2024 at 11:52:00PM +0100, Thomas Weißschuh wrote:
> > diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> > index 7b329057997ad2ec310133ca84617d9bfcdb7e9f..57d317a6fa444195d0806e6bd7a2af6e338a7f01 100644
> > --- a/kernel/module/Kconfig
> > +++ b/kernel/module/Kconfig
> > @@ -344,6 +344,17 @@ config MODULE_DECOMPRESS
> >  
> >  	  If unsure, say N.
> >  
> > +config MODULE_HASHES
> > +	bool "Module hash validation"
> > +	depends on !MODULE_SIG
> 
> Why are these mutually exclusive? Can't you want module signatures *and*
> this as well? What distro which is using module signatures would switch
> to this as an alternative instead? The help menu does not clarify any of
> this at all, and neither does the patch.

The exclusivity is to keep the initial RFC patch small.
The cover letter lists "Enable coexistence with MODULE_SIG" as
a further improvement.

In general this MODULE_HASHES would be used by distros which are
currently using the build-time generated signing key with
CONFIG_MODULE_SIG_KEY=certs/signing_key.pem.

More concretely the Arch Linux team has expressed interest.

> > +	select CRYPTO_LIB_SHA256
> > +	help
> > +	  Validate modules by their hashes.
> > +	  Only modules built together with the main kernel image can be
> > +	  validated that way.
> > +
> > +	  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..f19eccb0e3754e3edbf5cdea6d418da5c6ae6c65
> > --- /dev/null
> > +++ b/kernel/module/hashes.c
> > @@ -0,0 +1,51 @@
> > +// 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("Builtin 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;
> > +}
> > +
> > +#ifdef DEBUG
> 
> We have MODULE_DEBUG so just add depend on that and leverage that for
> this instead.

Ack.

> > diff --git a/scripts/module-hashes.sh b/scripts/module-hashes.sh
> > new file mode 100755
> > index 0000000000000000000000000000000000000000..7ca4e84f4c74266b9902d9f377aa2c901a06f995
> > --- /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="$(cksum -a sha256 --raw "$mod" | hexdump -v -e '"0x" 1/1 "%02x, "')"
> > +	fi
> > +	echo "	/* $mod */"
> > +	echo "	{ $modhash },"
> > +	echo
> > +done
> > +
> > +echo "};"
> 
> Parallelize this.

Ack.

Re: [PATCH RFC 2/2] module: Introduce hash-based integrity checking

[Luis Chamberlain]

On Sat, Jan 04, 2025 at 07:30:39AM +0100, Thomas Weißschuh wrote:
> Hi Luis,
> 
> On 2025-01-03 17:37:52-0800, Luis Chamberlain wrote:
> > On Wed, Dec 25, 2024 at 11:52:00PM +0100, Thomas Weißschuh wrote:
> > > diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> > > index 7b329057997ad2ec310133ca84617d9bfcdb7e9f..57d317a6fa444195d0806e6bd7a2af6e338a7f01 100644
> > > --- a/kernel/module/Kconfig
> > > +++ b/kernel/module/Kconfig
> > > @@ -344,6 +344,17 @@ config MODULE_DECOMPRESS
> > >  
> > >  	  If unsure, say N.
> > >  
> > > +config MODULE_HASHES
> > > +	bool "Module hash validation"
> > > +	depends on !MODULE_SIG
> > 
> > Why are these mutually exclusive? Can't you want module signatures *and*
> > this as well? What distro which is using module signatures would switch
> > to this as an alternative instead? The help menu does not clarify any of
> > this at all, and neither does the patch.
> 
> The exclusivity is to keep the initial RFC patch small.
> The cover letter lists "Enable coexistence with MODULE_SIG" as
> a further improvement.

Looking forward to that.

> In general this MODULE_HASHES would be used by distros which are
> currently using the build-time generated signing key with
> CONFIG_MODULE_SIG_KEY=certs/signing_key.pem.

The Kconfig needs to describe this, otherwise no one would sensibly
enable this.

> More concretely the Arch Linux team has expressed interest.

Interest sure, but will it be used? If not there is no point to this.
What do the other distro have to think about this?

  Luis

Re: [PATCH RFC 2/2] module: Introduce hash-based integrity checking

[Fabian Grünbichler]

On January 4, 2025 2:37 am, Luis Chamberlain wrote:
> On Wed, Dec 25, 2024 at 11:52:00PM +0100, Thomas Weißschuh wrote:
>> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
>> index 7b329057997ad2ec310133ca84617d9bfcdb7e9f..57d317a6fa444195d0806e6bd7a2af6e338a7f01 100644
>> --- a/kernel/module/Kconfig
>> +++ b/kernel/module/Kconfig
>> @@ -344,6 +344,17 @@ config MODULE_DECOMPRESS
>>  
>>  	  If unsure, say N.
>>  
>> +config MODULE_HASHES
>> +	bool "Module hash validation"
>> +	depends on !MODULE_SIG
> 
> Why are these mutually exclusive? Can't you want module signatures *and*
> this as well? What distro which is using module signatures would switch
> to this as an alternative instead? The help menu does not clarify any of
> this at all, and neither does the patch.

FWIW, I think we (Proxmox, a Debian derivative) would consider switching
to MODULE_HASHES for the modules shipped with our kernel packages, once
MODULE_HASHES does not conflict with user/MOK-signatures on DKMS- or
manually-built modules. we do prefer reproducible builds, but
extensibility via third-party modules is an important use case for us
(and I except many other more general purpose distros).

Re: [PATCH RFC 2/2] module: Introduce hash-based integrity checking

[Arnout Engelen]

On Fri, 3 Jan 2025 17:37:52 -0800, Luis Chamberlain wrote:
> What distro which is using module signatures would switch
> to this as an alternative instead?

In NixOS, we disable MODULE_SIG by default (because we value
reproducibility over having module signatures). Enabling
MODULE_HASHES on systems that do not need to load out-of-tree
modules would be a good step forward.


Kind regards,

Arnout Engelen

Re: [PATCH RFC 2/2] module: Introduce hash-based integrity checking

[Luis Chamberlain]

On Thu, Jan 09, 2025 at 11:52:27AM +0100, Arnout Engelen wrote:
> On Fri, 3 Jan 2025 17:37:52 -0800, Luis Chamberlain wrote:
> > What distro which is using module signatures would switch
> > to this as an alternative instead?
> 
> In NixOS, we disable MODULE_SIG by default (because we value
> reproducibility over having module signatures). Enabling
> MODULE_HASHES on systems that do not need to load out-of-tree
> modules would be a good step forward.
> 

Mentioning this in the cover letter will also be good. So two
distros seemt to want this.

  Luis

Re: [PATCH RFC 2/2] module: Introduce hash-based integrity checking

[Petr Pavlu]

On 1/10/25 20:16, Luis Chamberlain wrote:
> On Thu, Jan 09, 2025 at 11:52:27AM +0100, Arnout Engelen wrote:
>> On Fri, 3 Jan 2025 17:37:52 -0800, Luis Chamberlain wrote:
>>> What distro which is using module signatures would switch
>>> to this as an alternative instead?
>>
>> In NixOS, we disable MODULE_SIG by default (because we value
>> reproducibility over having module signatures). Enabling
>> MODULE_HASHES on systems that do not need to load out-of-tree
>> modules would be a good step forward.
>>
> 
> Mentioning this in the cover letter will also be good. So two
> distros seemt to want this.

I'm aware that folks from the reproducible build community have been
interested in this functionality [1, 2].

Some people at SUSE have been eyeing this as well. I've let them know
about this series. It would help with the mentioned build
reproducibility and from what I understood, it should also avoid in SUSE
case some bottlenecks with HSM needing to sign all modules.

I agree that we should make sure that whatever ends up added is
something that some distributions actually check it works for them and
they intend to use it.

From the SUSE side, I can also support that the feature should work
seamlessly with the current MODULE_SIG.

[1] https://lists.reproducible-builds.org/pipermail/rb-general/2024-September/003530.html
[2] https://gitlab.archlinux.org/archlinux/packaging/packages/linux/-/merge_requests/1

-- 
Thanks,
Petr