From 038597d295fa0ebd48ed4c00a6dfff0f30a2353a Mon Sep 17 00:00:00 2001 From: Phil Pennock Date: Tue, 26 May 2015 10:48:46 +0100 Subject: [PATCH 1/1] TLS: Enable ECDHE on OpenSSL, just the NIST P-256 curve. Bug 1397 Original by Phil Pennock; tweaked by JH. --- doc/doc-txt/ChangeLog | 2 + src/src/tls-openssl.c | 86 ++++++++++++++++++++++++++++++++++++--- src/src/transports/smtp.c | 4 +- test/confs/2102 | 3 +- test/confs/2119 | 3 +- test/confs/2132 | 3 +- 6 files changed, 91 insertions(+), 10 deletions(-) diff --git a/doc/doc-txt/ChangeLog b/doc/doc-txt/ChangeLog index a0d964926..dcae009b6 100644 --- a/doc/doc-txt/ChangeLog +++ b/doc/doc-txt/ChangeLog @@ -109,6 +109,8 @@ JH/30 Check the forward DNS lookup for DNSSEC, in addition to the reverse, JH/31 Check the HELO verification lookup for DNSSEC, adding new $sender_helo_dnssec variable. +JH/32 Bug 1397: Enable ECDHE on OpenSSL, just the NIST P-256 curve. + Exim version 4.85 ----------------- diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c index 64e2fb061..f183e8b45 100644 --- a/src/src/tls-openssl.c +++ b/src/src/tls-openssl.c @@ -521,6 +521,7 @@ DEBUG(D_tls) debug_printf("SSL info: %s\n", SSL_state_string_long(s)); /* If dhparam is set, expand it, and load up the parameters for DH encryption. Arguments: + sctx The current SSL CTX (inbound or outbound) dhparam DH parameter file or fixed parameter identity string host connected host, if client; NULL if server @@ -600,6 +601,75 @@ return TRUE; +/************************************************* +* Initialize for ECDH * +*************************************************/ + +/* Load parameters for ECDH encryption. + +For now, we stick to NIST P-256 because: it's simple and easy to configure; +it avoids any patent issues that might bite redistributors; despite events in +the news and concerns over curve choices, we're not cryptographers, we're not +pretending to be, and this is "good enough" to be better than no support, +protecting against most adversaries. Given another year or two, there might +be sufficient clarity about a "right" way forward to let us make an informed +decision, instead of a knee-jerk reaction. + +Longer-term, we should look at supporting both various named curves and +external files generated with "openssl ecparam", much as we do for init_dh(). +We should also support "none" as a value, to explicitly avoid initialisation. + +Patches welcome. + +Arguments: + sctx The current SSL CTX (inbound or outbound) + host connected host, if client; NULL if server + +Returns: TRUE if OK (nothing to set up, or setup worked) +*/ + +static BOOL +init_ecdh(SSL_CTX *sctx, host_item *host) +{ +if (host) /* No ECDH setup for clients, only for servers */ + return TRUE; + +#ifndef SSL_CTX_set_tmp_ecdh +/* No elliptic curve API in OpenSSL, skip it */ +DEBUG(D_tls) + debug_printf("No OpenSSL API to define ECDH parameters, skipping\n"); +return TRUE; +#else +# ifndef NID_X9_62_prime256v1 +/* For now, stick to NIST P-256 to get "something" running. +If that's not available, bail */ +DEBUG(D_tls) + debug_printf("NIST P-256 EC curve not available, skipping ECDH setup\n"); +return TRUE; +# else + { + EC_KEY * ecdh = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1); + BOOL rv; + + /* The "tmp" in the name here refers to setting a tempoary key + not to the stability of the interface. */ + + if ((rv = SSL_CTX_set_tmp_ecdh(sctx, ecdh) != 0)) + { + DEBUG(D_tls) debug_printf("ECDH: enable NIST P-256 curve\n"); + } + else + tls_error(US"Error enabling NIST P-256 curve", host, NULL); + EC_KEY_free(ecdh); + return rv; + } +# endif +#endif +} + + + + #ifndef DISABLE_OCSP /************************************************* * Load OCSP information into state * @@ -883,6 +953,12 @@ SSL_CTX_set_options(server_sni, SSL_CTX_get_options(server_ctx)); SSL_CTX_set_timeout(server_sni, SSL_CTX_get_timeout(server_ctx)); SSL_CTX_set_tlsext_servername_callback(server_sni, tls_servername_cb); SSL_CTX_set_tlsext_servername_arg(server_sni, cbinfo); + +if ( !init_dh(server_sni, cbinfo->dhparam, NULL) + || !init_ecdh(server_sni, NULL) + ) + return SSL_TLSEXT_ERR_NOACK; + if (cbinfo->server_cipher_list) SSL_CTX_set_cipher_list(server_sni, CS cbinfo->server_cipher_list); #ifndef DISABLE_OCSP @@ -898,10 +974,7 @@ if (rc != OK) return SSL_TLSEXT_ERR_NOACK; /* do this after setup_certs, because this can require the certs for verifying OCSP information. */ -rc = tls_expand_session_files(server_sni, cbinfo); -if (rc != OK) return SSL_TLSEXT_ERR_NOACK; - -if (!init_dh(server_sni, cbinfo->dhparam, NULL)) +if ((rc = tls_expand_session_files(server_sni, cbinfo)) != OK) return SSL_TLSEXT_ERR_NOACK; DEBUG(D_tls) debug_printf("Switching SSL context.\n"); @@ -1234,7 +1307,10 @@ else /* Initialize with DH parameters if supplied */ -if (!init_dh(*ctxp, dhparam, host)) return DEFER; +if ( !init_dh(*ctxp, dhparam, host) + || !init_ecdh(*ctxp, host) + ) + return DEFER; /* Set up certificate and key (and perhaps OCSP info) */ diff --git a/src/src/transports/smtp.c b/src/src/transports/smtp.c index 0e1aa308d..48bab9599 100644 --- a/src/src/transports/smtp.c +++ b/src/src/transports/smtp.c @@ -1461,14 +1461,14 @@ if (continue_hostname == NULL) || verify_check_given_host(&ob->hosts_try_dane, host) == OK ) && (rc = tlsa_lookup(host, &tlsa_dnsa, dane_required, &dane)) != OK - && dane_required + && dane_required /* do not error on only dane-requested */ ) { set_errno(addrlist, ERRNO_DNSDEFER, string_sprintf("DANE error: tlsa lookup %s", rc == DEFER ? "DEFER" : "FAIL"), rc, FALSE, NULL); - return rc; + return rc; } } else if (dane_required) diff --git a/test/confs/2102 b/test/confs/2102 index 38441e9d6..293474a71 100644 --- a/test/confs/2102 +++ b/test/confs/2102 @@ -42,7 +42,8 @@ check_recipient: DHE-RSA-AES256-SHA : \ DHE-RSA-AES256-GCM-SHA384 : \ DHE_RSA_AES_256_CBC_SHA1 : \ - DHE_RSA_3DES_EDE_CBC_SHA + DHE_RSA_3DES_EDE_CBC_SHA : \ + ECDHE-RSA-AES128-GCM-SHA256 warn logwrite = ${if def:tls_in_ourcert \ {Our cert SN: <${certextract{subject}{$tls_in_ourcert}}>} \ {We did not present a cert}} diff --git a/test/confs/2119 b/test/confs/2119 index de1cd2e22..3714f97c6 100644 --- a/test/confs/2119 +++ b/test/confs/2119 @@ -41,7 +41,8 @@ check_recipient: DHE-RSA-AES256-SHA:\ DHE-RSA-AES256-GCM-SHA384:\ DHE_RSA_AES_256_CBC_SHA1:\ - DHE_RSA_3DES_EDE_CBC_SHA + DHE_RSA_3DES_EDE_CBC_SHA:\ + ECDHE-RSA-AES128-GCM-SHA256 accept diff --git a/test/confs/2132 b/test/confs/2132 index 2b06d6c4a..f2d67d9a7 100644 --- a/test/confs/2132 +++ b/test/confs/2132 @@ -41,7 +41,8 @@ check_recipient: DHE-RSA-AES256-SHA : \ DHE-RSA-AES256-GCM-SHA384 : \ DHE_RSA_AES_256_CBC_SHA1 : \ - DHE_RSA_3DES_EDE_CBC_SHA + DHE_RSA_3DES_EDE_CBC_SHA : \ + ECDHE-RSA-AES128-GCM-SHA256 warn logwrite = ${if def:tls_in_ourcert \ {Our cert SN: <${certextract{subject}{$tls_in_ourcert}}>} \ {We did not present a cert}} -- 2.30.2