Patrick Williams | f1e5d69 | 2016-03-30 15:21:19 -0500 | [diff] [blame^] | 1 | From 878e2c5b13010329c203f309ed0c8f2113f85648 Mon Sep 17 00:00:00 2001 |
| 2 | From: Matt Caswell <matt@openssl.org> |
| 3 | Date: Mon, 18 Jan 2016 11:31:58 +0000 |
| 4 | Subject: [PATCH] Prevent small subgroup attacks on DH/DHE |
| 5 | |
| 6 | Historically OpenSSL only ever generated DH parameters based on "safe" |
| 7 | primes. More recently (in version 1.0.2) support was provided for |
| 8 | generating X9.42 style parameter files such as those required for RFC |
| 9 | 5114 support. The primes used in such files may not be "safe". Where an |
| 10 | application is using DH configured with parameters based on primes that |
| 11 | are not "safe" then an attacker could use this fact to find a peer's |
| 12 | private DH exponent. This attack requires that the attacker complete |
| 13 | multiple handshakes in which the peer uses the same DH exponent. |
| 14 | |
| 15 | A simple mitigation is to ensure that y^q (mod p) == 1 |
| 16 | |
| 17 | CVE-2016-0701 (fix part 1 of 2) |
| 18 | |
| 19 | Issue reported by Antonio Sanso. |
| 20 | |
| 21 | Reviewed-by: Viktor Dukhovni <viktor@openssl.org> |
| 22 | |
| 23 | Upstream-Status: Backport |
| 24 | |
| 25 | https://github.com/openssl/openssl/commit/878e2c5b13010329c203f309ed0c8f2113f85648 |
| 26 | |
| 27 | CVE: CVE-2016-0701 |
| 28 | Signed-of-by: Armin Kuster <akuster@mvisa.com> |
| 29 | |
| 30 | --- |
| 31 | crypto/dh/dh.h | 1 + |
| 32 | crypto/dh/dh_check.c | 35 +++++++++++++++++++++++++---------- |
| 33 | 2 files changed, 26 insertions(+), 10 deletions(-) |
| 34 | |
| 35 | diff --git a/crypto/dh/dh.h b/crypto/dh/dh.h |
| 36 | index b177673..5498a9d 100644 |
| 37 | --- a/crypto/dh/dh.h |
| 38 | +++ b/crypto/dh/dh.h |
| 39 | @@ -174,6 +174,7 @@ struct dh_st { |
| 40 | /* DH_check_pub_key error codes */ |
| 41 | # define DH_CHECK_PUBKEY_TOO_SMALL 0x01 |
| 42 | # define DH_CHECK_PUBKEY_TOO_LARGE 0x02 |
| 43 | +# define DH_CHECK_PUBKEY_INVALID 0x03 |
| 44 | |
| 45 | /* |
| 46 | * primes p where (p-1)/2 is prime too are called "safe"; we define this for |
| 47 | diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c |
| 48 | index 347467c..5adedc0 100644 |
| 49 | --- a/crypto/dh/dh_check.c |
| 50 | +++ b/crypto/dh/dh_check.c |
| 51 | @@ -151,23 +151,38 @@ int DH_check(const DH *dh, int *ret) |
| 52 | int DH_check_pub_key(const DH *dh, const BIGNUM *pub_key, int *ret) |
| 53 | { |
| 54 | int ok = 0; |
| 55 | - BIGNUM *q = NULL; |
| 56 | + BIGNUM *tmp = NULL; |
| 57 | + BN_CTX *ctx = NULL; |
| 58 | |
| 59 | *ret = 0; |
| 60 | - q = BN_new(); |
| 61 | - if (q == NULL) |
| 62 | + ctx = BN_CTX_new(); |
| 63 | + if (ctx == NULL) |
| 64 | goto err; |
| 65 | - BN_set_word(q, 1); |
| 66 | - if (BN_cmp(pub_key, q) <= 0) |
| 67 | + BN_CTX_start(ctx); |
| 68 | + tmp = BN_CTX_get(ctx); |
| 69 | + if (tmp == NULL) |
| 70 | + goto err; |
| 71 | + BN_set_word(tmp, 1); |
| 72 | + if (BN_cmp(pub_key, tmp) <= 0) |
| 73 | *ret |= DH_CHECK_PUBKEY_TOO_SMALL; |
| 74 | - BN_copy(q, dh->p); |
| 75 | - BN_sub_word(q, 1); |
| 76 | - if (BN_cmp(pub_key, q) >= 0) |
| 77 | + BN_copy(tmp, dh->p); |
| 78 | + BN_sub_word(tmp, 1); |
| 79 | + if (BN_cmp(pub_key, tmp) >= 0) |
| 80 | *ret |= DH_CHECK_PUBKEY_TOO_LARGE; |
| 81 | |
| 82 | + if (dh->q != NULL) { |
| 83 | + /* Check pub_key^q == 1 mod p */ |
| 84 | + if (!BN_mod_exp(tmp, pub_key, dh->q, dh->p, ctx)) |
| 85 | + goto err; |
| 86 | + if (!BN_is_one(tmp)) |
| 87 | + *ret |= DH_CHECK_PUBKEY_INVALID; |
| 88 | + } |
| 89 | + |
| 90 | ok = 1; |
| 91 | err: |
| 92 | - if (q != NULL) |
| 93 | - BN_free(q); |
| 94 | + if (ctx != NULL) { |
| 95 | + BN_CTX_end(ctx); |
| 96 | + BN_CTX_free(ctx); |
| 97 | + } |
| 98 | return (ok); |
| 99 | } |
| 100 | -- |
| 101 | 2.3.5 |
| 102 | |