All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] test/py: Check hashes produced by mkimage against known values
@ 2021-09-15 19:33 Alexandru Gagniuc
  2021-09-30  4:09 ` Simon Glass
  2021-10-05 22:02 ` Tom Rini
  0 siblings, 2 replies; 3+ messages in thread
From: Alexandru Gagniuc @ 2021-09-15 19:33 UTC (permalink / raw)
  To: u-boot, trini, sjg; +Cc: Alexandru Gagniuc

Target code and mkimage share the same hashing infrastructure. If one
is wrong, it's very likely that both are wrong in the same way. Thus
testing won't catch hash regressions. This already happened in
commit 92055e138f28 ("image: Drop if/elseif hash selection in
calculate_hash()"). None of the tests caught that CRC32 was broken.

Instead of testing hash_calculate() against itself, create a FIT with
containing a kernel with pre-calculated hashes. Then check the hashes
produced against the known good hashes.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>

---
Desired:
  $ ./test/py/test.py -k hash
  ...
  test_mkimage_hashes PASSED

Is very cryptic with regards to what is going on. It would be much
nicer to have tests named "crc32", "sha256", and so on. But I don't
know how to do that without several pyton functions each assembling
their own damn FIT.
I think it would also be nicer for the test log to show
    test_sha1  PASSED
    test_crc32 PASSED  
    test_md5   FAILED

 test/py/tests/test_fit_hashes.py    | 111 ++++++++++++++++++++++++++++
 test/py/tests/vboot/hash-images.its |  76 +++++++++++++++++++
 2 files changed, 187 insertions(+)
 create mode 100644 test/py/tests/test_fit_hashes.py
 create mode 100644 test/py/tests/vboot/hash-images.its

diff --git a/test/py/tests/test_fit_hashes.py b/test/py/tests/test_fit_hashes.py
new file mode 100644
index 0000000000..e228ea96d3
--- /dev/null
+++ b/test/py/tests/test_fit_hashes.py
@@ -0,0 +1,111 @@
+# SPDX-License-Identifier:	GPL-2.0+
+#
+# Copyright (c) 2021 Alexandru Gagniuc <mr.nuke.me@gmail.com>
+
+"""
+Check hashes produced by mkimage against known values
+
+This test checks the correctness of mkimage's hashes. by comparing the mkimage
+output of a fixed data block with known good hashes.
+This test doesn't run the sandbox. It only checks the host tool 'mkimage'
+"""
+
+import pytest
+import u_boot_utils as util
+
+kernel_hashes = {
+    "sha512" : "f18c1486a2c29f56360301576cdfce4dfd8e8e932d0ed8e239a1f314b8ae1d77b2a58cd7fe32e4075e69448e623ce53b0b6aa6ce5626d2c189a5beae29a68d93",
+    "sha384" : "16e28976740048485d08d793d8bf043ebc7826baf2bc15feac72825ad67530ceb3d09e0deb6932c62a5a0e9f3936baf4",
+    "sha256" : "2955c56bc1e5050c111ba6e089e0f5342bb47dedf77d87e3f429095feb98a7e5",
+    "sha1"   : "652383e1a6d946953e1f65092c9435f6452c2ab7",
+    "md5"    : "4879e5086e4c76128e525b5fe2af55f1",
+    "crc32"  : "32eddfdf",
+    "crc16-ccitt" : "d4be"
+}
+
+class ReadonlyFitImage(object):
+    """ Helper to manipulate a FIT image on disk """
+    def __init__(self, cons, file_name):
+        self.fit = file_name
+        self.cons = cons
+        self.hashable_nodes = set()
+
+    def __fdt_list(self, path):
+        return util.run_and_log(self.cons, f'fdtget -l {self.fit} {path}')
+
+    def __fdt_get(self, node, prop):
+        val = util.run_and_log(self.cons, f'fdtget {self.fit} {node} {prop}')
+        return val.rstrip('\n')
+
+    def __fdt_get_sexadecimal(self, node, prop):
+        numbers = util.run_and_log(self.cons, f'fdtget -tbx {self.fit} {node} {prop}')
+
+        sexadecimal = ''
+        for num in numbers.rstrip('\n').split(' '):
+            sexadecimal += num.zfill(2)
+        return sexadecimal
+
+    def find_hashable_image_nodes(self):
+        for node in self.__fdt_list('/images').split():
+            # We only have known hashes for the kernel node
+            if 'kernel' not in node:
+                continue
+            self.hashable_nodes.add(f'/images/{node}')
+
+        return self.hashable_nodes
+
+    def verify_hashes(self):
+        for image in self.hashable_nodes:
+            algos = set()
+            for node in self.__fdt_list(image).split():
+                if "hash-" not in node:
+                    continue
+
+                raw_hash = self.__fdt_get_sexadecimal(f'{image}/{node}', 'value')
+                algo = self.__fdt_get(f'{image}/{node}', 'algo')
+                algos.add(algo)
+
+                good_hash = kernel_hashes[algo]
+                if good_hash != raw_hash:
+                    raise ValueError(f'{image} Borked hash: {algo}');
+
+            # Did we test all the hashes we set out to test?
+            missing_algos = kernel_hashes.keys() - algos
+            if (missing_algos):
+                raise ValueError(f'Missing hashes from FIT: {missing_algos}')
+
+
+@pytest.mark.buildconfigspec('hash')
+@pytest.mark.requiredtool('dtc')
+@pytest.mark.requiredtool('fdtget')
+@pytest.mark.requiredtool('fdtput')
+def test_mkimage_hashes(u_boot_console):
+    """ Test that hashes generated by mkimage are correct. """
+
+    def assemble_fit_image(dest_fit, its, destdir):
+        dtc_args = f'-I dts -O dtb -i {destdir}'
+        util.run_and_log(cons, [mkimage, '-D', dtc_args, '-f', its, dest_fit])
+
+    def dtc(dts):
+        dtb = dts.replace('.dts', '.dtb')
+        util.run_and_log(cons, f'dtc {datadir}/{dts} -O dtb -o {tempdir}/{dtb}')
+
+    cons = u_boot_console
+    mkimage = cons.config.build_dir + '/tools/mkimage'
+    datadir = cons.config.source_dir + '/test/py/tests/vboot/'
+    tempdir = cons.config.result_dir
+    fit_file = f'{tempdir}/test.fit'
+    dtc('sandbox-kernel.dts')
+
+    # Create a fake kernel image -- Avoid zeroes or crc16 will be zero
+    with open(f'{tempdir}/test-kernel.bin', 'w') as fd:
+        fd.write(500 * chr(0xa5))
+
+    assemble_fit_image(fit_file, f'{datadir}/hash-images.its', tempdir)
+
+    fit = ReadonlyFitImage(cons, fit_file)
+    nodes = fit.find_hashable_image_nodes()
+    if len(nodes) == 0:
+        raise ValueError('FIT image has no "/image" nodes with "hash-..."')
+
+    fit.verify_hashes()
diff --git a/test/py/tests/vboot/hash-images.its b/test/py/tests/vboot/hash-images.its
new file mode 100644
index 0000000000..3ff797288c
--- /dev/null
+++ b/test/py/tests/vboot/hash-images.its
@@ -0,0 +1,76 @@
+/dts-v1/;
+
+/ {
+	description = "Chrome OS kernel image with one or more FDT blobs";
+	#address-cells = <1>;
+
+	images {
+		kernel {
+			data = /incbin/("test-kernel.bin");
+			type = "kernel_noload";
+			arch = "sandbox";
+			os = "linux";
+			compression = "none";
+			load = <0x4>;
+			entry = <0x8>;
+			kernel-version = <1>;
+			hash-0 {
+                                algo = "crc16-ccitt";
+                        };
+                        hash-1 {
+                                algo = "crc32";
+                        };
+                        hash-2 {
+                                algo = "md5";
+                        };
+                        hash-3 {
+                                algo = "sha1";
+                        };
+                        hash-4 {
+                                algo = "sha256";
+                        };
+                        hash-5 {
+                                algo = "sha384";
+                        };
+                        hash-6 {
+                                algo = "sha512";
+                        };
+		};
+		fdt-1 {
+			description = "snow";
+			data = /incbin/("sandbox-kernel.dtb");
+			type = "flat_dt";
+			arch = "sandbox";
+			compression = "none";
+			fdt-version = <1>;
+			hash-0 {
+                                algo = "crc16-ccitt";
+                        };
+                        hash-1 {
+                                algo = "crc32";
+                        };
+                        hash-2 {
+                                algo = "md5";
+                        };
+                        hash-3 {
+                                algo = "sha1";
+                        };
+                        hash-4 {
+                                algo = "sha256";
+                        };
+                        hash-5 {
+                                algo = "sha384";
+                        };
+                        hash-6 {
+                                algo = "sha512";
+                        };
+		};
+	};
+	configurations {
+		default = "conf-1";
+		conf-1 {
+			kernel = "kernel";
+			fdt = "fdt-1";
+		};
+	};
+};
-- 
2.31.1


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

* Re: [RFC PATCH] test/py: Check hashes produced by mkimage against known values
  2021-09-15 19:33 [RFC PATCH] test/py: Check hashes produced by mkimage against known values Alexandru Gagniuc
@ 2021-09-30  4:09 ` Simon Glass
  2021-10-05 22:02 ` Tom Rini
  1 sibling, 0 replies; 3+ messages in thread
From: Simon Glass @ 2021-09-30  4:09 UTC (permalink / raw)
  To: Alexandru Gagniuc; +Cc: U-Boot Mailing List, Tom Rini

On Wed, 15 Sept 2021 at 13:33, Alexandru Gagniuc <mr.nuke.me@gmail.com> wrote:
>
> Target code and mkimage share the same hashing infrastructure. If one
> is wrong, it's very likely that both are wrong in the same way. Thus
> testing won't catch hash regressions. This already happened in
> commit 92055e138f28 ("image: Drop if/elseif hash selection in
> calculate_hash()"). None of the tests caught that CRC32 was broken.
>
> Instead of testing hash_calculate() against itself, create a FIT with
> containing a kernel with pre-calculated hashes. Then check the hashes
> produced against the known good hashes.
>
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
>
> ---
> Desired:
>   $ ./test/py/test.py -k hash
>   ...
>   test_mkimage_hashes PASSED
>
> Is very cryptic with regards to what is going on. It would be much
> nicer to have tests named "crc32", "sha256", and so on. But I don't
> know how to do that without several pyton functions each assembling
> their own damn FIT.
> I think it would also be nicer for the test log to show
>     test_sha1  PASSED
>     test_crc32 PASSED
>     test_md5   FAILED
>
>  test/py/tests/test_fit_hashes.py    | 111 ++++++++++++++++++++++++++++
>  test/py/tests/vboot/hash-images.its |  76 +++++++++++++++++++
>  2 files changed, 187 insertions(+)
>  create mode 100644 test/py/tests/test_fit_hashes.py
>  create mode 100644 test/py/tests/vboot/hash-images.its
>

Reviewed-by: Simon Glass <sjg@chromium.org>

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

* Re: [RFC PATCH] test/py: Check hashes produced by mkimage against known values
  2021-09-15 19:33 [RFC PATCH] test/py: Check hashes produced by mkimage against known values Alexandru Gagniuc
  2021-09-30  4:09 ` Simon Glass
@ 2021-10-05 22:02 ` Tom Rini
  1 sibling, 0 replies; 3+ messages in thread
From: Tom Rini @ 2021-10-05 22:02 UTC (permalink / raw)
  To: Alexandru Gagniuc; +Cc: u-boot, sjg

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

On Wed, Sep 15, 2021 at 02:33:01PM -0500, Alexandru Gagniuc wrote:

> Target code and mkimage share the same hashing infrastructure. If one
> is wrong, it's very likely that both are wrong in the same way. Thus
> testing won't catch hash regressions. This already happened in
> commit 92055e138f28 ("image: Drop if/elseif hash selection in
> calculate_hash()"). None of the tests caught that CRC32 was broken.
> 
> Instead of testing hash_calculate() against itself, create a FIT with
> containing a kernel with pre-calculated hashes. Then check the hashes
> produced against the known good hashes.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!

-- 
Tom

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

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

end of thread, other threads:[~2021-10-05 22:04 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15 19:33 [RFC PATCH] test/py: Check hashes produced by mkimage against known values Alexandru Gagniuc
2021-09-30  4:09 ` Simon Glass
2021-10-05 22:02 ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.