* [PATCH v2 0/4] mkimage usability fixes
@ 2020-12-08 4:12 Joel Stanley
2020-12-08 4:12 ` [PATCH v2 1/4] tools/Makefile: FIT_CIPHER requires libssl Joel Stanley
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Joel Stanley @ 2020-12-08 4:12 UTC (permalink / raw)
To: u-boot
v2: Move -B and -E out from FIT_SIGNATURE check, and other fixes from
Philippe's review.
Original cover letter:
I was learning about signed FIT today and was unable to convince mkimage
to produce a signature node in my control device tree. It turns out that
Debian's packaged mkimage doesn't enable FIT_SIGNATURE, so the options I
was passing were silently ignored.
There's an existing Debian bug for that[1] but in the mean time this
changes mkimage's behaviour to fail loudly when FIT_SIGNATURE is
disabled.
The first two patches fix some issues I found when testing the third
patch under sandbox_defconfig with FIT_SIGNATURE=n.
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=972513
Joel Stanley (4):
tools/Makefile: FIT_CIPHER requires libssl
image-fit: Fix FIT_CIPHER linking
mkimage: Move padding commands outside of FIT_SIGNATURE
mkimge: Reject signing-related flags without FIT_SIGNATURE
common/image-fit-sig.c | 14 --------------
common/image-fit.c | 15 +++++++++++++++
tools/Makefile | 2 +-
tools/mkimage.c | 18 +++++++++++-------
4 files changed, 27 insertions(+), 22 deletions(-)
--
2.29.2
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/4] tools/Makefile: FIT_CIPHER requires libssl
2020-12-08 4:12 [PATCH v2 0/4] mkimage usability fixes Joel Stanley
@ 2020-12-08 4:12 ` Joel Stanley
2020-12-08 15:25 ` Philippe REYNES
2021-01-23 17:46 ` Tom Rini
2020-12-08 4:12 ` [PATCH v2 2/4] image-fit: Fix FIT_CIPHER linking Joel Stanley
` (2 subsequent siblings)
3 siblings, 2 replies; 14+ messages in thread
From: Joel Stanley @ 2020-12-08 4:12 UTC (permalink / raw)
To: u-boot
If CONFIG_FIT_CIPHER is enabled without CONFIG_FIT_SIGNATURE then
mkimage/dumpimage will fail to link:
/usr/bin/ld: tools/common/image-cipher.o: in function `fit_image_decrypt_data':
image-cipher.c:(.text+0x9a): undefined reference to `image_get_host_blob'
/usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x10): undefined reference to `EVP_aes_128_cbc'
/usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x40): undefined reference to `EVP_aes_192_cbc'
/usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x70): undefined reference to `EVP_aes_256_cbc'
/usr/bin/ld: tools/lib/aes/aes-encrypt.o: in function `image_aes_encrypt':
aes-encrypt.c:(.text+0x22): undefined reference to `EVP_CIPHER_CTX_new'
/usr/bin/ld: aes-encrypt.c:(.text+0x6f): undefined reference to `EVP_EncryptInit_ex'
/usr/bin/ld: aes-encrypt.c:(.text+0x8d): undefined reference to `EVP_EncryptUpdate'
/usr/bin/ld: aes-encrypt.c:(.text+0xac): undefined reference to `EVP_CIPHER_CTX_free'
/usr/bin/ld: aes-encrypt.c:(.text+0xf2): undefined reference to `EVP_EncryptFinal_ex'
collect2: error: ld returned 1 exit status
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
tools/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/Makefile b/tools/Makefile
index 253a6b97065b..99a931312cd8 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -154,7 +154,7 @@ HOSTCFLAGS_kwbimage.o += -DCONFIG_KWB_SECURE
endif
# MXSImage needs LibSSL
-ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_ARMADA_39X)$(CONFIG_FIT_SIGNATURE),)
+ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_ARMADA_39X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
HOSTCFLAGS_kwbimage.o += \
$(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
HOSTLDLIBS_mkimage += \
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] image-fit: Fix FIT_CIPHER linking
2020-12-08 4:12 [PATCH v2 0/4] mkimage usability fixes Joel Stanley
2020-12-08 4:12 ` [PATCH v2 1/4] tools/Makefile: FIT_CIPHER requires libssl Joel Stanley
@ 2020-12-08 4:12 ` Joel Stanley
2020-12-08 15:26 ` Philippe REYNES
2021-01-23 17:46 ` Tom Rini
2020-12-08 4:12 ` [PATCH v2 3/4] mkimage: Move padding commands outside of FIT_SIGNATURE Joel Stanley
2020-12-08 4:12 ` [PATCH v2 4/4] mkimge: Reject signing-related flags without FIT_SIGNATURE Joel Stanley
3 siblings, 2 replies; 14+ messages in thread
From: Joel Stanley @ 2020-12-08 4:12 UTC (permalink / raw)
To: u-boot
When CONFIG_FIT_CIPHER=y and CONFIG_FIT_SIGNATURE=n is there is no
implementation of image_get_host_blob for mkimage/dumpimage:
/usr/bin/ld: tools/common/image-cipher.o: in function `fit_image_decrypt_data':
image-cipher.c:(.text+0x9a): undefined reference to `image_get_host_blob'
Move the implementation to a common file so it can be shaed between
image-cipher.c and image-fit-sig.c.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
v2: Fix compilation when signature and ciphering are both enabled
---
common/image-fit-sig.c | 14 --------------
common/image-fit.c | 15 +++++++++++++++
2 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
index 5401d9411b98..d39741e9058f 100644
--- a/common/image-fit-sig.c
+++ b/common/image-fit-sig.c
@@ -19,20 +19,6 @@ DECLARE_GLOBAL_DATA_PTR;
#define IMAGE_MAX_HASHED_NODES 100
-#ifdef USE_HOSTCC
-void *host_blob;
-
-void image_set_host_blob(void *blob)
-{
- host_blob = blob;
-}
-
-void *image_get_host_blob(void)
-{
- return host_blob;
-}
-#endif
-
/**
* fit_region_make_list() - Make a list of image regions
*
diff --git a/common/image-fit.c b/common/image-fit.c
index c82d4d8015f0..664a0d00006c 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -112,6 +112,21 @@ int fit_parse_subimage(const char *spec, ulong addr_curr,
}
#endif /* !USE_HOSTCC */
+#ifdef USE_HOSTCC
+/* Host tools use these implementations for Cipher and Signature support */
+static void *host_blob;
+
+void image_set_host_blob(void *blob)
+{
+ host_blob = blob;
+}
+
+void *image_get_host_blob(void)
+{
+ return host_blob;
+}
+#endif /* USE_HOSTCC */
+
static void fit_get_debug(const void *fit, int noffset,
char *prop_name, int err)
{
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] mkimage: Move padding commands outside of FIT_SIGNATURE
2020-12-08 4:12 [PATCH v2 0/4] mkimage usability fixes Joel Stanley
2020-12-08 4:12 ` [PATCH v2 1/4] tools/Makefile: FIT_CIPHER requires libssl Joel Stanley
2020-12-08 4:12 ` [PATCH v2 2/4] image-fit: Fix FIT_CIPHER linking Joel Stanley
@ 2020-12-08 4:12 ` Joel Stanley
2020-12-08 15:35 ` Philippe REYNES
2021-01-23 17:46 ` Tom Rini
2020-12-08 4:12 ` [PATCH v2 4/4] mkimge: Reject signing-related flags without FIT_SIGNATURE Joel Stanley
3 siblings, 2 replies; 14+ messages in thread
From: Joel Stanley @ 2020-12-08 4:12 UTC (permalink / raw)
To: u-boot
These commands were disabled when CONFIG_FIT_SIGNATURE is disabled, but
they do not depend on crypto support so they can be unconditionally
enabled.
Signed-off-by: Joel Stanley <joel@jms.id.au>
--
v2: New patch
---
tools/mkimage.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c
index e78608293e72..68d5206cb4fd 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -94,18 +94,18 @@ static void usage(const char *msg)
" -x ==> set XIP (execute in place)\n",
params.cmdname);
fprintf(stderr,
- " %s [-D dtc_options] [-f fit-image.its|-f auto|-F] [-b <dtb> [-b <dtb>]] [-i <ramdisk.cpio.gz>] fit-image\n"
+ " %s [-D dtc_options] [-f fit-image.its|-f auto|-F] [-b <dtb> [-b <dtb>]] [-E] [-B size] [-i <ramdisk.cpio.gz>] fit-image\n"
" <dtb> file is used with -f auto, it may occur multiple times.\n",
params.cmdname);
fprintf(stderr,
" -D => set all options for device tree compiler\n"
" -f => input filename for FIT source\n"
- " -i => input filename for ramdisk file\n");
+ " -i => input filename for ramdisk file\n"
+ " -E => place data outside of the FIT structure\n"
+ " -B => align size in hex for FIT structure and header\n");
#ifdef CONFIG_FIT_SIGNATURE
fprintf(stderr,
- "Signing / verified boot options: [-E] [-B size] [-k keydir] [-K dtb] [ -c <comment>] [-p addr] [-r] [-N engine]\n"
- " -E => place data outside of the FIT structure\n"
- " -B => align size in hex for FIT structure and header\n"
+ "Signing / verified boot options: [-k keydir] [-K dtb] [ -c <comment>] [-p addr] [-r] [-N engine]\n"
" -k => set directory containing private keys\n"
" -K => write public keys to this .dtb file\n"
" -c => add comment in signature node\n"
@@ -142,6 +142,7 @@ static int add_content(int type, const char *fname)
return 0;
}
+#define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
static void process_args(int argc, char **argv)
{
char *ptr;
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] mkimge: Reject signing-related flags without FIT_SIGNATURE
2020-12-08 4:12 [PATCH v2 0/4] mkimage usability fixes Joel Stanley
` (2 preceding siblings ...)
2020-12-08 4:12 ` [PATCH v2 3/4] mkimage: Move padding commands outside of FIT_SIGNATURE Joel Stanley
@ 2020-12-08 4:12 ` Joel Stanley
2020-12-08 15:38 ` Philippe REYNES
2021-01-22 21:58 ` Tom Rini
3 siblings, 2 replies; 14+ messages in thread
From: Joel Stanley @ 2020-12-08 4:12 UTC (permalink / raw)
To: u-boot
When CONFIG_FIT_SIGNATURE=n the signing options are not available. If a
user is careful they will notice this when looking at the help output.
If they are not careful they will waste several hours wondering why
their FIT doesn't contain a /signature node, as mkimage will silently
ingore the signing related options.
Make it obvious that the commands don't work by removing them from the
getopt opt_string.
$ mkimage -f machine.its -k keys -K u-boot-pubkey.dtb -r image.fit
mkimage: invalid option -- 'k'
Error: Invalid option
Signed-off-by: Joel Stanley <joel@jms.id.au>
--
v2: Leave padding related options in the CONFIG_FIT_SIGNATURE=y optargs
---
tools/mkimage.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c
index 68d5206cb4fd..f7d3ac40e681 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -142,7 +142,11 @@ static int add_content(int type, const char *fname)
return 0;
}
+#ifdef CONFIG_FIT_SIGNATURE
#define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
+#else
+#define OPT_STRING "a:A:b:B:C:d:D:e:Ef:i:ln:O:R:qstT:vVx"
+#endif
static void process_args(int argc, char **argv)
{
char *ptr;
@@ -150,8 +154,7 @@ static void process_args(int argc, char **argv)
char *datafile = NULL;
int opt;
- while ((opt = getopt(argc, argv,
- "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) {
+ while ((opt = getopt(argc, argv, OPT_STRING)) != -1) {
switch (opt) {
case 'a':
params.addr = strtoull(optarg, &ptr, 16);
--
2.29.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 1/4] tools/Makefile: FIT_CIPHER requires libssl
2020-12-08 4:12 ` [PATCH v2 1/4] tools/Makefile: FIT_CIPHER requires libssl Joel Stanley
@ 2020-12-08 15:25 ` Philippe REYNES
2021-01-23 17:46 ` Tom Rini
1 sibling, 0 replies; 14+ messages in thread
From: Philippe REYNES @ 2020-12-08 15:25 UTC (permalink / raw)
To: u-boot
Hi Joel,
Le 08/12/2020 ? 05:12, Joel Stanley a ?crit?:
> If CONFIG_FIT_CIPHER is enabled without CONFIG_FIT_SIGNATURE then
> mkimage/dumpimage will fail to link:
>
> /usr/bin/ld: tools/common/image-cipher.o: in function `fit_image_decrypt_data':
> image-cipher.c:(.text+0x9a): undefined reference to `image_get_host_blob'
> /usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x10): undefined reference to `EVP_aes_128_cbc'
> /usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x40): undefined reference to `EVP_aes_192_cbc'
> /usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x70): undefined reference to `EVP_aes_256_cbc'
> /usr/bin/ld: tools/lib/aes/aes-encrypt.o: in function `image_aes_encrypt':
> aes-encrypt.c:(.text+0x22): undefined reference to `EVP_CIPHER_CTX_new'
> /usr/bin/ld: aes-encrypt.c:(.text+0x6f): undefined reference to `EVP_EncryptInit_ex'
> /usr/bin/ld: aes-encrypt.c:(.text+0x8d): undefined reference to `EVP_EncryptUpdate'
> /usr/bin/ld: aes-encrypt.c:(.text+0xac): undefined reference to `EVP_CIPHER_CTX_free'
> /usr/bin/ld: aes-encrypt.c:(.text+0xf2): undefined reference to `EVP_EncryptFinal_ex'
> collect2: error: ld returned 1 exit status
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
??? Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
> tools/Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 253a6b97065b..99a931312cd8 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -154,7 +154,7 @@ HOSTCFLAGS_kwbimage.o += -DCONFIG_KWB_SECURE
> endif
>
> # MXSImage needs LibSSL
> -ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_ARMADA_39X)$(CONFIG_FIT_SIGNATURE),)
> +ifneq ($(CONFIG_MX23)$(CONFIG_MX28)$(CONFIG_ARMADA_38X)$(CONFIG_ARMADA_39X)$(CONFIG_FIT_SIGNATURE)$(CONFIG_FIT_CIPHER),)
> HOSTCFLAGS_kwbimage.o += \
> $(shell pkg-config --cflags libssl libcrypto 2> /dev/null || echo "")
> HOSTLDLIBS_mkimage += \
Regards,
Philippe
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] image-fit: Fix FIT_CIPHER linking
2020-12-08 4:12 ` [PATCH v2 2/4] image-fit: Fix FIT_CIPHER linking Joel Stanley
@ 2020-12-08 15:26 ` Philippe REYNES
2021-01-23 17:46 ` Tom Rini
1 sibling, 0 replies; 14+ messages in thread
From: Philippe REYNES @ 2020-12-08 15:26 UTC (permalink / raw)
To: u-boot
Hi Joel
Le 08/12/2020 ? 05:12, Joel Stanley a ?crit?:
> When CONFIG_FIT_CIPHER=y and CONFIG_FIT_SIGNATURE=n is there is no
> implementation of image_get_host_blob for mkimage/dumpimage:
>
> /usr/bin/ld: tools/common/image-cipher.o: in function `fit_image_decrypt_data':
> image-cipher.c:(.text+0x9a): undefined reference to `image_get_host_blob'
>
> Move the implementation to a common file so it can be shaed between
> image-cipher.c and image-fit-sig.c.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
??? Reviewed-by: Philippe Reynes <philippe.reynes@softathome.com>
> ---
> v2: Fix compilation when signature and ciphering are both enabled
> ---
> common/image-fit-sig.c | 14 --------------
> common/image-fit.c | 15 +++++++++++++++
> 2 files changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
> index 5401d9411b98..d39741e9058f 100644
> --- a/common/image-fit-sig.c
> +++ b/common/image-fit-sig.c
> @@ -19,20 +19,6 @@ DECLARE_GLOBAL_DATA_PTR;
>
> #define IMAGE_MAX_HASHED_NODES 100
>
> -#ifdef USE_HOSTCC
> -void *host_blob;
> -
> -void image_set_host_blob(void *blob)
> -{
> - host_blob = blob;
> -}
> -
> -void *image_get_host_blob(void)
> -{
> - return host_blob;
> -}
> -#endif
> -
> /**
> * fit_region_make_list() - Make a list of image regions
> *
> diff --git a/common/image-fit.c b/common/image-fit.c
> index c82d4d8015f0..664a0d00006c 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -112,6 +112,21 @@ int fit_parse_subimage(const char *spec, ulong addr_curr,
> }
> #endif /* !USE_HOSTCC */
>
> +#ifdef USE_HOSTCC
> +/* Host tools use these implementations for Cipher and Signature support */
> +static void *host_blob;
> +
> +void image_set_host_blob(void *blob)
> +{
> + host_blob = blob;
> +}
> +
> +void *image_get_host_blob(void)
> +{
> + return host_blob;
> +}
> +#endif /* USE_HOSTCC */
> +
> static void fit_get_debug(const void *fit, int noffset,
> char *prop_name, int err)
> {
Regards,
Philippe
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] mkimage: Move padding commands outside of FIT_SIGNATURE
2020-12-08 4:12 ` [PATCH v2 3/4] mkimage: Move padding commands outside of FIT_SIGNATURE Joel Stanley
@ 2020-12-08 15:35 ` Philippe REYNES
2021-01-23 17:46 ` Tom Rini
1 sibling, 0 replies; 14+ messages in thread
From: Philippe REYNES @ 2020-12-08 15:35 UTC (permalink / raw)
To: u-boot
Hi Joel
Le 08/12/2020 ? 05:12, Joel Stanley a ?crit?:
> These commands were disabled when CONFIG_FIT_SIGNATURE is disabled, but
> they do not depend on crypto support so they can be unconditionally
> enabled.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> --
> v2: New patch
> ---
> tools/mkimage.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index e78608293e72..68d5206cb4fd 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -94,18 +94,18 @@ static void usage(const char *msg)
> " -x ==> set XIP (execute in place)\n",
> params.cmdname);
> fprintf(stderr,
> - " %s [-D dtc_options] [-f fit-image.its|-f auto|-F] [-b <dtb> [-b <dtb>]] [-i <ramdisk.cpio.gz>] fit-image\n"
> + " %s [-D dtc_options] [-f fit-image.its|-f auto|-F] [-b <dtb> [-b <dtb>]] [-E] [-B size] [-i <ramdisk.cpio.gz>] fit-image\n"
> " <dtb> file is used with -f auto, it may occur multiple times.\n",
> params.cmdname);
> fprintf(stderr,
> " -D => set all options for device tree compiler\n"
> " -f => input filename for FIT source\n"
> - " -i => input filename for ramdisk file\n");
> + " -i => input filename for ramdisk file\n"
> + " -E => place data outside of the FIT structure\n"
> + " -B => align size in hex for FIT structure and header\n");
> #ifdef CONFIG_FIT_SIGNATURE
> fprintf(stderr,
> - "Signing / verified boot options: [-E] [-B size] [-k keydir] [-K dtb] [ -c <comment>] [-p addr] [-r] [-N engine]\n"
> - " -E => place data outside of the FIT structure\n"
> - " -B => align size in hex for FIT structure and header\n"
> + "Signing / verified boot options: [-k keydir] [-K dtb] [ -c <comment>] [-p addr] [-r] [-N engine]\n"
> " -k => set directory containing private keys\n"
> " -K => write public keys to this .dtb file\n"
> " -c => add comment in signature node\n"
Option -p is a generic option, not a signing option. it defines the
offset of the padding.
So it should be moved above.
Option -k, -K are also used for ciphering. So it signature is disabled
and ciphering is enabled, the help won't display those options.
I think it could be acceptable to? use #if
CONFIG_IS_ENABLED(FIT_SIGNATURE) || CONFIG_IS_ENABLED(FIT_CIPHER)
and also change a bit the text "Signing / verified option ..."
> @@ -142,6 +142,7 @@ static int add_content(int type, const char
> *fname) return 0; } +#define OPT_STRING
> "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
I think that OPT_STRING should be defined in the next patch.
> static void process_args(int argc, char **argv)
> {
> char *ptr;
Regards,
Philippe
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] mkimge: Reject signing-related flags without FIT_SIGNATURE
2020-12-08 4:12 ` [PATCH v2 4/4] mkimge: Reject signing-related flags without FIT_SIGNATURE Joel Stanley
@ 2020-12-08 15:38 ` Philippe REYNES
2020-12-12 15:39 ` Simon Glass
2021-01-22 21:58 ` Tom Rini
1 sibling, 1 reply; 14+ messages in thread
From: Philippe REYNES @ 2020-12-08 15:38 UTC (permalink / raw)
To: u-boot
Hi Joel
Le 08/12/2020 ? 05:12, Joel Stanley a ?crit?:
> When CONFIG_FIT_SIGNATURE=n the signing options are not available. If a
> user is careful they will notice this when looking at the help output.
>
> If they are not careful they will waste several hours wondering why
> their FIT doesn't contain a /signature node, as mkimage will silently
> ingore the signing related options.
>
> Make it obvious that the commands don't work by removing them from the
> getopt opt_string.
>
> $ mkimage -f machine.its -k keys -K u-boot-pubkey.dtb -r image.fit
> mkimage: invalid option -- 'k'
> Error: Invalid option
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> --
> v2: Leave padding related options in the CONFIG_FIT_SIGNATURE=y optargs
> ---
> tools/mkimage.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index 68d5206cb4fd..f7d3ac40e681 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -142,7 +142,11 @@ static int add_content(int type, const char *fname)
> return 0;
> }
>
> +#ifdef CONFIG_FIT_SIGNATURE
> #define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
Option -k and -K is also needed for ciphering. So we should also check
FIT_CIPHER.
Option -p is "generic", it is used to define the size of the padding.
> +#else
> +#define OPT_STRING "a:A:b:B:C:d:D:e:Ef:i:ln:O:R:qstT:vVx"
> +#endif
> static void process_args(int argc, char **argv)
> {
> char *ptr;
> @@ -150,8 +154,7 @@ static void process_args(int argc, char **argv)
> char *datafile = NULL;
> int opt;
>
> - while ((opt = getopt(argc, argv,
> - "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) {
> + while ((opt = getopt(argc, argv, OPT_STRING)) != -1) {
> switch (opt) {
> case 'a':
> params.addr = strtoull(optarg, &ptr, 16);
Regards,
Philippe
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] mkimge: Reject signing-related flags without FIT_SIGNATURE
2020-12-08 15:38 ` Philippe REYNES
@ 2020-12-12 15:39 ` Simon Glass
0 siblings, 0 replies; 14+ messages in thread
From: Simon Glass @ 2020-12-12 15:39 UTC (permalink / raw)
To: u-boot
Hi,
On Tue, 8 Dec 2020 at 08:38, Philippe REYNES
<philippe.reynes@softathome.com> wrote:
>
> Hi Joel
>
>
> Le 08/12/2020 ? 05:12, Joel Stanley a ?crit :
> > When CONFIG_FIT_SIGNATURE=n the signing options are not available. If a
> > user is careful they will notice this when looking at the help output.
> >
> > If they are not careful they will waste several hours wondering why
> > their FIT doesn't contain a /signature node, as mkimage will silently
> > ingore the signing related options.
> >
> > Make it obvious that the commands don't work by removing them from the
> > getopt opt_string.
> >
> > $ mkimage -f machine.its -k keys -K u-boot-pubkey.dtb -r image.fit
> > mkimage: invalid option -- 'k'
> > Error: Invalid option
> >
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > --
> > v2: Leave padding related options in the CONFIG_FIT_SIGNATURE=y optargs
> > ---
> > tools/mkimage.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
I have somehow missed these patches on the mailing list. I'm not sure
why, but I'm not going to review them as is.
Regards,
Simon
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 4/4] mkimge: Reject signing-related flags without FIT_SIGNATURE
2020-12-08 4:12 ` [PATCH v2 4/4] mkimge: Reject signing-related flags without FIT_SIGNATURE Joel Stanley
2020-12-08 15:38 ` Philippe REYNES
@ 2021-01-22 21:58 ` Tom Rini
1 sibling, 0 replies; 14+ messages in thread
From: Tom Rini @ 2021-01-22 21:58 UTC (permalink / raw)
To: u-boot
On Tue, Dec 08, 2020 at 02:42:16PM +1030, Joel Stanley wrote:
> When CONFIG_FIT_SIGNATURE=n the signing options are not available. If a
> user is careful they will notice this when looking at the help output.
>
> If they are not careful they will waste several hours wondering why
> their FIT doesn't contain a /signature node, as mkimage will silently
> ingore the signing related options.
>
> Make it obvious that the commands don't work by removing them from the
> getopt opt_string.
>
> $ mkimage -f machine.its -k keys -K u-boot-pubkey.dtb -r image.fit
> mkimage: invalid option -- 'k'
> Error: Invalid option
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> v2: Leave padding related options in the CONFIG_FIT_SIGNATURE=y optargs
> ---
> tools/mkimage.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tools/mkimage.c b/tools/mkimage.c
> index 68d5206cb4fd..f7d3ac40e681 100644
> --- a/tools/mkimage.c
> +++ b/tools/mkimage.c
> @@ -142,7 +142,11 @@ static int add_content(int type, const char *fname)
> return 0;
> }
>
> +#ifdef CONFIG_FIT_SIGNATURE
> #define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
> +#else
> +#define OPT_STRING "a:A:b:B:C:d:D:e:Ef:i:ln:O:R:qstT:vVx"
> +#endif
> static void process_args(int argc, char **argv)
> {
> char *ptr;
> @@ -150,8 +154,7 @@ static void process_args(int argc, char **argv)
> char *datafile = NULL;
> int opt;
>
> - while ((opt = getopt(argc, argv,
> - "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) {
> + while ((opt = getopt(argc, argv, OPT_STRING)) != -1) {
> switch (opt) {
> case 'a':
> params.addr = strtoull(optarg, &ptr, 16);
This part of the series breaks a huge number of platforms default build
because we don't have 'p' which is:
-p => place external data at a static position
and not strictly used in signature only options and is used for
FIT_EXTERNAL_OFFSET. While I could adjust the patch to keep 'p' in both
cases I wanted to bring this up so it's not a surprise and ask you to
update just this patch, or suggest things need to be further adjusted.
The rest of the series seems fine and is under review. 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/20210122/7af9768f/attachment.sig>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 1/4] tools/Makefile: FIT_CIPHER requires libssl
2020-12-08 4:12 ` [PATCH v2 1/4] tools/Makefile: FIT_CIPHER requires libssl Joel Stanley
2020-12-08 15:25 ` Philippe REYNES
@ 2021-01-23 17:46 ` Tom Rini
1 sibling, 0 replies; 14+ messages in thread
From: Tom Rini @ 2021-01-23 17:46 UTC (permalink / raw)
To: u-boot
On Tue, Dec 08, 2020 at 02:42:13PM +1030, Joel Stanley wrote:
> If CONFIG_FIT_CIPHER is enabled without CONFIG_FIT_SIGNATURE then
> mkimage/dumpimage will fail to link:
>
> /usr/bin/ld: tools/common/image-cipher.o: in function `fit_image_decrypt_data':
> image-cipher.c:(.text+0x9a): undefined reference to `image_get_host_blob'
> /usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x10): undefined reference to `EVP_aes_128_cbc'
> /usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x40): undefined reference to `EVP_aes_192_cbc'
> /usr/bin/ld: tools/common/image-cipher.o:(.data.rel+0x70): undefined reference to `EVP_aes_256_cbc'
> /usr/bin/ld: tools/lib/aes/aes-encrypt.o: in function `image_aes_encrypt':
> aes-encrypt.c:(.text+0x22): undefined reference to `EVP_CIPHER_CTX_new'
> /usr/bin/ld: aes-encrypt.c:(.text+0x6f): undefined reference to `EVP_EncryptInit_ex'
> /usr/bin/ld: aes-encrypt.c:(.text+0x8d): undefined reference to `EVP_EncryptUpdate'
> /usr/bin/ld: aes-encrypt.c:(.text+0xac): undefined reference to `EVP_CIPHER_CTX_free'
> /usr/bin/ld: aes-encrypt.c:(.text+0xf2): undefined reference to `EVP_EncryptFinal_ex'
> collect2: error: ld returned 1 exit status
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
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/20210123/054873f4/attachment.sig>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] image-fit: Fix FIT_CIPHER linking
2020-12-08 4:12 ` [PATCH v2 2/4] image-fit: Fix FIT_CIPHER linking Joel Stanley
2020-12-08 15:26 ` Philippe REYNES
@ 2021-01-23 17:46 ` Tom Rini
1 sibling, 0 replies; 14+ messages in thread
From: Tom Rini @ 2021-01-23 17:46 UTC (permalink / raw)
To: u-boot
On Tue, Dec 08, 2020 at 02:42:14PM +1030, Joel Stanley wrote:
> When CONFIG_FIT_CIPHER=y and CONFIG_FIT_SIGNATURE=n is there is no
> implementation of image_get_host_blob for mkimage/dumpimage:
>
> /usr/bin/ld: tools/common/image-cipher.o: in function `fit_image_decrypt_data':
> image-cipher.c:(.text+0x9a): undefined reference to `image_get_host_blob'
>
> Move the implementation to a common file so it can be shaed between
> image-cipher.c and image-fit-sig.c.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
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/20210123/1622cf07/attachment.sig>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 3/4] mkimage: Move padding commands outside of FIT_SIGNATURE
2020-12-08 4:12 ` [PATCH v2 3/4] mkimage: Move padding commands outside of FIT_SIGNATURE Joel Stanley
2020-12-08 15:35 ` Philippe REYNES
@ 2021-01-23 17:46 ` Tom Rini
1 sibling, 0 replies; 14+ messages in thread
From: Tom Rini @ 2021-01-23 17:46 UTC (permalink / raw)
To: u-boot
On Tue, Dec 08, 2020 at 02:42:15PM +1030, Joel Stanley wrote:
> These commands were disabled when CONFIG_FIT_SIGNATURE is disabled, but
> they do not depend on crypto support so they can be unconditionally
> enabled.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
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/20210123/ae105a72/attachment.sig>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-01-23 17:46 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-08 4:12 [PATCH v2 0/4] mkimage usability fixes Joel Stanley
2020-12-08 4:12 ` [PATCH v2 1/4] tools/Makefile: FIT_CIPHER requires libssl Joel Stanley
2020-12-08 15:25 ` Philippe REYNES
2021-01-23 17:46 ` Tom Rini
2020-12-08 4:12 ` [PATCH v2 2/4] image-fit: Fix FIT_CIPHER linking Joel Stanley
2020-12-08 15:26 ` Philippe REYNES
2021-01-23 17:46 ` Tom Rini
2020-12-08 4:12 ` [PATCH v2 3/4] mkimage: Move padding commands outside of FIT_SIGNATURE Joel Stanley
2020-12-08 15:35 ` Philippe REYNES
2021-01-23 17:46 ` Tom Rini
2020-12-08 4:12 ` [PATCH v2 4/4] mkimge: Reject signing-related flags without FIT_SIGNATURE Joel Stanley
2020-12-08 15:38 ` Philippe REYNES
2020-12-12 15:39 ` Simon Glass
2021-01-22 21:58 ` 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.