Hi Joel, Sorry, for this very late answer.

Le 11/11/2020 à 12:18, Joel Stanley a écrit :

If CONFIG_FIT_CIPHER=y and CONFIG_FIT_SIGNATURE=n then mkimage and 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 51123fd92983..103b3ab8a7f2 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

Hi Joel,

sorry for this very late answer ..

This patch fix this issue when only the ciphering is enabled. But it breaks the compilation when signature and ciphering are enabled, because both functions image_set_host_blob and image_get_host_blob are defined twice. So it is a NAK for me.

A simple way to fix it to move this block of code from image-fit-sig.c to iamge-fit.c

Regards, Philippe

Le 11/11/2020 à 12:18, 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 or 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'

The implementation is the same as common/image-fit-sig.c.

Signed-off-by: Joel Stanley joel@jms.id.au

common/image-cipher.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)

diff --git a/common/image-cipher.c b/common/image-cipher.c index 4ca9eec4ef15..fcbbceb847a5 100644 --- a/common/image-cipher.c +++ b/common/image-cipher.c @@ -15,6 +15,20 @@ DECLARE_GLOBAL_DATA_PTR; #include <uboot_aes.h> #include <u-boot/aes.h>

+#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

• struct cipher_algo cipher_algos[] = { { .name = "aes128",

On Mon, 7 Dec 2020 at 17:57, Philippe REYNES philippe.reynes@softathome.com wrote:

Hi Joel,

Sorry for this very late answer.

You're right, this is a bad behaviour, but I think that this solution remove too many options. For example, If signature is disabled, this solution also disable the padding in the fit image.

Ok. I can amend my patch but it wasn't obvious which commands should be moved from outside the FIT_SIGNATURE guard.

Just -E and -B?

Looking a bit deeper, this patch removes all options that are not displayed by the help of mkimage when signature is disabled. But I think that this help is not correct. If the signature is disabled, the padding is still available. So I think that we have an issue in the help too.

Regards, Philippe

Le 11/11/2020 à 12:18, 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 what they got wrong, as mkimage will silently ignore the signing related options.

Make it obvious that the commands don't work by removing them from the getopt opt_string.

The tool will now behave as follows:

$ 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

tools/mkimage.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c index e78608293e72..10a1b3dc8c18 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -142,6 +142,12 @@ 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:C:d:D:e:f:i:ln:O:R:qstT:vVx" +#endif

• static void process_args(int argc, char **argv) { char *ptr;

@@ -149,8 +155,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);