Improved image, keygen, sign, test lib messages. Polish

Author: gojimmypi

hal/library.c

@@ -124,14 +124,17 @@ int wolfBoot_start(void)
     os_image.hdr = (uint8_t*)gImage;
 
     if ((ret = wolfBoot_open_image_address(&os_image, (uint8_t*)gImage)) < 0) {
+        wolfBoot_printf("Failed to open image address.\n");
         goto exit;
     }
 
     if ((ret = wolfBoot_verify_integrity(&os_image)) < 0) {
+        wolfBoot_printf("Failed to verify integrity.\n");
         goto exit;
     }
 
     if ((ret = wolfBoot_verify_authenticity(&os_image)) < 0) {
+        wolfBoot_printf("Failed to verify authenticity.\n");
         goto exit;
     }
 
@@ -142,11 +145,13 @@ int wolfBoot_start(void)
  exit:
     if (ret < 0) {
         wolfBoot_printf("Failure %d: Hdr %d, Hash %d, Sig %d\n", ret,
-                        (int)os_image.hdr_ok, (int)os_image.sha_ok,
-                        (int)os_image.signature_ok);
+                        os_image.hdr_ok, os_image.sha_ok, os_image.signature_ok);
+        return -1;
+    }
+    else {
+        return 0;
     }
 
-    return 0;
 }
 
 
@@ -155,39 +160,62 @@ int main(int argc, const char* argv[])
     int ret = 0;
 
 #ifdef NO_FILESYSTEM
+    wolfBoot_printf("NO_FILESYSTEM is defined, looking at test_img");
     gImage = (uintptr_t)test_img;
 #else
-    if (argc > 1) {
-        size_t sz = 0, bread;
+    if (argc == 2) {
+        size_t sz = 0, bread = 0;
         FILE* img = fopen(argv[1], "rb");
         if (img == NULL) {
-            wolfBoot_printf("failed to open %s!\n", argv[1]);
+            wolfBoot_printf("Failed to open file: %s!\n\n", argv[1]);
+            wolfBoot_printf("Usage: %s image_file.bin\n", argv[0]);
             return -3;
         }
-        fseek(img, 0, SEEK_END);
-        sz = ftell(img);
-        fseek(img, 0, SEEK_SET);
+        else {
+            wolfBoot_printf("Looking at image file: %s\n", argv[1]);
+            fseek(img, 0, SEEK_END);
+            sz = ftell(img);
+            fseek(img, 0, SEEK_SET);
+
+            gImage = (uintptr_t)malloc(sz);
+        }
 
-        gImage = (uintptr_t)malloc(sz);
         if (((void*)gImage) == NULL) {
-            wolfBoot_printf("failed to malloc %zu bytes for image\n", sz);
+            wolfBoot_printf("Failed to malloc %zu bytes for image.\n", sz);
             ret = -1;
         }
+        else {
+            /* check the image */
+            bread = fread((void*)gImage, 1, sz, img);
+        }
 
-        bread = fread((void*)gImage, 1, sz, img);
-        if (bread != sz) {
+        if (bread == sz) {
+            wolfBoot_printf("Confirmed expected size: %zu bytes.\n", bread);
+        }
+        else {
             ret = -2;
-            wolfBoot_printf("read %zu of %zu bytes from %s\n", bread, sz, argv[1]);
+            wolfBoot_printf("Read %zu of %zu bytes from %s\n", bread, sz, argv[1]);
         }
         fclose(img);
-    } else {
+    }
+    else {
         wolfBoot_printf("usage: %s image_file.bin\n", argv[0]);
         ret = 255;
     }
 #endif
     if (ret == 0) {
+        wolfBoot_printf("Checking image... ");
         ret = wolfBoot_start();
     }
+    if (ret == 0) {
+        wolfBoot_printf("Success!\n");
+    }
+    else {
+        if (ret != 255) {
+            /* Only show error if we actually processed file, not missing params */
+            wolfBoot_printf("Failed to verify with wolfBoot_start\n");
+        }
+    }
 
 #ifndef NO_FILESYSTEM
     if ((void*)gImage != NULL)

src/image.c

@@ -941,7 +941,7 @@ static int header_sha256(wc_Sha256 *sha256_ctx, struct wolfBoot_image *img)
     while (p < end_sha) {
         blksz = WOLFBOOT_SHA_BLOCK_SIZE;
         if (end_sha - p < blksz)
-            blksz = end_sha - p;
+            blksz = (int)(end_sha - p);
         wc_Sha256Update(sha256_ctx, p, blksz);
         p += blksz;
     }
@@ -988,20 +988,32 @@ static int image_sha256(struct wolfBoot_image *img, uint8_t *hash)
  * @param key_slot The key slot ID to calculate the hash for.
  * @param hash A pointer to store the resulting SHA256 hash.
  */
-static void key_sha256(uint8_t key_slot, uint8_t *hash)
+static int key_sha256(uint8_t key_slot, uint8_t *hash)
 {
     uint8_t *pubkey = keystore_get_buffer(key_slot);
     int pubkey_sz = keystore_get_size(key_slot);
     wc_Sha256 sha256_ctx;
+    int ret = 0;
+
+    if (!pubkey || (pubkey_sz < 0)) {
+        return -1;
+    }
+#ifndef WOLFBOOT_KEYHASH_HAS_RESULT
+    wolfBoot_printf("This hash result must define WOLFBOOT_KEYHASH_HAS_RESULT");
+    return -1;
+#endif
 
     memset(hash, 0, SHA256_DIGEST_SIZE);
-    if (!pubkey || (pubkey_sz < 0))
-        return;
 
-    wc_InitSha256(&sha256_ctx);
-    wc_Sha256Update(&sha256_ctx, pubkey, (word32)pubkey_sz);
-    wc_Sha256Final(&sha256_ctx, hash);
+    ret = wc_InitSha256(&sha256_ctx);
+    if (ret == 0) {
+        ret = wc_Sha256Update(&sha256_ctx, pubkey, (word32)pubkey_sz);
+    }
+    if (ret == 0) {
+        ret = wc_Sha256Final(&sha256_ctx, hash);
+    }
     wc_Sha256Free(&sha256_ctx);
+    return ret;
 }
 #endif /* WOLFBOOT_NO_SIGN */
 #endif /* SHA2-256 */
@@ -2192,6 +2204,17 @@ uint8_t* wolfBoot_peek_image(struct wolfBoot_image *img, uint32_t offset,
 
 #if !defined(WOLFBOOT_NO_SIGN) && !defined(WOLFBOOT_RENESAS_SCEPROTECT)
 
+/* Normalize the call so we always get an int status: 0 == OK, <0 == error */
+static inline int key_hash_ok(int id, uint8_t* digest)
+{
+#ifdef WOLFBOOT_KEYHASH_HAS_RET
+    return key_hash(id, digest);
+#else
+    key_hash(id, digest);
+    return 0;
+#endif
+}
+
 /**
  * @brief Get the key slot ID by SHA hash.
  *
@@ -2200,18 +2223,37 @@ uint8_t* wolfBoot_peek_image(struct wolfBoot_image *img, uint32_t offset,
  *
  * @param hint The SHA hash of the public key to search for.
  * @return The key slot ID if found, -1 if the key was not found.
+ *         Other negative values if the key_hash function failed.
  */
+
 int keyslot_id_by_sha(const uint8_t *hint)
 {
     int id;
+    int ret = -1;
+    int ct = 0;
+    if (hint == NULL) {
+        return -1;
+    }
+    if (WOLFBOOT_SHA_DIGEST_SIZE <= 0) {
+        return -1;
+    }
 
     for (id = 0; id < keystore_num_pubkeys(); id++) {
-        key_hash(id, digest);
-        if (memcmp(digest, hint, WOLFBOOT_SHA_DIGEST_SIZE) == 0) {
+        ct++;
+        ret = key_hash_ok(id, digest);
+        if ((ret == 0) && memcmp(digest, hint, WOLFBOOT_SHA_DIGEST_SIZE) == 0) {
+            wolfBoot_printf("Found matching digest in slot %d\n", id);
             return id;
         }
     }
-    return -1;
+
+    if (ret == 0) {
+        /* Calls to key_hash were successful, but we did not find one. Fail: */
+        wolfBoot_printf("No matching key hash found. Looked at %d slot(s)", ct);
+        ret = -1;
+    }
+    /* Reminder: zero based slot array. */
+    return ret;
 }
 #endif /* !WOLFBOOT_NO_SIGN && !WOLFBOOT_RENESAS_SCEPROTECT */
 

tools/keytools/keygen.c

@@ -503,10 +503,10 @@ void keystore_add(uint32_t ktype, uint8_t *key, uint32_t sz, const char *keyfile
     }
     fprintf(fpub, Pubkey_footer);
     fprintf(fpub, Slot_footer);
-    printf("Associated key file:   %s\n", keyfile);
+    printf("Associated key file:  %s\n", keyfile);
     printf("Partition ids mask:   %08x\n", id_mask);
-    printf("Key type   :           %s\n", KName[ktype]);
-    printf("Public key slot:       %u\n", id_slot);
+    printf("Key type:             %s\n", KName[ktype]);
+    printf("Public key slot:      %u\n", id_slot);
     if (noLocalKeys) {
         printf("WARNING: --nolocalkeys flag used, keystore.c public key is zeroed\n");
     }
@@ -1089,7 +1089,7 @@ static void keygen_ml_dsa(const char *priv_fname, uint32_t id_mask)
     if (exportPubKey) {
         if (saveAsDer) {
             uint8_t*  pubDer;
-            size_t    pubDerSz;
+            word32    pubDerSz;
             int       pubOutLen;
             const int WITH_ALG_SPKI = 1;
 
@@ -1349,10 +1349,11 @@ int main(int argc, char** argv)
     uint32_t n_pubkeys = 0;
     uint32_t part_id_mask = 0xFFFFFFFF; /* Default: key verify all */
 
+    printf("wolfBoot KeyGen\n");
 #ifdef DEBUG_SIGNTOOL
     wolfSSL_Debugging_ON();
 #endif
-    printf("Keystore size: %lu\n", (unsigned long)sizeof(struct keystore_slot));
+    printf("Keystore size:        %lu\n", (unsigned long)sizeof(struct keystore_slot));
 
     /* Check arguments and print usage */
     if (argc < 2)
@@ -1436,6 +1437,7 @@ int main(int argc, char** argv)
             i++;
             sprintf(pubkeyfile,"%s%s", argv[i], "/keystore.c");
             sprintf(pubkeyimg, "%s%s", argv[i], "/keystore.der");
+            printf("Saving keystore file: %s\n", pubkeyfile);
             i++;
             continue;
         }
@@ -1453,20 +1455,23 @@ int main(int argc, char** argv)
             usage(argv[0]);
         }
     }
-    printf("Keytype: %s\n", KName[keytype]);
-    if (keytype == 0)
+    printf("Selected Keytype:     %s\n", KName[keytype]);
+    if (keytype == 0) {
+        fprintf(stderr, "No keytype, exiting...");
         exit(0);
+    }
     fpub = fopen(pubkeyfile, "rb");
     if (!force && (fpub != NULL)) {
         char reply[40];
         int replySz;
-        printf("** Warning: keystore already exists! Are you sure you want to generate a new key and overwrite the existing key? [Type 'Yes']: ");
+        printf("** Warning: keystore file already exists! %s\n", pubkeyfile);
+        printf("Are you sure you want to generate a new key and overwrite the existing key ? [Type 'Yes'] : ");
         fflush(stdout);
         replySz = scanf("%s", reply);
         printf("Reply is [%s]\n", reply);
         fclose(fpub);
         if (replySz < 0 || strcmp(reply, "Yes") != 0) {
-            printf("Operation aborted by user.");
+            printf("Operation aborted by user.\n");
             exit(5);
         } else {
             unlink(pubkeyfile);