On Thu, 2023-03-16 at 07:59 -0600, Simon Glass wrote: > Hi Ivan, > > On Wed, 15 Mar 2023 at 19:17, Ivan Mikhaylov <fr0st6...@gmail.com> > wrote: > > > > On Fri, 2023-03-10 at 17:46 -0800, Simon Glass wrote: > > > Hi Ivan, > > > > > > On Tue, 7 Mar 2023 at 14:13, Ivan Mikhaylov <fr0st6...@gmail.com> > > > wrote: > > > > > > > > From: Roman Kopytin <roman.kopy...@kaspersky.com> > > > > > > > > Signed-off-by: Roman Kopytin <roman.kopy...@kaspersky.com> > > > > Cc: Rasmus Villemoes <rasmus.villem...@prevas.dk> > > > > --- > > > > test/py/tests/test_vboot.py | 8 ++++++++ > > > > 1 file changed, 8 insertions(+) > > > > > > > > diff --git a/test/py/tests/test_vboot.py > > > > b/test/py/tests/test_vboot.py > > > > index e3e7ca4b21..956b8fcd43 100644 > > > > --- a/test/py/tests/test_vboot.py > > > > +++ b/test/py/tests/test_vboot.py > > > > @@ -313,6 +313,13 @@ def test_vboot(u_boot_console, name, > > > > sha_algo, > > > > padding, sign_options, required, > > > > > > > > util.run_and_log(cons, [fit_check_sign, '-f', fit, '- > > > > k', > > > > dtb]) > > > > > > > > + # Create a fresh .dtb without the public keys > > > > + dtc('sandbox-u-boot.dts') > > > > + # Then add the dev key via the fdt_add_pubkey tool > > > > + util.run_and_log(cons, [fdt_add_pubkey, '-a', > > > > '%s,rsa2048' > > > > % sha_algo, > > > > + '-k', tmpdir, '-n', 'dev', '- > > > > r', > > > > 'conf', dtb]) > > > > + util.run_and_log(cons, [fit_check_sign, '-f', fit, '- > > > > k', > > > > dtb]) > > > > + > > > > if full_test: > > > > # 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. > > > > @@ -500,6 +507,7 @@ def test_vboot(u_boot_console, name, > > > > sha_algo, > > > > padding, sign_options, required, > > > > mkimage = cons.config.build_dir + '/tools/mkimage' > > > > binman = cons.config.source_dir + '/tools/binman/binman' > > > > fit_check_sign = cons.config.build_dir + > > > > '/tools/fit_check_sign' > > > > + fdt_add_pubkey = cons.config.build_dir + > > > > '/tools/fdt_add_pubkey' > > > > dtc_args = '-I dts -O dtb -i %s' % tmpdir > > > > dtb = '%ssandbox-u-boot.dtb' % tmpdir > > > > sig_node = '/configurations/conf-1/signature' > > > > -- > > > > 2.39.1 > > > > > > > > > > Unfortunately this test fails on sandbox: > > > > > > https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/591975 > > > > > > I think it would be better to put it in its own test (perhaps in > > > the > > > same file) so we are not doing it on every test run. Also you > > > could > > > check (in a very basic way) that it adds the key correctly since > > > we > > > don't really need another test of the logic of doing that. We are > > > just > > > checking that your tool calls that logic correctly. > > > > > > I'll drop this one when applying, for now. Please take a look. > > > > > > Regards, > > > Simon > > > > Simon, does it look ok? Test for test_vboot is passed fine. > > Please see my message before: > > > Unfortunately this test fails on sandbox: > > > > https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/591975 > > > > I think it would be better to put it in its own test (perhaps in > > the > > same file) so we are not doing it on every test run. Also you could > > check (in a very basic way) that it adds the key correctly since we > > don't really need another test of the logic of doing that. We are > > just > > checking that your tool calls that logic correctly. > > > > I'll drop this one when applying, for now. Please take a look. > > I have not applied this patch due to the problem. > > Regards, > Simon >
Simon, but I've changed the test and put it in previous note, maybe you didn't notice, I did what you asked: - made own test "test_fdt_add_pubkey" - simple check that with clear dtb you can add keys with fdt_add_pubkey and check with fit_check_sign with signed fit. I've checked that change with sandbox and it passes test_vboot well. Need I re-submit that patch or whole series? Thanks. > > > > > > Thanks. > > > > From 5484d525d4950b064adf1204f5bf055229c942ac Mon Sep 17 00:00:00 > > 2001 > > From: Roman Kopytin <roman.kopy...@kaspersky.com> > > Date: Thu, 11 Nov 2021 11:15:12 +0300 > > Subject: [PATCH v3] test_vboot.py: include test of fdt_add_pubkey > > tool > > > > Signed-off-by: Roman Kopytin <roman.kopy...@kaspersky.com> > > Signed-off-by: Ivan Mikhaylov <fr0st6...@gmail.com> > > Cc: Rasmus Villemoes <rasmus.villem...@prevas.dk> > > --- > > test/py/tests/test_vboot.py | 33 +++++++++++++++++++++++++++++++++ > > 1 file changed, 33 insertions(+) > > > > diff --git a/test/py/tests/test_vboot.py > > b/test/py/tests/test_vboot.py > > index e3e7ca4b21..5ae622fe21 100644 > > --- a/test/py/tests/test_vboot.py > > +++ b/test/py/tests/test_vboot.py > > @@ -491,6 +491,37 @@ def test_vboot(u_boot_console, name, sha_algo, > > padding, sign_options, required, > > # Check that the boot fails if the global signature is not > > provided > > run_bootm(sha_algo, 'global image signature', 'signature > > is > > mandatory', False) > > > > + def test_fdt_add_pubkey(sha_algo, padding, sign_options): > > + """Test fdt_add_pubkey utility with given hash algorithm > > and > > padding. > > + > > + This function tests if fdt_add_pubkey utility may add > > public > > keys into dtb. > > + > > + 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. > > + """ > > + > > + # Create a fresh .dtb without the public keys > > + dtc('sandbox-u-boot.dts') > > + make_fit('sign-configs-%s%s.its' % (sha_algo, padding)) > > + > > + # Sign images with our dev keys > > + sign_fit(sha_algo, sign_options) > > + > > + # Create a fresh .dtb without the public keys > > + dtc('sandbox-u-boot.dts') > > + > > + cons.log.action('%s: Test fdt_add_pubkey with signed > > configuration' % sha_algo) > > + # Then add the dev key via the fdt_add_pubkey tool > > + util.run_and_log(cons, [fdt_add_pubkey, '-a', '%s,%s' % > > ('sha256' if algo_arg else sha_algo, \ > > + 'rsa3072' if sha_algo == 'sha384' > > else > > 'rsa2048'), > > + '-k', tmpdir, '-n', 'dev', '-r', > > 'conf', dtb]) > > + > > + # Check with fit_check_sign that FIT is signed with key > > + util.run_and_log(cons, [fit_check_sign, '-f', fit, '-k', > > dtb]) > > + > > cons = u_boot_console > > tmpdir = os.path.join(cons.config.result_dir, name) + '/' > > if not os.path.exists(tmpdir): > > @@ -500,6 +531,7 @@ def test_vboot(u_boot_console, name, sha_algo, > > padding, sign_options, required, > > mkimage = cons.config.build_dir + '/tools/mkimage' > > binman = cons.config.source_dir + '/tools/binman/binman' > > fit_check_sign = cons.config.build_dir + > > '/tools/fit_check_sign' > > + fdt_add_pubkey = cons.config.build_dir + > > '/tools/fdt_add_pubkey' > > dtc_args = '-I dts -O dtb -i %s' % tmpdir > > dtb = '%ssandbox-u-boot.dtb' % tmpdir > > sig_node = '/configurations/conf-1/signature' > > @@ -516,6 +548,7 @@ def test_vboot(u_boot_console, name, sha_algo, > > padding, sign_options, required, > > with open(evil_kernel, 'wb') as fd: > > fd.write(500 * b'\x01') > > > > + test_fdt_add_pubkey(sha_algo, padding, sign_options) > > try: > > # We need to use our own device tree file. Remember to > > restore > > it > > # afterwards. > > -- > > 2.39.1 > > > >