Merge pull request #7978 from douzzer/20240916-wc_DhAgree_ct-sp-math

20240916-wc_DhAgree_ct-sp-math
wolfSSL · Sep 16, 2024 · 5ef617a · 5ef617a
2 parents 9666394 + 04c781a
commit 5ef617a
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 7 deletions.
diff --git a/src/ssl_load.c b/src/ssl_load.c
@@ -137,6 +137,7 @@ static int DataToDerBuffer(const unsigned char* buff, word32 len, int format,
             FreeDer(der);
         }
     #else
+        (void)algId;
         ret = NOT_COMPILED_IN;
     #endif
     }

diff --git a/wolfcrypt/src/dh.c b/wolfcrypt/src/dh.c
@@ -2138,6 +2138,13 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
 #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);
@@ -2181,13 +2188,11 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
 
     if (ret == 0) {
         if (ct) {
-            if (mp_to_unsigned_bin_len_ct(z, agree, (int)*agreeSz) != MP_OKAY)
-                ret = MP_TO_E;
+            ret = mp_to_unsigned_bin_len_ct(z, agree, (int)*agreeSz);
         }
         else {
-            if (mp_to_unsigned_bin(z, agree) != MP_OKAY)
-                ret = MP_TO_E;
-            if (ret == 0)
+            ret = mp_to_unsigned_bin(z, agree);
+            if (ret == MP_OKAY)
                 *agreeSz = (word32)mp_unsigned_bin_size(z);
         }
     }
@@ -2296,13 +2301,57 @@ 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;
     }
 
-    return wc_DhAgree_Sync(key, agree, agreeSz, priv, privSz, otherPub, pubSz,
-                           1);
+    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, 0, requested_agreeSz);
+    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;
+    }
+
+#ifndef WOLFSSL_NO_MALLOC
+    XFREE(agree_buffer, key->heap, DYNAMIC_TYPE_DH);
+#endif
+
+    return ret;
 }
 
 #ifdef WOLFSSL_DH_EXTRA