Re: [resent][PATCH 3/3] mkimage: Allow to specify the signature algorithm on the command line

Hi Jan,

On Fri, 14 Jan 2022 at 02:21, Jan Kiszka jan.kiszka@siemens.com wrote:

From: Jan Kiszka jan.kiszka@siemens.com

This permits to prepare FIT image description that do not hard-code the final choice of the signature algorithm, possibly requiring the user to patch the sources.

When -o <algo> is specified, this information is used in favor of the 'algo' property in the signature node. Furthermore, that property is set accordingly when writing the image.

Signed-off-by: Jan Kiszka jan.kiszka@siemens.com

doc/mkimage.1 | 5 +++++ include/image.h | 3 ++- tools/fit_image.c | 3 ++- tools/image-host.c | 48 +++++++++++++++++++++++++++------------------- tools/imagetool.h | 1 + tools/mkimage.c | 5 ++++- 6 files changed, 42 insertions(+), 23 deletions(-)

Please add a test to test_vboot for this case.

diff --git a/doc/mkimage.1 b/doc/mkimage.1 index fea5288784..0734bd36a1 100644 --- a/doc/mkimage.1 +++ b/doc/mkimage.1 @@ -155,6 +155,11 @@ the corresponding public key is written into this file for for run-time verification. Typically the file here is the device tree binary used by CONFIG_OF_CONTROL in U-Boot.

+.TP +.BI "-o [" "signing algorithm" "]" +Specifies the algorithm to be used for signing a FIT image. The default is +taken from the target signature nodes 'algo' properties.

What does 'target' mean in this case?

Perhaps 'taken from the signature node's 'algo' properties' ?

.TP .BI "-p [" "external position" "]" Place external data at a static external position. See -E. Instead of writing diff --git a/include/image.h b/include/image.h index 16ccc5b10f..4a7e9bc9a1 100644 --- a/include/image.h +++ b/include/image.h @@ -1031,6 +1031,7 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit,

• @require_keys: Mark all keys as 'required'
• @engine_id: Engine to use for signing
• @cmdname: Command name used when reporting errors

• • @algo_name: Algorithm name, or NULL if to be read from FIT
  • Adds hash values for all component images in the FIT blob.
  • Hashes are calculated for all component images which have hash subnodes

@@ -1045,7 +1046,7 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit, int fit_add_verification_data(const char *keydir, const char *keyfile, void *keydest, void *fit, const char *comment, int require_keys, const char *engine_id,

•                         const char *cmdname);
  

•                         const char *cmdname, const char *algo_name);
  
  

int fit_image_verify_with_data(const void *fit, int image_noffset, const void *data, size_t size); diff --git a/tools/fit_image.c b/tools/fit_image.c index f4f372ba62..428ddcf881 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -73,7 +73,8 @@ static int fit_add_file_data(struct image_tool_params *params, size_t size_inc, params->comment, params->require_keys, params->engine_id,

•                                           params->cmdname);
  

•                                           params->cmdname,
  

•                                           params->algo_name);
    }
  
    if (dest_blob) {
  

diff --git a/tools/image-host.c b/tools/image-host.c index a027155f3c..d2e67a06aa 100644 --- a/tools/image-host.c +++ b/tools/image-host.c @@ -107,7 +107,7 @@ static int fit_image_process_hash(void *fit, const char *image_name, */ 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, const char *cmdname)
  

•           int region_proplen, const char *cmdname, const char *algo_name)
  

{ int string_size; int ret; @@ -150,6 +150,8 @@ static int fit_image_write_sig(void *fit, int noffset, uint8_t *value, strdata, sizeof(strdata)); } }

•   if (algo_name && !ret)
  

•           ret = fdt_setprop_string(fit, noffset, "algo", algo_name);
  
    return ret;
  

} @@ -157,17 +159,18 @@ static int fit_image_write_sig(void *fit, int noffset, uint8_t *value, static int fit_image_setup_sig(struct image_sign_info *info, const char *keydir, const char *keyfile, void *fit, const char *image_name, int noffset, const char *require_keys,

•           const char *engine_id)
  

•           const char *engine_id, const char *algo_name)
  

{ const char *node_name;

•   const char *algo_name;
    const char *padding_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;
  

•   if (!algo_name) {
  

•           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;
  

•           }
    }
  
    padding_name = fdt_getprop(fit, noffset, "padding", NULL);
  

@@ -215,7 +218,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile, void *keydest, void *fit, const char *image_name, int noffset, const void *data, size_t size, const char *comment, int require_keys, const char *engine_id,

•           const char *cmdname)
  

•           const char *cmdname, const char *algo_name)
  

{ struct image_sign_info info; struct image_region region; @@ -226,7 +229,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile,

    if (fit_image_setup_sig(&info, keydir, keyfile, fit, image_name,
                            noffset, require_keys ? "image" : NULL,

•                           engine_id))
  

•                           engine_id, algo_name))
            return -1;
  
    node_name = fit_get_name(fit, noffset, NULL);
  

@@ -244,7 +247,7 @@ static int fit_image_process_sig(const char *keydir, const char *keyfile, }

    ret = fit_image_write_sig(fit, noffset, value, value_len, comment,

•                   NULL, 0, cmdname);
  

•                   NULL, 0, cmdname, algo_name);
    if (ret) {
            if (ret == -FDT_ERR_NOSPACE)
                    return -ENOSPC;
  

@@ -606,7 +609,7 @@ int fit_image_cipher_data(const char *keydir, void *keydest, int fit_image_add_verification_data(const char *keydir, const char *keyfile, void *keydest, void *fit, int image_noffset, const char *comment, int require_keys, const char *engine_id,

•           const char *cmdname)
  

•           const char *cmdname, const char* algo_name)
  

{ const char *image_name; const void *data; @@ -643,7 +646,8 @@ int fit_image_add_verification_data(const char *keydir, const char *keyfile, strlen(FIT_SIG_NODENAME))) { ret = fit_image_process_sig(keydir, keyfile, keydest, fit, image_name, noffset, data, size,

•                           comment, require_keys, engine_id, cmdname);
  

•                           comment, require_keys, engine_id, cmdname,
  

•                           algo_name);
            }
            if (ret)
                    return ret;
  

@@ -927,7 +931,8 @@ static int fit_config_get_data(void *fit, int conf_noffset, int noffset, static int fit_config_process_sig(const char *keydir, const char *keyfile, void *keydest, void *fit, const char *conf_name, int conf_noffset, int noffset, const char *comment,

•           int require_keys, const char *engine_id, const char *cmdname)
  

•           int require_keys, const char *engine_id, const char *cmdname,
  

•           const char *algo_name)
  

{ struct image_sign_info info; const char *node_name; @@ -945,7 +950,8 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile, return -1;

    if (fit_image_setup_sig(&info, keydir, keyfile, fit, conf_name, noffset,

•                           require_keys ? "conf" : NULL, engine_id))
  

•                           require_keys ? "conf" : NULL, engine_id,
  

•                           algo_name))
            return -1;
  
    ret = info.crypto->sign(&info, region, region_count, &value,
  

@@ -962,7 +968,8 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile, }

    ret = fit_image_write_sig(fit, noffset, value, value_len, comment,

•                           region_prop, region_proplen, cmdname);
  

•                             region_prop, region_proplen, cmdname,
  

•                             algo_name);
    if (ret) {
            if (ret == -FDT_ERR_NOSPACE)
                    return -ENOSPC;
  

@@ -992,7 +999,7 @@ static int fit_config_process_sig(const char *keydir, const char *keyfile, static int fit_config_add_verification_data(const char *keydir, const char *keyfile, void *keydest, void *fit, int conf_noffset, const char *comment, int require_keys, const char *engine_id,

•           const char *cmdname)
  

•           const char *cmdname, const char *algo_name)
  

{ const char *conf_name; int noffset; @@ -1011,7 +1018,7 @@ static int fit_config_add_verification_data(const char *keydir, strlen(FIT_SIG_NODENAME))) { ret = fit_config_process_sig(keydir, keyfile, keydest, fit, conf_name, conf_noffset, noffset, comment,

•                           require_keys, engine_id, cmdname);
  

•                           require_keys, engine_id, cmdname, algo_name);
            }
            if (ret)
                    return ret;
  

@@ -1058,7 +1065,7 @@ int fit_cipher_data(const char *keydir, void *keydest, void *fit, int fit_add_verification_data(const char *keydir, const char *keyfile, void *keydest, void *fit, const char *comment, int require_keys, const char *engine_id,

•                         const char *cmdname)
  

•                         const char *cmdname, const char *algo_name)
  

{ int images_noffset, confs_noffset; int noffset; @@ -1082,7 +1089,7 @@ int fit_add_verification_data(const char *keydir, const char *keyfile, */ ret = fit_image_add_verification_data(keydir, keyfile, keydest, fit, noffset, comment, require_keys, engine_id,

•                           cmdname);
  

•                           cmdname, algo_name);
            if (ret)
                    return ret;
    }
  

@@ -1106,7 +1113,8 @@ int fit_add_verification_data(const char *keydir, const char *keyfile, ret = fit_config_add_verification_data(keydir, keyfile, keydest, fit, noffset, comment, require_keys,

•                                                  engine_id, cmdname);
  

•                                                  engine_id, cmdname,
  

•                                                  algo_name);
            if (ret)
                    return ret;
    }
  

diff --git a/tools/imagetool.h b/tools/imagetool.h index e229a34ffc..d71027317f 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -69,6 +69,7 @@ struct image_tool_params { const char *keydest; /* Destination .dtb for public key */ const char *keyfile; /* Filename of private or public key */ const char *comment; /* Comment to add to signature node */

•   const char *algo_name;  /* Algorithm name to use hashing/signing */
  

NULL to use the one in the .its ?

    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 */

diff --git a/tools/mkimage.c b/tools/mkimage.c index a4844d0f18..ddb79331a6 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -154,7 +154,7 @@ static void process_args(int argc, char **argv) int opt;

    while ((opt = getopt(argc, argv,

•              "a:A:b:B:c:C:d:D:e:Ef:FG:k:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) {
  

•              "a:A:b:B:c:C:d:D:e:Ef:FG:k:i:K:ln:N:p:o:O:rR:qstT:vVx")) != -1) {
            switch (opt) {
            case 'a':
                    params.addr = strtoull(optarg, &ptr, 16);
  

@@ -250,6 +250,9 @@ static void process_args(int argc, char **argv) case 'N': params.engine_id = optarg; break;

•           case 'o':
  

•                   params.algo_name = optarg;
  

•                   break;
            case 'O':
                    params.os = genimg_get_os_id(optarg);
                    if (params.os < 0) {
  

-- 2.31.1