Hi Ayoub,
On 2026-10-28T10:31:01+11:00, Ayoub Zaki <[email protected]> wrote:
> test: fit: Cover private key filename fallbacks
>
> Test RSA and ECDSA signing with each suffix alone. Test both suffixes
> with different keys to check which key is selected. Verify each image
> signature against the expected public key.
>
> Check that an invalid or unreadable preferred key prevents fallback.
> Check that an unreadable fallback key makes mkimage fail. Check that
> missing-key errors name both attempted paths.
>
> Check that explicit -G paths do not fall back to another suffix.
> Missing keys remain optional.
>
> Signed-off-by: Ayoub Zaki <[email protected]>
>
> test/py/tests/test_fit_key_fallback.py | 121
> +++++++++++++++++++++++++++++++++
> 1 file changed, 121 insertions(+)
> diff --git a/test/py/tests/test_fit_key_fallback.py
> b/test/py/tests/test_fit_key_fallback.py
> @@ -0,0 +1,121 @@
> + utils.run_and_log(ubman, [mkimage, '-D', f'-I dts -O dtb -i {tmp_path}',
> + '-f', str(datadir / 'sign-images-sha256.its'),
> + fit])
> + nodes = ['/images/kernel', '/images/fdt-1']
> + for node in nodes:
> + utils.run_and_log(ubman, ['fdtput', '-ts', fit, f'{node}/signature',
> + 'algo', f'sha256,{algo}'])
One thing I'd like to check: 'sign-images-sha256.its' no longer
exists. Commit a6351dbef18 ("test: py: vboot: prepare integration test
for ecdsa") renames it to 'sign-images-sha256-rsa2048.its' and adds
'sign-images-sha256-ecdsa256.its' alongside it, so every case should
fail at this step. Did this pass for you on a tree from before that
rename? Please can you rebase and use the per-algorithm ITS files,
e.g. f'sign-images-sha256-{algo}.its' ? Then you can drop the fdtput
loop and the fdtput requiredtool marker.
> diff --git a/test/py/tests/test_fit_key_fallback.py
> b/test/py/tests/test_fit_key_fallback.py
> @@ -0,0 +1,121 @@
> + if case in ('preferred', 'both', 'unreadable'):
> + write_key(preferred, key)
> + if case == 'both':
> + write_key(fallback, other_key)
> + elif case != 'preferred' and case != 'missing':
> + write_key(fallback, key)
> + if case in ('invalid', 'explicit'):
> + preferred.write_text('Not a PEM private key\n')
This is hard to follow: to know what is on disk for a given case you
have to work through three overlapping conditions, and 'invalid'
writes the preferred file only after the fallback one. The checks at
the end have the same problem. It would be worth making the
parametrisation a table, with one tuple per case giving the preferred
file contents, the fallback file contents, whether to use -G and the
expected result, so the body has no case-name checks at all. What do
you think?
> diff --git a/test/py/tests/test_fit_key_fallback.py
> b/test/py/tests/test_fit_key_fallback.py
> @@ -0,0 +1,121 @@
> + if case == 'explicit':
> + key_args = ['-G', str(fallback)]
The commit message says this checks 'that explicit -G paths do not
fall back to another suffix', but the path given to -G here is the
valid key, so a fallback would never be triggered. It is
'explicit-missing' that actually tests no-fallback; this one just
shows that -G uses the file it is given, even when the other suffix
holds an invalid key. Perhaps reword the commit message to match?
Regards,
Simon
