Hi Eva,
On 2026-09-25T07:27:39, Eva Crystal <[email protected]> wrote:
> test: vboot: add coverage for a control FDT with no /signature node
>
> Build a control devicetree with no "/signature" node and an unsigned
> FIT, then check that the boot is refused when
> CONFIG_FIT_REQUIRE_IMAGE_SIGS is enabled, and that the historical
> fail-open behaviour is kept when it is not.
>
> The missing node is reported whether or not the option is enabled, so
> the warning is expected in both cases and only the boot outcome
> differs.
>
> Cc: Jonas Karlman <[email protected]>
> Signed-off-by: Eva Crystal <[email protected]>
>
> test/py/tests/test_vboot.py | 109 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 109 insertions(+)
> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> @@ -853,3 +853,112 @@ def test_fdt_add_pubkey(ubman, name, sha_algo, padding,
> sign_options, algo_arg):
> + bcfg = ubman.config.buildconfig
> + require_image_sigs = bcfg.get('config_fit_require_image_sigs', False)
No defconfig enables FIT_REQUIRE_IMAGE_SIGS, so CI only runs the
fail-open branch, never the one that tests the new option. Also, as
1/2 notes, enabling the option on sandbox makes 13 existing test_vboot
cases fail, so nobody can turn it on without the suite going red.
Please can you deal with both? One approach is to add
@pytest.mark.notbuildconfigspec('fit_require_image_sigs') to
test_vboot(), or have it expect the refusal where relevant, then
enable the option in a sandbox config that CI builds (e.g.
sandbox_flattree or a new small defconfig) or a CI job, so the refused
path runs. Without that, the most important half of this test is dead
code.
> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> @@ -853,3 +853,112 @@ def test_fdt_add_pubkey(ubman, name, sha_algo, padding,
> sign_options, algo_arg):
> + else:
> + # Fail-open is kept: the image still verifies and the board
> + # boots, but the condition is no longer silent.
> + run_bootm('no signature node',
> + [warning, 'Verifying Hash Integrity'],
> + True)
test_vboot() already covers this case: test_with_algo() builds a fresh
sandbox-u-boot.dtb with no keys, builds sign-configs-*.its and checks
that 'unsigned config' boots. The only new thing here is the warning
check. I suspect it would be simpler to add the warning assertion (and
the refusal expectation, per the comment above) to that existing
run_bootm() call, rather than adding a near-copy of the setup and of
run_bootm(). What do you think?
Also, 'Verifying Hash Integrity' is printed before any check is done,
so it shows nothing here. If you keep this, 'sha256+ OK' would at
least pin down the output you describe in patch 1.
> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> @@ -853,3 +853,112 @@ def test_fdt_add_pubkey(ubman, name, sha_algo, padding,
> sign_options, algo_arg):
> [email protected]('fdtget')
> [email protected]('openssl')
> +def test_vboot_no_sig_node(ubman):
No keys are created here, so openssl isn't needed. Please drop that marker.
> diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> @@ -853,3 +853,112 @@ def test_fdt_add_pubkey(ubman, name, sha_algo, padding,
> sign_options, algo_arg):
> + run_bootm('no signature node',
> + [warning, 'Unable to verify required signature'],
> + False)
The full message here is "Unable to verify required signature for ''
hash node in 'kernel' image node". The empty name is misleading, since
no hash node is involved when the control FDT has no /signature node.
The message comes from 1/2, so it may be worth fixing there. It would
also be good to assert that the warning appears only once, since 'once
per boot' is part of the contract in 1/2. The FIT has two images, so a
count check would catch a regression.
Regards,
Simon
