Merge pull request #7802 from douzzer/20240725-wc_DhAgree_ct

20240725-wc_DhAgree_ct

src/pk.c

@@ -8672,20 +8672,8 @@ int wolfSSL_DH_generate_key(WOLFSSL_DH* dh)
 }
 
 
-/* Compute the shared key from the private key and peer's public key.
+static int _DH_compute_key(unsigned char* key, const WOLFSSL_BIGNUM* otherPub,
- *
+    WOLFSSL_DH* dh, int ct)
- * Return code compliant with OpenSSL.
- * OpenSSL returns 0 when number of bits in p are smaller than minimum
- * supported.
- *
- * @param [out] key       Buffer to place shared key.
- * @param [in]  otherPub  Peer's public key.
- * @param [in]  dh        DH key containing private key.
- * @return  -1 on error.
- * @return  Size of shared secret in bytes on success.
- */
-int wolfSSL_DH_compute_key(unsigned char* key, const WOLFSSL_BIGNUM* otherPub,
-    WOLFSSL_DH* dh)
 {
     int            ret    = 0;
     word32         keySz  = 0;
@@ -8773,10 +8761,39 @@ int wolfSSL_DH_compute_key(unsigned char* key, const WOLFSSL_BIGNUM* otherPub,
 
     PRIVATE_KEY_UNLOCK();
     /* Calculate shared secret from private and public keys. */
-    if ((ret == 0) && (wc_DhAgree((DhKey*)dh->internal, key, &keySz, priv,
+    if (ret == 0) {
-            (word32)privSz, pub, (word32)pubSz) < 0)) {
+        word32 padded_keySz = keySz;
-        WOLFSSL_ERROR_MSG("wc_DhAgree failed");
+#if (!defined(HAVE_FIPS) || FIPS_VERSION_GE(7,0)) && !defined(HAVE_SELFTEST)
-        ret = WOLFSSL_FATAL_ERROR;
+        if (ct) {
+            if (wc_DhAgree_ct((DhKey*)dh->internal, key, &keySz, priv,
+                           (word32)privSz, pub, (word32)pubSz) < 0) {
+                WOLFSSL_ERROR_MSG("wc_DhAgree_ct failed");
+                ret = WOLFSSL_FATAL_ERROR;
+            }
+        }
+        else
+#endif /* (!HAVE_FIPS || FIPS_VERSION_GE(7,0)) && !HAVE_SELFTEST */
+        {
+            if (wc_DhAgree((DhKey*)dh->internal, key, &keySz, priv,
+                           (word32)privSz, pub, (word32)pubSz) < 0) {
+                WOLFSSL_ERROR_MSG("wc_DhAgree failed");
+                ret = WOLFSSL_FATAL_ERROR;
+            }
+        }
+
+        if ((ret == 0) && ct) {
+            /* Arrange for correct fixed-length, right-justified key, even if
+             * the crypto back end doesn't support it.  With some crypto back
+             * ends this forgoes formal constant-timeness on the key agreement,
+             * but assured that wolfSSL_DH_compute_key_padded() functions
+             * correctly.
+             */
+            if (keySz < padded_keySz) {
+                XMEMMOVE(key, key + (padded_keySz - keySz),
+                         padded_keySz - keySz);
+                XMEMSET(key, 0, padded_keySz - keySz);
+            }
+        }
     }
     if (ret == 0) {
         /* Return actual length. */
@@ -8800,6 +8817,45 @@ int wolfSSL_DH_compute_key(unsigned char* key, const WOLFSSL_BIGNUM* otherPub,
 
     return ret;
 }
+
+/* Compute the shared key from the private key and peer's public key.
+ *
+ * Return code compliant with OpenSSL.
+ * OpenSSL returns 0 when number of bits in p are smaller than minimum
+ * supported.
+ *
+ * @param [out] key       Buffer to place shared key.
+ * @param [in]  otherPub  Peer's public key.
+ * @param [in]  dh        DH key containing private key.
+ * @return  -1 on error.
+ * @return  Size of shared secret in bytes on success.
+ */
+int wolfSSL_DH_compute_key(unsigned char* key, const WOLFSSL_BIGNUM* otherPub,
+    WOLFSSL_DH* dh)
+{
+    return _DH_compute_key(key, otherPub, dh, 0);
+}
+
+/* Compute the shared key from the private key and peer's public key as in
+ * wolfSSL_DH_compute_key, but using constant time processing, with an output
+ * key length fixed at the nominal DH key size.  Leading zeros are retained.
+ *
+ * Return code compliant with OpenSSL.
+ * OpenSSL returns 0 when number of bits in p are smaller than minimum
+ * supported.
+ *
+ * @param [out] key       Buffer to place shared key.
+ * @param [in]  otherPub  Peer's public key.
+ * @param [in]  dh        DH key containing private key.
+ * @return  -1 on error.
+ * @return  Size of shared secret in bytes on success.
+ */
+int wolfSSL_DH_compute_key_padded(unsigned char* key,
+    const WOLFSSL_BIGNUM* otherPub, WOLFSSL_DH* dh)
+{
+    return _DH_compute_key(key, otherPub, dh, 1);
+}
+
 #endif /* !HAVE_FIPS || (HAVE_FIPS && !WOLFSSL_DH_EXTRA) ||
         * HAVE_FIPS_VERSION > 2 */
 

tests/api.c

@@ -82334,6 +82334,30 @@ static int test_wolfSSL_DH(void)
     ExpectNotNull(dh->g);
     ExpectTrue(pt == buf);
     ExpectIntEQ(DH_generate_key(dh), 1);
+
+    /* first, test for expected successful key agreement. */
+    if (EXPECT_SUCCESS()) {
+        DH *dh2 = NULL;
+        unsigned char buf2[268];
+        int sz1 = 0, sz2 = 0;
+
+        ExpectNotNull(dh2 = d2i_DHparams(NULL, &pt, len));
+        ExpectIntEQ(DH_generate_key(dh2), 1);
+
+        ExpectIntGT(sz1=DH_compute_key(buf, dh2->pub_key, dh), 0);
+        ExpectIntGT(sz2=DH_compute_key(buf2, dh->pub_key, dh2), 0);
+        ExpectIntEQ(sz1, sz2);
+        ExpectIntEQ(XMEMCMP(buf, buf2, (size_t)sz1), 0);
+
+        ExpectIntNE(sz1 = DH_size(dh), 0);
+        ExpectIntEQ(DH_compute_key_padded(buf, dh2->pub_key, dh), sz1);
+        ExpectIntEQ(DH_compute_key_padded(buf2, dh->pub_key, dh2), sz1);
+        ExpectIntEQ(XMEMCMP(buf, buf2, (size_t)sz1), 0);
+
+        if (dh2 != NULL)
+            DH_free(dh2);
+    }
+
     ExpectIntEQ(DH_generate_key(dh), 1);
     ExpectIntEQ(DH_compute_key(NULL, NULL, NULL), -1);
     ExpectNotNull(pub = BN_new());

wolfcrypt/src/dh.c

@@ -1981,7 +1981,7 @@ int wc_DhGenerateKeyPair(DhKey* key, WC_RNG* rng,
 
 #ifndef WOLFSSL_KCAPI_DH
 static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
-    const byte* priv, word32 privSz, const byte* otherPub, word32 pubSz)
+    const byte* priv, word32 privSz, const byte* otherPub, word32 pubSz, int ct)
 {
     int ret = 0;
 #if defined(WOLFSSL_SMALL_STACK) && !defined(WOLFSSL_NO_MALLOC)
@@ -2159,8 +2159,17 @@ 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 && mp_exptmod(y, x, &key->p, z) != MP_OKAY)
+    if (ret == 0) {
-        ret = MP_EXPTMOD_E;
+        if (ct)
+            ret = mp_exptmod_ex(y, x,
+                                ((int)*agreeSz + DIGIT_BIT - 1) / DIGIT_BIT,
+                                &key->p, z);
+        else
+            ret = mp_exptmod(y, x, &key->p, z);
+        if (ret != MP_OKAY)
+            ret = MP_EXPTMOD_E;
+    }
+
 #ifdef WOLFSSL_CHECK_MEM_ZERO
     if (ret == 0)
         mp_memzero_add("wc_DhAgree_Sync z", z);
@@ -2170,11 +2179,18 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
     if (ret == 0 && (mp_cmp_d(z, 1) == MP_EQ))
         ret = MP_VAL;
 
-    if (ret == 0 && mp_to_unsigned_bin(z, agree) != MP_OKAY)
+    if (ret == 0) {
-        ret = MP_TO_E;
+        if (ct) {
-
+            if (mp_to_unsigned_bin_len_ct(z, agree, (int)*agreeSz) != MP_OKAY)
-    if (ret == 0)
+                ret = MP_TO_E;
-        *agreeSz = (word32)mp_unsigned_bin_size(z);
+        }
+        else {
+            if (mp_to_unsigned_bin(z, agree) != MP_OKAY)
+                ret = MP_TO_E;
+            if (ret == 0)
+                *agreeSz = (word32)mp_unsigned_bin_size(z);
+        }
+    }
 
     mp_forcezero(z);
     mp_clear(y);
@@ -2183,6 +2199,7 @@ static int wc_DhAgree_Sync(DhKey* key, byte* agree, word32* agreeSz,
     RESTORE_VECTOR_REGISTERS();
 
 #else
+    (void)ct;
     ret = WC_KEY_SIZE_E;
 #endif
 
@@ -2238,7 +2255,8 @@ static int wc_DhAgree_Async(DhKey* key, byte* agree, word32* agreeSz,
 #endif
 
     /* otherwise use software DH */
-    ret = wc_DhAgree_Sync(key, agree, agreeSz, priv, privSz, otherPub, pubSz);
+    ret = wc_DhAgree_Sync(key, agree, agreeSz, priv, privSz, otherPub, pubSz,
+                          0);
 
     return ret;
 }
@@ -2267,13 +2285,26 @@ int wc_DhAgree(DhKey* key, byte* agree, word32* agreeSz, const byte* priv,
     else
 #endif
     {
-        ret = wc_DhAgree_Sync(key, agree, agreeSz, priv, privSz, otherPub, pubSz);
+        ret = wc_DhAgree_Sync(key, agree, agreeSz, priv, privSz, otherPub,
+                              pubSz, 0);
     }
 #endif /* WOLFSSL_KCAPI_DH */
 
     return ret;
 }
 
+int wc_DhAgree_ct(DhKey* key, byte* agree, word32 *agreeSz, const byte* priv,
+            word32 privSz, const byte* otherPub, word32 pubSz)
+{
+    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);
+}
+
 #ifdef WOLFSSL_DH_EXTRA
 WOLFSSL_LOCAL int wc_DhKeyCopy(DhKey* src, DhKey* dst)
 {

wolfcrypt/test/test.c

@@ -22785,6 +22785,38 @@ WOLFSSL_TEST_SUBROUTINE wc_test_ret_t dh_test(void)
     if (agreeSz != agreeSz2 || XMEMCMP(agree, agree2, agreeSz)) {
         ERROR_OUT(WC_TEST_RET_ENC_NC, done);
     }
+
+#if (!defined(HAVE_FIPS) || FIPS_VERSION_GE(7,0)) && \
+            !defined(HAVE_SELFTEST)
+    agreeSz = DH_TEST_BUF_SIZE;
+    agreeSz2 = DH_TEST_BUF_SIZE;
+
+    ret = wc_DhAgree_ct(key, agree, &agreeSz, priv, privSz, pub2, pubSz2);
+    if (ret != 0)
+        ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done);
+
+    ret = wc_DhAgree_ct(key2, agree2, &agreeSz2, priv2, privSz2, pub, pubSz);
+    if (ret != 0)
+        ERROR_OUT(WC_TEST_RET_ENC_EC(ret), done);
+
+#ifdef WOLFSSL_PUBLIC_MP
+    if (agreeSz != (word32)mp_unsigned_bin_size(&key->p))
+    {
+        ERROR_OUT(WC_TEST_RET_ENC_NC, done);
+    }
+#endif
+
+    if (agreeSz != agreeSz2)
+    {
+        ERROR_OUT(WC_TEST_RET_ENC_NC, done);
+    }
+
+    if (XMEMCMP(agree, agree2, agreeSz) != 0)
+    {
+        ERROR_OUT(WC_TEST_RET_ENC_NC, done);
+    }
+#endif /* (!HAVE_FIPS || FIPS_VERSION_GE(7,0)) && !HAVE_SELFTEST */
+
 #endif /* !WC_NO_RNG */
 
 #if defined(WOLFSSL_KEY_GEN) && !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST)

wolfssl/openssl/dh.h

@@ -67,6 +67,9 @@ WOLFSSL_API int wolfSSL_DH_size(WOLFSSL_DH* dh);
 WOLFSSL_API int wolfSSL_DH_generate_key(WOLFSSL_DH* dh);
 WOLFSSL_API int wolfSSL_DH_compute_key(unsigned char* key, const WOLFSSL_BIGNUM* pub,
                                      WOLFSSL_DH* dh);
+WOLFSSL_API int wolfSSL_DH_compute_key_padded(unsigned char* key,
+                                const WOLFSSL_BIGNUM* otherPub, WOLFSSL_DH* dh);
+
 WOLFSSL_API int wolfSSL_DH_LoadDer(WOLFSSL_DH* dh, const unsigned char* derBuf,
                                    int derSz);
 WOLFSSL_API int wolfSSL_DH_set_length(WOLFSSL_DH* dh, long len);
@@ -91,6 +94,7 @@ typedef WOLFSSL_DH                   DH;
 #define DH_size         wolfSSL_DH_size
 #define DH_generate_key wolfSSL_DH_generate_key
 #define DH_compute_key  wolfSSL_DH_compute_key
+#define DH_compute_key_padded wolfSSL_DH_compute_key_padded
 #define DH_set_length   wolfSSL_DH_set_length
 #define DH_set0_pqg     wolfSSL_DH_set0_pqg
 #define DH_get0_pqg     wolfSSL_DH_get0_pqg

wolfssl/wolfcrypt/dh.h

@@ -151,6 +151,9 @@ WOLFSSL_API int wc_DhGenerateKeyPair(DhKey* key, WC_RNG* rng, byte* priv,
 WOLFSSL_API int wc_DhAgree(DhKey* key, byte* agree, word32* agreeSz,
                        const byte* priv, word32 privSz, const byte* otherPub,
                        word32 pubSz);
+WOLFSSL_API int wc_DhAgree_ct(DhKey* key, byte* agree, word32* agreeSz,
+                       const byte* priv, word32 privSz, const byte* otherPub,
+                       word32 pubSz);
 
 WOLFSSL_API int wc_DhKeyDecode(const byte* input, word32* inOutIdx, DhKey* key,
                            word32 inSz); /* wc_DhKeyDecode is in asn.c */