Re: [PATCH v3 1/1] rsa: adds rsa3072 algorithm

[Jamin Lin <jamin_lin@aspeedtech.com>]

The 01/18/2022 07:02, Jamin Lin wrote:
> The 01/14/2022 18:14, Tom Rini wrote:
> > On Fri, Dec 10, 2021 at 02:00:55PM +0800, Jamin Lin wrote:
> > 
> > > Add to support rsa 3072 bits algorithm in tools
> > > for image sign at host side and adds rsa 3072 bits
> > > verification in the image binary.
> > > 
> > > Add test case in vboot for sha384 with rsa3072 algorithm testing.
> > > 
> > > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > > ---
> > >  include/u-boot/rsa.h                        |  1 +
> > >  lib/rsa/rsa-verify.c                        |  6 +++
> > >  test/py/tests/test_vboot.py                 | 12 +++++-
> > >  test/py/tests/vboot/sign-configs-sha384.its | 45 +++++++++++++++++++++
> > >  test/py/tests/vboot/sign-images-sha384.its  | 42 +++++++++++++++++++
> > >  tools/image-sig-host.c                      |  7 ++++
> > >  6 files changed, 111 insertions(+), 2 deletions(-)
> > >  create mode 100644 test/py/tests/vboot/sign-configs-sha384.its
> > >  create mode 100644 test/py/tests/vboot/sign-images-sha384.its
> > > 
> > > diff --git a/include/u-boot/rsa.h b/include/u-boot/rsa.h
> > > index 7556aa5b4b..bb56c2243c 100644
> > > --- a/include/u-boot/rsa.h
> > > +++ b/include/u-boot/rsa.h
> > > @@ -110,6 +110,7 @@ int padding_pss_verify(struct image_sign_info *info,
> > >  #define RSA_DEFAULT_PADDING_NAME		"pkcs-1.5"
> > >  
> > >  #define RSA2048_BYTES	(2048 / 8)
> > > +#define RSA3072_BYTES	(3072 / 8)
> > >  #define RSA4096_BYTES	(4096 / 8)
> > >  
> > >  /* This is the minimum/maximum key size we support, in bits */
> > > diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
> > > index 83f7564101..4fe487d7e5 100644
> > > --- a/lib/rsa/rsa-verify.c
> > > +++ b/lib/rsa/rsa-verify.c
> > > @@ -588,6 +588,12 @@ U_BOOT_CRYPTO_ALGO(rsa2048) = {
> > >  	.verify = rsa_verify,
> > >  };
> > >  
> > > +U_BOOT_CRYPTO_ALGO(rsa3072) = {
> > > +	.name = "rsa3072",
> > > +	.key_len = RSA3072_BYTES,
> > > +	.verify = rsa_verify,
> > > +};
> > > +
> > >  U_BOOT_CRYPTO_ALGO(rsa4096) = {
> > >  	.name = "rsa4096",
> > >  	.key_len = RSA4096_BYTES,
> > > diff --git a/test/py/tests/test_vboot.py b/test/py/tests/test_vboot.py
> > > index 095e00cce3..b080d482af 100644
> > > --- a/test/py/tests/test_vboot.py
> > > +++ b/test/py/tests/test_vboot.py
> > > @@ -45,6 +45,8 @@ TESTDATA = [
> > >      ['sha256-pss-pad', 'sha256', '-pss', '-E -p 0x10000', False, False],
> > >      ['sha256-pss-required', 'sha256', '-pss', None, True, False],
> > >      ['sha256-pss-pad-required', 'sha256', '-pss', '-E -p 0x10000', True, True],
> > > +    ['sha384-basic', 'sha384', '', None, False, False],
> > > +    ['sha384-pad', 'sha384', '', '-E -p 0x10000', False, False],
> > >  ]
> > >  
> > >  @pytest.mark.boardspec('sandbox')
> > > @@ -180,10 +182,16 @@ def test_vboot(u_boot_console, name, sha_algo, padding, sign_options, required,
> > >              name: Name of of the key (e.g. 'dev')
> > >          """
> > >          public_exponent = 65537
> > > +
> > > +        if sha_algo == "sha384":
> > > +            rsa_keygen_bits = 3072
> > > +        else:
> > > +            rsa_keygen_bits = 2048
> > > +
> > >          util.run_and_log(cons, 'openssl genpkey -algorithm RSA -out %s%s.key '
> > > -                     '-pkeyopt rsa_keygen_bits:2048 '
> > > +                     '-pkeyopt rsa_keygen_bits:%d '
> > >                       '-pkeyopt rsa_keygen_pubexp:%d' %
> > > -                     (tmpdir, name, public_exponent))
> > > +                     (tmpdir, name, rsa_keygen_bits, public_exponent))
> > >  
> > >          # Create a certificate containing the public key
> > >          util.run_and_log(cons, 'openssl req -batch -new -x509 -key %s%s.key '
> > > diff --git a/test/py/tests/vboot/sign-configs-sha384.its b/test/py/tests/vboot/sign-configs-sha384.its
> > > new file mode 100644
> > > index 0000000000..2869401991
> > > --- /dev/null
> > > +++ b/test/py/tests/vboot/sign-configs-sha384.its
> > > @@ -0,0 +1,45 @@
> > > +/dts-v1/;
> > > +
> > > +/ {
> > > +	description = "Chrome OS kernel image with one or more FDT blobs";
> > > +	#address-cells = <1>;
> > > +
> > > +	images {
> > > +		kernel {
> > > +			data = /incbin/("test-kernel.bin");
> > > +			type = "kernel_noload";
> > > +			arch = "sandbox";
> > > +			os = "linux";
> > > +			compression = "none";
> > > +			load = <0x4>;
> > > +			entry = <0x8>;
> > > +			kernel-version = <1>;
> > > +			hash-1 {
> > > +				algo = "sha384";
> > > +			};
> > > +		};
> > > +		fdt-1 {
> > > +			description = "snow";
> > > +			data = /incbin/("sandbox-kernel.dtb");
> > > +			type = "flat_dt";
> > > +			arch = "sandbox";
> > > +			compression = "none";
> > > +			fdt-version = <1>;
> > > +			hash-1 {
> > > +				algo = "sha384";
> > > +			};
> > > +		};
> > > +	};
> > > +	configurations {
> > > +		default = "conf-1";
> > > +		conf-1 {
> > > +			kernel = "kernel";
> > > +			fdt = "fdt-1";
> > > +			signature {
> > > +				algo = "sha384,rsa3072";
> > > +				key-name-hint = "dev";
> > > +				sign-images = "fdt", "kernel";
> > > +			};
> > > +		};
> > > +	};
> > > +};
> > > diff --git a/test/py/tests/vboot/sign-images-sha384.its b/test/py/tests/vboot/sign-images-sha384.its
> > > new file mode 100644
> > > index 0000000000..be1a9a653c
> > > --- /dev/null
> > > +++ b/test/py/tests/vboot/sign-images-sha384.its
> > > @@ -0,0 +1,42 @@
> > > +/dts-v1/;
> > > +
> > > +/ {
> > > +	description = "Chrome OS kernel image with one or more FDT blobs";
> > > +	#address-cells = <1>;
> > > +
> > > +	images {
> > > +		kernel {
> > > +			data = /incbin/("test-kernel.bin");
> > > +			type = "kernel_noload";
> > > +			arch = "sandbox";
> > > +			os = "linux";
> > > +			compression = "none";
> > > +			load = <0x4>;
> > > +			entry = <0x8>;
> > > +			kernel-version = <1>;
> > > +			signature {
> > > +				algo = "sha384,rsa3072";
> > > +				key-name-hint = "dev";
> > > +			};
> > > +		};
> > > +		fdt-1 {
> > > +			description = "snow";
> > > +			data = /incbin/("sandbox-kernel.dtb");
> > > +			type = "flat_dt";
> > > +			arch = "sandbox";
> > > +			compression = "none";
> > > +			fdt-version = <1>;
> > > +			signature {
> > > +				algo = "sha384,rsa3072";
> > > +				key-name-hint = "dev";
> > > +			};
> > > +		};
> > > +	};
> > > +	configurations {
> > > +		default = "conf-1";
> > > +		conf-1 {
> > > +			kernel = "kernel";
> > > +			fdt = "fdt-1";
> > > +		};
> > > +	};
> > > +};
> > > diff --git a/tools/image-sig-host.c b/tools/image-sig-host.c
> > > index 8ed6998dab..d0133aec4c 100644
> > > --- a/tools/image-sig-host.c
> > > +++ b/tools/image-sig-host.c
> > > @@ -55,6 +55,13 @@ struct crypto_algo crypto_algos[] = {
> > >  		.add_verify_data = rsa_add_verify_data,
> > >  		.verify = rsa_verify,
> > >  	},
> > > +	{
> > > +		.name = "rsa3072",
> > > +		.key_len = RSA3072_BYTES,
> > > +		.sign = rsa_sign,
> > > +		.add_verify_data = rsa_add_verify_data,
> > > +		.verify = rsa_verify,
> > > +	},
> > >  	{
> > >  		.name = "rsa4096",
> > >  		.key_len = RSA4096_BYTES,
> > 
> > With current master these tests run and fail:
> > https://source.denx.de/u-boot/u-boot/-/jobs/376757 (and also fail for me
> > when running locally), please re-check and resubmit, thanks.
> > 
> > -- 
> > Tom
> 
> Hi Tom,
> Thanks for review.
> I noticed that the latest version of u-boot test vboot failed for sha384.
> So far, u-boot support sha256, sha384 and sha512 for hash algorithm.
> And supports RSA 2048 and 4096 bits.
> 
> I tried to add test cases in test_vboot.py but I encountered verified failed issue
> only if hash algorithm was "sha384"
> 
> For example:
> I created two test files which were sign-images-sha384.its and sign-configs-sha384.its and
> placed them here, https://source.denx.de/u-boot/u-boot/-/tree/master/test/py/tests/vboot 
> The contents of both files were very similar sign-images-sha256.its and sign-images-sha256.its.
> The difference was that I modified to use sha384 with RSA 2048.
> I tested sha256/rsa2048, sha256/rsa512 pass but failed in sha384.
Sorry for typo, sha512/rsa4096
> Do you have any idea or could you please give any suggestion?
> Could you please help me to check this issue?
> 
> https://source.denx.de/u-boot/u-boot/-/jobs/376757 
> The CI showed incorrect hash data for sha384.
> My local got the same test result
> Thanks-Jamin
> 
> ## Loading kernel from FIT Image at 00000100 ...
>    Using 'conf-1' configuration
>    Verifying Hash Integrity ... OK
>    Trying 'kernel' kernel subimage
>      Description:  unavailable
>      Created:      2022-01-18   6:39:31 UTC
>      Type:         Kernel Image (no loading done)
>      Compression:  uncompressed
>      Data Start:   0x000001c4
>      Data Size:    500 Bytes = 500 Bytes
>      Sign algo:    sha384,rsa2048:dev
>      Sign value:   2cf3105d0f3d455f3c234b817279af7091a89e7f4f33df0acb03ebb04405606d2bd8bd612cf17d1932c8fe142a8f7ccdd952245497c5b98cbaa4b7ffda06a5802db5da8d32b9111c9a3ed6587b5bb1eb9eb4665af5da317d19fefa471c6a5ee596340921923102f622b850738061aeab11fdf8387312e610b41a7b62e491c30e22a64177020a7df0f09e08211a66b61fb04763cd447d08459b82f19baa226b3e6f40f29ee28edd001d2d5a23549834aaf83160a0cd73285836d3e2fe039b22371bd313989e5a47329d046054c043944a451bf2b5046ff14869121c2bcc1f7e3029d7bdfc5f488e76584234631c91627a1203834051e7e6dc11f5d210501a5467
>      Timestamp:    2022-01-18   6:39:32 UTC
>    Verifying Hash Integrity ... sha384,rsa2048:dev- Failed to verify required signature 'key-dev'
>  error!
> Unable to verify required signature for '' hash node in 'kernel' image node
> Bad Data Hash
> ERROR: can't get kernel image!
> 
> 
> My test data:
> /dts-v1/;
> 
> / {
> 	description = "Chrome OS kernel image with one or more FDT blobs";
> 	#address-cells = <1>;
> 
> 	images {
> 		kernel {
> 			data = /incbin/("test-kernel.bin");
> 			type = "kernel_noload";
> 			arch = "sandbox";
> 			os = "linux";
> 			compression = "none";
> 			load = <0x4>;
> 			entry = <0x8>;
> 			kernel-version = <1>;
> 			signature {
> 				algo = "sha384,rsa2048";
> 				key-name-hint = "dev";
> 			};
> 		};
> 		fdt-1 {
> 			description = "snow";
> 			data = /incbin/("sandbox-kernel.dtb");
> 			type = "flat_dt";
> 			arch = "sandbox";
> 			compression = "none";
> 			fdt-version = <1>;
> 			signature {
> 				algo = "sha384,rsa2048";
> 				key-name-hint = "dev";
> 			};
> 		};
> 	};
> 	configurations {
> 		default = "conf-1";
> 		conf-1 {
> 			kernel = "kernel";
> 			fdt = "fdt-1";
> 		};
> 	};
> };
>