Better check of DH parameters in TLS data

When the client reads DH parameters from the TLS stream, we only
checked that they all are non-zero.  This change updates the check
as follows:

    check that p is odd
    check that 1 < g < p - 1

Reviewed-by: Matt Caswell <matt@openssl.org>

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 218534734dd7e7467a14d1d0914207a77343e218..32f2f1aeed2b19a4b9946466cbc76f095a1fc9c5 100644 (file)
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -1710,12 +1710,6 @@ int ssl3_get_key_exchange(SSL *s)
         }
         p += i;
 
-        if (BN_is_zero(dh->p)) {
-            SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_DH_P_VALUE);
-            goto f_err;
-        }
-
-
         if (2 > n - param_len) {
             SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_LENGTH_TOO_SHORT);
             goto f_err;
@@ -1736,11 +1730,6 @@ int ssl3_get_key_exchange(SSL *s)
         }
         p += i;
 
-        if (BN_is_zero(dh->g)) {
-            SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_DH_G_VALUE);
-            goto f_err;
-        }
-
         if (2 > n - param_len) {
             SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_LENGTH_TOO_SHORT);
             goto f_err;
@@ -1767,6 +1756,39 @@ int ssl3_get_key_exchange(SSL *s)
             goto f_err;
         }
 
+        /*-
+         * Check that p and g are suitable enough
+         *
+         * p is odd
+         * 1 < g < p - 1
+         */
+        {
+            BIGNUM *tmp = NULL;
+
+            if (!BN_is_odd(dh->p)) {
+                SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_DH_P_VALUE);
+                goto f_err;
+            }
+            if (BN_is_negative(dh->g) || BN_is_zero(dh->g)
+                || BN_is_one(dh->g)) {
+                SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_DH_G_VALUE);
+                goto f_err;
+            }
+            if ((tmp = BN_new()) == NULL
+                || BN_copy(tmp, dh->p) == NULL
+                || !BN_sub_word(tmp, 1)) {
+                BN_free(tmp);
+                SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, ERR_R_BN_LIB);
+                goto err;
+            }
+            if (BN_cmp(dh->g, tmp) >= 0) {
+                BN_free(tmp);
+                SSLerr(SSL_F_SSL3_GET_KEY_EXCHANGE, SSL_R_BAD_DH_G_VALUE);
+                goto f_err;
+            }
+            BN_free(tmp);
+        }
+
 # ifndef OPENSSL_NO_RSA
         if (alg_a & SSL_aRSA)
             pkey =