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

.github/workflows/test-library.yml

@@ -20,7 +20,7 @@ jobs:
       fail-fast: false
 
       # Limit concurrent jobs for scheduling problem on GitHub's hosted runner pool.
-      max-parallel: 12
+      max-parallel: 10
 
       matrix:
         math:
@@ -31,7 +31,7 @@ jobs:
           - "SPMATH=0 SPMATHALL=0 WOLFBOOT_SMALL_STACK=0"
           - "SPMATH=0 SPMATHALL=0 WOLFBOOT_SMALL_STACK=1"
         asym: [ed25519, ecc256, ecc384, ecc521, rsa2048, rsa3072, rsa4096, ed448]
-        hash: [sha256, sha384, sha3]
+        hash: [sha256, sha384, sha3] # --sha256 for commandline, SHA256 for make
 
         # See https://github.com/wolfSSL/wolfBoot/issues/614 regarding exclusions:
         exclude:
@@ -41,41 +41,119 @@ jobs:
     steps:
       - uses: actions/checkout@v4
         with:
+          clean: true
           submodules: true
 
-      - name: make clean
+      - name: hard clean
         run: |
-          make keysclean && make -C tools/keytools clean && rm -f include/target.h
+          # Ensure parallel build did not leave behind any debris
+          make clean || true
+          make keysclean || true
+          make -C tools/keytools clean || true
+
+          # The brute-force clean:
+          rm -rf build build-*     \
+                 test-app          \
+                 build/vis_hal     \
+                 build/vis_cmsis   \
+                 **/*.o            \
+                 **/*.d            \
+                 **/*.a            \
+                 include/target.h  \
+                 src/keystore.c    \
+                 keystore.der      \
+                 pubkey_*.der
 
       - name: Build test-lib
         env:
           shell: bash
-          ASYM: ${{ matrix.asym }}
+          ASYM: ${{ matrix.asym }} # MAKE_SIGN in upper case
           HASH: ${{ matrix.hash }}
           MATH: ${{ matrix.math }}
         run: |
+          # Test various library parameters
+
+          export MAKE_SIGN="${ASYM^^}"
+          export MAKE_HASH="${HASH^^}"
+          export MAKE_MATH='${{ matrix.math }}' # e.g., "SPMATH=1 WOLFBOOT_SMALL_STACK=1"
+          export PRIVATE_KEY="wolfboot_signing_private_key.der"
+
+          echo "This MAKE_SIGN=$MAKE_SIGN"
+          echo "This MAKE_HASH=$MAKE_HASH"
+          echo "This MAKE_MATH=$MAKE_MATH"
+
           # Sample build
           build_once() {
               # Convert asym and hash to upper case, optionally add additional param
-              make -j test-lib SIGN=${ASYM^^} HASH=${HASH^^} ${MATH} "$@"
+              echo "Build test-lib..."
+              echo "make -j1 test-lib SIGN=${MAKE_SIGN} HASH=${MAKE_HASH} ${MATH} \"$@\""
+                    make -j1 test-lib SIGN=${MAKE_SIGN} HASH=${MAKE_HASH} ${MATH}  "$@"
           }
 
           set -euo pipefail
 
           # Get the reference config
           cp config/examples/library.config .config
 
+          # peek
+          echo "Existing files?"
+          if [ -f "src/keystore.c" ]; then
+            echo "WARNING: Found unexpected src/keystore.c"
+          fi
+          if [ -f "include/target.h" ]; then
+            echo "WARNING: Found unexpected include/target.h"
+          fi
+          if [ -f "keystore.der" ]; then
+            echo "WARNING: Found unexpected keystore.der"
+          fi
+          if [ -f "wolfboot_signing_private_key.der" ]; then
+            echo "WARNING: Found unexpected wolfboot_signing_private_key.der"
+          fi
+          if [ -f "./tools/keytools/keystore.der" ]; then
+            echo "WARNING: Found unexpected ./tools/keytools/keystore.der"
+          fi
+          if [ -f "./tools/keytools/wolfboot_signing_private_key.der" ]; then
+            echo "WARNING: Found unexpected ./tools/keytools/wolfboot_signing_private_key.der"
+          fi
+
           # Keytools
-          make keytools
-          ./tools/keytools/keygen --${ASYM} -g wolfboot_signing_private_key.der
+          echo ""
+          echo "make -j1 keytools SIGN=\"${MAKE_SIGN}\" HASH=\"${MAKE_HASH}\" $MATH"
+                make -j1 keytools SIGN="${MAKE_SIGN}"   HASH="${MAKE_HASH}"   $MATH
+
+          # Generate keys
+          echo ""
+          echo "./tools/keytools/keygen --${ASYM} -g wolfboot_signing_private_key.der"
+                ./tools/keytools/keygen --${ASYM} -g wolfboot_signing_private_key.der
+
+          # Force fresh files
+          # peek
+          echo "Existing files?"
+          if [ -f "src/keystore.c" ]; then
+            echo "Found unexpected src/keystore.c"
+          fi
+          if [ -f "include/target.h" ]; then
+            echo "Found unexpected include/target.h"
+          fi
+          if [ -f "keystore.der" ]; then
+            echo "Found unexpected keystore.der"
+          fi
+          if [ -f "wolfboot_signing_private_key.der" ]; then
+            echo "Found unexpected wolfboot_signing_private_key.der"
+          fi
 
           # Sign
+          echo ""
           echo "Test" > test.bin
-          ./tools/keytools/sign --${ASYM} --${HASH} test.bin wolfboot_signing_private_key.der 1
+          echo "Sign test.bin"
+          echo "./tools/keytools/sign --${ASYM} --${HASH} test.bin wolfboot_signing_private_key.der 1"
+                ./tools/keytools/sign --${ASYM} --${HASH} test.bin wolfboot_signing_private_key.der 1
 
           # First attempt
           if build_once >build.out 2>build.err; then
             echo "Success on first attempt, WOLFBOOT_HUGE_STACK not applied."
+            cat build.out
+            cat build.err
             exit 0
           fi
 
@@ -90,12 +168,16 @@ jobs:
             build_once WOLFBOOT_HUGE_STACK=1
           else
             echo "Build failed for another reason:"
+            cat build.out
             cat build.err
             exit 1
           fi
 
       - name: Run test-lib
         run: |
+          # Check test_v1_signed.bin
+
+          echo "./test-lib test_v1_signed.bin"
           ./test-lib test_v1_signed.bin
           ./test-lib test_v1_signed.bin 2>&1 | grep "Firmware Valid"
 
@@ -104,5 +186,25 @@ jobs:
           # Corrupt signed binary
           truncate -s -1 test_v1_signed.bin
           echo "A" >> test_v1_signed.bin
-          ./test-lib test_v1_signed.bin
-          ./test-lib test_v1_signed.bin 2>&1 | grep "Failure"
+
+          # Run once, capture output and status
+          set +e
+          output=$(./test-lib test_v1_signed.bin 2>&1)
+          status=$?
+          set -e
+
+          echo "$output"
+
+          # Must have failed (non-zero exit)
+          if [ $status -eq 0 ]; then
+            echo "Expected failure, but exit code was 0"
+            exit 1
+          fi
+
+          # Must include the expected Failure message
+          echo "$output" | grep -F "Failure" >/dev/null || {
+            echo "Expected 'Failure' not found in output"
+            exit 1
+          }
+
+          echo "Got expected non-zero exit and 'Failure' message."

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)

include/printf.h

@@ -66,6 +66,9 @@
 #   elif defined(WOLFBOOT_LOG_PRINTF)
         /* allow output to stdout */
 #       define wolfBoot_printf(_f_, ...) printf(_f_, ##__VA_ARGS__)
+#   elif defined(_MSC_VER)
+#       include <stdio.h>
+#       define wolfBoot_printf(...) do { fprintf(stderr, __VA_ARGS__); } while (0)
 #   else
         /* use stderr by default */
 #       define wolfBoot_printf(_f_, ...) fprintf(stderr, _f_, ##__VA_ARGS__)

include/wolfboot/wolfboot.h

@@ -178,6 +178,7 @@ extern "C" {
 
 /* Hashing configuration */
 #if defined(WOLFBOOT_HASH_SHA256)
+    #define WOLFBOOT_KEYHASH_HAS_RESULT
     #include "wolfssl/wolfcrypt/sha256.h"
 #   ifndef WOLFBOOT_SHA_BLOCK_SIZE
 #     define WOLFBOOT_SHA_BLOCK_SIZE (256)