From 477f559edf1144f95e29173d290818250aa57ef8 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 18 Mar 2020 11:43:58 -0600 Subject: test: vboot: Drop unnecessary parameter for fit_check_sign This tool only uses the last -k parameter provided. Drop the earlier one since it has no effect. Signed-off-by: Simon Glass --- test/py/tests/test_vboot.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'test/py/tests/test_vboot.py') diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 9c41ee56b15..3dd8e3cb664 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -180,8 +180,7 @@ def test_vboot(u_boot_console): cons.log.action('%s: Check signed config on the host' % sha_algo) - util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', tmpdir, - '-k', dtb]) + util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb]) # Replace header bytes bcfg = u_boot_console.config.buildconfig -- cgit v1.2.3 From c021971e132234667eb80bc29bdd4ad6c8d04458 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 18 Mar 2020 11:43:59 -0600 Subject: test: vboot: Add a test for a forged configuration Add a check to make sure that it is not possible to add a new configuration and use the hashed nodes and hash of another configuration. Signed-off-by: Simon Glass --- test/py/tests/test_vboot.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'test/py/tests/test_vboot.py') diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 3dd8e3cb664..22c79ef3136 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -28,6 +28,7 @@ import pytest import sys import struct import u_boot_utils as util +import vboot_forge @pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('fit_signature') @@ -182,7 +183,22 @@ def test_vboot(u_boot_console): util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', dtb]) - # Replace header bytes + # Make sure that U-Boot checks that the config is in the list of hashed + # nodes. If it isn't, a security bypass is possible. + with open(fit, 'rb') as fp: + root, strblock = vboot_forge.read_fdt(fp) + root, strblock = vboot_forge.manipulate(root, strblock) + with open(fit, 'w+b') as fp: + vboot_forge.write_fdt(root, strblock, fp) + util.run_and_log_expect_exception(cons, + [fit_check_sign, '-f', fit, '-k', dtb], + 1, 'Failed to verify required signature') + + run_bootm(sha_algo, 'forged config', 'Bad Data Hash', False) + + # Create a new properly signed fit and replace header bytes + make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) + sign_fit(sha_algo) bcfg = u_boot_console.config.buildconfig max_size = int(bcfg.get('config_fit_signature_max_size', 0x10000000), 0) existing_size = replace_fit_totalsize(max_size + 1) -- cgit v1.2.3 From 1b090032029b35080a5a87c9f1047882d894ab37 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 18 Mar 2020 11:44:00 -0600 Subject: test: vboot: Parameterise the test This test is actually made up of five separate tests. Split them out so that they appear as separate tests. Unfortunately this restarts U-Boot multiple times which adds about a second to the already-long vboot test, about 8 seconds total on my machine. We could add a special 'teardown' test afterwards but if the tests are executed out of order that would not work. Changing test_vboot into a class causes it not to be discovered and makes it different from all other tests. Signed-off-by: Simon Glass --- test/py/tests/test_vboot.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'test/py/tests/test_vboot.py') diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 22c79ef3136..b1badaad737 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -30,13 +30,22 @@ import struct import u_boot_utils as util import vboot_forge +TESTDATA = [ + ['sha1', '', False], + ['sha1', '-pss', False], + ['sha256', '', False], + ['sha256', '-pss', False], + ['sha256', '-pss', True], +] + @pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('fit_signature') @pytest.mark.requiredtool('dtc') @pytest.mark.requiredtool('fdtget') @pytest.mark.requiredtool('fdtput') @pytest.mark.requiredtool('openssl') -def test_vboot(u_boot_console): +@pytest.mark.parametrize("sha_algo,padding,required", TESTDATA) +def test_vboot(u_boot_console, sha_algo, padding, required): """Test verified boot signing with mkimage and verification with 'bootm'. This works using sandbox only as it needs to update the device tree used @@ -297,11 +306,10 @@ def test_vboot(u_boot_console): # afterwards. old_dtb = cons.config.dtb cons.config.dtb = dtb - test_with_algo('sha1','') - test_with_algo('sha1','-pss') - test_with_algo('sha256','') - test_with_algo('sha256','-pss') - test_required_key('sha256','-pss') + if required: + test_required_key(sha_algo, padding) + else: + test_with_algo(sha_algo, padding) finally: # Go back to the original U-Boot with the correct dtb. cons.config.dtb = old_dtb -- cgit v1.2.3 From 3156ee35a3f11e578442ec7f2f3b96179cb07c94 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 18 Mar 2020 11:44:04 -0600 Subject: test: vboot: Tidy up the code a little Fix some long lines and comments. Use a distinct name for the 'required key' test. Signed-off-by: Simon Glass --- test/py/tests/test_vboot.py | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) (limited to 'test/py/tests/test_vboot.py') diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index b1badaad737..817f2a99d2c 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -91,7 +91,8 @@ def test_vboot(u_boot_console, sha_algo, padding, required): if boots: assert('sandbox: continuing, as we cannot run' in ''.join(output)) else: - assert('sandbox: continuing, as we cannot run' not in ''.join(output)) + assert('sandbox: continuing, as we cannot run' + not in ''.join(output)) def make_fit(its): """Make a new FIT from the .its source file. @@ -211,7 +212,8 @@ def test_vboot(u_boot_console, sha_algo, padding, required): bcfg = u_boot_console.config.buildconfig max_size = int(bcfg.get('config_fit_signature_max_size', 0x10000000), 0) existing_size = replace_fit_totalsize(max_size + 1) - run_bootm(sha_algo, 'Signed config with bad hash', 'Bad Data Hash', False) + run_bootm(sha_algo, 'Signed config with bad hash', 'Bad Data Hash', + False) cons.log.action('%s: Check overflowed FIT header totalsize' % sha_algo) # Replace with existing header bytes @@ -229,7 +231,8 @@ def test_vboot(u_boot_console, sha_algo, padding, required): util.run_and_log(cons, 'fdtput -t bx %s %s value %s' % (fit, sig_node, sig)) - run_bootm(sha_algo, 'Signed config with bad hash', 'Bad Data Hash', False) + run_bootm(sha_algo, 'Signed config with bad hash', 'Bad Data Hash', + False) cons.log.action('%s: Check bad config on the host' % sha_algo) util.run_and_log_expect_exception(cons, [fit_check_sign, '-f', fit, @@ -238,12 +241,11 @@ def test_vboot(u_boot_console, sha_algo, padding, required): def test_required_key(sha_algo, padding): """Test verified boot with the given hash algorithm. - This function test if u-boot reject an image when a required - key isn't used to sign a FIT. + This function tests if U-Boot rejects an image when a required key isn't + used to sign a FIT. Args: - sha_algo: Either 'sha1' or 'sha256', to select the algorithm to - use. + sha_algo: Either 'sha1' or 'sha256', to select the algorithm to use """ # Compile our device tree files for kernel and U-Boot. These are # regenerated here since mkimage will modify them (by adding a @@ -251,18 +253,24 @@ def test_vboot(u_boot_console, sha_algo, padding, required): dtc('sandbox-kernel.dts') dtc('sandbox-u-boot.dts') - # Build the FIT with prod key (keys required) - # Build the FIT with dev key (keys NOT required) - # The dtb contain the key prod and dev and the key prod are set as required. - # Then try to boot the FIT with dev key - # This FIT should not be accepted by u-boot because the key prod is required cons.log.action('%s: Test FIT with configs images' % sha_algo) + + # Build the FIT with prod key (keys required) and sign it. This puts the + # signature into sandbox-u-boot.dtb, marked 'required' make_fit('sign-configs-%s%s-prod.its' % (sha_algo , padding)) sign_fit(sha_algo) + + # Build the FIT with dev key (keys NOT required). This adds the + # signature into sandbox-u-boot.dtb, NOT marked 'required'. make_fit('sign-configs-%s%s.its' % (sha_algo , padding)) sign_fit(sha_algo) - run_bootm(sha_algo, 'signed configs', '', False) + # So now sandbox-u-boot.dtb two signatures, for the prod and dev keys. + # Only the prod key is set as 'required'. But FIT we just built has + # a dev signature only (sign_fit() overwrites the FIT). + # Try to boot the FIT with dev key. This FIT should not be accepted by + # U-Boot because the prod key is required. + run_bootm(sha_algo, 'required key', '', False) cons = u_boot_console tmpdir = cons.config.result_dir + '/' -- cgit v1.2.3 From b008677daf2a9dc0335260c7c4e24390487fe0ca Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 18 Mar 2020 11:44:05 -0600 Subject: test: vboot: Fix pylint errors Fix various minor things noticed by pylint. Signed-off-by: Simon Glass --- test/py/tests/test_vboot.py | 53 ++++++++++++++++----------------------------- 1 file changed, 19 insertions(+), 34 deletions(-) (limited to 'test/py/tests/test_vboot.py') diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 817f2a99d2c..0b8bd9ac0b0 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -24,9 +24,8 @@ For configuration verification: Tests run with both SHA1 and SHA256 hashing. """ -import pytest -import sys import struct +import pytest import u_boot_utils as util import vboot_forge @@ -85,11 +84,11 @@ def test_vboot(u_boot_console, sha_algo, padding, required): with cons.log.section('Verified boot %s %s' % (sha_algo, test_type)): output = cons.run_command_list( ['host load hostfs - 100 %stest.fit' % tmpdir, - 'fdt addr 100', - 'bootm 100']) - assert(expect_string in ''.join(output)) + 'fdt addr 100', + 'bootm 100']) + assert expect_string in ''.join(output) if boots: - assert('sandbox: continuing, as we cannot run' in ''.join(output)) + assert 'sandbox: continuing, as we cannot run' in ''.join(output) else: assert('sandbox: continuing, as we cannot run' not in ''.join(output)) @@ -119,20 +118,6 @@ def test_vboot(u_boot_console, sha_algo, padding, required): util.run_and_log(cons, [mkimage, '-F', '-k', tmpdir, '-K', dtb, '-r', fit]) - def sign_fit_norequire(sha_algo): - """Sign the FIT - - Signs the FIT and writes the signature into it. It also writes the - public key into the dtb. - - Args: - sha_algo: Either 'sha1' or 'sha256', to select the algorithm to - use. - """ - cons.log.action('%s: Sign images' % sha_algo) - util.run_and_log(cons, [mkimage, '-F', '-k', tmpdir, '-K', dtb, - fit]) - def replace_fit_totalsize(size): """Replace FIT header's totalsize with something greater. @@ -171,7 +156,7 @@ def test_vboot(u_boot_console, sha_algo, padding, required): # Build the FIT, but don't sign anything yet cons.log.action('%s: Test FIT with signed images' % sha_algo) - make_fit('sign-images-%s%s.its' % (sha_algo , padding)) + make_fit('sign-images-%s%s.its' % (sha_algo, padding)) run_bootm(sha_algo, 'unsigned images', 'dev-', True) # Sign images with our dev keys @@ -182,7 +167,7 @@ def test_vboot(u_boot_console, sha_algo, padding, required): dtc('sandbox-u-boot.dts') cons.log.action('%s: Test FIT with signed configuration' % sha_algo) - make_fit('sign-configs-%s%s.its' % (sha_algo , padding)) + make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) run_bootm(sha_algo, 'unsigned config', '%s+ OK' % sha_algo, True) # Sign images with our dev keys @@ -195,14 +180,14 @@ def test_vboot(u_boot_console, sha_algo, padding, required): # Make sure that U-Boot checks that the config is in the list of hashed # nodes. If it isn't, a security bypass is possible. - with open(fit, 'rb') as fp: - root, strblock = vboot_forge.read_fdt(fp) + with open(fit, 'rb') as fd: + root, strblock = vboot_forge.read_fdt(fd) root, strblock = vboot_forge.manipulate(root, strblock) - with open(fit, 'w+b') as fp: - vboot_forge.write_fdt(root, strblock, fp) - util.run_and_log_expect_exception(cons, - [fit_check_sign, '-f', fit, '-k', dtb], - 1, 'Failed to verify required signature') + with open(fit, 'w+b') as fd: + vboot_forge.write_fdt(root, strblock, fd) + util.run_and_log_expect_exception( + cons, [fit_check_sign, '-f', fit, '-k', dtb], + 1, 'Failed to verify required signature') run_bootm(sha_algo, 'forged config', 'Bad Data Hash', False) @@ -235,8 +220,9 @@ def test_vboot(u_boot_console, sha_algo, padding, required): False) cons.log.action('%s: Check bad config on the host' % sha_algo) - util.run_and_log_expect_exception(cons, [fit_check_sign, '-f', fit, - '-k', dtb], 1, 'Failed to verify required signature') + util.run_and_log_expect_exception( + cons, [fit_check_sign, '-f', fit, '-k', dtb], + 1, 'Failed to verify required signature') def test_required_key(sha_algo, padding): """Test verified boot with the given hash algorithm. @@ -257,12 +243,12 @@ def test_vboot(u_boot_console, sha_algo, padding, required): # Build the FIT with prod key (keys required) and sign it. This puts the # signature into sandbox-u-boot.dtb, marked 'required' - make_fit('sign-configs-%s%s-prod.its' % (sha_algo , padding)) + make_fit('sign-configs-%s%s-prod.its' % (sha_algo, padding)) sign_fit(sha_algo) # Build the FIT with dev key (keys NOT required). This adds the # signature into sandbox-u-boot.dtb, NOT marked 'required'. - make_fit('sign-configs-%s%s.its' % (sha_algo , padding)) + make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) sign_fit(sha_algo) # So now sandbox-u-boot.dtb two signatures, for the prod and dev keys. @@ -274,7 +260,6 @@ def test_vboot(u_boot_console, sha_algo, padding, required): cons = u_boot_console tmpdir = cons.config.result_dir + '/' - tmp = tmpdir + 'vboot.tmp' datadir = cons.config.source_dir + '/test/py/tests/vboot/' fit = '%stest.fit' % tmpdir mkimage = cons.config.build_dir + '/tools/mkimage' -- cgit v1.2.3 From da76ed2795f2679ff0fa3c43f2b906157ec7c0b0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 18 Mar 2020 11:44:07 -0600 Subject: test: vboot: Move key creation into a function This code is repeated so move it into a function with a parameter. Signed-off-by: Simon Glass --- test/py/tests/test_vboot.py | 39 ++++++++++++++++++--------------------- 1 file changed, 18 insertions(+), 21 deletions(-) (limited to 'test/py/tests/test_vboot.py') diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 0b8bd9ac0b0..91305a4f0fc 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -138,6 +138,22 @@ def test_vboot(u_boot_console, sha_algo, padding, required): handle.write(struct.pack(">I", size)) return struct.unpack(">I", total_size)[0] + def create_rsa_pair(name): + """Generate a new RSA key paid and certificate + + Args: + name: Name of of the key (e.g. 'dev') + """ + public_exponent = 65537 + util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %s%s.key ' + '-pkeyopt rsa_keygen_bits:2048 ' + '-pkeyopt rsa_keygen_pubexp:%d' % + (tmpdir, name, public_exponent)) + + # Create a certificate containing the public key + util.run_and_log(cons, 'openssl req -batch -new -x509 -key %s%s.key ' + '-out %s%s.crt' % (tmpdir, name, tmpdir, name)) + def test_with_algo(sha_algo, padding): """Test verified boot with the given hash algorithm. @@ -268,27 +284,8 @@ def test_vboot(u_boot_console, sha_algo, padding, required): dtb = '%ssandbox-u-boot.dtb' % tmpdir sig_node = '/configurations/conf-1/signature' - # Create an RSA key pair - public_exponent = 65537 - util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %sdev.key ' - '-pkeyopt rsa_keygen_bits:2048 ' - '-pkeyopt rsa_keygen_pubexp:%d' % - (tmpdir, public_exponent)) - - # Create a certificate containing the public key - util.run_and_log(cons, 'openssl req -batch -new -x509 -key %sdev.key -out ' - '%sdev.crt' % (tmpdir, tmpdir)) - - # Create an RSA key pair (prod) - public_exponent = 65537 - util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %sprod.key ' - '-pkeyopt rsa_keygen_bits:2048 ' - '-pkeyopt rsa_keygen_pubexp:%d' % - (tmpdir, public_exponent)) - - # Create a certificate containing the public key (prod) - util.run_and_log(cons, 'openssl req -batch -new -x509 -key %sprod.key -out ' - '%sprod.crt' % (tmpdir, tmpdir)) + create_rsa_pair('dev') + create_rsa_pair('prod') # Create a number kernel image with zeroes with open('%stest-kernel.bin' % tmpdir, 'w') as fd: -- cgit v1.2.3 From 0e29648f8e7e0aa60c0f7efe9d2efed98f8c0c6e Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 18 Mar 2020 11:44:08 -0600 Subject: test: vboot: Reduce fake kernel size to 500 bytes We don't need 5KB to test things out. A smaller size makes it easier to look at the FIT with fdtdump. Signed-off-by: Simon Glass --- test/py/tests/test_vboot.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'test/py/tests/test_vboot.py') diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 91305a4f0fc..e67f2b3d0f6 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -289,7 +289,7 @@ def test_vboot(u_boot_console, sha_algo, padding, required): # Create a number kernel image with zeroes with open('%stest-kernel.bin' % tmpdir, 'w') as fd: - fd.write(5000 * chr(0)) + fd.write(500 * chr(0)) try: # We need to use our own device tree file. Remember to restore it -- cgit v1.2.3