Regression testing

Fixes to get WOLFSSL_PUBLIC_MP testing passing.
Fix DH constant time agreement:
  - implement constant time encoding to big-endian byte array in TFM
- only force x to be zero for SP math as others implementations ensure
unused words are zero
- exponentiate in constant time to the smallest number of words
possible
- no need to encode into separate buffer anymore as encoding is
constant time and front padded
- make requested_sz be the maximum size for the parameters and check
against agreeSz
- update agreeSz to be the maximum valid size instead of filling all
the buffer which may be many times too big
- fix SP result to front pad when doing constant time

Author: SparkiDev

wolfcrypt/src/dh.c

@@ -2058,44 +2058,19 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
 #endif
 
 #ifdef WOLFSSL_HAVE_SP_DH
+    if (0
 #ifndef WOLFSSL_SP_NO_2048
-    if (mp_count_bits(&key->p) == 2048) {
-        if (mp_init(y) != MP_OKAY)
-            ret = MP_INIT_E;
-
-        if (ret == 0) {
-            SAVE_VECTOR_REGISTERS(ret = _svr_ret;);
-
-            if (ret == 0 && mp_read_unsigned_bin(y, otherPub, pubSz) != MP_OKAY)
-                ret = MP_READ_E;
-
-            if (ret == 0)
-                ret = sp_DhExp_2048(y, priv, privSz, &key->p, agree, agreeSz);
-
-            mp_clear(y);
-
-            RESTORE_VECTOR_REGISTERS();
-        }
-
-        /* make sure agree is > 1 (SP800-56A, 5.7.1.1) */
-        if ((ret == 0) &&
-            ((*agreeSz == 0) || ((*agreeSz == 1) && (agree[0] == 1))))
-        {
-            ret = MP_VAL;
-        }
-
-    #if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
-    #if !defined(WOLFSSL_SP_MATH)
-        XFREE(z, key->heap, DYNAMIC_TYPE_DH);
-        XFREE(x, key->heap, DYNAMIC_TYPE_DH);
-    #endif
-        XFREE(y, key->heap, DYNAMIC_TYPE_DH);
-    #endif
-        return ret;
-    }
+        || mp_count_bits(&key->p) == 2048
 #endif
 #ifndef WOLFSSL_SP_NO_3072
-    if (mp_count_bits(&key->p) == 3072) {
+        || mp_count_bits(&key->p) == 3072
+#endif
+#ifdef WOLFSSL_SP_4096
+        || mp_count_bits(&key->p) == 4096
+#endif
+       ) {
+        int i = (int)*agreeSz - 1;
+
         if (mp_init(y) != MP_OKAY)
             ret = MP_INIT_E;
 
@@ -2105,8 +2080,26 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
             if (ret == 0 && mp_read_unsigned_bin(y, otherPub, pubSz) != MP_OKAY)
                 ret = MP_READ_E;
 
-            if (ret == 0)
-                ret = sp_DhExp_3072(y, priv, privSz, &key->p, agree, agreeSz);
+            if (ret == 0) {
+            #ifndef WOLFSSL_SP_NO_2048
+                if (mp_count_bits(&key->p) == 2048) {
+                    ret = sp_DhExp_2048(y, priv, privSz, &key->p, agree,
+                                        agreeSz);
+                }
+            #endif
+            #ifndef WOLFSSL_SP_NO_3072
+                if (mp_count_bits(&key->p) == 3072) {
+                    ret = sp_DhExp_3072(y, priv, privSz, &key->p, agree,
+                                        agreeSz);
+                }
+            #endif
+            #ifdef WOLFSSL_SP_4096
+                if (mp_count_bits(&key->p) == 4096) {
+                    ret = sp_DhExp_4096(y, priv, privSz, &key->p, agree,
+                                        agreeSz);
+                }
+            #endif
+            }
 
             mp_clear(y);
 
@@ -2120,40 +2113,16 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
             ret = MP_VAL;
         }
 
-    #if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
-    #if !defined(WOLFSSL_SP_MATH)
-        XFREE(z, key->heap, DYNAMIC_TYPE_DH);
-        XFREE(x, key->heap, DYNAMIC_TYPE_DH);
-    #endif
-        XFREE(y, key->heap, DYNAMIC_TYPE_DH);
-    #endif
-        return ret;
-    }
-#endif
-#ifdef WOLFSSL_SP_4096
-    if (mp_count_bits(&key->p) == 4096) {
-        if (mp_init(y) != MP_OKAY)
-            ret = MP_INIT_E;
-
-        if (ret == 0) {
-            SAVE_VECTOR_REGISTERS(ret = _svr_ret;);
-
-            if (ret == 0 && mp_read_unsigned_bin(y, otherPub, pubSz) != MP_OKAY)
-                ret = MP_READ_E;
-
-            if (ret == 0)
-                ret = sp_DhExp_4096(y, priv, privSz, &key->p, agree, agreeSz);
-
-            mp_clear(y);
-
-            RESTORE_VECTOR_REGISTERS();
-        }
+        if ((ret == 0) && ct) {
+            word16 mask = 0xff;
+            sword16 o = (sword16)(*agreeSz - 1);
 
-        /* make sure agree is > 1 (SP800-56A, 5.7.1.1) */
-        if ((ret == 0) &&
-            ((*agreeSz == 0) || ((*agreeSz == 1) && (agree[0] == 1))))
-        {
-            ret = MP_VAL;
+            *agreeSz = (word32)(i + 1);
+            for (; i >= 0 ; i--) {
+                agree[i] = agree[o] & (byte)mask;
+                mask = ctMask16LT(0, (int)o);
+                o = (sword16)(o + (sword16)mask);
+            }
         }
 
     #if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
@@ -2166,16 +2135,8 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
         return ret;
     }
 #endif
-#endif
 
 #if !defined(WOLFSSL_SP_MATH)
-    if (ct) {
-        /* for the constant-time variant, we will probably use more bits in x for
-         * the modexp than we read from the private key, and those extra bits need
-         * to be zeroed.
-         */
-        XMEMSET(x, 0, sizeof *x);
-    }
     if (mp_init_multi(x, y, z, 0, 0, 0) != MP_OKAY) {
     #if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
         XFREE(z, key->heap, DYNAMIC_TYPE_DH);
@@ -2184,6 +2145,14 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
     #endif
         return MP_INIT_E;
     }
+#if defined(WOLFSSL_SP_MATH_ALL)
+    if (ct) {
+        /* TFM and Integer implementations keep high words zero.
+         * SP math implementation needs all words set to zero as it doesn't
+         * ensure unused words are zero. */
+        mp_forcezero(x);
+    }
+#endif
 
     SAVE_VECTOR_REGISTERS(ret = _svr_ret;);
 
@@ -2198,12 +2167,24 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
         ret = MP_READ_E;
 
     if (ret == 0) {
-        if (ct)
-            ret = mp_exptmod_ex(y, x,
-                                ((int)*agreeSz + DIGIT_BIT - 1) / DIGIT_BIT,
+        if (ct) {
+            int bits;
+
+            /* x is mod q but if q not available, use p (> q). */
+            if (mp_iszero(&key->q) == MP_NO) {
+                bits = mp_count_bits(&key->q);
+            }
+            else {
+                bits = mp_count_bits(&key->p);
+            }
+            /* Exponentiate to the maximum words of a valid x to ensure a
+             * constant time operation. */
+            ret = mp_exptmod_ex(y, x, (bits + DIGIT_BIT - 1) / DIGIT_BIT,
                                 &key->p, z);
-        else
+        }
+        else {
             ret = mp_exptmod(y, x, &key->p, z);
+        }
         if (ret != MP_OKAY)
             ret = MP_EXPTMOD_E;
     }
@@ -2219,6 +2200,7 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
 
     if (ret == 0) {
         if (ct) {
+            /* Put the secret into a buffer in constant time. */
             ret = mp_to_unsigned_bin_len_ct(z, agree, (int)*agreeSz);
         }
         else {
@@ -2316,7 +2298,8 @@ int wc_DhAgree(DhKey* key, byte* agree, word32* agreeSz, const byte* priv,
 #else
 #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_DH)
     if (key->asyncDev.marker == WOLFSSL_ASYNC_MARKER_DH) {
-        ret = wc_DhAgree_Async(key, agree, agreeSz, priv, privSz, otherPub, pubSz);
+        ret = wc_DhAgree_Async(key, agree, agreeSz, priv, privSz, otherPub,
+                               pubSz);
     }
     else
 #endif
@@ -2332,56 +2315,21 @@ int wc_DhAgree(DhKey* key, byte* agree, word32* agreeSz, const byte* priv,
 int wc_DhAgree_ct(DhKey* key, byte* agree, word32 *agreeSz, const byte* priv,
             word32 privSz, const byte* otherPub, word32 pubSz)
 {
-    int ret;
     word32 requested_agreeSz;
-#ifndef WOLFSSL_NO_MALLOC
-    byte *agree_buffer = NULL;
-#else
-    byte agree_buffer[DH_MAX_SIZE / 8];
-#endif
 
     if (key == NULL || agree == NULL || agreeSz == NULL || priv == NULL ||
                                                             otherPub == NULL) {
         return BAD_FUNC_ARG;
     }
 
-    requested_agreeSz = *agreeSz;
-
-#ifndef WOLFSSL_NO_MALLOC
-    agree_buffer = (byte *)XMALLOC(requested_agreeSz, key->heap,
-                                   DYNAMIC_TYPE_DH);
-    if (agree_buffer == NULL)
-        return MEMORY_E;
-#endif
-
-    XMEMSET(agree_buffer, 0, requested_agreeSz);
-
-    ret = wc_DhAgree_Sync(key, agree_buffer, agreeSz, priv, privSz, otherPub,
-                          pubSz, 1);
-
-    if (ret == 0) {
-        /* Arrange for correct fixed-length, right-justified key, even if the
-         * crypto back end doesn't support it.  This assures that the key is
-         * unconditionally agreed correctly.  With some crypto back ends,
-         * e.g. heapmath, there are no provisions for actual constant time, but
-         * with others the key computation and clamping is constant time, and
-         * the unclamping here is also constant time.
-         */
-        byte *agree_src = agree_buffer + *agreeSz - 1,
-            *agree_dst = agree + requested_agreeSz - 1;
-        while (agree_dst >= agree) {
-            word32 mask = (agree_src >= agree_buffer) - 1U;
-            agree_src += (mask & requested_agreeSz);
-            *agree_dst-- = *agree_src--;
-        }
-        *agreeSz = requested_agreeSz;
+    requested_agreeSz = (word32)mp_unsigned_bin_size(&key->p);
+    if (requested_agreeSz > *agreeSz) {
+        return BUFFER_E;
     }
+    *agreeSz = requested_agreeSz;
 
-#ifndef WOLFSSL_NO_MALLOC
-    XFREE(agree_buffer, key->heap, DYNAMIC_TYPE_DH);
-#endif
-
-    return ret;
+    return wc_DhAgree_Sync(key, agree, agreeSz, priv, privSz, otherPub, pubSz,
+                           1);
 }
 
 #ifdef WOLFSSL_DH_EXTRA

wolfcrypt/src/sp_int.c

@@ -5241,7 +5241,7 @@ int sp_grow(sp_int* a, int l)
 #endif /* (!NO_RSA && !WOLFSSL_RSA_VERIFY_ONLY) || !NO_DH || HAVE_ECC */
 
 #if (!defined(NO_RSA) && !defined(WOLFSSL_RSA_VERIFY_ONLY)) || \
-    defined(HAVE_ECC)
+    defined(HAVE_ECC) || defined(WOLFSSL_PUBLIC_MP)
 /* Set the multi-precision number to zero.
  *
  * @param  [out]  a  SP integer to set to zero.
@@ -5826,7 +5826,7 @@ int sp_cmp_ct(const sp_int* a, const sp_int* b, unsigned int n)
 
 #if (!defined(NO_RSA) && !defined(WOLFSSL_RSA_VERIFY_ONLY)) || \
     ((defined(WOLFSSL_SP_MATH_ALL) || defined(WOLFSSL_SP_SM2)) && \
-     defined(HAVE_ECC)) || defined(OPENSSL_EXTRA)
+     defined(HAVE_ECC)) || defined(OPENSSL_EXTRA) || defined(WOLFSSL_PUBLIC_MP)
 /* Check if a bit is set
  *
  * When a is NULL, result is 0.

wolfcrypt/src/tfm.c

@@ -4198,6 +4198,58 @@ int fp_to_unsigned_bin(fp_int *a, unsigned char *b)
   return FP_OKAY;
 }
 
+int fp_to_unsigned_bin_len_ct(fp_int *a, unsigned char *out, int outSz)
+{
+  int err = MP_OKAY;
+
+  /* Validate parameters. */
+  if ((a == NULL) || (out == NULL) || (outSz < 0)) {
+    err = MP_VAL;
+  }
+
+#if DIGIT_BIT > 8
+  if (err == MP_OKAY) {
+    /* Start at the end of the buffer - least significant byte. */
+    int j;
+    unsigned int i;
+    fp_digit mask = (fp_digit)-1;
+    fp_digit d;
+
+    /* Put each digit in. */
+    i = 0;
+    for (j = outSz - 1; j >= 0; ) {
+      unsigned int b;
+      d = a->dp[i];
+      /* Place each byte of a digit into the buffer. */
+      for (b = 0; (j >= 0) && (b < (DIGIT_BIT / 8)); b++) {
+        out[j--] = (byte)(d & mask);
+        d >>= 8;
+      }
+      mask &= (fp_digit)0 - (i < (unsigned int)a->used - 1);
+      i += (unsigned int)(1 & mask);
+    }
+  }
+#else
+  if ((err == MP_OKAY) && ((unsigned int)outSz < a->used)) {
+    err = MP_VAL;
+  }
+  if (err == MP_OKAY) {
+    unsigned int i;
+    int j;
+    fp_digit mask = (fp_digit)-1;
+
+    i = 0;
+    for (j = outSz - 1; j >= 0; j--) {
+      out[j] = a->dp[i] & mask;
+      mask &= (fp_digit)0 - (i < (unsigned int)a->used - 1);
+      i += (unsigned int)(1 & mask);
+    }
+  }
+#endif
+
+  return err;
+}
+
 int fp_to_unsigned_bin_len(fp_int *a, unsigned char *b, int c)
 {
 #if DIGIT_BIT == 64 || DIGIT_BIT == 32 || DIGIT_BIT == 16
@@ -4823,6 +4875,11 @@ int mp_to_unsigned_bin (mp_int * a, unsigned char *b)
   return fp_to_unsigned_bin(a,b);
 }
 
+int mp_to_unsigned_bin_len_ct(mp_int * a, unsigned char *b, int c)
+{
+  return fp_to_unsigned_bin_len_ct(a, b, c);
+}
+
 int mp_to_unsigned_bin_len(mp_int * a, unsigned char *b, int c)
 {
   return fp_to_unsigned_bin_len(a, b, c);