misc: reject a negative length in ConstantCompare()

Returning 0 for a negative length reported "equal" for a comparison
that never ran. The sentinel is positive because AES_GCM_decrypt_C()
folds this return into a mask that reads a negative value as a match.

Author: stenslae

ChangeLog.md

@@ -101,6 +101,15 @@
   leak must stop doing so, and the returned key is shared with the
   certificate, so it must be treated as read only.
 
+* **Behavioral change (`ConstantCompare()` rejects a negative length)**:
+  `ConstantCompare()` and `wc_ConstantCompare()` now return 1, not 0
+  ("equal"), for a negative `length`, so a caller whose length arithmetic
+  underflows no longer gets a false match.  Differences are normalized to 1,
+  so wide-byte targets (such as C28x) never return negative on a high-bit
+  difference.  `length == 0` still returns 0.  The sentinel is positive
+  because callers fold the result into constant-time masks that read a
+  negative value as a match.
+
 ## Post-Quantum Cryptography (PQC)
 
 * Added opt-in per-key Falcon signing caches (`--enable-falcon=cache-key`, `cache-basis`), roughly doubling signing speed with the default integer fpr backend. by @Frauschi

wolfcrypt/src/aes.c

@@ -12602,9 +12602,7 @@ int WARN_UNUSED_RESULT AES_GCM_decrypt_C(
         return ret;
     xorbuf(Tprime, EKY0, sizeof(Tprime));
 #ifdef WC_AES_GCM_DEC_AUTH_EARLY
-    /* ConstantCompare returns the cumulative bitwise or of the bitwise xor of
-     * the pairwise bytes in the strings.
-     */
+    /* ConstantCompare returns 0 when the tags match and 1 otherwise. */
     res = ConstantCompare(authTag, Tprime, authTagSz);
     /* convert positive retval from ConstantCompare() to all-1s word, in
      * constant time.
@@ -12685,9 +12683,7 @@ int WARN_UNUSED_RESULT AES_GCM_decrypt_C(
     }
 
 #ifndef WC_AES_GCM_DEC_AUTH_EARLY
-    /* ConstantCompare returns the cumulative bitwise or of the bitwise xor of
-     * the pairwise bytes in the strings.
-     */
+    /* ConstantCompare returns 0 when the tags match and 1 otherwise. */
     res = ConstantCompare(authTag, Tprime, (int)authTagSz);
     /* convert positive retval from ConstantCompare() to all-1s word, in
      * constant time.

wolfcrypt/src/misc.c

@@ -826,18 +826,26 @@ WC_MISC_STATIC WC_INLINE void ForceZero(void* mem, size_t len)
 
 
 #ifndef WOLFSSL_NO_CONST_CMP
-/* check all length bytes for equality, return 0 on success */
+/* Compare length bytes of a and b in constant time.
+ * Returns 0 if equal, 1 if not or if length is negative. */
 WC_MISC_STATIC WC_INLINE int ConstantCompare(const byte* a, const byte* b,
                                              int length)
 {
     int i;
-    int compareSum = 0;
+    word32 compareSum = 0;
+
+    /* length is public, so branching is safe. Return positive: callers
+     * such as AES_GCM_decrypt_C() build masks that read negative as equal. */
+    if (length < 0) {
+        return 1;
+    }
 
     for (i = 0; i < length; i++) {
-        compareSum |= a[i] ^ b[i];
+        compareSum |= (word32)(a[i] ^ b[i]);
     }
 
-    return compareSum;
+    /* Normalize to 0/1 without a compare on secret data. */
+    return (int)(((compareSum | ((word32)0 - compareSum)) >> 31) & 1U);
 }
 #endif
 

wolfcrypt/test/test.c

@@ -864,6 +864,9 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  macro_test(void);
 WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  error_test(void);
 WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  flags_test(void);
 WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  octets_test(void);
+#ifdef WC_TEST_CONSTANTCOMPARE
+WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  constantcompare_test(void);
+#endif
 WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  base64_test(void);
 WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  base16_test(void);
 WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  asn_test(void);
@@ -2729,6 +2732,13 @@ options: [-s max_relative_stack_bytes] [-m max_relative_heap_memory_bytes]\n\
     else
         TEST_PASS("MEMORY   test passed!\n");
 
+#ifdef WC_TEST_CONSTANTCOMPARE
+    if ( (ret = constantcompare_test()) != 0)
+        TEST_FAIL("constantcompare test failed!\n", ret);
+    else
+        TEST_PASS("constantcompare test passed!\n");
+#endif
+
     if ( (ret = octets_test()) != 0)
         TEST_FAIL("octets   test failed!\n", ret);
     else
@@ -5523,6 +5533,75 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t octets_test(void)
     return 0;
 }
 
+#ifdef WC_TEST_CONSTANTCOMPARE
+
+typedef int (*constantcompare_fn)(const byte* a, const byte* b, int length);
+
+/* Run the ConstantCompare() checks against cmp.
+ * Returns 0 on success, a test error code on failure. */
+static wc_test_ret_t constantcompare_sweep(constantcompare_fn cmp)
+{
+    byte a[9] = { 0, 1, 2, 3, 4, 5, 6, 7, 8 };
+    byte b[9];
+    int i;
+    int length = (int)sizeof(a) - 1; /* leave a[8]/b[8] out of range */
+
+    XMEMCPY(b, a, sizeof(a));
+    if (cmp(a, b, length) != 0)
+        return WC_TEST_RET_ENC_NC;
+    b[0] ^= 0x01; /* length 0 must ignore even a differing first byte */
+    if (cmp(a, b, 0) != 0)
+        return WC_TEST_RET_ENC_NC;
+
+    /* A difference at any index must return exactly 1.
+     * Exercise both low and high bits (e.g. bit 15 on WOLFSSL_WIDE_BYTE). */
+    for (i = 0; i < length; i++) {
+        XMEMCPY(b, a, sizeof(a));
+        b[i] ^= 0x01;
+        if (cmp(a, b, length) != 1)
+            return WC_TEST_RET_ENC_I(i);
+        b[i] ^= (byte)~0;
+        if (cmp(a, b, length) != 1)
+            return WC_TEST_RET_ENC_I(i);
+    }
+
+    /* A difference past length must be ignored. */
+    XMEMCPY(b, a, sizeof(a));
+    b[length] ^= 0x01;
+    if (cmp(a, b, length) != 0)
+        return WC_TEST_RET_ENC_NC;
+
+    /* Negative length must return 1, not 0. */
+    XMEMSET(b, (byte)~a[0], sizeof(b));
+    if (cmp(a, b, -1) != 1)
+        return WC_TEST_RET_ENC_NC;
+
+    return 0;
+}
+
+/* Test ConstantCompare() and wc_ConstantCompare().
+ * Returns 0 on success, a test error code on failure. */
+WOLFSSL_TEST_SUBROUTINE wc_test_ret_t constantcompare_test(void)
+{
+    wc_test_ret_t ret = 0;
+
+    WOLFSSL_ENTER("constantcompare_test");
+
+    /* Inline copy from misc.c.  Under NO_INLINE it is hidden in the library
+     * and cannot be linked; wc_ConstantCompare() covers the same code. */
+#ifndef NO_INLINE
+    ret = constantcompare_sweep(ConstantCompare);
+    if (ret != 0)
+        return ret;
+#endif
+
+#ifdef WC_TEST_HAVE_WC_CONSTANTCOMPARE
+    ret = constantcompare_sweep(wc_ConstantCompare);
+#endif
+    return ret;
+}
+#endif /* WC_TEST_CONSTANTCOMPARE */
+
 #ifndef NO_CODING
 
 WOLFSSL_TEST_SUBROUTINE wc_test_ret_t base64_test(void)

wolfcrypt/test/test.h

@@ -138,6 +138,18 @@ wc_static_assert(-(long)MIN_CODE_E < 0x7ffL);
 
 #endif /* !WC_TEST_RET_HAVE_CUSTOM_MACROS */
 
+#ifndef WOLFSSL_NO_CONST_CMP
+    /* wc_ConstantCompare() lives in memory.c, which is not built under
+     * NO_WOLFSSL_MEMORY or, with autotools, WOLFSSL_LEANPSK. */
+    #if !defined(NO_WOLFSSL_MEMORY) && !defined(WOLFSSL_LEANPSK)
+        #define WC_TEST_HAVE_WC_CONSTANTCOMPARE
+    #endif
+    /* Test only when ConstantCompare() or wc_ConstantCompare() is linkable. */
+    #if !defined(NO_INLINE) || defined(WC_TEST_HAVE_WC_CONSTANTCOMPARE)
+        #define WC_TEST_CONSTANTCOMPARE
+    #endif
+#endif
+
 #ifdef WC_TEST_EXPORT_SUBTESTS
 
 #if defined(NO_FILESYSTEM) || defined(WC_NO_RNG)
@@ -158,6 +170,9 @@ extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  macro_test(void);
 extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  error_test(void);
 extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  flags_test(void);
 extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  octets_test(void);
+#ifdef WC_TEST_CONSTANTCOMPARE
+extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  constantcompare_test(void);
+#endif
 extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  base64_test(void);
 extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  base16_test(void);
 extern WOLFSSL_TEST_SUBROUTINE wc_test_ret_t  asn_test(void);

wolfssl/wolfcrypt/memory.h

@@ -336,6 +336,8 @@ WOLFSSL_API void wc_ForceZero(void *mem, size_t len);
 #endif
 
 #ifndef WOLFSSL_NO_CONST_CMP
+/* Compare length bytes of a and b in constant time.
+ * Returns 0 if equal, 1 if they differ or if length is negative. */
 WOLFSSL_API int wc_ConstantCompare(const byte* a, const byte* b, int length);
 #endif