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

[Sean Anderson]

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 at 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