Re: [PATCH 3/3] binman: add test for sign option

On 3/21/22 5:43 PM, Ivan Mikhaylov wrote:

Add the test which provides sequence of actions:

1. create the image from binman dts
2. create public and private keys
3. add public key into dtb with fdt_add_pubkey
4. sign image with new sign option
5. check with fit_check_sign

Signed-off-by: Ivan Mikhaylov ivan.mikhaylov@siemens.com

tools/binman/ftest.py | 42 +++++++++++++++++++ tools/binman/test/225_fit_sign.dts | 67 ++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+) create mode 100644 tools/binman/test/225_fit_sign.dts

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 8f00db6945..8a3d3720c4 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3088,6 +3088,48 @@ class TestFunctional(unittest.TestCase): self.assertEqual(len(U_BOOT_DATA), entry.contents_size) self.assertEqual(len(U_BOOT_DATA), entry.size)

• def testSignSimple(self):

•    """Test signing a single file"""
  

•    data = self._DoReadFileRealDtb('225_fit_sign.dts')
  

•    updated_fname = tools.GetOutputFilename('image-updated.bin')
  

•    tools.WriteFile(updated_fname, data)
  

•    outdir = os.path.join(self._indir, 'extract')
  

•    einfos = control.ExtractEntries(updated_fname, None, outdir, [])
  

•    dtb = tools.GetOutputFilename('source.dtb')
  

•    private_key = tools.GetOutputFilename('test_key.key')
  

•    public_key = tools.GetOutputFilename('test_key.crt')
  

•    fit = tools.GetOutputFilename('fit.fit')
  

•    key_dir = tools.GetOutputDir()
  

•    def check_sign(fit, key):
  

please inline this, since it is only called once

•        try:
  

•            tools.Run('fit_check_sign', '-k', key, '-f', fit)
  

•        except:
  

•            return False
  

you can just do a bare tools.Run() and if an exception is raised it will cause the test to fail.

•        return True
  

•    is_signed = False
  

•    try:
  

•        tools.Run('openssl', 'req', '-batch' , '-newkey', 'rsa:4096',
  

•                  '-sha256', '-new',  '-nodes',  '-x509', '-keyout',
  

•                  private_key, '-out', public_key)
  

•        tools.Run('fdt_add_pubkey', '-a', 'sha256,rsa4096', '-k', key_dir,
  

•                  '-n', 'test_key', dtb)
  

•        with test_util.capture_sys_output() as (stdout, stderr):
  

•            # do sign with private key
  

•            self._DoBinman('sign', '-i', updated_fname, '-k', private_key,
  

•                           '-a', 'sha256,rsa4096', '-f', fit, 'fit')
  

•            is_signed = check_sign(fit, dtb)
  

•    finally:
  

•        shutil.rmtree(key_dir)
  

•    self.assertEqual(is_signed, True)
  

so no need for this assert here

• def _RunReplaceCmd(self, entry_name, data, decomp=True, allow_resize=True, dts='132_replace.dts'): """Replace an entry in an image

diff --git a/tools/binman/test/225_fit_sign.dts b/tools/binman/test/225_fit_sign.dts new file mode 100644 index 0000000000..2bfa826106 --- /dev/null +++ b/tools/binman/test/225_fit_sign.dts @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+

+/dts-v1/;

+/ {

• #address-cells = <1>;
• #size-cells = <1>;
• binman {

I don't really understand what's going on in this test case.

• size = <0x100000>;
  

• allow-repack;
  

• u-boot {
  

• };
  

• fit {
  

• 	description = "U-Boot";
  

• 	offset = <0x10000>;
  

• 	images {
  

• 		u-boot-1 {
  

• 			description = "U-Boot";
  

• 			type = "standalone";
  

• 			arch = "arm64";
  

• 			os = "u-boot";
  

• 			compression = "none";
  

• 			hash-1 {
  

• 				algo = "sha256";
  

• 			};
  

Shouldn't there be some kind of data here?

• 		};
  

• 		fdt-1 {
  

Maybe use things like @fdt-SEQ, as seen in tools/binman/test/170_fit_fdt.dts?

• 			description = "test.dtb";
  

• 			type = "flat_dt";
  

• 			arch = "arm64";
  

• 			compression = "none";
  

• 			hash-1 {
  

• 				algo = "sha256";
  

• 			};
  

• 			u-boot-spl-dtb {
  

• 			};
  

• 		};
  

• 	};
  

• 	configurations {
  

• 		default = "conf-1";
  

• 		conf-1 {
  

• 			description = "u-boot with fdt";
  

• 			firmware = "u-boot-1";
  

• 			fdt = "fdt-1";
  

• 			signature-1 {
  

• 				algo = "sha256,rsa4096";
  

• 				key-name-hint = "test_key";
  

• 				sign-images = "firmware", "fdt";
  

• 			};
  

• 		};
  

• 	};
  

• };
  

• u-boot-nodtb {
  

• };
  

Why do we have U-Boot again here without a device tree?

• fdtmap {
  

• };
  

• };

+};

What is U-Boot supposed to be verified by? Shouldn't this package SPL? I guess that is out of scope?

--Sean