Re: OpenLDAP and DH parameter size / LogJam vulnerability

[Emmanuel Dreyfus <manu@netbsd.org>]

On Tue, Jul 14, 2015 at 05:25:54PM +0200, Jens Vagelpohl wrote:
> Server Temp Key: DH, 1024 bits

Indeed I confirm OpenLDAP 2.4.40 support for TLSDHParamFile is broken.
The problems seems that slapd wants to set the DH parameters through 
a callback, and I do not see how we can tell OpenSSL what DH parameter
length we want in that case. Hence it defaults to 1024 bits.

The attached patch is a first attempt to fix the problem:
- set DH parameter for once if they are supplied through TLSDHParamFile,
  instead of using a callback
- Do use SSL_OP_SINGLE_DH_USE (sendmail does that). I do not know whether
  it should also be used in the callback case.
- And while there add the code to support ECDH, it is simple and it does
  not hurt (This is the same code I contributed to sendmail).

Opinions? Appart that I must file an ITS?

-- 
Emmanuel Dreyfus
manu@netbsd.org

--- libraries/libldap/tls_o.c.orig	2015-07-15 14:38:11.000000000 +0200
+++ libraries/libldap/tls_o.c	2015-07-15 16:58:03.000000000 +0200
@@ -307,28 +307,48 @@
 		tlso_report_error();
 		return -1;
 	}
 
+#ifdef SSL_OP_SINGLE_ECDH_USE
+	{
+		EC_KEY *ecdh;
+
+		ecdh = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1);
+		if ( ecdh != NULL ) {
+			SSL_CTX_set_options(ctx, SSL_OP_SINGLE_ECDH_USE);
+			SSL_CTX_set_tmp_ecdh(ctx, ecdh);
+			EC_KEY_free(ecdh);
+		}
+	}
+#endif /* SSL_OP_SINGLE_ECDH_USE */
+
 	if ( lo->ldo_tls_dhfile ) {
 		DH *dh = NULL;
 		BIO *bio;
 		dhplist *p;
 
 		if (( bio=BIO_new_file( lt->lt_dhfile,"r" )) == NULL ) {
 			Debug( LDAP_DEBUG_ANY,
-				"TLS: could not use DH parameters file `%s'.\n",
+				"TLS: could not load DH parameters `%s'.\n",
 				lo->ldo_tls_dhfile,0,0);
 			tlso_report_error();
 			return -1;
 		}
-		while (( dh=PEM_read_bio_DHparams( bio, NULL, NULL, NULL ))) {
-			p = LDAP_MALLOC( sizeof(dhplist) );
-			if ( p != NULL ) {
-				p->keylength = DH_size( dh ) * 8;
-				p->param = dh;
-				p->next = tlso_dhparams;
-				tlso_dhparams = p;
-			}
+		if (( dh=PEM_read_bio_DHparams( bio, NULL,
+						NULL, NULL )) == NULL ) {
+			Debug( LDAP_DEBUG_ANY,
+				"TLS: could not use DH parameters `%s'.\n",
+				lo->ldo_tls_dhfile,0,0);
+			tlso_report_error();
+			BIO_free( bio );
+			return -1;
+		}
+		p = LDAP_MALLOC( sizeof(dhplist) );
+		if ( p != NULL ) {
+			p->keylength = DH_size( dh ) * 8;
+			p->param = dh;
+			p->next = tlso_dhparams;
+			tlso_dhparams = p;
 		}
 		BIO_free( bio );
 	}
 
@@ -348,10 +368,13 @@
 	SSL_CTX_set_verify( ctx, i,
 		lo->ldo_tls_require_cert == LDAP_OPT_X_TLS_ALLOW ?
 		tlso_verify_ok : tlso_verify_cb );
 	SSL_CTX_set_tmp_rsa_callback( ctx, tlso_tmp_rsa_cb );
-	if ( lo->ldo_tls_dhfile ) {
-		SSL_CTX_set_tmp_dh_callback( ctx, tlso_tmp_dh_cb );
+	if ( lo->ldo_tls_dhfile && tlso_dhparams->param ) {
+		SSL_CTX_set_options ( ctx, SSL_OP_SINGLE_DH_USE );
+		SSL_CTX_set_tmp_dh ( ctx, tlso_dhparams->param );
+	} else {
+	 	SSL_CTX_set_tmp_dh_callback( ctx, tlso_tmp_dh_cb );
 	}
 #ifdef HAVE_OPENSSL_CRL
 	if ( lo->ldo_tls_crlcheck ) {
 		X509_STORE *x509_s = SSL_CTX_get_cert_store( ctx );