[PATCH v2 2/2] crypto: asymmetric_keys - add KUnit tests for the PE parser
Fabrice Derepas <[email protected]>
| Newsgroups | org.kernel.vger.keyrings,org.infradead.lists.kexec,org.kernel.vger.linux-crypto,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <73155550248025620873094809432c88ffa4c3d6.1786802052.git.fabrice.derepas@canonical.com> |
Add self-contained KUnit tests for pefile_parse_binary(), exercised
through verify_pefile_signature() -- which parses the image before any
signature check, so no keyring or real signature is needed. The cases
build small malformed PE images in memory:
- a PE declaring 0 data-directory entries: the certificate table entry
at fixed index 4 is absent and must be rejected (-ELIBBAD), not read
out of bounds;
- a PE declaring 5 entries with a zero (unsigned) certificate entry
gets past the directory bound and is rejected as unsigned (-ENODATA).
verify_pefile_signature() is not exported, so the test is built-in only
(bool, not tristate).
Signed-off-by: Fabrice Derepas <[email protected]>
---
crypto/asymmetric_keys/Kconfig | 12 +++
crypto/asymmetric_keys/Makefile | 2 +
crypto/asymmetric_keys/verify_pefile_test.c | 100 ++++++++++++++++++++
3 files changed, 114 insertions(+)
create mode 100644 crypto/asymmetric_keys/verify_pefile_test.c
diff --git a/crypto/asymmetric_keys/Kconfig b/crypto/asymmetric_keys/Kconfig
index 6a2f66404..c73979b2e 100644
--- a/crypto/asymmetric_keys/Kconfig
+++ b/crypto/asymmetric_keys/Kconfig
@@ -114,4 +114,16 @@ config FIPS_SIGNATURE_SELFTEST_ECDSA
depends on CRYPTO_SHA256=y || CRYPTO_SHA256=FIPS_SIGNATURE_SELFTEST
depends on CRYPTO_ECDSA=y || CRYPTO_ECDSA=FIPS_SIGNATURE_SELFTEST
+config VERIFY_PEFILE_KUNIT_TEST
+ bool "KUnit tests for signed PE file parsing" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y && SIGNED_PE_FILE_VERIFICATION
+ default KUNIT_ALL_TESTS
+ help
+ Enable KUnit tests for the PE binary parser used by signed PE file
+ signature verification. The tests build small malformed PE images in
+ memory and check that pefile_parse_binary() rejects them instead of
+ reading out of bounds.
+
+ If unsure, say N.
+
endif # ASYMMETRIC_KEY_TYPE
diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index bc65d3b98..35172685c 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -79,3 +79,5 @@ verify_signed_pefile-y := \
$(obj)/mscode_parser.o: $(obj)/mscode.asn1.h $(obj)/mscode.asn1.h
$(obj)/mscode.asn1.o: $(obj)/mscode.asn1.c $(obj)/mscode.asn1.h
+
+obj-$(CONFIG_VERIFY_PEFILE_KUNIT_TEST) += verify_pefile_test.o
diff --git a/crypto/asymmetric_keys/verify_pefile_test.c b/crypto/asymmetric_keys/verify_pefile_test.c
new file mode 100644
index 000000000..8b42eb41e
--- /dev/null
+++ b/crypto/asymmetric_keys/verify_pefile_test.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for the signed PE binary parser in verify_pefile.c.
+ *
+ * The cases build small malformed PE images in memory and call
+ * verify_pefile_signature(), which parses the image before any signature
+ * check, so the parser is exercised without a keyring or a real signature.
+ */
+#include <kunit/test.h>
+#include <linux/slab.h>
+#include <linux/pe.h>
+#include <linux/verification.h>
+
+/*
+ * Build a minimal PE32 image whose optional header declares @data_dirs data
+ * directory entries. header_size is sized to hold them and the image is a few
+ * bytes larger. With @data_dirs < 5 the certificate table entry (fixed index
+ * 4) lies outside the declared directory.
+ */
+static void *build_pe32(u16 data_dirs, unsigned int *out_len)
+{
+ size_t cursor = sizeof(struct mz_hdr) + sizeof(struct pe_hdr) +
+ sizeof(struct pe32_opt_hdr);
+ size_t header_size = cursor + ((size_t)data_dirs + 1) *
+ sizeof(struct data_dirent);
+ size_t pelen = header_size + 8;
+ struct mz_hdr *mz;
+ struct pe_hdr *pe;
+ struct pe32_opt_hdr *opt;
+ void *buf = kzalloc(pelen, GFP_KERNEL);
+
+ if (!buf)
+ return NULL;
+
+ mz = buf;
+ mz->magic = IMAGE_DOS_SIGNATURE;
+ mz->peaddr = sizeof(struct mz_hdr);
+
+ pe = buf + sizeof(struct mz_hdr);
+ pe->magic = IMAGE_NT_SIGNATURE;
+
+ opt = buf + sizeof(struct mz_hdr) + sizeof(struct pe_hdr);
+ opt->magic = IMAGE_NT_OPTIONAL_HDR32_MAGIC;
+ opt->header_size = header_size;
+ opt->data_dirs = data_dirs;
+
+ *out_len = pelen;
+ return buf;
+}
+
+/*
+ * data_dirs = 0: the certificate table entry (index 4) is absent, so the
+ * parser must reject the image instead of reading ddir->certs past the end.
+ */
+static void pefile_missing_certs_dirent(struct kunit *test)
+{
+ unsigned int len;
+ void *buf = build_pe32(0, &len);
+ int ret;
+
+ KUNIT_ASSERT_NOT_NULL(test, buf);
+ ret = verify_pefile_signature(buf, len, NULL,
+ VERIFYING_KEXEC_PE_SIGNATURE);
+ KUNIT_EXPECT_EQ(test, ret, -ELIBBAD);
+ kfree(buf);
+}
+
+/*
+ * data_dirs = 5: the certificate table entry is present (and zero, i.e.
+ * unsigned), so the parser gets past the directory bound and rejects the image
+ * as unsigned (-ENODATA) rather than -ELIBBAD.
+ */
+static void pefile_present_certs_dirent(struct kunit *test)
+{
+ unsigned int len;
+ void *buf = build_pe32(5, &len);
+ int ret;
+
+ KUNIT_ASSERT_NOT_NULL(test, buf);
+ ret = verify_pefile_signature(buf, len, NULL,
+ VERIFYING_KEXEC_PE_SIGNATURE);
+ KUNIT_EXPECT_EQ(test, ret, -ENODATA);
+ kfree(buf);
+}
+
+static struct kunit_case verify_pefile_cases[] = {
+ KUNIT_CASE(pefile_missing_certs_dirent),
+ KUNIT_CASE(pefile_present_certs_dirent),
+ {}
+};
+
+static struct kunit_suite verify_pefile_suite = {
+ .name = "verify_pefile",
+ .test_cases = verify_pefile_cases,
+};
+
+kunit_test_suite(verify_pefile_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("KUnit tests for the signed PE binary parser");
--
2.53.0