[PATCH 0/2] test/py: vboot: fix signature check on config node

The signature check of config node is broken when used on fit with padding. We didn't see it before because this case is not covered by vboot test.
When check the signature for a config nde, u-boot uses all the properties of the node referenced in the config node, except the property data. When padding is used on fit, the property data is replaced by two properties: data-offset and data-size, and u-boot uses those properties when checking the signature. To fix this signature check, we simply ignore the properties data-offset and data_size.
The first commit add some vboot tests that check signature on fit with padding. The second commit fixes the signature check on config node for fit with padding.
Philippe Reynes (2): test/py: vboot: add a test to check fit signature on fit with padding rsa: sig: fix config signature check for fit with padding
common/image-sig.c | 2 +- test/py/tests/test_vboot.py | 42 ++++++++++++++++++++++++++++-------------- 2 files changed, 29 insertions(+), 15 deletions(-)

The pytest vboot does all his tests on fit without padding. We add the same tests on fit with padding.
Signed-off-by: Philippe Reynes philippe.reynes@softathome.com --- test/py/tests/test_vboot.py | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-)
diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py index 9c41ee5..32299be 100644 --- a/test/py/tests/test_vboot.py +++ b/test/py/tests/test_vboot.py @@ -94,7 +94,7 @@ def test_vboot(u_boot_console): util.run_and_log(cons, [mkimage, '-D', dtc_args, '-f', '%s%s' % (datadir, its), fit])
- def sign_fit(sha_algo): + def sign_fit(sha_algo, options): """Sign the FIT
Signs the FIT and writes the signature into it. It also writes the @@ -103,10 +103,13 @@ def test_vboot(u_boot_console): Args: sha_algo: Either 'sha1' or 'sha256', to select the algorithm to use. + options: Options to provide to mkimage. """ + args = [mkimage, '-F', '-k', tmpdir, '-K', dtb, '-r', fit] + if options: + args += options.split(' ') cons.log.action('%s: Sign images' % sha_algo) - util.run_and_log(cons, [mkimage, '-F', '-k', tmpdir, '-K', dtb, - '-r', fit]) + util.run_and_log(cons, args)
def sign_fit_norequire(sha_algo): """Sign the FIT @@ -142,7 +145,7 @@ def test_vboot(u_boot_console): handle.write(struct.pack(">I", size)) return struct.unpack(">I", total_size)[0]
- def test_with_algo(sha_algo, padding): + def test_with_algo(sha_algo, padding, sign_options): """Test verified boot with the given hash algorithm.
This is the main part of the test code. The same procedure is followed @@ -151,6 +154,9 @@ def test_vboot(u_boot_console): Args: sha_algo: Either 'sha1' or 'sha256', to select the algorithm to use. + padding: Either '' or '-pss', to select the padding to use for the + rsa signature algorithm. + sign_options: Options to mkimage when signing a fit image. """ # Compile our device tree files for kernel and U-Boot. These are # regenerated here since mkimage will modify them (by adding a @@ -164,7 +170,7 @@ def test_vboot(u_boot_console): run_bootm(sha_algo, 'unsigned images', 'dev-', True)
# Sign images with our dev keys - sign_fit(sha_algo) + sign_fit(sha_algo, sign_options) run_bootm(sha_algo, 'signed images', 'dev+', True)
# Create a fresh .dtb without the public keys @@ -175,7 +181,7 @@ def test_vboot(u_boot_console): run_bootm(sha_algo, 'unsigned config', '%s+ OK' % sha_algo, True)
# Sign images with our dev keys - sign_fit(sha_algo) + sign_fit(sha_algo, sign_options) run_bootm(sha_algo, 'signed config', 'dev+', True)
cons.log.action('%s: Check signed config on the host' % sha_algo) @@ -211,7 +217,7 @@ def test_vboot(u_boot_console): 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): + def test_required_key(sha_algo, padding, sign_options): """Test verified boot with the given hash algorithm.
This function test if u-boot reject an image when a required @@ -220,6 +226,9 @@ def test_vboot(u_boot_console): Args: sha_algo: Either 'sha1' or 'sha256', to select the algorithm to use. + padding: Either '' or '-pss', to select the padding to use for the + rsa signature algorithm. + sign_options: Options to mkimage when signing a fit image. """ # Compile our device tree files for kernel and U-Boot. These are # regenerated here since mkimage will modify them (by adding a @@ -234,9 +243,9 @@ def test_vboot(u_boot_console): # 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) make_fit('sign-configs-%s%s-prod.its' % (sha_algo , padding)) - sign_fit(sha_algo) + sign_fit(sha_algo, sign_options) make_fit('sign-configs-%s%s.its' % (sha_algo , padding)) - sign_fit(sha_algo) + sign_fit(sha_algo, sign_options)
run_bootm(sha_algo, 'signed configs', '', False)
@@ -282,11 +291,16 @@ 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') + test_with_algo('sha1','', None) + test_with_algo('sha1','', '-E -p 0x10000') + test_with_algo('sha1','-pss', None) + test_with_algo('sha1','-pss', '-E -p 0x10000') + test_with_algo('sha256','', None) + test_with_algo('sha256','', '-E -p 0x10000') + test_with_algo('sha256','-pss', None) + test_with_algo('sha256','-pss', '-E -p 0x10000') + test_required_key('sha256','-pss', None) + test_required_key('sha256','-pss', '-E -p 0x10000') finally: # Go back to the original U-Boot with the correct dtb. cons.config.dtb = old_dtb

On Fri, 27 Mar 2020 at 08:55, Philippe Reynes philippe.reynes@softathome.com wrote:
The pytest vboot does all his tests on fit without padding. We add the same tests on fit with padding.
Signed-off-by: Philippe Reynes philippe.reynes@softathome.com
test/py/tests/test_vboot.py | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

The signature check on config node is broken on fit with padding. To compute the signature for config node, u-boot compute the signature on all properties of requested node for this config, except for the property "data". But, when padding is used for binary in a fit, there isn't a property "data" but two properties: "data-offset" and "data-size". So to fix the check of signature, we also dont use the properties "data-offset" and "data-size" when checking the signature on config node.
Signed-off-by: Philippe Reynes philippe.reynes@softathome.com --- common/image-sig.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/image-sig.c b/common/image-sig.c index 639a112..8a0ea28 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -362,7 +362,7 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset, int fit_config_check_sig(const void *fit, int noffset, int required_keynode, char **err_msgp) { - char * const exc_prop[] = {"data"}; + char * const exc_prop[] = {"data", "data-size", "data-position"}; const char *prop, *end, *name; struct image_sign_info info; const uint32_t *strings;

Hi Philippe,
On Fri, 27 Mar 2020 at 08:55, Philippe Reynes philippe.reynes@softathome.com wrote:
The signature check on config node is broken on fit with padding. To compute the signature for config node, u-boot compute the
U-Boot
signature on all properties of requested node for this config, except for the property "data". But, when padding is used for binary in a fit, there isn't a property "data" but two properties: "data-offset" and "data-size". So to fix the check of signature, we also dont use the properties "data-offset" and "data-size"
don't
when checking the signature on config node.
Signed-off-by: Philippe Reynes philippe.reynes@softathome.com
common/image-sig.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/common/image-sig.c b/common/image-sig.c index 639a112..8a0ea28 100644 --- a/common/image-sig.c +++ b/common/image-sig.c @@ -362,7 +362,7 @@ int fit_image_verify_required_sigs(const void *fit, int image_noffset, int fit_config_check_sig(const void *fit, int noffset, int required_keynode, char **err_msgp) {
char * const exc_prop[] = {"data"};
char * const exc_prop[] = {"data", "data-size", "data-position"}; const char *prop, *end, *name; struct image_sign_info info; const uint32_t *strings;
-- 2.7.4
participants (2)
-
Philippe Reynes
-
Simon Glass