TLS: Enable ECDHE on OpenSSL, just the NIST P-256 curve. Bug 1397

Original by Phil Pennock; tweaked by JH.

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}}