* [PATCH v2] mkimage: FIT ciphering bug fixes
@ 2020-07-30 4:22 patrick.oppenlander at gmail.com
2020-07-30 4:22 ` [PATCH v2 1/3] mkimage: fit: only process one cipher node patrick.oppenlander at gmail.com
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: patrick.oppenlander at gmail.com @ 2020-07-30 4:22 UTC (permalink / raw)
To: u-boot
The v2 series addresses review comments from Philippe Reynes:
* Use FIT_CIPHER_NODENAME instead of hard coding "cipher"
* Simplify handling of FDT_ERR_NOSPACE
* Simplify detection of previously ciphered data
The last two points are possible as I overlooked that the retry loop
handling ENOSPC in fit_handle_file() reloads the original FIT before
retrying.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] mkimage: fit: only process one cipher node
2020-07-30 4:22 [PATCH v2] mkimage: FIT ciphering bug fixes patrick.oppenlander at gmail.com
@ 2020-07-30 4:22 ` patrick.oppenlander at gmail.com
2020-08-08 12:29 ` Tom Rini
2020-07-30 4:22 ` [PATCH v2 2/3] mkimage: fit: handle FDT_ERR_NOSPACE when ciphering patrick.oppenlander at gmail.com
2020-07-30 4:22 ` [PATCH v2 3/3] mkimage: fit: don't cipher ciphered data patrick.oppenlander at gmail.com
2 siblings, 1 reply; 9+ messages in thread
From: patrick.oppenlander at gmail.com @ 2020-07-30 4:22 UTC (permalink / raw)
To: u-boot
From: Patrick Oppenlander <patrick.oppenlander@gmail.com>
Previously mkimage would process any node matching the regex cipher.*
and apply the ciphers to the image data in the order they appeared in
the FDT. This meant that data could be inadvertently ciphered multiple
times.
Switch to processing a single cipher node which exactly matches
FIT_CIPHER_NODENAME.
Signed-off-by: Patrick Oppenlander <patrick.oppenlander@gmail.com>
Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
---
tools/image-host.c | 57 ++++++++++++++++++----------------------------
1 file changed, 22 insertions(+), 35 deletions(-)
diff --git a/tools/image-host.c b/tools/image-host.c
index 9a83b7f675..dd7ecc4b60 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -323,15 +323,15 @@ err:
static int fit_image_setup_cipher(struct image_cipher_info *info,
const char *keydir, void *fit,
const char *image_name, int image_noffset,
- const char *node_name, int noffset)
+ int noffset)
{
char *algo_name;
char filename[128];
int ret = -1;
if (fit_image_cipher_get_algo(fit, noffset, &algo_name)) {
- printf("Can't get algo name for cipher '%s' in image '%s'\n",
- node_name, image_name);
+ printf("Can't get algo name for cipher in image '%s'\n",
+ image_name);
goto out;
}
@@ -340,16 +340,16 @@ static int fit_image_setup_cipher(struct image_cipher_info *info,
/* Read the key name */
info->keyname = fdt_getprop(fit, noffset, FIT_KEY_HINT, NULL);
if (!info->keyname) {
- printf("Can't get key name for cipher '%s' in image '%s'\n",
- node_name, image_name);
+ printf("Can't get key name for cipher in image '%s'\n",
+ image_name);
goto out;
}
/* Read the IV name */
info->ivname = fdt_getprop(fit, noffset, "iv-name-hint", NULL);
if (!info->ivname) {
- printf("Can't get iv name for cipher '%s' in image '%s'\n",
- node_name, image_name);
+ printf("Can't get iv name for cipher in image '%s'\n",
+ image_name);
goto out;
}
@@ -428,8 +428,7 @@ int fit_image_write_cipher(void *fit, int image_noffset, int noffset,
static int
fit_image_process_cipher(const char *keydir, void *keydest, void *fit,
const char *image_name, int image_noffset,
- const char *node_name, int node_noffset,
- const void *data, size_t size,
+ int node_noffset, const void *data, size_t size,
const char *cmdname)
{
struct image_cipher_info info;
@@ -440,7 +439,7 @@ fit_image_process_cipher(const char *keydir, void *keydest, void *fit,
memset(&info, 0, sizeof(info));
ret = fit_image_setup_cipher(&info, keydir, fit, image_name,
- image_noffset, node_name, node_noffset);
+ image_noffset, node_noffset);
if (ret)
goto out;
@@ -482,7 +481,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
const char *image_name;
const void *data;
size_t size;
- int node_noffset;
+ int cipher_node_offset;
/* Get image name */
image_name = fit_get_name(fit, image_noffset, NULL);
@@ -497,32 +496,20 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
return -1;
}
- /* Process all hash subnodes of the component image node */
- for (node_noffset = fdt_first_subnode(fit, image_noffset);
- node_noffset >= 0;
- node_noffset = fdt_next_subnode(fit, node_noffset)) {
- const char *node_name;
- int ret = 0;
-
- node_name = fit_get_name(fit, node_noffset, NULL);
- if (!node_name) {
- printf("Can't get node name\n");
- return -1;
- }
- if (IMAGE_ENABLE_ENCRYPT && keydir &&
- !strncmp(node_name, FIT_CIPHER_NODENAME,
- strlen(FIT_CIPHER_NODENAME)))
- ret = fit_image_process_cipher(keydir, keydest,
- fit, image_name,
- image_noffset,
- node_name, node_noffset,
- data, size, cmdname);
- if (ret)
- return ret;
+ /* Process cipher node if present */
+ cipher_node_offset = fdt_subnode_offset(fit, image_noffset,
+ FIT_CIPHER_NODENAME);
+ if (cipher_node_offset == -FDT_ERR_NOTFOUND)
+ return 0;
+ if (cipher_node_offset < 0) {
+ printf("Failure getting cipher node\n");
+ return -1;
}
-
- return 0;
+ if (!IMAGE_ENABLE_ENCRYPT || !keydir)
+ return 0;
+ return fit_image_process_cipher(keydir, keydest, fit, image_name,
+ image_noffset, cipher_node_offset, data, size, cmdname);
}
/**
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] mkimage: fit: handle FDT_ERR_NOSPACE when ciphering
2020-07-30 4:22 [PATCH v2] mkimage: FIT ciphering bug fixes patrick.oppenlander at gmail.com
2020-07-30 4:22 ` [PATCH v2 1/3] mkimage: fit: only process one cipher node patrick.oppenlander at gmail.com
@ 2020-07-30 4:22 ` patrick.oppenlander at gmail.com
2020-07-30 13:53 ` Philippe REYNES
2020-08-08 12:29 ` Tom Rini
2020-07-30 4:22 ` [PATCH v2 3/3] mkimage: fit: don't cipher ciphered data patrick.oppenlander at gmail.com
2 siblings, 2 replies; 9+ messages in thread
From: patrick.oppenlander at gmail.com @ 2020-07-30 4:22 UTC (permalink / raw)
To: u-boot
From: Patrick Oppenlander <patrick.oppenlander@gmail.com>
Also replace fdt_delprop/fdt_setprop with fdt_setprop as fdt_setprop can
replace an existing property value.
Signed-off-by: Patrick Oppenlander <patrick.oppenlander@gmail.com>
---
tools/image-host.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/tools/image-host.c b/tools/image-host.c
index dd7ecc4b60..b4603c5f01 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -399,23 +399,24 @@ int fit_image_write_cipher(void *fit, int image_noffset, int noffset,
{
int ret = -1;
- /* Remove unciphered data */
- ret = fdt_delprop(fit, image_noffset, FIT_DATA_PROP);
- if (ret) {
- printf("Can't remove data (err = %d)\n", ret);
- goto out;
- }
-
- /* Add ciphered data */
+ /* Replace data with ciphered data */
ret = fdt_setprop(fit, image_noffset, FIT_DATA_PROP,
data_ciphered, data_ciphered_len);
+ if (ret == -FDT_ERR_NOSPACE) {
+ ret = -ENOSPC;
+ goto out;
+ }
if (ret) {
- printf("Can't add ciphered data (err = %d)\n", ret);
+ printf("Can't replace data with ciphered data (err = %d)\n", ret);
goto out;
}
/* add non ciphered data size */
ret = fdt_setprop_u32(fit, image_noffset, "data-size-unciphered", size);
+ if (ret == -FDT_ERR_NOSPACE) {
+ ret = -ENOSPC;
+ goto out;
+ }
if (ret) {
printf("Can't add unciphered data size (err = %d)\n", ret);
goto out;
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] mkimage: fit: don't cipher ciphered data
2020-07-30 4:22 [PATCH v2] mkimage: FIT ciphering bug fixes patrick.oppenlander at gmail.com
2020-07-30 4:22 ` [PATCH v2 1/3] mkimage: fit: only process one cipher node patrick.oppenlander at gmail.com
2020-07-30 4:22 ` [PATCH v2 2/3] mkimage: fit: handle FDT_ERR_NOSPACE when ciphering patrick.oppenlander at gmail.com
@ 2020-07-30 4:22 ` patrick.oppenlander at gmail.com
2020-07-30 13:58 ` Philippe REYNES
2020-08-08 12:29 ` Tom Rini
2 siblings, 2 replies; 9+ messages in thread
From: patrick.oppenlander at gmail.com @ 2020-07-30 4:22 UTC (permalink / raw)
To: u-boot
From: Patrick Oppenlander <patrick.oppenlander@gmail.com>
Previously, mkimage -F could be run multiple times causing already
ciphered image data to be ciphered again.
Signed-off-by: Patrick Oppenlander <patrick.oppenlander@gmail.com>
---
tools/image-host.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/tools/image-host.c b/tools/image-host.c
index b4603c5f01..e5417beee5 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -482,7 +482,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
const char *image_name;
const void *data;
size_t size;
- int cipher_node_offset;
+ int cipher_node_offset, len;
/* Get image name */
image_name = fit_get_name(fit, image_noffset, NULL);
@@ -497,6 +497,19 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
return -1;
}
+ /*
+ * Don't cipher ciphered data.
+ *
+ * If the data-size-unciphered property is present the data for this
+ * image is already encrypted. This is important as 'mkimage -F' can be
+ * run multiple times on a FIT image.
+ */
+ if (fdt_getprop(fit, image_noffset, "data-size-unciphered", &len))
+ return 0;
+ if (len != -FDT_ERR_NOTFOUND) {
+ printf("Failure testing for data-size-unciphered\n");
+ return -1;
+ }
/* Process cipher node if present */
cipher_node_offset = fdt_subnode_offset(fit, image_noffset,
--
2.27.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] mkimage: fit: handle FDT_ERR_NOSPACE when ciphering
2020-07-30 4:22 ` [PATCH v2 2/3] mkimage: fit: handle FDT_ERR_NOSPACE when ciphering patrick.oppenlander at gmail.com
@ 2020-07-30 13:53 ` Philippe REYNES
2020-08-08 12:29 ` Tom Rini
1 sibling, 0 replies; 9+ messages in thread
From: Philippe REYNES @ 2020-07-30 13:53 UTC (permalink / raw)
To: u-boot
Hi Patrick,
> From: Patrick Oppenlander <patrick.oppenlander@gmail.com>
>
> Also replace fdt_delprop/fdt_setprop with fdt_setprop as fdt_setprop can
> replace an existing property value.
Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
> Signed-off-by: Patrick Oppenlander <patrick.oppenlander@gmail.com>
Regards,
Philippe
> ---
> tools/image-host.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/tools/image-host.c b/tools/image-host.c
> index dd7ecc4b60..b4603c5f01 100644
> --- a/tools/image-host.c
> +++ b/tools/image-host.c
> @@ -399,23 +399,24 @@ int fit_image_write_cipher(void *fit, int image_noffset,
> int noffset,
> {
> int ret = -1;
>
> - /* Remove unciphered data */
> - ret = fdt_delprop(fit, image_noffset, FIT_DATA_PROP);
> - if (ret) {
> - printf("Can't remove data (err = %d)\n", ret);
> - goto out;
> - }
> -
> - /* Add ciphered data */
> + /* Replace data with ciphered data */
> ret = fdt_setprop(fit, image_noffset, FIT_DATA_PROP,
> data_ciphered, data_ciphered_len);
> + if (ret == -FDT_ERR_NOSPACE) {
> + ret = -ENOSPC;
> + goto out;
> + }
> if (ret) {
> - printf("Can't add ciphered data (err = %d)\n", ret);
> + printf("Can't replace data with ciphered data (err = %d)\n", ret);
> goto out;
> }
>
> /* add non ciphered data size */
> ret = fdt_setprop_u32(fit, image_noffset, "data-size-unciphered", size);
> + if (ret == -FDT_ERR_NOSPACE) {
> + ret = -ENOSPC;
> + goto out;
> + }
> if (ret) {
> printf("Can't add unciphered data size (err = %d)\n", ret);
> goto out;
> --
> 2.27.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] mkimage: fit: don't cipher ciphered data
2020-07-30 4:22 ` [PATCH v2 3/3] mkimage: fit: don't cipher ciphered data patrick.oppenlander at gmail.com
@ 2020-07-30 13:58 ` Philippe REYNES
2020-08-08 12:29 ` Tom Rini
1 sibling, 0 replies; 9+ messages in thread
From: Philippe REYNES @ 2020-07-30 13:58 UTC (permalink / raw)
To: u-boot
Hi Patrick,
> From: Patrick Oppenlander <patrick.oppenlander@gmail.com>
>
> Previously, mkimage -F could be run multiple times causing already
> ciphered image data to be ciphered again.
Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
> Signed-off-by: Patrick Oppenlander <patrick.oppenlander@gmail.com>
Regards,
Philippe
> ---
> tools/image-host.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/tools/image-host.c b/tools/image-host.c
> index b4603c5f01..e5417beee5 100644
> --- a/tools/image-host.c
> +++ b/tools/image-host.c
> @@ -482,7 +482,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest,
> const char *image_name;
> const void *data;
> size_t size;
> - int cipher_node_offset;
> + int cipher_node_offset, len;
>
> /* Get image name */
> image_name = fit_get_name(fit, image_noffset, NULL);
> @@ -497,6 +497,19 @@ int fit_image_cipher_data(const char *keydir, void
> *keydest,
> return -1;
> }
>
> + /*
> + * Don't cipher ciphered data.
> + *
> + * If the data-size-unciphered property is present the data for this
> + * image is already encrypted. This is important as 'mkimage -F' can be
> + * run multiple times on a FIT image.
> + */
> + if (fdt_getprop(fit, image_noffset, "data-size-unciphered", &len))
> + return 0;
> + if (len != -FDT_ERR_NOTFOUND) {
> + printf("Failure testing for data-size-unciphered\n");
> + return -1;
> + }
>
> /* Process cipher node if present */
> cipher_node_offset = fdt_subnode_offset(fit, image_noffset,
> --
> 2.27.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] mkimage: fit: only process one cipher node
2020-07-30 4:22 ` [PATCH v2 1/3] mkimage: fit: only process one cipher node patrick.oppenlander at gmail.com
@ 2020-08-08 12:29 ` Tom Rini
0 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2020-08-08 12:29 UTC (permalink / raw)
To: u-boot
On Thu, Jul 30, 2020 at 02:22:13PM +1000, patrick.oppenlander at gmail.com wrote:
> From: Patrick Oppenlander <patrick.oppenlander@gmail.com>
>
> Previously mkimage would process any node matching the regex cipher.*
> and apply the ciphers to the image data in the order they appeared in
> the FDT. This meant that data could be inadvertently ciphered multiple
> times.
>
> Switch to processing a single cipher node which exactly matches
> FIT_CIPHER_NODENAME.
>
> Signed-off-by: Patrick Oppenlander <patrick.oppenlander@gmail.com>
> Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200808/abe10912/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] mkimage: fit: handle FDT_ERR_NOSPACE when ciphering
2020-07-30 4:22 ` [PATCH v2 2/3] mkimage: fit: handle FDT_ERR_NOSPACE when ciphering patrick.oppenlander at gmail.com
2020-07-30 13:53 ` Philippe REYNES
@ 2020-08-08 12:29 ` Tom Rini
1 sibling, 0 replies; 9+ messages in thread
From: Tom Rini @ 2020-08-08 12:29 UTC (permalink / raw)
To: u-boot
On Thu, Jul 30, 2020 at 02:22:14PM +1000, patrick.oppenlander at gmail.com wrote:
> From: Patrick Oppenlander <patrick.oppenlander@gmail.com>
>
> Also replace fdt_delprop/fdt_setprop with fdt_setprop as fdt_setprop can
> replace an existing property value.
>
> Signed-off-by: Patrick Oppenlander <patrick.oppenlander@gmail.com>
> Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200808/3ca47d9b/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] mkimage: fit: don't cipher ciphered data
2020-07-30 4:22 ` [PATCH v2 3/3] mkimage: fit: don't cipher ciphered data patrick.oppenlander at gmail.com
2020-07-30 13:58 ` Philippe REYNES
@ 2020-08-08 12:29 ` Tom Rini
1 sibling, 0 replies; 9+ messages in thread
From: Tom Rini @ 2020-08-08 12:29 UTC (permalink / raw)
To: u-boot
On Thu, Jul 30, 2020 at 02:22:15PM +1000, patrick.oppenlander at gmail.com wrote:
> From: Patrick Oppenlander <patrick.oppenlander@gmail.com>
>
> Previously, mkimage -F could be run multiple times causing already
> ciphered image data to be ciphered again.
>
> Signed-off-by: Patrick Oppenlander <patrick.oppenlander@gmail.com>
> Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200808/8bee209e/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-08-08 12:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-30 4:22 [PATCH v2] mkimage: FIT ciphering bug fixes patrick.oppenlander at gmail.com
2020-07-30 4:22 ` [PATCH v2 1/3] mkimage: fit: only process one cipher node patrick.oppenlander at gmail.com
2020-08-08 12:29 ` Tom Rini
2020-07-30 4:22 ` [PATCH v2 2/3] mkimage: fit: handle FDT_ERR_NOSPACE when ciphering patrick.oppenlander at gmail.com
2020-07-30 13:53 ` Philippe REYNES
2020-08-08 12:29 ` Tom Rini
2020-07-30 4:22 ` [PATCH v2 3/3] mkimage: fit: don't cipher ciphered data patrick.oppenlander at gmail.com
2020-07-30 13:58 ` Philippe REYNES
2020-08-08 12:29 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.