fix(secure-boot): a debug lock that failed must not report a successful boot

core/ed25519_verify.c

@@ -289,6 +289,16 @@ static int point_is_identity(gf p[4])
     return diff == 0;
 }
 
+static void scalarbase(gf r[4], const uint8_t *s)
+{
+    gf q[4];
+    fe_copy16(q[0], BX);
+    fe_copy16(q[1], BY);
+    fe_copy16(q[2], gf1);
+    fe_mul(q[3], BX, BY);
+    scalarmult(r, q, s);
+}
+
 /* Reject a public key outside the prime-order subgroup.
  *
  * Decoding a point is not enough. Ed25519 has eight points of low order, and
@@ -304,51 +314,9 @@ static int point_is_identity(gf p[4])
  * so there is no separate constant to transcribe wrongly: a mistyped L would
  * reject valid keys, and only in the field.
  *
- * A arrives negated from unpackneg(). [L](-A) = -[L]A and the identity is its
- * own negation, so neither condition is affected by the sign.
- *
- * Formulation taken from eBoot#57 by @muhammadburhandevv-hub, which reached
- * this before I did and states both conditions in one expression.
+ * The key arrives negated from unpackneg(). [L](-A) = -[L]A and the identity
+ * is its own negation, so neither condition is affected by the sign.
  */
-static int key_has_prime_order(gf A[4])
-{
-    uint8_t order_l[32];
-    gf q[4], multiple[4];
-    int i;
-
-    for (i = 0; i < 32; i++)
-        order_l[i] = (uint8_t)ORDER_L[i];
-    for (i = 0; i < 4; i++)
-        fe_copy16(q[i], A[i]);
-
-    scalarmult(multiple, q, order_l);
-    return point_is_identity(multiple) && !point_is_identity(A);
-}
-
-static void scalarbase(gf r[4], const uint8_t *s)
-{
-    gf q[4];
-    fe_copy16(q[0], BX);
-    fe_copy16(q[1], BY);
-    fe_copy16(q[2], gf1);
-    fe_mul(q[3], BX, BY);
-    scalarmult(r, q, s);
-}
-
-static int point_is_identity(gf p[4])
-{
-    uint8_t encoded[32];
-    point_pack(encoded, p);
-
-    uint8_t diff = (uint8_t)(encoded[0] ^ 1U);
-    for (int i = 1; i < 32; i++)
-        diff |= encoded[i];
-    return diff == 0;
-}
-
-/* Public keys must be non-identity points in Ed25519's prime-order subgroup.
- * Merely decoding a point is insufficient: an identity or torsion key can
- * make the verification equation true without knowledge of a private key. */
 static int public_key_is_valid_subgroup(gf public_key[4])
 {
     uint8_t order_l[32];

core/secure_boot.c

@@ -102,18 +102,41 @@ eos_secure_boot_result_t eos_secure_boot(const eos_secure_boot_config_t *cfg,
         }
 
         /* ---- Step 4: Verify signing key against OTP root-of-trust ---- */
-        /* Extract key hash from TLV area */
-        /* The TLV_KEYHASH entry contains SHA-256 of the public key */
+        /*
+         * This step is PLANNED, not implemented: comparing the image's
+         * TLV_KEYHASH against the OTP anchor needs TLV extraction that does
+         * not exist here yet. What it must not do in the meantime is report
+         * success.
+         *
+         * It used to. An otp_read failure was discarded and boot continued,
+         * and when the read succeeded and the anchor was provisioned the body
+         * of the `if` was a comment -- so a device whose root of trust *is*
+         * provisioned booted an image signed by any key the image carried,
+         * and step 8 recorded EOS_SBOOT_OK. That is the same fail-open this
+         * PR removes from step 7, two steps earlier.
+         *
+         * Until the comparison exists, a provisioned device fails closed. An
+         * unprovisioned one (all-zero anchor) is unchanged: there is nothing
+         * to check against, and refusing would brick every un-provisioned
+         * board.
+         */
         uint8_t otp_key_hash[32];
         rc = eos_hal_otp_read(OTP_KEY_HASH_OFFSET, otp_key_hash, OTP_KEY_HASH_SIZE);
-        if (rc == EOS_OK) {
-            /* Check if OTP key hash is provisioned (not all-zeros) */
-            uint8_t zeros[32] = {0};
-            if (secure_compare(otp_key_hash, zeros, 32) != 0) {
-                /* OTP is provisioned — must match */
-                /* In a full implementation, extract key hash from TLV and compare */
-                /* For now, the signature verification implicitly uses the embedded key */
-            }
+        if (rc != EOS_OK) {
+            /* The anchor could not be read, so it cannot be checked. A
+             * verification step that cannot run must fail, not pass. */
+            attest_record(2, hdr.image_version, hdr.hash, NULL,
+                          EOS_SBOOT_ERR_SIGNATURE);
+            return EOS_SBOOT_ERR_SIGNATURE;
+        }
+
+        uint8_t zeros[32] = {0};
+        if (secure_compare(otp_key_hash, zeros, 32) != 0) {
+            /* Provisioned, and nothing here compares against it. Refuse
+             * rather than boot on a key this function has not checked. */
+            attest_record(2, hdr.image_version, hdr.hash, NULL,
+                          EOS_SBOOT_ERR_SIGNATURE);
+            return EOS_SBOOT_ERR_SIGNATURE;
         }
     }
 
@@ -182,8 +205,19 @@ eos_secure_boot_result_t eos_secure_boot(const eos_secure_boot_config_t *cfg,
     }
 
     /* ---- Step 7: Lock debug interfaces ---- */
+    /*
+     * A policy that asked for the debug port to be closed and did not get
+     * it is a policy violation, not a detail. The result used to be
+     * discarded, so a board whose OTP write failed -- or one with no
+     * otp_write at all, where the HAL returns EOS_ERR_NOT_SUPPORTED --
+     * booted with SWD/JTAG open while attestation recorded EOS_SBOOT_OK.
+     */
     if (cfg->lock_debug) {
-        eos_secure_boot_lock_debug();
+        if (eos_secure_boot_lock_debug() != EOS_OK) {
+            attest_record(2, hdr.image_version, hdr.hash, NULL,
+                          EOS_SBOOT_ERR_POLICY);
+            return EOS_SBOOT_ERR_POLICY;
+        }
     }
 
     /* ---- Step 8: Record successful attestation ---- */
@@ -212,17 +246,21 @@ int eos_secure_boot_verify_key(const uint8_t key_hash[32])
     return secure_compare(key_hash, otp_hash, 32);
 }
 
-void eos_secure_boot_lock_debug(void)
+int eos_secure_boot_lock_debug(void)
 {
     /* Write lock pattern to eFuse debug lock register */
     uint8_t lock = 0xFF;
-    eos_hal_otp_write(OTP_DEBUG_LOCK_OFFSET, &lock, 1);
+    int rc = eos_hal_otp_write(OTP_DEBUG_LOCK_OFFSET, &lock, 1);
+    if (rc != EOS_OK)
+        return rc;
 
     /* On Cortex-M: disable DAP access via DHCSR if supported */
 #if defined(__ARM_ARCH)
     /* Some MCUs support disabling debug via DBGMCU register */
     /* *((volatile uint32_t *)0xE0042004) = 0; */
 #endif
+
+    return EOS_OK;
 }
 
 int eos_secure_boot_update_rollback(uint32_t new_version)

include/eos_image.h

@@ -108,7 +108,8 @@ EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, tlv_hash) +
 
 /* Every remaining field, pinned.
  *
- * Four of the fourteen fields were asserted. Transposing two adjacent
+ * Three of the thirteen field offsets were asserted (the fourth pre-existing
+ * assert is sizeof, which is not a field). Transposing two adjacent
  * same-width fields moves neither sizeof nor any of those four offsets, so it
  * compiled clean: with load_addr and entry_addr swapped, all four existing
  * asserts still passed and the bootloader would load an image at its entry
@@ -132,15 +133,18 @@ EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, flags) == 24,
                       "flags must stay at offset 24");
 EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, sig_len) == 61,
                       "sig_len must stay at offset 61");
-EOS_IMG_STATIC_ASSERT(offsetof(eos_image_header_t, reserved) == 62,
-                      "reserved[] must stay at offset 62");
+/* tlv_len and tlv_hash are asserted above, where #93 introduced them; the
+ * 30 bytes they occupy are the ones this block used to pin as reserved[]. */
 
 /* Field widths. An offset assert cannot see a field growing into padding that
- * happens to keep every later offset -- reserved[] absorbs exactly that. */
+ * happens to keep every later offset -- the 30 bytes at 62 absorb exactly
+ * that, which is why both halves of that span carry a width assert. */
 EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->hash) == 32,
                       "hash[] is 32 bytes on the wire");
-EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->reserved) == 30,
-                      "reserved[] is 30 bytes on the wire");
+EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->tlv_len) == 2,
+                      "tlv_len is 2 bytes on the wire");
+EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->tlv_hash) == 28,
+                      "tlv_hash is 28 bytes on the wire");
 EOS_IMG_STATIC_ASSERT(sizeof(((eos_image_header_t *)0)->signature) == 64,
                       "signature[] is 64 bytes on the wire");
 

include/eos_secure_boot.h

@@ -106,8 +106,13 @@ int eos_secure_boot_verify_key(const uint8_t key_hash[32]);
 /**
  * @brief Lock debug interfaces (SWD/JTAG) permanently.
  * Only effective on real hardware with eFuse support.
+ *
+ * @return EOS_OK if the lock was written, otherwise the HAL error --
+ *         EOS_ERR_NOT_SUPPORTED on a board with no otp_write. Callers that
+ *         asked for the debug port to be closed must treat a non-OK result
+ *         as a failure to boot: the port is still open.
  */
-void eos_secure_boot_lock_debug(void);
+int eos_secure_boot_lock_debug(void);
 
 /**
  * @brief Update the anti-rollback counter in OTP.

tests/CMakeLists.txt

@@ -90,6 +90,11 @@ add_executable(eboot_test_keystore unit/test_keystore.c)
 target_link_libraries(eboot_test_keystore PRIVATE eboot_core)
 add_test(NAME test_keystore COMMAND eboot_test_keystore)
 
+# --- test_secure_boot_policy: debug-lock policy enforcement ---
+add_executable(eboot_test_secure_boot_policy unit/test_secure_boot_policy.c)
+target_link_libraries(eboot_test_secure_boot_policy PRIVATE eboot_core)
+add_test(NAME test_secure_boot_policy COMMAND eboot_test_secure_boot_policy)
+
 # --- test_rollback: Anti-rollback security counter ---
 add_executable(eboot_test_rollback unit/test_rollback.c)
 target_link_libraries(eboot_test_rollback PRIVATE eboot_core)
@@ -119,7 +124,8 @@ if(VALGRIND)
                       test_multicore test_board_registry test_slot_manager
                       test_boot_log test_image_verify test_image_abi
                       test_recovery test_slot_size_bounds test_fw_transport
-                      test_tlv_auth)
+                      test_tlv_auth test_secure_boot_policy test_fdt_loader
+                      test_fw_decrypt)
         add_test(
             NAME valgrind_${TEST_NAME}
             COMMAND ${VALGRIND} ${VALGRIND_OPTS} $<TARGET_FILE:eboot_${TEST_NAME}>