Merge pull request #2301 from SparkiDev/dh_pubval_check

Simple checks of DH public value from peer.

src/internal.c

@@ -4194,11 +4194,23 @@ int DhAgree(WOLFSSL* ssl, DhKey* dhKey,
                     otherPub, otherPubSz, agree, agreeSz, ctx);
     }
     else
+#endif
+    {
+#if !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST)
+        ret = wc_DhCheckPubValue(ssl->buffers.serverDH_P.buffer,
+                    ssl->buffers.serverDH_P.length, otherPub, otherPubSz);
+        if (ret != 0) {
+    #ifdef OPENSSL_EXTRA
+            SendAlert(ssl, alert_fatal, illegal_parameter);
+    #endif
+        }
+        else
 #endif
         {
             ret = wc_DhAgree(dhKey, agree, agreeSz, priv, privSz, otherPub,
                     otherPubSz);
         }
+    }
 
     /* Handle async pending response */
 #ifdef WOLFSSL_ASYNC_CRYPT

wolfcrypt/src/dh.c

@@ -1522,6 +1522,46 @@ int wc_DhCheckPubKey(DhKey* key, const byte* pub, word32 pubSz)
 }
 
 
+/**
+ * Quick validity check of public key value agaist prime.
+ * Checks are:
+ *   - Public key not 0 or 1
+ *   - Public key not equal to prime or prime - 1
+ *   - Public key not bigger than prime.
+ *
+ * prime    Big-endian encoding of prime in bytes.
+ * primeSz  Size of prime in bytes.
+ * pub      Big-endian encoding of public key in bytes.
+ * pubSz    Size of public key in bytes.
+ */
+int wc_DhCheckPubValue(const byte* prime, word32 primeSz, const byte* pub,
+                       word32 pubSz)
+{
+    int ret = 0;
+    word32 i;
+
+    for (i = 0; i < pubSz && pub[i] == 0; i++) {
+    }
+    pubSz -= i;
+    pub += i;
+
+    if (pubSz == 0 || (pubSz == 1 && pub[0] == 1))
+        ret = MP_VAL;
+    else if (pubSz == primeSz) {
+        for (i = 0; i < pubSz-1 && pub[i] == prime[i]; i++) {
+        }
+        if (i == pubSz-1 && (pub[i] == prime[i] || pub[i] == prime[i] - 1))
+            ret = MP_VAL;
+        else if (pub[i] > prime[i])
+            ret = MP_VAL;
+    }
+    else if (pubSz > primeSz)
+        ret = MP_VAL;
+
+    return ret;
+}
+
+
 /* Check DH Private Key for invalid numbers, optionally allowing
  * the private key to be checked against the large prime (q).
  * Check per process in SP 800-56Ar3, section 5.6.2.1.2.
@@ -1987,6 +2027,7 @@ static int _DhSetKey(DhKey* key, const byte* p, word32 pSz, const byte* g,
     int ret = 0;
     mp_int* keyP = NULL;
     mp_int* keyG = NULL;
+    mp_int* keyQ = NULL;
 
     if (key == NULL || p == NULL || g == NULL || pSz == 0 || gSz == 0) {
         ret = BAD_FUNC_ARG;
@@ -2051,9 +2092,13 @@ static int _DhSetKey(DhKey* key, const byte* p, word32 pSz, const byte* g,
     if (ret == 0 && q != NULL) {
         if (mp_read_unsigned_bin(&key->q, q, qSz) != MP_OKAY)
             ret = MP_INIT_E;
+        else
+            keyQ = &key->q;
     }
 
     if (ret != 0 && key != NULL) {
+        if (keyQ)
+            mp_clear(keyQ);
         if (keyG)
             mp_clear(keyG);
         if (keyP)
@@ -2096,8 +2141,7 @@ int wc_DhGenerateParams(WC_RNG *rng, int modSz, DhKey *dh)
     int     groupSz = 0, bufSz = 0,
             primeCheckCount = 0,
             primeCheck = MP_NO,
-            ret = 0,
+            ret = 0;
-            tmp_valid = 0;
     unsigned char *buf = NULL;
 
     if (rng == NULL || dh == NULL)
@@ -2147,10 +2191,6 @@ int wc_DhGenerateParams(WC_RNG *rng, int modSz, DhKey *dh)
                 != MP_OKAY) {
             ret = MP_INIT_E;
         }
-        else {
-            /* tmp and tmp2 are initialized */
-            tmp_valid = 1;
-        }
     }
 
     if (ret == 0) {
@@ -2237,11 +2277,8 @@ int wc_DhGenerateParams(WC_RNG *rng, int modSz, DhKey *dh)
             XFREE(buf, dh->heap, DYNAMIC_TYPE_TMP_BUFFER);
         }
     }
-
-    if (tmp_valid) {
     mp_clear(&tmp);
     mp_clear(&tmp2);
-    }
 
     return ret;
 }

wolfcrypt/test/test.c

@@ -12830,6 +12830,71 @@ exit_gen_test:
     return ret;
 }
 
+#if !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST)
+typedef struct dh_pubvalue_test {
+    const byte* data;
+    word32 len;
+} dh_pubvalue_test;
+
+static int dh_test_check_pubvalue(void)
+{
+    int ret;
+    word32 i;
+    const byte prime[] = {0x01, 0x00, 0x01};
+    const byte pubValZero[] = { 0x00 };
+    const byte pubValZeroLong[] = { 0x00, 0x00, 0x00 };
+    const byte pubValOne[] = { 0x01 };
+    const byte pubValOneLong[] = { 0x00, 0x00, 0x01 };
+    const byte pubValPrimeMinusOne[] = { 0x01, 0x00, 0x00 };
+    const byte pubValPrimeLong[] = {0x00, 0x01, 0x00, 0x01};
+    const byte pubValPrimePlusOne[] = { 0x01, 0x00, 0x02 };
+    const byte pubValTooBig0[] = { 0x02, 0x00, 0x01 };
+    const byte pubValTooBig1[] = { 0x01, 0x01, 0x01 };
+    const byte pubValTooLong[] = { 0x01, 0x00, 0x00, 0x01 };
+    const dh_pubvalue_test dh_pubval_fail[] = {
+        { prime, sizeof(prime) },
+        { pubValZero, sizeof(pubValZero) },
+        { pubValZeroLong, sizeof(pubValZeroLong) },
+        { pubValOne, sizeof(pubValOne) },
+        { pubValOneLong, sizeof(pubValOneLong) },
+        { pubValPrimeMinusOne, sizeof(pubValPrimeMinusOne) },
+        { pubValPrimeLong, sizeof(pubValPrimeLong) },
+        { pubValPrimePlusOne, sizeof(pubValPrimePlusOne) },
+        { pubValTooBig0, sizeof(pubValTooBig0) },
+        { pubValTooBig1, sizeof(pubValTooBig1) },
+        { pubValTooLong, sizeof(pubValTooLong) },
+    };
+    const byte pubValTwo[] = { 0x02 };
+    const byte pubValTwoLong[] = { 0x00, 0x00, 0x02 };
+    const byte pubValGood[] = { 0x12, 0x34 };
+    const byte pubValGoodLen[] = { 0x00, 0x12, 0x34 };
+    const byte pubValGoodLong[] = { 0x00, 0x00, 0x12, 0x34 };
+    const dh_pubvalue_test dh_pubval_pass[] = {
+        { pubValTwo, sizeof(pubValTwo) },
+        { pubValTwoLong, sizeof(pubValTwoLong) },
+        { pubValGood, sizeof(pubValGood) },
+        { pubValGoodLen, sizeof(pubValGoodLen) },
+        { pubValGoodLong, sizeof(pubValGoodLong) },
+    };
+
+    for (i = 0; i < sizeof(dh_pubval_fail) / sizeof(*dh_pubval_fail); i++) {
+        ret = wc_DhCheckPubValue(prime, sizeof(prime), dh_pubval_fail[i].data,
+                                                         dh_pubval_fail[i].len);
+        if (ret != MP_VAL)
+            return -7150 - i;
+    }
+
+    for (i = 0; i < sizeof(dh_pubval_pass) / sizeof(*dh_pubval_pass); i++) {
+        ret = wc_DhCheckPubValue(prime, sizeof(prime), dh_pubval_pass[i].data,
+                                                         dh_pubval_pass[i].len);
+        if (ret != 0)
+            return -7160 - i;
+    }
+
+    return 0;
+}
+#endif
+
 int dh_test(void)
 {
     int    ret;
@@ -12977,6 +13042,10 @@ int dh_test(void)
     ret = dh_generate_test(&rng);
     if (ret == 0)
         ret = dh_fips_generate_test(&rng);
+#if !defined(HAVE_FIPS) && !defined(HAVE_SELFTEST)
+    if (ret == 0)
+        ret = dh_test_check_pubvalue();
+#endif
 
 
     wc_FreeDhKey(&key);

wolfssl/wolfcrypt/dh.h

@@ -106,6 +106,8 @@ WOLFSSL_API int wc_DhParamsLoad(const byte* input, word32 inSz, byte* p,
 WOLFSSL_API int wc_DhCheckPubKey(DhKey* key, const byte* pub, word32 pubSz);
 WOLFSSL_API int wc_DhCheckPubKey_ex(DhKey* key, const byte* pub, word32 pubSz,
                             const byte* prime, word32 primeSz);
+WOLFSSL_API int wc_DhCheckPubValue(const byte* prime, word32 primeSz,
+                                   const byte* pub, word32 pubSz);
 WOLFSSL_API int wc_DhCheckPrivKey(DhKey* key, const byte* priv, word32 pubSz);
 WOLFSSL_API int wc_DhCheckPrivKey_ex(DhKey* key, const byte* priv, word32 pubSz,
                             const byte* prime, word32 primeSz);