From b93a65209c4afae3f929262761b48b228ef58828 Mon Sep 17 00:00:00 2001 From: Massimo Pegorer Date: Thu, 5 Jan 2023 10:31:09 +0100 Subject: [PATCH 1/1] mkimage: fit: Support signed configurations in 'auto' FITs Extend support for signing in auto-generated (-f auto) FIT. Previously, it was possible to get signed 'images' subnodes in the FIT using options -g and -o together with -f auto. This patch allows signing 'configurations' subnodes instead of 'images' ones (which are hashed), using option -f auto-conf instead of -f auto. Adding also -K and -r options, will add public key to file with required = "conf" property. Summary: -f auto => FIT with crc32 images -f auto -g ... -o ... => FIT with signed images -f auto-conf -g ... -o ... => FIT with sha1 images and signed confs Example: FIT with kernel, two device tree files, and signed configurations; public key (needed to verify signatures) is added to u-boot.dtb with required = "conf" property. mkimage -f auto-conf -A arm -O linux -T kernel -C none -a 43e00000 \ -e 0 -d vmlinuz -b /path/to/first.dtb -b /path/to/second.dtb \ -k /folder/with/key-files -g keyname -o sha256,rsa4096 \ -K u-boot.dtb -r kernel.itb Example: Add public key with required = "conf" property to u-boot.dtb without needing to sign anything. This will also create a useless FIT named unused.itb. mkimage -f auto-conf -d /dev/null -k /folder/with/key-files \ -g keyname -o sha256,rsa4096 -K u-boot.dtb -r unused.itb Signed-off-by: Massimo Pegorer Reviewed-by: Simon Glass --- doc/mkimage.1 | 119 ++++++++++++++------- test/py/tests/test_fit_auto_signed.py | 195 ++++++++++++++++++++++++++++++++++ tools/fit_image.c | 75 ++++++++----- tools/imagetool.h | 10 +- tools/mkimage.c | 21 +++- 5 files changed, 353 insertions(+), 67 deletions(-) create mode 100644 test/py/tests/test_fit_auto_signed.py diff --git a/doc/mkimage.1 b/doc/mkimage.1 index 353ea8b..d8727ec 100644 --- a/doc/mkimage.1 +++ b/doc/mkimage.1 @@ -22,7 +22,8 @@ mkimage \- generate images for U-Boot .SY mkimage .RI [ option\~ .\|.\|.\&] .BI \-f\~ image-tree-source-file\c -.RB | auto +.RB | auto\c +.RB | auto-conf .I image-file-name .YS . @@ -296,9 +297,9 @@ FIT. See for details on using external data. . .TP -\fB\-f \fIimage-tree-source-file\fR | \fBauto +\fB\-f \fIimage-tree-source-file\fR | \fBauto\fR | \fBauto-conf .TQ -\fB\-\-fit \fIimage-tree-source-file\fR | \fBauto +\fB\-\-fit \fIimage-tree-source-file\fR | \fBauto\fR | \fBauto-conf Image tree source file that describes the structure and contents of the FIT image. .IP @@ -317,7 +318,25 @@ and options may be used to specify the image to include in the FIT and its attributes. No .I image-tree-source-file -is required. +is required. The +.BR \-g , +.BR \-o , +and +.B \-k +or +.B \-G +options may be used to get \(oqimages\(cq signed subnodes in the generated +auto FIT. Instead, to get \(oqconfigurations\(cq signed subnodes and +\(oqimages\(cq hashed subnodes, pass +.BR "\-f auto-conf". +In this case +.BR \-g , +.BR \-o , +and +.B \-k +or +.B \-G +are mandatory options. . .TP .B \-F @@ -348,16 +367,16 @@ for use with signing, and a certificate necessary when embedding it into another device tree using .BR \-K . .I name -defaults to the value of the signature node's \(oqkey-name-hint\(cq property, -but may be overridden using -.BR \-g . +is the value of the signature node's \(oqkey-name-hint\(cq property. . .TP .BI \-G " key-file" .TQ .BI \-\-key\-file " key-file" Specifies the private key file to use when signing. This option may be used -instead of \-k. +instead of \-k. Useful when the private key file basename does not match +\(oqkey-name-hint\(cq value. But note that it may lead to unexpected results +when used together with -K and/or -k options. . .TP .BI \-K " key-destination" @@ -373,49 +392,50 @@ CONFIG_OF_CONTROL in U-Boot. .BI \-g " key-name-hint" .TQ .BI \-\-key\-name\-hint " key-name-hint" -Overrides the signature node's \(oqkey-name-hint\(cq property. This is -especially useful when signing an image with -.BR "\-f auto" . -This is the -.I name -part of the key. The directory part is set by -.BR \-k . -This option also indicates that the images included in the FIT should be signed. -If this option is specified, then +Specifies the value of signature node \(oqkey-name-hint\(cq property for +an automatically generated FIT image. It makes sense only when used with +.B "\-f auto" +or +.BR "\-f auto-conf". +This option also indicates that the images or configurations included in +the FIT should be signed. If this option is specified, then .B \-o must be specified as well. . .TP -.BI \-o " crypto" , checksum +.BI \-o " checksum" , crypto .TQ -.BI \-\-algo " crypto" , checksum -Specifies the algorithm to be used for signing a FIT image. The default is -taken from the signature node's \(oqalgo\(cq property. +.BI \-\-algo " checksum" , crypto +Specifies the algorithm to be used for signing a FIT image, overriding value +taken from the signature node \(oqalgo\(cq property in the +.IR image-tree-source-file . +It is mandatory for automatically generated FIT. +.IP The valid values for -.I crypto +.I checksum are: .RS .IP .TS lb. -rsa2048 -rsa3072 -rsa4096 -ecdsa256 +sha1 +sha256 +sha384 +sha512 .TE .RE .IP The valid values for -.I checksum -are +.I crypto +are: .RS .IP .TS lb. -sha1 -sha256 -sha384 -sha512 +rsa2048 +rsa3072 +rsa4096 +ecdsa256 .TE .RE . @@ -423,9 +443,13 @@ sha512 .B \-r .TQ .B \-\-key\-required -Specifies that keys used to sign the FIT are required. This means that they -must be verified for the image to boot. Without this option, the verification -will be optional (useful for testing but not for release). +Specifies that keys used to sign the FIT are required. This means that images +or configurations signatures must be verified before using them (i.e. to +boot). Without this option, the verification will be optional (useful for +testing but not for release). It makes sense only when used with +.BR \-K. +When both, images and configurations, are signed, \(oqrequired\(cq property +value will be "conf". . .TP .BI \-N " engine" @@ -716,7 +740,7 @@ skipping those for which keys cannot be found. Also add a comment. .EE .RE .P -Add public keys to u\-boot.dtb without needing a FIT to sign. This will also +Add public key to u\-boot.dtb without needing a FIT to sign. This will also create a FIT containing an images node with no data named unused.itb. .RS .P @@ -726,6 +750,16 @@ create a FIT containing an images node with no data named unused.itb. .EE .RE .P +Add public key with required = "conf" property to u\-boot.dtb without needing +a FIT to sign. This will also create a useless FIT named unused.itb. +.RS +.P +.EX +\fBmkimage \-f auto-conf \-d /dev/null \-k /public/signing\-keys \-g dev \\ + \-o sha256,rsa2048 \-K u\-boot.dtb -r unused.itb +.EE +.RE +.P Update an existing FIT image, signing it with additional keys. Add corresponding public keys into u\-boot.dtb. This will resign all images with keys that are available in the new directory. Images that request signing @@ -768,6 +802,19 @@ file is required. \-d vmlinuz \-k /secret/signing\-keys \-g dev \-o sha256,rsa2048 kernel.itb .EE .RE +.P +Create a FIT image containing a kernel and some device tree files, signing +each configuration, using automatic mode. Moreover, the public key needed to +verify signatures is added to u\-boot.dtb with required = "conf" property. +.RS +.P +.EX +\fBmkimage \-f auto-conf \-A arm \-O linux \-T kernel \-C none \-a 43e00000 \\ + \-e 0 \-d vmlinuz \-b /path/to/file\-1.dtb \-b /path/to/file\-2.dtb \\ + \-k /folder/with/signing\-keys \-g dev \-o sha256,rsa2048 \\ + \-K u\-boot.dtb -r kernel.itb +.EE +.RE . .SH SEE ALSO .BR dtc (1), diff --git a/test/py/tests/test_fit_auto_signed.py b/test/py/tests/test_fit_auto_signed.py new file mode 100644 index 0000000..9ea3351 --- /dev/null +++ b/test/py/tests/test_fit_auto_signed.py @@ -0,0 +1,195 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2022 Massimo Pegorer + +""" +Test that mkimage generates auto-FIT with signatures and/or hashes as expected. + +The mkimage tool can create auto generated (i.e. without an ITS file +provided as input) FIT in three different flavours: with crc32 checksums +of 'images' subnodes; with signatures of 'images' subnodes; with sha1 +hashes of 'images' subnodes and signatures of 'configurations' subnodes. +This test verifies that auto-FIT are generated as expected, in all of +the three flavours, including check of hashes and signatures (except for +configurations ones). + +The test does not run the sandbox. It only checks the host tool mkimage. +""" + +import os +import pytest +import u_boot_utils as util +import binascii +from Cryptodome.Hash import SHA1 +from Cryptodome.Hash import SHA256 +from Cryptodome.PublicKey import RSA +from Cryptodome.Signature import pkcs1_15 + +class SignedFitHelper(object): + """Helper to manipulate a FIT with signed/hashed images/configs.""" + def __init__(self, cons, file_name): + self.fit = file_name + self.cons = cons + self.images_nodes = set() + self.confgs_nodes = set() + + def __fdt_list(self, path): + return util.run_and_log(self.cons, + f'fdtget -l {self.fit} {path}') + + def __fdt_get_string(self, node, prop): + return util.run_and_log(self.cons, + f'fdtget -ts {self.fit} {node} {prop}') + + def __fdt_get_binary(self, node, prop): + numbers = util.run_and_log(self.cons, + f'fdtget -tbi {self.fit} {node} {prop}') + + bignum = bytearray() + for little_num in numbers.split(): + bignum.append(int(little_num)) + + return bignum + + def build_nodes_sets(self): + """Fill sets with FIT images and configurations subnodes.""" + for node in self.__fdt_list('/images').split(): + subnode = f'/images/{node}' + self.images_nodes.add(subnode) + + for node in self.__fdt_list('/configurations').split(): + subnode = f'/configurations/{node}' + self.confgs_nodes.add(subnode) + + return len(self.images_nodes) + len(self.confgs_nodes) + + def check_fit_crc32_images(self): + """Test that all images in the set are hashed as expected. + + Each image must have an hash with algo=crc32 and hash value must match + the one calculated over image data. + """ + for node in self.images_nodes: + algo = self.__fdt_get_string(f'{node}/hash', 'algo') + assert algo == "crc32\n", "Missing expected crc32 image hash!" + + raw_crc32 = self.__fdt_get_binary(f'{node}/hash', 'value') + raw_bin = self.__fdt_get_binary(node, 'data') + assert raw_crc32 == (binascii.crc32(raw_bin) & + 0xffffffff).to_bytes(4, 'big'), "Wrong crc32 hash!" + + def check_fit_signed_images(self, key_name, sign_algo, verifier): + """Test that all images in the set are signed as expected. + + Each image must have a signature with: key-name-hint matching key_name + argument; algo matching sign_algo argument; value matching the one + calculated over image data using verifier argument. + """ + for node in self.images_nodes: + hint = self.__fdt_get_string(f'{node}/signature', 'key-name-hint') + assert hint == key_name + "\n", "Missing expected key name hint!" + algo = self.__fdt_get_string(f'{node}/signature', 'algo') + assert algo == sign_algo + "\n", "Missing expected signature algo!" + + raw_sig = self.__fdt_get_binary(f'{node}/signature', 'value') + raw_bin = self.__fdt_get_binary(node, 'data') + verifier.verify(SHA256.new(raw_bin), bytes(raw_sig)) + + def check_fit_signed_confgs(self, key_name, sign_algo): + """Test that all configs are signed, and images hashed, as expected. + + Each image must have an hash with algo=sha1 and hash value must match + the one calculated over image data. Each configuration must have a + signature with key-name-hint matching key_name argument and algo + matching sign_algo argument. + TODO: configurations signature checking. + """ + for node in self.images_nodes: + algo = self.__fdt_get_string(f'{node}/hash', 'algo') + assert algo == "sha1\n", "Missing expected sha1 image hash!" + + raw_hash = self.__fdt_get_binary(f'{node}/hash', 'value') + raw_bin = self.__fdt_get_binary(node, 'data') + assert raw_hash == SHA1.new(raw_bin).digest(), "Wrong sha1 hash!" + + for node in self.confgs_nodes: + hint = self.__fdt_get_string(f'{node}/signature', 'key-name-hint') + assert hint == key_name + "\n", "Missing expected key name hint!" + algo = self.__fdt_get_string(f'{node}/signature', 'algo') + assert algo == sign_algo + "\n", "Missing expected signature algo!" + + +@pytest.mark.buildconfigspec('fit_signature') +@pytest.mark.requiredtool('fdtget') +def test_fit_auto_signed(u_boot_console): + """Test that mkimage generates auto-FIT with signatures/hashes as expected. + + The mkimage tool can create auto generated (i.e. without an ITS file + provided as input) FIT in three different flavours: with crc32 checksums + of 'images' subnodes; with signatures of 'images' subnodes; with sha1 + hashes of 'images' subnodes and signatures of 'configurations' subnodes. + This test verifies that auto-FIT are generated as expected, in all of + the three flavours, including check of hashes and signatures (except for + configurations ones). + + The test does not run the sandbox. It only checks the host tool mkimage. + """ + cons = u_boot_console + mkimage = cons.config.build_dir + '/tools/mkimage' + tempdir = os.path.join(cons.config.result_dir, 'auto_fit') + os.makedirs(tempdir, exist_ok=True) + kernel_file = f'{tempdir}/vmlinuz' + dt1_file = f'{tempdir}/dt-1.dtb' + dt2_file = f'{tempdir}/dt-2.dtb' + key_name = 'sign-key' + sign_algo = 'sha256,rsa4096' + key_file = f'{tempdir}/{key_name}.key' + fit_file = f'{tempdir}/test.fit' + + # Create a fake kernel image and two dtb files with random data + with open(kernel_file, 'wb') as fd: + fd.write(os.urandom(512)) + + with open(dt1_file, 'wb') as fd: + fd.write(os.urandom(256)) + + with open(dt2_file, 'wb') as fd: + fd.write(os.urandom(256)) + + # Create 4096 RSA key and write to file to be read by mkimage + key = RSA.generate(bits=4096) + verifier = pkcs1_15.new(key) + + with open(key_file, 'w') as fd: + fd.write(str(key.export_key(format='PEM').decode('ascii'))) + + b_args = " -d" + kernel_file + " -b" + dt1_file + " -b" + dt2_file + s_args = " -k" + tempdir + " -g" + key_name + " -o" + sign_algo + + # 1 - Create auto FIT with images crc32 checksum, and verify it + util.run_and_log(cons, mkimage + ' -fauto' + b_args + " " + fit_file) + + fit = SignedFitHelper(cons, fit_file) + if fit.build_nodes_sets() == 0: + raise ValueError('FIT-1 has no "/image" nor "/configuration" nodes') + + fit.check_fit_crc32_images() + + # 2 - Create auto FIT with signed images, and verify it + util.run_and_log(cons, mkimage + ' -fauto' + b_args + s_args + " " + + fit_file) + + fit = SignedFitHelper(cons, fit_file) + if fit.build_nodes_sets() == 0: + raise ValueError('FIT-2 has no "/image" nor "/configuration" nodes') + + fit.check_fit_signed_images(key_name, sign_algo, verifier) + + # 3 - Create auto FIT with signed configs and hashed images, and verify it + util.run_and_log(cons, mkimage + ' -fauto-conf' + b_args + s_args + " " + + fit_file) + + fit = SignedFitHelper(cons, fit_file) + if fit.build_nodes_sets() == 0: + raise ValueError('FIT-3 has no "/image" nor "/configuration" nodes') + + fit.check_fit_signed_confgs(key_name, sign_algo) diff --git a/tools/fit_image.c b/tools/fit_image.c index 8a18b1b..8763a36 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -201,36 +201,59 @@ static void get_basename(char *str, int size, const char *fname) } /** - * add_hash_node() - Add a hash or signature node + * fit_add_hash_or_sign() - Add a hash or signature node * * @params: Image parameters * @fdt: Device tree to add to (in sequential-write mode) + * @is_images_subnode: true to add hash even if key name hint is provided * - * If there is a key name hint, try to sign the images. Otherwise, just add a - * CRC. - * - * Return: 0 on success, or -1 on failure + * If do_add_hash is false (default) and there is a key name hint, try to add + * a sign node to parent. Otherwise, just add a CRC. Rationale: if conf have + * to be signed, image/dt have to be hashed even if there is a key name hint. */ -static int add_hash_node(struct image_tool_params *params, void *fdt) +static void fit_add_hash_or_sign(struct image_tool_params *params, void *fdt, + bool is_images_subnode) { - if (params->keyname) { - if (!params->algo_name) { - fprintf(stderr, - "%s: Algorithm name must be specified\n", - params->cmdname); - return -1; + const char *hash_algo = "crc32"; + bool do_hash = false; + bool do_sign = false; + + switch (params->auto_fit) { + case AF_OFF: + break; + case AF_HASHED_IMG: + do_hash = is_images_subnode; + break; + case AF_SIGNED_IMG: + do_sign = is_images_subnode; + break; + case AF_SIGNED_CONF: + if (is_images_subnode) { + do_hash = true; + hash_algo = "sha1"; + } else { + do_sign = true; } + break; + default: + fprintf(stderr, + "%s: Unsupported auto FIT mode %u\n", + params->cmdname, params->auto_fit); + break; + } + + if (do_hash) { + fdt_begin_node(fdt, FIT_HASH_NODENAME); + fdt_property_string(fdt, FIT_ALGO_PROP, hash_algo); + fdt_end_node(fdt); + } - fdt_begin_node(fdt, "signature-1"); + if (do_sign) { + fdt_begin_node(fdt, FIT_SIG_NODENAME); fdt_property_string(fdt, FIT_ALGO_PROP, params->algo_name); fdt_property_string(fdt, FIT_KEY_HINT, params->keyname); - } else { - fdt_begin_node(fdt, "hash-1"); - fdt_property_string(fdt, FIT_ALGO_PROP, "crc32"); + fdt_end_node(fdt); } - - fdt_end_node(fdt); - return 0; } /** @@ -271,9 +294,7 @@ static int fit_write_images(struct image_tool_params *params, char *fdt) ret = fdt_property_file(params, fdt, FIT_DATA_PROP, params->datafile); if (ret) return ret; - ret = add_hash_node(params, fdt); - if (ret) - return ret; + fit_add_hash_or_sign(params, fdt, true); fdt_end_node(fdt); /* Now the device tree files if available */ @@ -296,7 +317,7 @@ static int fit_write_images(struct image_tool_params *params, char *fdt) genimg_get_arch_short_name(params->arch)); fdt_property_string(fdt, FIT_COMP_PROP, genimg_get_comp_short_name(IH_COMP_NONE)); - ret = add_hash_node(params, fdt); + fit_add_hash_or_sign(params, fdt, true); if (ret) return ret; fdt_end_node(fdt); @@ -316,7 +337,7 @@ static int fit_write_images(struct image_tool_params *params, char *fdt) params->fit_ramdisk); if (ret) return ret; - ret = add_hash_node(params, fdt); + fit_add_hash_or_sign(params, fdt, true); if (ret) return ret; fdt_end_node(fdt); @@ -368,6 +389,7 @@ static void fit_write_configs(struct image_tool_params *params, char *fdt) snprintf(str, sizeof(str), FIT_FDT_PROP "-%d", upto); fdt_property_string(fdt, FIT_FDT_PROP, str); + fit_add_hash_or_sign(params, fdt, false); fdt_end_node(fdt); } @@ -380,6 +402,7 @@ static void fit_write_configs(struct image_tool_params *params, char *fdt) if (params->fit_ramdisk) fdt_property_string(fdt, FIT_RAMDISK_PROP, FIT_RAMDISK_PROP "-1"); + fit_add_hash_or_sign(params, fdt, false); fdt_end_node(fdt); } @@ -723,7 +746,7 @@ static int fit_handle_file(struct image_tool_params *params) sprintf (tmpfile, "%s%s", params->imagefile, MKIMAGE_TMPFILE_SUFFIX); /* We either compile the source file, or use the existing FIT image */ - if (params->auto_its) { + if (params->auto_fit) { if (fit_build(params, tmpfile)) { fprintf(stderr, "%s: failed to build FIT\n", params->cmdname); @@ -907,7 +930,7 @@ static int fit_extract_contents(void *ptr, struct image_tool_params *params) static int fit_check_params(struct image_tool_params *params) { - if (params->auto_its) + if (params->auto_fit) return 0; return ((params->dflag && params->fflag) || (params->fflag && params->lflag) || diff --git a/tools/imagetool.h b/tools/imagetool.h index ca7c2e4..fdceea4 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -39,6 +39,14 @@ struct content_info { const char *fname; }; +/* FIT auto generation modes */ +enum af_mode { + AF_OFF = 0, /* Needs .its or existing FIT to be provided */ + AF_HASHED_IMG, /* Auto FIT with crc32 hashed images subnodes */ + AF_SIGNED_IMG, /* Auto FIT with signed images subnodes */ + AF_SIGNED_CONF, /* Auto FIT with sha1 images and signed configs */ +}; + /* * This structure defines all such variables those are initialized by * mkimage and dumpimage main core and need to be referred by image @@ -79,7 +87,7 @@ struct image_tool_params { int require_keys; /* 1 to mark signing keys as 'required' */ int file_size; /* Total size of output file */ int orig_file_size; /* Original size for file before padding */ - bool auto_its; /* Automatically create the .its file */ + enum af_mode auto_fit; /* Automatically create the FIT */ int fit_image_type; /* Image type to put into the FIT */ char *fit_ramdisk; /* Ramdisk file to include */ struct content_info *content_head; /* List of files to include */ diff --git a/tools/mkimage.c b/tools/mkimage.c index 8306861..af7b0e0 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -104,7 +104,7 @@ static void usage(const char *msg) " -v ==> verbose\n", params.cmdname); fprintf(stderr, - " %s [-D dtc_options] [-f fit-image.its|-f auto|-F] [-b [-b ]] [-E] [-B size] [-i ] fit-image\n" + " %s [-D dtc_options] [-f fit-image.its|-f auto|-f auto-conf|-F] [-b [-b ]] [-E] [-B size] [-i ] fit-image\n" " file is used with -f auto, it may occur multiple times.\n", params.cmdname); fprintf(stderr, @@ -271,7 +271,10 @@ static void process_args(int argc, char **argv) break; case 'f': datafile = optarg; - params.auto_its = !strcmp(datafile, "auto"); + if (!strcmp(datafile, "auto")) + params.auto_fit = AF_HASHED_IMG; + else if (!strcmp(datafile, "auto-conf")) + params.auto_fit = AF_SIGNED_CONF; /* fallthrough */ case 'F': /* @@ -283,6 +286,7 @@ static void process_args(int argc, char **argv) break; case 'g': params.keyname = optarg; + break; case 'G': params.keyfile = optarg; break; @@ -370,6 +374,15 @@ static void process_args(int argc, char **argv) if (optind < argc) params.imagefile = argv[optind]; + if (params.auto_fit == AF_SIGNED_CONF) { + if (!params.keyname || !params.algo_name) + usage("Missing key/algo for auto-FIT with signed configs (use -g -o)"); + } else if (params.auto_fit == AF_HASHED_IMG && params.keyname) { + params.auto_fit = AF_SIGNED_IMG; + if (!params.algo_name) + usage("Missing algorithm for auto-FIT with signed images (use -g)"); + } + /* * For auto-generated FIT images we need to know the image type to put * in the FIT, which is separate from the file's image type (which @@ -377,8 +390,8 @@ static void process_args(int argc, char **argv) */ if (params.type == IH_TYPE_FLATDT) { params.fit_image_type = type ? type : IH_TYPE_KERNEL; - /* For auto_its, datafile is always 'auto' */ - if (!params.auto_its) + /* For auto-FIT, datafile has to be provided with -d */ + if (!params.auto_fit) params.datafile = datafile; else if (!params.datafile) usage("Missing data file for auto-FIT (use -d)"); -- 2.7.4