Minor code cleanups.

Author: dgarske

.github/workflows/make-test-swtpm.yml

@@ -68,7 +68,7 @@ jobs:
             wolftpm_config: --enable-st33
           # STMicro ST33KTPM2
           - name: st33ktpm2 firmware
-            wolftpm_config: --enable-st33 --enable-firmware --enable-st
+            wolftpm_config: --enable-st33 --enable-firmware
           # Microchip
           - name: microchip
             wolftpm_config: --enable-microchip

.gitignore

@@ -84,6 +84,7 @@ examples/boot/secret_seal
 examples/boot/secret_unseal
 examples/firmware/ifx_fw_extract
 examples/firmware/ifx_fw_update
+examples/firmware/st33_fw_update
 examples/endorsement/get_ek_certs
 examples/endorsement/verify_ek_cert
 

examples/firmware/include.am

@@ -8,28 +8,31 @@ EXTRA_DIST += examples/firmware/Makefile
 EXTRA_DIST += examples/firmware/ifx_fw_extract.c
 
 if BUILD_EXAMPLES
-if BUILD_INFINEON
 if BUILD_FIRMWARE
+
+if BUILD_INFINEON
 noinst_PROGRAMS += examples/firmware/ifx_fw_update
 noinst_HEADERS  += examples/firmware/ifx_fw_update.h
 examples_firmware_ifx_fw_update_SOURCES      = examples/firmware/ifx_fw_update.c \
                                                examples/tpm_test_keys.c
 examples_firmware_ifx_fw_update_LDADD        = src/libwolftpm.la $(LIB_STATIC_ADD)
 examples_firmware_ifx_fw_update_DEPENDENCIES = src/libwolftpm.la
 endif
-endif
+
 if BUILD_ST33
-if BUILD_FIRMWARE
 noinst_PROGRAMS += examples/firmware/st33_fw_update
 examples_firmware_st33_fw_update_SOURCES     = examples/firmware/st33_fw_update.c \
                                                examples/tpm_test_keys.c
 examples_firmware_st33_fw_update_LDADD       = src/libwolftpm.la $(LIB_STATIC_ADD)
 examples_firmware_st33_fw_update_DEPENDENCIES = src/libwolftpm.la
+
 endif
 endif
 endif
 
 example_firmwaredir = $(exampledir)/firmware
-dist_example_firmware_DATA = examples/firmware/ifx_fw_update.c
+dist_example_firmware_DATA = examples/firmware/ifx_fw_update.c \
+                             examples/firmware/st33_fw_update.c
 
-DISTCLEANFILES+= examples/firmware/.libs/ifx_fw_update
+DISTCLEANFILES+= examples/firmware/.libs/ifx_fw_update \
+                 examples/firmware/.libs/st33_fw_update

examples/firmware/st33_fw_update.c

@@ -138,20 +138,25 @@ static int TPM2_ST33_SendFirmwareData(fw_info_t* fwinfo)
     return rc;
 }
 
+/* Callback function for firmware data access
+ * Returns the actual number of bytes copied (may be less than requested at end of buffer)
+ * Returns BUFFER_E on error (offset out of bounds) */
 static int TPM2_ST33_FwData_Cb(uint8_t* data, uint32_t data_req_sz,
     uint32_t offset, void* cb_ctx)
 {
     fw_info_t* fwinfo = (fw_info_t*)cb_ctx;
+    uint32_t actual_sz = data_req_sz;
+    
     if (offset > fwinfo->firmware_bufSz) {
         return BUFFER_E;
     }
     if (offset + data_req_sz > (uint32_t)fwinfo->firmware_bufSz) {
-        data_req_sz = (uint32_t)fwinfo->firmware_bufSz - offset;
+        actual_sz = (uint32_t)fwinfo->firmware_bufSz - offset;
     }
-    if (data_req_sz > 0) {
-        XMEMCPY(data, &fwinfo->firmware_buf[offset], data_req_sz);
+    if (actual_sz > 0) {
+        XMEMCPY(data, &fwinfo->firmware_buf[offset], actual_sz);
     }
-    return data_req_sz;
+    return actual_sz;  /* Return actual bytes copied */
 }
 
 static void TPM2_ST33_PrintInfo(WOLFTPM2_CAPS* caps)

src/tpm2_wrap.c

@@ -8695,21 +8695,26 @@ int wolfTPM2_FirmwareUpgradeCancel(WOLFTPM2_DEV* dev)
 
 /* ST33 uses password auth (TPM_RS_PW) for firmware update, not policy */
 
-/* Start firmware upgrade (non-LMS): sends manifest (blob0=177 bytes) 
- * via FieldUpgradeStart */
-static int tpm2_st33_firmware_start(WOLFTPM2_DEV* dev,
-    uint8_t* manifest, uint32_t manifest_sz)
+/* Common firmware upgrade start function
+ * Sends manifest (blob0) via FieldUpgradeStart
+ * 300ms delay: ST reference implementation uses this delay to allow
+ * TPM to switch modes after FieldUpgradeStart command */
+static int tpm2_st33_firmware_start_common(WOLFTPM2_DEV* dev,
+    uint8_t* manifest, uint32_t manifest_sz, int is_lms)
 {
     int rc;
 
     (void)dev;
 
     /* ST33 uses password auth (TPM_RS_PW) for FieldUpgradeStart.
-     * This matches the ST reference implementation behavior. */
+     * This matches the ST reference implementation behavior.
+     * For LMS format, the manifest (blob0) already contains the embedded
+     * LMS signature. Send the full manifest directly. */
     rc = TPM2_ST33_FieldUpgradeStart(TPM_RS_PW, manifest, manifest_sz);
 
     if (rc == TPM_RC_SUCCESS) {
-        /* delay to give the TPM time to switch modes */
+        /* 300ms delay: ST reference implementation uses this delay to allow
+         * TPM to switch modes after FieldUpgradeStart command */
         tpm2_firmware_sleep_ms(300);
 
     #if !defined(WOLFTPM_LINUX_DEV) && !defined(WOLFTPM_SWTPM) && \
@@ -8720,43 +8725,28 @@ static int tpm2_st33_firmware_start(WOLFTPM2_DEV* dev,
     }
 #ifdef DEBUG_WOLFTPM
     if (rc != TPM_RC_SUCCESS) {
-        printf("ST33 Firmware upgrade start failed 0x%x: %s\n",
-            rc, TPM2_GetRCString(rc));
+        printf("ST33 Firmware upgrade start%s failed 0x%x: %s\n",
+            is_lms ? " (LMS)" : "", rc, TPM2_GetRCString(rc));
     }
+#else
+    (void)is_lms;  /* Suppress unused parameter warning when DEBUG_WOLFTPM not defined */
 #endif
     return rc;
 }
 
+/* Start firmware upgrade (non-LMS): sends manifest (blob0=177 bytes) 
+ * via FieldUpgradeStart */
+static int tpm2_st33_firmware_start(WOLFTPM2_DEV* dev,
+    uint8_t* manifest, uint32_t manifest_sz)
+{
+    return tpm2_st33_firmware_start_common(dev, manifest, manifest_sz, 0);
+}
+
 /* Start firmware upgrade (LMS): sends manifest (blob0=2697 bytes with embedded signature) via FieldUpgradeStart */
 static int tpm2_st33_firmware_start_lms(WOLFTPM2_DEV* dev,
     uint8_t* manifest, uint32_t manifest_sz)
 {
-    int rc;
-
-    (void)dev;
-
-    /* ST33 LMS: The manifest (blob0) already contains the embedded
-     * LMS signature. Send the full manifest directly.
-     * Uses password auth (TPM_RS_PW) like ST reference implementation. */
-    rc = TPM2_ST33_FieldUpgradeStart(TPM_RS_PW, manifest, manifest_sz);
-
-    if (rc == TPM_RC_SUCCESS) {
-        /* delay to give the TPM time to switch modes */
-        tpm2_firmware_sleep_ms(300);
-
-    #if !defined(WOLFTPM_LINUX_DEV) && !defined(WOLFTPM_SWTPM) && \
-        !defined(WOLFTPM_WINAPI)
-        /* Do chip startup and request locality again */
-        rc = TPM2_ChipStartup(&dev->ctx, 10);
-    #endif
-    }
-#ifdef DEBUG_WOLFTPM
-    if (rc != TPM_RC_SUCCESS) {
-        printf("ST33 Firmware upgrade start (LMS) failed 0x%x: %s\n",
-            rc, TPM2_GetRCString(rc));
-    }
-#endif
-    return rc;
+    return tpm2_st33_firmware_start_common(dev, manifest, manifest_sz, 1);
 }
 
 /* ST33 sends full manifest (blob0) directly in FieldUpgradeStart */
@@ -8790,16 +8780,27 @@ static int tpm2_st33_firmware_data(WOLFTPM2_DEV* dev,
         /* Read blob header: type (1 byte) + length (2 bytes big-endian) */
         rc = cb(blob_header, 3, offset, cb_ctx);
         if (rc < 3) {
-            /* End of data or error */
-            if (rc == 0 || blob_header[0] == 0) {
-            #ifdef DEBUG_WOLFTPM
-                printf("ST33 Firmware data done\n");
-            #endif
-                rc = TPM_RC_SUCCESS;
+            /* Handle end of data or error conditions */
+            if (rc == 0) {
+                /* End of data - check if we're at a valid end marker */
+                if (offset == 0 || blob_header[0] == 0) {
+                #ifdef DEBUG_WOLFTPM
+                    printf("ST33 Firmware data done\n");
+                #endif
+                    rc = TPM_RC_SUCCESS;
+                }
+                else {
+                #ifdef DEBUG_WOLFTPM
+                    printf("ST33 Firmware data incomplete at offset %u\n", offset);
+                #endif
+                    rc = BUFFER_E;
+                }
             }
             else {
+                /* Partial read (< 3 bytes) is an error - need complete header */
             #ifdef DEBUG_WOLFTPM
-                printf("ST33 Firmware data read error at offset %u\n", offset);
+                printf("ST33 Firmware data read error at offset %u (got %d bytes, need 3)\n",
+                    offset, rc);
             #endif
                 rc = BUFFER_E;
             }
@@ -8817,6 +8818,16 @@ static int tpm2_st33_firmware_data(WOLFTPM2_DEV* dev,
 
         /* Parse blob length from bytes 1-2 (big-endian) */
         blob_len = ((uint32_t)blob_header[1] << 8) | blob_header[2];
+        
+        /* Check for integer overflow before calculating total size */
+        if (blob_len > UINT32_MAX - 3) {
+        #ifdef DEBUG_WOLFTPM
+            printf("ST33 Blob length overflow at offset %u: %u\n", offset, blob_len);
+        #endif
+            rc = BUFFER_E;
+            break;
+        }
+        
         blob_total = blob_len + 3; /* total blob size including header */
 
         if (blob_total > ST33_FW_MAX_CHUNK_SZ + 3) {
@@ -8860,7 +8871,8 @@ static int tpm2_st33_firmware_data(WOLFTPM2_DEV* dev,
     XFREE(blob_buf, NULL, DYNAMIC_TYPE_TMP_BUFFER);
 
     if (rc == TPM_RC_SUCCESS) {
-        /* Give the TPM time to process */
+        /* 300ms delay: ST reference implementation uses this delay to allow
+         * TPM to process firmware data blobs */
         tpm2_firmware_sleep_ms(300);
 
     #if !defined(WOLFTPM_LINUX_DEV) && !defined(WOLFTPM_SWTPM) && \

tests/unit_tests.c

@@ -343,9 +343,65 @@ static void test_wolfTPM2_ST33_FirmwareUpgrade(void)
         rc = wolfTPM2_FirmwareUpgradeWithLMS(&dev, dummy_manifest,
             sizeof(dummy_manifest), NULL, NULL, dummy_sig, sizeof(dummy_sig));
         AssertIntNE(rc, 0);
+
+        /* ===== Test version detection edge cases ===== */
+        
+        /* Test version threshold boundary: exactly 511 (< 512, non-LMS) */
+        if (caps.fwVerMinor == 511) {
+            /* Should require non-LMS format */
+            rc = wolfTPM2_FirmwareUpgradeHashWithLMS(&dev, TPM_ALG_SHA384, NULL, 0,
+                dummy_manifest, sizeof(dummy_manifest), NULL, NULL,
+                dummy_sig, sizeof(dummy_sig));
+            /* Should fail because LMS signature provided but version < 512 */
+            AssertIntNE(rc, 0);
+        }
+        
+        /* Test version threshold boundary: exactly 512 (>= 512, LMS required) */
+        if (caps.fwVerMinor == 512) {
+            /* Should require LMS format */
+            rc = wolfTPM2_FirmwareUpgradeHash(&dev, TPM_ALG_SHA384, NULL, 0,
+                dummy_manifest, sizeof(dummy_manifest), NULL, NULL);
+            /* Should fail because LMS signature required but not provided */
+            AssertIntNE(rc, 0);
+        }
+        
+        /* Test version threshold boundary: exactly 513 (>= 512, LMS required) */
+        if (caps.fwVerMinor == 513) {
+            /* Should require LMS format */
+            rc = wolfTPM2_FirmwareUpgradeHash(&dev, TPM_ALG_SHA384, NULL, 0,
+                dummy_manifest, sizeof(dummy_manifest), NULL, NULL);
+            /* Should fail because LMS signature required but not provided */
+            AssertIntNE(rc, 0);
+        }
     }
 #endif /* !WOLFTPM2_NO_WOLFCRYPT */
 
+    /* ===== Test callback edge cases ===== */
+    {
+        /* Test callback with offset beyond buffer size */
+        typedef struct {
+            uint8_t* buf;
+            size_t bufSz;
+        } test_cb_ctx_t;
+        
+        uint8_t test_buf[10] = {0};
+        test_cb_ctx_t test_ctx = {test_buf, sizeof(test_buf)};
+        int cb_rc;
+        
+        /* Simulate callback behavior: offset beyond buffer */
+        if (sizeof(test_buf) + 1 > test_ctx.bufSz) {
+            cb_rc = BUFFER_E;  /* Should return error */
+            AssertIntNE(cb_rc, 0);
+        }
+        
+        /* Simulate callback behavior: partial read at end of buffer */
+        if (sizeof(test_buf) - 2 + 5 > test_ctx.bufSz) {
+            /* Should adjust requested size to fit */
+            size_t actual_sz = test_ctx.bufSz - (sizeof(test_buf) - 2);
+            AssertIntEQ(actual_sz, 2);  /* Should only read 2 bytes */
+        }
+    }
+
     rc = 0;
 
     wolfTPM2_Cleanup(&dev);

wolftpm/tpm2_wrap.h

@@ -4211,9 +4211,8 @@ WOLFTPM_API int wolfTPM2_FirmwareUpgrade(WOLFTPM2_DEV* dev,
 /*!
     \ingroup wolfTPM2_Wrappers
     \brief Perform TPM firmware upgrade with LMS signature (ST33KTPM)
-    \note For ST33KTPM devices with firmware version >= 256, supports LMS signature
-    \note For firmware version >= 915, LMS signature is required
-    \note For firmware version 256-914, LMS signature is optional
+    \note For ST33KTPM devices with firmware version < 512, non-LMS format required (legacy firmware, e.g., 9.257)
+    \note For ST33KTPM devices with firmware version >= 512, LMS format required (modern firmware, e.g., 9.512)
 
     \return TPM_RC_SUCCESS: successful
     \return TPM_RC_FAILURE: generic failure (check TPM IO and TPM return code)