From: Richard Levitte <levitte@openssl.org>
Date: Thu, 26 Jan 2017 10:47:36 +0000 (+0100)
Subject: Better check of DH parameters in TLS data
X-Git-Tag: OpenSSL_1_0_2k~2
X-Git-Url: https://git.openssl.org/gitweb/?a=commitdiff_plain;h=918d8eadb35746456fd1a9d4e219c63ff706173e;p=openssl.git

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 218534734d..32f2f1aeed 100644
--- 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 =