Patchwork [U-Boot,RFC,32/44] image: Support signing of images

login
register
mail settings
Submitter Simon Glass
Date Jan. 5, 2013, 1:52 a.m.
Message ID <1357350734-13737-33-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/209630/
State Superseded, archived
Headers show

Comments

Simon Glass - Jan. 5, 2013, 1:52 a.m.
Add support for signing images using a new signature node. The process
is handled by fdt_add_verification_data() which now takes parameters to
provide the keys and related information.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 doc/uImage.FIT/sign-images.its |   42 +++++++++
 include/image.h                |   22 ++++-
 tools/fit_image.c              |    2 +-
 tools/image-host.c             |  181 +++++++++++++++++++++++++++++++++++++--
 4 files changed, 232 insertions(+), 15 deletions(-)
 create mode 100644 doc/uImage.FIT/sign-images.its
Marek Vasut - Jan. 5, 2013, 8:19 a.m.
Dear Simon Glass,

[...]

> +	int string_size;
> +	int ret;
> +
> +	/*
> +	 * Get the current string size, before we update the FIT and add
> +	 * more
> +	 */
> +	string_size = fdt_size_dt_strings(fit);
> +
> +	ret = fdt_setprop(fit, noffset, FIT_VALUE_PROP, value, value_len);
> +	ret |= fdt_setprop_string(fit, noffset, "signer-name", "mkimage");
> +	ret |= fdt_setprop_string(fit, noffset, "signer-version",
> +				  PLAIN_VERSION);

Can you really be ORR'ing a signed variable such as "ret" ?
[...]
Best regards,
Marek Vasut
Simon Glass - Jan. 5, 2013, 9:50 p.m.
Hi Marek,

On Sat, Jan 5, 2013 at 12:19 AM, Marek Vasut <marex@denx.de> wrote:
> Dear Simon Glass,
>
> [...]
>
>> +     int string_size;
>> +     int ret;
>> +
>> +     /*
>> +      * Get the current string size, before we update the FIT and add
>> +      * more
>> +      */
>> +     string_size = fdt_size_dt_strings(fit);
>> +
>> +     ret = fdt_setprop(fit, noffset, FIT_VALUE_PROP, value, value_len);
>> +     ret |= fdt_setprop_string(fit, noffset, "signer-name", "mkimage");
>> +     ret |= fdt_setprop_string(fit, noffset, "signer-version",
>> +                               PLAIN_VERSION);
>
> Can you really be ORR'ing a signed variable such as "ret" ?

It's pretty ugly, and it means that I don't get a proper error
message, so I will change it, particularly as this is only used by
mkimage and code size is not a concern.

Thanks very much for reviewing these patches, and for comments. The
image code is actually mostly pretty nice so it hasn't been difficult
to work with.

Regards,
Simon

> [...]
> Best regards,
> Marek Vasut

Patch

diff --git a/doc/uImage.FIT/sign-images.its b/doc/uImage.FIT/sign-images.its
new file mode 100644
index 0000000..f69326a
--- /dev/null
+++ b/doc/uImage.FIT/sign-images.its
@@ -0,0 +1,42 @@ 
+/dts-v1/;
+
+/ {
+	description = "Chrome OS kernel image with one or more FDT blobs";
+	#address-cells = <1>;
+
+	images {
+		kernel@1 {
+			data = /incbin/("test-kernel.bin");
+			type = "kernel_noload";
+			arch = "sandbox";
+			os = "linux";
+			compression = "none";
+			load = <0x4>;
+			entry = <0x8>;
+			kernel-version = <1>;
+			signature@1 {
+				algo = "sha1,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@1 {
+				algo = "sha1,rsa2048";
+				key-name-hint = "dev";
+			};
+		};
+	};
+	configurations {
+		default = "conf@1";
+		conf@1 {
+			kernel = "kernel@1";
+			fdt = "fdt@1";
+		};
+	};
+};
diff --git a/include/image.h b/include/image.h
index 8c648dc..a1072c0 100644
--- a/include/image.h
+++ b/include/image.h
@@ -611,12 +611,26 @@  int fit_image_hash_get_ignore(const void *fit, int noffset, int *ignore);
 int fit_set_timestamp(void *fit, int noffset, time_t timestamp);
 
 /**
- * fit_add_verification_data() - Calculate and add hashes to FIT
+ * fit_add_verification_data() - add verification data to FIT image nodes
  *
- * @fit:	Fit image to process
- * @return 0 if ok, <0 for error
+ * @keydir:	Directory containing keys
+ * @kwydest:	FDT blob to write public key information to
+ * @fit:	Pointer to the FIT format image header
+ * @comment:	Comment to add to signature nodes
+ * @require_keys: Mark all keys as 'required'
+ *
+ * Adds hash values for all component images in the FIT blob.
+ * Hashes are calculated for all component images which have hash subnodes
+ * with algorithm property set to one of the supported hash algorithms.
+ *
+ * Also add signatures if signature nodes are present.
+ *
+ * returns
+ *     0, on success
+ *     libfdt error code, on failure
  */
-int fit_add_verification_data(void *fit);
+int fit_add_verification_data(const char *keydir, void *keydest, void *fit,
+			      const char *comment, int require_keys);
 
 int fit_image_verify(const void *fit, int noffset);
 int fit_all_image_verify(const void *fit);
diff --git a/tools/fit_image.c b/tools/fit_image.c
index 8f51159..e0675d7 100644
--- a/tools/fit_image.c
+++ b/tools/fit_image.c
@@ -125,7 +125,7 @@  static int fit_handle_file (struct mkimage_params *params)
 	}
 
 	/* set hashes for images in the blob */
-	if (fit_add_verification_data(ptr)) {
+	if (fit_add_verification_data(NULL, NULL, ptr, NULL, 0)) {
 		fprintf (stderr, "%s Can't add hashes to FIT blob",
 				params->cmdname);
 		unlink (tmpfile);
diff --git a/tools/image-host.c b/tools/image-host.c
index 4c589af..4196190 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -26,12 +26,8 @@ 
  */
 
 #include "mkimage.h"
-#include <bootstage.h>
 #include <image.h>
-#include <sha1.h>
-#include <time.h>
-#include <u-boot/crc.h>
-#include <u-boot/md5.h>
+#include <version.h>
 
 /**
  * fit_set_hash_value - set hash value in requested has node
@@ -113,9 +109,161 @@  static int fit_image_process_hash(void *fit, const char *image_name,
 }
 
 /**
- * fit_image_add_verification_data() - calculate/set hash data for image node
+ * fit_image_write_sig() - write the signature to a FIT
  *
- * This adds hash values for a component image node.
+ * This writes the signature and signer data to the FIT.
+ *
+ * @fit: pointer to the FIT format image header
+ * @noffset: hash node offset
+ * @value: signature value to be set
+ * @value_len: signature value length
+ * @comment: Text comment to write (NULL for none)
+ *
+ * returns
+ *     0, on success
+ *     -1, on failure
+ */
+static int fit_image_write_sig(void *fit, int noffset, uint8_t *value,
+		int value_len, const char *comment, const char *region_prop,
+		int region_proplen)
+{
+	int string_size;
+	int ret;
+
+	/*
+	 * Get the current string size, before we update the FIT and add
+	 * more
+	 */
+	string_size = fdt_size_dt_strings(fit);
+
+	ret = fdt_setprop(fit, noffset, FIT_VALUE_PROP, value, value_len);
+	ret |= fdt_setprop_string(fit, noffset, "signer-name", "mkimage");
+	ret |= fdt_setprop_string(fit, noffset, "signer-version",
+				  PLAIN_VERSION);
+	if (comment)
+		ret |= fdt_setprop_string(fit, noffset, "comment", comment);
+	ret |= fit_set_timestamp(fit, noffset, time(NULL));
+	if (region_prop) {
+		uint32_t strdata[2];
+
+		ret |= fdt_setprop(fit, noffset, "hashed-nodes",
+				   region_prop, region_proplen);
+		strdata[0] = 0;
+		strdata[1] = cpu_to_fdt32(string_size);
+		ret |= fdt_setprop(fit, noffset, "hashed-strings", strdata,
+				   sizeof(strdata));
+	}
+
+	return ret;
+}
+
+static int fit_image_setup_sig(struct image_sign_info *info,
+		const char *keydir, void *fit, const char *image_name,
+		int noffset, const char *require_keys)
+{
+	const char *node_name;
+	char *algo_name;
+
+	node_name = fit_get_name(fit, noffset, NULL);
+	if (fit_image_hash_get_algo(fit, noffset, &algo_name)) {
+		printf("Can't get algo property for "
+			"'%s' signature node in '%s' image node\n",
+			node_name, image_name);
+		return -1;
+	}
+
+	memset(info, '\0', sizeof(*info));
+	info->keydir = keydir;
+	info->keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL);
+	info->fit = fit;
+	info->node_offset = noffset;
+	info->algo = image_get_sig_algo(algo_name);
+	info->require_keys = require_keys;
+	if (!info->algo) {
+		printf("Unsupported signature algorithm (%s) for "
+			"'%s' signature node in '%s' image node\n",
+			algo_name, node_name, image_name);
+		return -1;
+	}
+
+	return 0;
+}
+
+/**
+ * fit_image_process_sig- Process a single subnode of the images/ node
+ *
+ * Check each subnode and process accordingly. For signature nodes we
+ * generate a signed hash of the supplised data and store it in the node.
+ *
+ * @keydir:	Directory containing keys to use for signing
+ * @keydest:	Destination FDT blob to write public keys into
+ * @fit:	pointer to the FIT format image header
+ * @image_name:	name of image being processes (used to display errors)
+ * @noffset:	subnode offset
+ * @data:	data to process
+ * @size:	size of data in bytes
+ * @comment:	Comment to add to signature nodes
+ * @require_keys: Mark all keys as 'required'
+ * @return 0 if ok, -1 on error
+ */
+static int fit_image_process_sig(const char *keydir, void *keydest,
+		void *fit, const char *image_name,
+		int noffset, const void *data, size_t size,
+		const char *comment, int require_keys)
+{
+	struct image_sign_info info;
+	struct image_region region;
+	const char *node_name;
+	uint8_t *value;
+	uint value_len;
+	int ret;
+
+	if (fit_image_setup_sig(&info, keydir, fit, image_name, noffset,
+			require_keys ? "image" : NULL))
+		return -1;
+
+	node_name = fit_get_name(fit, noffset, NULL);
+	region.data = data;
+	region.size = size;
+	ret = info.algo->sign(&info, &region, 1, &value, &value_len);
+	if (ret) {
+		printf("Failed to sign '%s' signature node in '%s'"
+			" image node: %d\n",
+			node_name, image_name, ret);
+
+		/* We allow keys to be missing */
+		if (ret == -ENOENT)
+			return 0;
+		return -1;
+	}
+
+	if (fit_image_write_sig(fit, noffset, value, value_len, comment,
+			NULL, 0)) {
+		printf("Can't write signature for "
+			"'%s' signature node in '%s' image node\n",
+			node_name, image_name);
+		return -1;
+	}
+	free(value);
+
+	/* Get keyname again, as FDT has changed and invalidated our pointer */
+	info.keyname = fdt_getprop(fit, noffset, "key-name-hint", NULL);
+
+	/* Write the public key into the supplied FDT file */
+	if (keydest && info.algo->add_verify_data(&info, keydest)) {
+		printf("Failed to add verification data for "
+			"'%s' signature node in '%s' image node\n",
+			node_name, image_name);
+		return -1;
+	}
+
+	return 0;
+}
+
+/**
+ * fit_image_add_verification_data() - calculate/set verig. data for image node
+ *
+ * This adds hash and signature values for an component image node.
  *
  * All existing hash subnodes are checked, if algorithm property is set to
  * one of the supported hash algorithms, hash value is computed and
@@ -138,11 +286,17 @@  static int fit_image_process_hash(void *fit, const char *image_name,
  *
  * For signature details, please see doc/uImage.FIT/signature.txt
  *
+ * @keydir	Directory containing *.key and *.crt files (or NULL)
+ * @keydest	FDT Blob to write public keys into (NULL if none)
  * @fit:	Pointer to the FIT format image header
  * @image_noffset: Requested component image node
+ * @comment:	Comment to add to signature nodes
+ * @require_keys: Mark all keys as 'required'
  * @return: 0 on success, <0 on failure
  */
-int fit_image_add_verification_data(void *fit, int image_noffset)
+int fit_image_add_verification_data(const char *keydir, void *keydest,
+		void *fit, int image_noffset, const char *comment,
+		int require_keys)
 {
 	const char *image_name;
 	const void *data;
@@ -176,6 +330,11 @@  int fit_image_add_verification_data(void *fit, int image_noffset)
 				strlen(FIT_HASH_NODENAME))) {
 			ret = fit_image_process_hash(fit, image_name, noffset,
 						data, size);
+		} else if (keydir && !strncmp(node_name, FIT_SIG_NODENAME,
+				strlen(FIT_SIG_NODENAME))) {
+			ret = fit_image_process_sig(keydir, keydest,
+				fit, image_name, noffset, data, size,
+				comment, require_keys);
 		}
 		if (ret)
 			return -1;
@@ -184,7 +343,8 @@  int fit_image_add_verification_data(void *fit, int image_noffset)
 	return 0;
 }
 
-int fit_add_verification_data(void *fit)
+int fit_add_verification_data(const char *keydir, void *keydest, void *fit,
+			      const char *comment, int require_keys)
 {
 	int images_noffset;
 	int noffset;
@@ -209,7 +369,8 @@  int fit_add_verification_data(void *fit)
 		 * Direct child node of the images parent node,
 		 * i.e. component image node.
 		 */
-		ret = fit_image_add_verification_data(fit, noffset);
+		ret = fit_image_add_verification_data(keydir, keydest,
+				fit, noffset, comment, require_keys);
 		if (ret)
 			return ret;
 	}