Hi Ludwig, On 2026-05-13T14:08:10, Ludwig Nussel <[email protected]> wrote: > test: vboot: add iminfo checks for config signature verification > > Co-authored-by: Copilot <[email protected]> > > Signed-off-by: Ludwig Nussel <[email protected]> > > test/py/tests/test_vboot.py | 47 ++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 46 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass <[email protected]> Various improvements below. > diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py > @@ -231,6 +231,34 @@ def test_vboot(ubman, name, sha_algo, padding, > sign_options, required, > + def run_iminfo(sha_algo, test_type, expect_string, succeeds, fit=None): > + """Run an 'iminfo' command in U-Boot. > + > + This always starts a fresh U-Boot instance since the device tree may > + contain a new public key. The commit message is just a subject - please add a body summarising which cases are covered (unsigned config under FIT_REQUIRE_CONFIG_SIGS, signed config, corrupted-signature) and why -E is skipped. > diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py > @@ -231,6 +231,34 @@ def test_vboot(ubman, name, sha_algo, padding, > sign_options, required, > + assert expect_string in ''.join(output) > + if succeeds: > + assert 'FAIL' not in ''.join(output) > + else: > + assert 'FAIL' in ''.join(output) This is a near-duplicate of run_bootm() - same arg list, same restart, same expect_string assertion. The only real difference is the command list and the 'FAIL' vs 'sandbox: continuing' tail check. Please factor out a helper that takes the command list and success sentinel, and make run_bootm()/run_iminfo() thin wrappers. The current shape will tempt the next person to copy-paste again. > diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py > @@ -326,9 +354,11 @@ def test_vboot(ubman, name, sha_algo, padding, > sign_options, required, > if require_config_sigs: > # DTB has no /signature node; FIT_REQUIRE_CONFIG_SIGS makes this > - # fail-closed, so U-Boot must reject the unsigned config FIT. > + # fail-closed, so both bootm and iminfo must reject the unsigned > FIT. > run_bootm(sha_algo, 'unsigned config', > 'No signature node found', False) > + run_iminfo(sha_algo, 'unsigned config', 'No signature node > found', > + False) > else: > # No required keys in the DTB, so an unsigned config FIT is fine. > run_bootm(sha_algo, 'unsigned config', The else branch is left without an iminfo check. Since patch 5 changed iminfo to call fit_all_configurations_verify() in both modes, please run iminfo here too and assert it succeeds - that's the path where the new -ENOENT relaxation in image_info() actually matters, and it's currently uncovered. > diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py > @@ -438,6 +474,15 @@ def test_vboot(ubman, name, sha_algo, padding, > sign_options, required, > + # Test that iminfo also detects the corrupted signature. > + # Skip for external-data FIT (-E): fdtput rewrites only the FDT > + # portion of the file, truncating the appended image data, so > + # fit_all_image_verify() would fail on a missing-data hash error > + # before fit_all_configurations_verify() is ever reached. > + if '-E' not in (sign_options or ''): > + run_iminfo(sha_algo, 'corrupted config', > + 'Checking configuration signatures', False) The expect_string is just the banner that fit_all_configurations_verify() always prints - it tells us the path ran but says nothing about the failure. Combined with 'FAIL' (also produced by fit_all_image_verify() on a bad image hash), the assertion is loose. Please key on something more specific such as the per-config FAIL line preceded by the config name, or 'Bad Data Hash' / the signature-verify error string, so a regression in fit_all_image_verify() ordering can't silently satisfy this test. Regards, Simon
