Prevent small subgroup attacks on DH/DHE

Historically OpenSSL only ever generated DH parameters based on "safe"
primes. More recently (in version 1.0.2) support was provided for
generating X9.42 style parameter files such as those required for RFC
5114 support. The primes used in such files may not be "safe". Where an
application is using DH configured with parameters based on primes that
are not "safe" then an attacker could use this fact to find a peer's
private DH exponent. This attack requires that the attacker complete
multiple handshakes in which the peer uses the same DH exponent.

A simple mitigation is to ensure that y^q (mod p) == 1

CVE-2016-0701 (fix part 1 of 2)

Issue reported by Antonio Sanso.

Reviewed-by: Viktor Dukhovni <viktor@openssl.org>

diff --git a/crypto/dh/dh.h b/crypto/dh/dh.h
index b17767328183e011c36d15738d4f9e737aca53d7..5498a9dc106044f505577ae9659dac6d1a3efa76 100644 (file)
--- a/crypto/dh/dh.h
+++ b/crypto/dh/dh.h
@@ -174,6 +174,7 @@ struct dh_st {
 /* DH_check_pub_key error codes */
 # define DH_CHECK_PUBKEY_TOO_SMALL       0x01
 # define DH_CHECK_PUBKEY_TOO_LARGE       0x02
+# define DH_CHECK_PUBKEY_INVALID         0x03
 
 /*
  * primes p where (p-1)/2 is prime too are called "safe"; we define this for

diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c
index 347467c6a43369dfe948f05fd33373ab0f1b0ae9..5adedc0d264e99aae5d68ef2e4c5585dcfd33de3 100644 (file)
--- a/crypto/dh/dh_check.c
+++ b/crypto/dh/dh_check.c
@@ -151,23 +151,38 @@ int DH_check(const DH *dh, int *ret)
 int DH_check_pub_key(const DH *dh, const BIGNUM *pub_key, int *ret)
 {
     int ok = 0;
-    BIGNUM *q = NULL;
+    BIGNUM *tmp = NULL;
+    BN_CTX *ctx = NULL;
 
     *ret = 0;
-    q = BN_new();
-    if (q == NULL)
+    ctx = BN_CTX_new();
+    if (ctx == NULL)
         goto err;
-    BN_set_word(q, 1);
-    if (BN_cmp(pub_key, q) <= 0)
+    BN_CTX_start(ctx);
+    tmp = BN_CTX_get(ctx);
+    if (tmp == NULL)
+        goto err;
+    BN_set_word(tmp, 1);
+    if (BN_cmp(pub_key, tmp) <= 0)
         *ret |= DH_CHECK_PUBKEY_TOO_SMALL;
-    BN_copy(q, dh->p);
-    BN_sub_word(q, 1);
-    if (BN_cmp(pub_key, q) >= 0)
+    BN_copy(tmp, dh->p);
+    BN_sub_word(tmp, 1);
+    if (BN_cmp(pub_key, tmp) >= 0)
         *ret |= DH_CHECK_PUBKEY_TOO_LARGE;
 
+    if (dh->q != NULL) {
+        /* Check pub_key^q == 1 mod p */
+        if (!BN_mod_exp(tmp, pub_key, dh->q, dh->p, ctx))
+            goto err;
+        if (!BN_is_one(tmp))
+            *ret |= DH_CHECK_PUBKEY_INVALID;
+    }
+
     ok = 1;
  err:
-    if (q != NULL)
-        BN_free(q);
+    if (ctx != NULL) {
+        BN_CTX_end(ctx);
+        BN_CTX_free(ctx);
+    }
     return (ok);
 }