Re: [EXT] Re: about Warning: Setting tune.ssl.default-dh-param to 1024

[Remi Gacogne]

Hello,

On 5/7/20 12:01 AM, Lukas Tribus wrote:
>> I'm fine with that, most people use at least a value of 2048 because of
>> the warning, their modern distribution will probably deny a lower value,
>> and we add this warning a long time ago.
> 
> I agree, we should default to 2048 and remove warnings.

I agree as well, let's use a more secure default and let those who need
to support older clients do so explicitly.

Cheers,
-- 
Remi



signature.asc
Description: OpenPGP digital signature

Re: Re: [PATCH] MINOR: dns: support advertising UDP message size.

[Remi Gacogne]

Hi,

> On Fri, Jun 24, 2016 at 04:13:56PM +0200, Conrad Hoffmann wrote:
>> Yeah, I was pondering the same thing. DNS servers not capable of that
>> extensions (very few, I think) would ignore it, so always adding the OPT
>> record would be safe indeed.

I would be very careful about that. A lot of DNS implementations have
code to deal with servers choking on EDNS0 and either responding with
FORMERR or NOTIMPL, or simply dropping the query.

See for example "EDNS fallback" at
https://www.unbound.net/documentation/requirements.html, or the
corresponding PowerDNS logic at
https://github.com/PowerDNS/pdns/blob/master/pdns/syncres.cc#L325

It's very nice having support for EDNS0, but IMHO it shouldn't be
enabled by default if it doesn't fallback.


Best regards,

Remi




signature.asc
Description: OpenPGP digital signature

Re: Re: [PATCH] BUG/MINOR: ssl: fix potential memory leak in ssl_sock_load_dh_params()

[Remi Gacogne]

Hi Willy, Roberto, Emeric,

On 06/30/2016 08:12 PM, Willy Tarreau wrote:
> I checked the man page for SSL_CTX_set_tmp_dh() and it does not mention
> anything regarding the need to free the param or not. And the example
> that comes with it doesn't involve a call to DH_free().

It's a mess, I recall having the same issue with a previous version of
the DH code.

> Thus for now I'd rather leave valgrind unhappy until someone finds what
> exactly needs to be done to make it happy and not cause this issue.

I've attached what I think is the right fix, simply checking if
local_dh_2014 is NULL before calling ssl_get_dh_1024(), thus only
generating it at most one time. It's then freed in __ssl_sock_deinit()
so valgrind should be happy.


Best regards,

Remi

From cd90e8cca7ac9156e769d4580448cf89f45cd929 Mon Sep 17 00:00:00 2001
From: Remi Gacogne <rgacogne-git...@coredump.fr>
Date: Sat, 2 Jul 2016 16:26:10 +0200
Subject: [PATCH] BUG/MINOR: ssl: fix potential memory leak in
 ssl_sock_load_dh_params()

Roberto Guimaraes reported that Valgrind complains about a leak
in ssl_get_dh_1024().
This is caused caused by an oversight in ssl_sock_load_dh_params(),
where local_dh_1024 is always replaced by a new DH object even if
it already holds one. This patch simply checks whether local_dh_1024
is NULL before calling ssl_get_dh_1024().
---
 src/ssl_sock.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index f247618..e5a6f0a 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1638,7 +1638,9 @@ int ssl_sock_load_dh_params(SSL_CTX *ctx, const char *file)
 
 		if (global.tune.ssl_default_dh_param <= 1024) {
 			/* we are limited to DH parameter of 1024 bits anyway */
-			local_dh_1024 = ssl_get_dh_1024();
+			if (local_dh_1024 == NULL)
+local_dh_1024 = ssl_get_dh_1024();
+
 			if (local_dh_1024 == NULL)
 goto end;
 
-- 
2.9.0



signature.asc
Description: OpenPGP digital signature

Re: Re: haproxy 1.6.0 crashes

[Remi Gacogne]

Hi,

On 10/19/2015 05:01 PM, Willy Tarreau wrote:
>> [1] https://www.mail-archive.com/haproxy@formilux.org/msg19962.html
>> [2] https://www.mail-archive.com/haproxy@formilux.org/msg19995.html
> 
> Regarding the second one, maybe Rémi's review could help. I noticed that
> you used gen_ssl_ctx_ptr_index = -1 which is the same value used for
> dh_params. Based on its name it makes me think it's a position in an
> array, so I'm not sure whether we can make them collide for example. I
> really don't know this API at all.

I am not familiar with the generated certificate cache, so I will just
comment on the use of SSL_CTX_set_ex_data() and SSL_CTX_get_ex_data() in
the second patch, at least for now.

The value of gen_ssl_ctx_ptr_index is correctly initialized to -1, and a
valid index is then obtained by calling SSL_CTX_get_ex_new_index() in
the __ssl_sock_init() constructor, so there should not be any collision
with other indexes.

My only minor remark is that, though unlikely,
SSL_CTX_get_ex_new_index() might return -1 in case of error. In the DH
code we handle this by checking that ssl_dh_ptr_index is != -1 before
using it, and I think it would be better to do the same check before
using gen_ssl_ctx_ptr_index.


Kind regards,

Remi






signature.asc
Description: OpenPGP digital signature

Re: Re: haproxy 1.6.0 crashes

[Remi Gacogne]

Hi Willy, Christopher,

> Ideally we'd have the info in the ssl_ctx itself, but I remember that Emeric
> told me a while ago that we couldn't store anything in an ssl_ctx. Thus I
> can understand that we can't easily "tag" the ssl_ctx as being statically
> or dynamically allocated, which is why I understand the need for the flag
> on the connection as an alternative.

Well, I am not sure it will suit your needs, but it is possible to store
some info in a SSL_CTX using SSL_CTX_set_ex_data(). We are already doing
that for DH parameters and Certificate Transparency data.

-- 
Remi





signature.asc
Description: OpenPGP digital signature

Re: Re: haproxy resolvers "nameserver: can't connect socket" (on FreeBSD)

[Remi Gacogne]

Hi,

On 09/07/2015 10:47 AM, Baptiste wrote:
>> It fails that way:
>>
>> socket(PF_INET,SOCK_DGRAM,17)= (0x4)
>> connect(4,{ AF_INET 8.8.8.8:53 },128)ERR#22 'Invalid argument'
>>
>> 3rd argument for connect() looks wrong for ipv4:
>>
>> ERRORS
>>  The connect() system call fails if:
>>
>>  [EINVAL]   The namelen argument is not a valid length for the
>> address family.
>>
>>
> 
> Ok, excellent.
> I wonder how this could happen :)

It looks like this code is passing the size of a struct
sockaddr_storage to connect(), instead of the size corresponding to the
underlying socket family. Some OS are forgiving, other not so much :)

diff --git a/src/dns.c b/src/dns.c
index 4bc5448..f725ff4 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -819,7 +819,7 @@ int dns_init_resolvers(void)
}

/* "connect" the UDP socket to the name server IP */
-   if (connect(fd, (struct
sockaddr*)>addr, sizeof(curnameserver->addr)) == -1) {
+   if (connect(fd, (struct
sockaddr*)>addr, get_addr_len(>addr)) == -1) {
Alert("Starting [%s/%s] nameserver:
can't connect socket.\n", curr_resolvers->id,
curnameserver->id);
close(fd);





signature.asc
Description: OpenPGP digital signature

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

[Remi Gacogne]

Hi,

I haven't had time to look at the entire patch set, so this is just some
remarks on the points that Emeric mentioned.

 Code initially use 'ctx-default_passwd_callback_userdata' to allow us to
 manage a further way to manage passphrase via configuration.
 
 As far as I could tell, this field was not getting set anywhere in the
 code. It¹s set with SSL_CTX_set_default_passwd_cb, which I did not find in
 the codebase. I had then assumed that this was just a harmless copy/paste
 of example code, which generally always use this CB.

I faced the same issue with the patch I sent earlier to provide support
for OpenSSL 1.1.x, because the default_passwd_callback_userdata can't be
accessed anymore (opaque struct). I couldn't find a place where
callback/callback_userdata are used in the code either, so I just
replaced them with NULL, which is equivalent when the callback is not set.
Note that PEM_read_bio_PrivateKey accepts a pem_password_cb and the
associated user data, so I don't think it's an issue if we want to
handle password-protected key in a different way in the future.

 I notice also you continue to load DH parameters for each files. It was
 not a big deal by the past because for each file correspond a uniq
 certificate.

 With your patch, i don't know what will be the behavior. In the way it is
 loaded on SSL_CTX, the DH parameter doesn't seem specific to the used
 DSA/RSA/ECDSA certificate. So which one will be used, first or latest
 loaded?

 Some users will expect to use the DH parameter defined in the same file
 than the used certificate, but i'm really not sure it will be the case.
 
 Unsure about this. I can change it to only load the first time the context
 is created.

The last DH parameters set with SSL_CTX_set_tmp_dh() will be used,
because the DSA/RSA/ECDSA certs will share the same SSL_CTX (that's the
whole point actually) and OpenSSL only supports one in a SSL_CTX.
We could change the way DH parameters are loaded for a bind, splitting
the certificate and the DH parameters in separate files. It would be
clearer but would not play nice with existing configuration.
Maybe we should just issue a warning when we detect that different DH
parameters are being loaded on the same SSL_CTX (ie, check in
ssl_sock_load_dh_params() if the SSL_CTX already has a tmp_dh, and if so
check if it's a different one)?





signature.asc
Description: OpenPGP digital signature

[PATCH] MEDIUM: ssl: add support for using cryptographic keys stored in a PKCS#11 HSM

[Remi Gacogne]

Hi,

This patch enables the use of cryptographic keys stored in a
PKCS#11-compatible hardware security module. It uses the OpenSSL engine
interface, and has been tested with engine-pkcs11 and two different
kinds of HSM:

- SoftHSM
- Yubikey

The main benefit of storing cryptographic keys in a HSM is to prevent
them from being accessed or stolen should the host been compromised, by
example using a heartbleed-like vulnerability. Note that while being
very useful for testing purpose, SoftHSM keeps the key in the same
memory space than the main process, and a such should not be considered
as secure as an hardware HSM. While a Yubikey is a nice device and a
hardware HSM, it provides a very low transactions/s rate, and as such
might probably not be suited for production use with HAproxy.

This patch should be considered more a proof-of-concept/WIP than a
production-ready one. It adds the possibility to load a private key from
a HSM while the corresponding certificate is still loaded from a file,
using the following syntax:

bind 127.0.0.1:8443 ssl crt pkcs11:slot_SLOT NUMBER-id_OBJECT
ID:/path/to/corresponding/cert.pem

For example, this would load the private key from the object named
deadbeef in the slot 0 of softhsm:

bind 127.0.0.1:8443 ssl crt
pkcs11:slot_0-id_deadbeef:/path/to/corresponding/cert.pem

Or from the slot 9a of a yubikey in PIV mode:

bind 127.0.0.1:8443 ssl crt
pkcs11:slot_1-id_1:/path/to/corresponding/cert.pem

This patch also allows the configuration of the OpenSSL application name
via the environment variable HAPROXY_SSL_APP_NAME, in order to use
different configurations for different applications without polluting
the default OpenSSL namespace.

For these examples to work, engine-pkcs11 must be installed and OpenSSL
be configured to use it. For example, these sections can be defined in
/etc/ssl/openssl.conf:

haproxy_yubi_conf= haproxy_yubi_def
haproxy_shsm_conf= haproxy_shsm_def

[...]

[haproxy_yubi_def]
engines = engine_ha_yubi_section

[engine_ha_yubi_section]
pkcs11 = pkcs11_ha_yubi_section

[pkcs11_ha_yubi_section]
engine_id = pkcs11
dynamic_path = /usr/lib/engines/engine_pkcs11.so
init = 0
MODULE_PATH = /usr/lib/pkcs11/opensc-pkcs11.so

[haproxy_shsm_def]
engines = engine_ha_shsm_section

[engine_ha_shsm_section]
pkcs11 = pkcs11_ha_shsm_section

[pkcs11_ha_shsm_section]
engine_id = pkcs11
dynamic_path = /usr/lib/engines/engine_pkcs11.so
init = 0
MODULE_PATH = /usr/lib/libsofthsm.so

In this example, in order to load keys from SoftHSM,
HAPROXY_SSL_APP_NAME has to be set to haproxy_shsm_conf. For loading
from a yubikey (via OpenSC), HAPROXY_SSL_APP_NAME has to be set to
haproxy_yubi_conf.

There is a few things I am not happy with in this patch:
- the hijacking of the crt syntax, prepending pkcs11: to load the key
from a PKCS#11 engine ;
- the use of a environment variable to specify the OpenSSL application
name. However it has to be set before __ssl_sock_init() constructor is
loaded, so the configuration parsing has not been done yet ;
- the PKCS#11 interface specifies that all PKCS#11 handles should be
reinitialized after a fork(), which might be an issue with nbproc  1.
In practice this is correctly handled by some drivers but it's not
guaranteed to work for every one of them.

I will appreciate any feedback regarding those points or the patch itself.

-- 
Remi
From 5f16d441e7fa5dfbce033faae8a866f178d05967 Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne-git...@coredump.fr
Date: Thu, 23 Jul 2015 16:50:49 +0200
Subject: [PATCH] MEDIUM: ssl: add support for using cryptographic keys stored
 in a PKCS#11 HSM

---
 src/ssl_sock.c | 127 ++---
 1 file changed, 121 insertions(+), 6 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 7f1a070..15460b5 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -50,6 +50,9 @@
 #ifndef OPENSSL_NO_DH
 #include openssl/dh.h
 #endif
+#ifndef OPENSSL_NO_ENGINE
+#include openssl/engine.h
+#endif
 
 #include import/lru.h
 #include import/xxhash.h
@@ -134,6 +137,10 @@ static DH *local_dh_2048 = NULL;
 static DH *local_dh_4096 = NULL;
 #endif /* OPENSSL_NO_DH */
 
+#ifndef OPENSSL_NO_ENGINE
+static ENGINE * pkcs11_engine = NULL;
+#endif /* OPENSSL_NO_ENGINE */
+
 #ifdef SSL_CTRL_SET_TLSEXT_HOSTNAME
 /* X509V3 Extensions that will be added on generated certificates */
 #define X509V3_EXT_SIZE 5
@@ -1632,8 +1639,14 @@ end:
 
 static int ssl_sock_load_cert_file(const char *path, struct bind_conf *bind_conf, struct proxy *curproxy, char **sni_filter, int fcount, char **err)
 {
+	static const char * const pkcs11_engine_name = pkcs11;
+	static const char pkcs11_prefix[] = pkcs11:;
+	static const size_t pkcs11_prefix_len = sizeof pkcs11_prefix - 1;
 	int ret;
 	SSL_CTX *ctx;
+	char * pkcs11_slot = NULL;
+	const char * ptr = NULL;
+	EVP_PKEY * pkey = NULL;
 
 	ctx = SSL_CTX_new(SSLv23_server_method());
 	if (!ctx) {
@@ -1642,11 +1655,83 @@ static int ssl_sock_load_cert_file(const char *path

[PATCH] MINOR: stream: initialize the current_rule field to NULL on stream init

[Remi Gacogne]

I stumbled upon a minor valgrind warning when testing a SSL change:

==16783== Conditional jump or move depends on uninitialised value(s)
==16783==at 0x44E662: http_res_get_intercept_rule (proto_http.c:3730)
==16783==by 0x44E662: http_process_res_common (proto_http.c:6528)
==16783==by 0x4797B7: process_stream (stream.c:1851)
==16783==by 0x414634: process_runnable_tasks (task.c:238)
==16783==by 0x40B02F: run_poll_loop (haproxy.c:1528)
==16783==by 0x407F25: main (haproxy.c:1887)

It looks like it's currently possible for the current_rule field to be
evaluated before being set, probably since commit
152b81e7b2565862956af883820d4f79177d0651.

-- 
Remi
From 99895c9786558d159ab35e0dde30123cc7633cab Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne-git...@coredump.fr
Date: Wed, 22 Jul 2015 17:10:58 +0200
Subject: [PATCH] MINOR: stream: initialize the current_rule field to NULL on
 stream init

Currently it is possible for the current_rule field to be evaluated before
being set, leading to valgrind complaining:

==16783== Conditional jump or move depends on uninitialised value(s)
==16783==at 0x44E662: http_res_get_intercept_rule (proto_http.c:3730)
==16783==by 0x44E662: http_process_res_common (proto_http.c:6528)
==16783==by 0x4797B7: process_stream (stream.c:1851)
==16783==by 0x414634: process_runnable_tasks (task.c:238)
==16783==by 0x40B02F: run_poll_loop (haproxy.c:1528)
==16783==by 0x407F25: main (haproxy.c:1887)

This was introduced by commit 152b81e7b2565862956af883820d4f79177d0651.
---
 src/stream.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/stream.c b/src/stream.c
index ff6b32b..e6f1fc6 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -105,6 +105,7 @@ struct stream *stream_new(struct session *sess, struct task *t, enum obj_type *o
 	 * any rulelist match the NULL pointer.
 	 */
 	s-current_rule_list = NULL;
+	s-current_rule = NULL;
 
 	memset(s-stkctr, 0, sizeof(s-stkctr));
 
-- 
2.4.6



signature.asc
Description: OpenPGP digital signature

[PATCH] Add compatibility with OpenSSL 1.1.x

[Remi Gacogne]

Hi,

A while back, Lukas Tribus mentioned that HAproxy used quite a few
OpenSSL internals that were not going to be usable in the 1.1.x branch,
and that we would better take a look at it. As I am currently having
unexpected free time, here is a first attempt at fixing it.

This patch tries to make HAproxy compatible with the OpenSSL 1.1.x
branch, which is still in development, by using accessors instead of
directly using OpenSSL internals when possible, and replacing the use of
deprecated functions by the new ones.

There is still some issues left with this patch:

- in src/shctx.c, the context size increases because I didn't find a way
to alter the session_id_length and sid_ctx_length fields in the same way
it was done before ;
- in ssl_sock_handshake(), we have now slightly less accurate SSL
handshake error messages, because I couldn't find how to retrieve the
information contained in (SSL *)conn-xprt_ctx)-packet_length in a
clean way ;
- in ssl_sock_load_ocsp_response(), we still access the certId field
from a OCSP_SINGLERESP struct, which is becoming opaque in 1.1. I
couldn't find an accessor for this field so I proposed to add one in a
pull request to OpenSSL [1].

Of course these only occur when built against the OpenSSL 1.1.x.

Any comment would be welcome!


Regards,

[1]: https://github.com/openssl/openssl/pull/334

--
Remi Gacogne


From 5b5a5a89b542c9a24c79babf6c1066b18553ecb8 Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne-git...@coredump.fr
Date: Thu, 23 Jul 2015 16:39:43 +0200
Subject: [PATCH] MEDIUM: ssl: add compatibility with openssl 1.1.x

This patch tries to make HAproxy compatible with the OpenSSL 1.1.x branch, which is still in development, by using accessors instead of directly using OpenSSL internals when possible, and replacing the use of deprecated functions by the new ones.
---
 src/shctx.c|  37 ++---
 src/ssl_sock.c | 122 +
 2 files changed, 129 insertions(+), 30 deletions(-)

diff --git a/src/shctx.c b/src/shctx.c
index a22730a..b0ff108 100644
--- a/src/shctx.c
+++ b/src/shctx.c
@@ -390,8 +390,18 @@ int shctx_new_cb(SSL *ssl, SSL_SESSION *sess)
 	unsigned char encsess[sizeof(struct shsess_packet)+SHSESS_MAX_DATA_LEN];
 	struct shsess_packet *packet = (struct shsess_packet *)encsess;
 	unsigned char *p;
-	int data_len, sid_length, sid_ctx_length;
+	const unsigned char *sid;
+	int data_len;
+	unsigned int sid_length;
+#if OPENSSL_VERSION_NUMBER  0x1010L
+	unsigned int sid_ctx_length;
+#endif
 
+#if OPENSSL_VERSION_NUMBER = 0x1010L
+	sid = SSL_SESSION_get_id(sess,
+ sid_length);
+#else
+	sid = sess-session_id;
 
 	/* Session id is already stored in to key and session id is known
 	 * so we dont store it to keep size.
@@ -400,6 +410,7 @@ int shctx_new_cb(SSL *ssl, SSL_SESSION *sess)
 	sess-session_id_length = 0;
 	sid_ctx_length = sess-sid_ctx_length;
 	sess-sid_ctx_length = 0;
+#endif
 
 	/* check if buffer is large enough for the ASN1 encoded session */
 	data_len = i2d_SSL_SESSION(sess, NULL);
@@ -410,7 +421,7 @@ int shctx_new_cb(SSL *ssl, SSL_SESSION *sess)
 	p = packet-data;
 	i2d_SSL_SESSION(sess, p);
 
-	memcpy(packet-hdr.id, sess-session_id, sid_length);
+	memcpy(packet-hdr.id, sid, sid_length);
 	if (sid_length  SSL_MAX_SSL_SESSION_ID_LENGTH)
 		memset(packet-hdr.id[sid_length], 0, SSL_MAX_SSL_SESSION_ID_LENGTH-sid_length);
 
@@ -422,9 +433,11 @@ int shctx_new_cb(SSL *ssl, SSL_SESSION *sess)
 	shared_context_unlock();
 
 err:
+#if OPENSSL_VERSION_NUMBER  0x1010L
 	/* reset original length values */
 	sess-session_id_length = sid_length;
 	sess-sid_ctx_length = sid_ctx_length;
+#endif
 
 	return 0; /* do not increment session reference count */
 }
@@ -501,6 +514,8 @@ SSL_SESSION *shctx_get_cb(SSL *ssl, unsigned char *key, int key_len, int *do_cop
 	/* decode ASN1 session */
 	p = data;
 	sess = d2i_SSL_SESSION(NULL, (const unsigned char **)p, data_len);
+
+#if OPENSSL_VERSION_NUMBER  0x1010L
 	/* Reset session id and session id contenxt */
 	if (sess) {
 		memcpy(sess-session_id, key, key_len);
@@ -508,6 +523,7 @@ SSL_SESSION *shctx_get_cb(SSL *ssl, unsigned char *key, int key_len, int *do_cop
 		memcpy(sess-sid_ctx, (const unsigned char *)SHCTX_APPNAME, strlen(SHCTX_APPNAME));
 		sess-sid_ctx_length = ssl-sid_ctx_length;
 	}
+#endif
 
 	return sess;
 }
@@ -517,13 +533,22 @@ void shctx_remove_cb(SSL_CTX *ctx, SSL_SESSION *sess)
 {
 	struct shared_session *shsess;
 	unsigned char tmpkey[SSL_MAX_SSL_SESSION_ID_LENGTH];
-	unsigned char *key = sess-session_id;
+	const unsigned char *key;
+	unsigned int key_length = 0;
+
+#if OPENSSL_VERSION_NUMBER = 0x1010L
+	key = SSL_SESSION_get_id(sess,
+ key_length);
+#else
+	key = sess-session_id;
+	key_length = sess-session_id_length;
+#endif
 	(void)ctx;
 
 	/* tree key is zeros padded sessionid */
-	if (sess-session_id_length  SSL_MAX_SSL_SESSION_ID_LENGTH) {
-		memcpy(tmpkey, sess-session_id, sess-session_id_length);
-		memset

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

[Remi Gacogne]

Hi,

 I was unaware that BoringSSL removed the callback, but in that case, could
 we limit this feature to only OpenSSL? I¹m also not seeing how using this
 callback prevents rfc5077, could you please elaborate.

Yes, sorry, I didn't realize it earlier but that's not true for all
OpenSSL versions. Starting with OpenSSL 1.0.0, tls1_process_ticket()
will decline decrypting session tickets sent by the client if the
session_secret_cb is in use:

if (s-tls_session_secret_cb)
{
/* Indicate cache miss here and instead
of

 * generating the session from ticket
now,

 * trigger abbreviated handshake based
on

 * external mechanism to calculate the
master

 * secret later. */
return 0;
}

There is even a nice comment about it, starting from 1.0.1 I believe:

 * If s-tls_session_secret_cb is set then we are expecting a pre-shared
key

 * ciphersuite, in which case we have no use for session tickets and one
will

 * never be decrypted, nor will s-tlsext_ticket_expected be set to 1.

 I see that now. But in the current implementation, doesn¹t it always just
 use the default CTX if the client doesn¹t specify SNI? In that case, you¹d
 still be limited to the signature algorithm of the default CTX. It¹s
 possible to take the server preferences in mind. However, wouldn¹t that
 lead to a situation where the server wants to use Signature Algorithm A,
 but the client only supports B, and then the client can¹t connect?

Yes, of course, but remember that the client sends not one choice but an
ordered list of ciphersuites he is willing to use. If none of these is
accepted by the server, the connection will fail, true, but in practice
it seldom happens.

 In my mind, choosing the signature algorithm based on what the client
 supports is a good idea as it guarantees that the client can use that
 algorithm. Then, the server can use w/e cipher suite it wants in terms of
 Key-Exchange and symmetric-encryption.

Yes, choosing a common suite supported by both side is a necessity. But
when there is more than one common suite, which happens most of the
time, you can either follow the client's preference or the server one.
Right now, it seems that we have a consensus to follow the server's
choice (see ssl_prefer_server_ciphers on for nginx, SSLHonorCipherOrder
on for Apache HTTPd, ..) and I believe we should continue to do that in
HAproxy because legacy clients have a long history of choosing crappy
ciphersuite (look at the recent export fiascos, for example).

 Tying this feature into 1.0.2 would definitely make it easier, I agree. It
 just will hinder adoption.

That's true, but I am afraid doing otherwise would require adding a
complex logic in the TLS stack of HAproxy, so sadly I am more enclined
to require 1.0.2 for people willing to use this feature.





signature.asc
Description: OpenPGP digital signature

Re: Contribution for HAProxy: Peer Cipher based SSL CTX switching

[Remi Gacogne]

Hi all,

Dave, thank you for proposing this feature, I truly think that being
able to serve RSA or ECDSA certificates depending on what the client
supports would be an awesome addition to HAproxy.

However, I have some concerns about the use of
SSL_set_session_secret_cb() for this feature, because it was clearly not
designed for this kind of manipulation. It has been removed from
BoringSSL [1] and simply enabling this callback in OpenSSL, regardless
of what is done in the callback itself, disables TLS session ticket
(rfc5077) [2], which I believe is an issue.
It also means that you have to use flags that are internals to OpenSSL,
and may change in the future. Note that this could probably be avoided
in 1.0.2+ with the use of ssl_cipher_get_cert_index().
Lastly, switching the SSL_CTX based only on the suites sent by the
client in ClientHello means that you effectively bypass the suites
preference set on HAproxy, if any. If the first suite sent by the client
requires an ECDSA certificate, you will switch to a SSL_CTX supporting
only ECDSA suites, even if the server preferences prefer some suites
using RSA better than ECDSA ones. This is quite a change from the
HAproxy default, which is to take into account the server preferences
first [3].

IMHO, I would tend to prefer letting OpenSSL handle both the RSA and
ECDSA pairs in the same SSL_CTX. It could probably be done when a new
certificate is loaded by looking up in the existing certificates list if
there is already is one with the same subject name / SAN and a different
key type, and merging it to the existing SSL_CTX if it does.
I do realize that it would only work with 1.0.2+ because of the
inability of previous OpenSSL version of supporting more than one chain
in a given SSL_CTX, though.

Just my two cents :)

[1]
https://boringssl.googlesource.com/boringssl/+/d1681e614f8c3b4105efc42fb1fd35bd3ec09547%5E!/
[2] see tls1_process_ticket(), in ssl/t1_lib.c
[3] the default ssloptions contains SSL_OP_CIPHER_SERVER_PREFERENCE, in
ssl_sock_prepare_ctx(), src/ssl_sock.c




signature.asc
Description: OpenPGP digital signature

Re: A few thoughts on Haproxy and weakdh/logjam

[Remi Gacogne]

Hi Willy,

On 05/28/2015 06:19 PM, Willy Tarreau wrote:
 RSA 1024 is (at last) being phased out, and we have
 seen on this mailing-list that a DH greater than 2048-bit is simply too
 costly for a lot of users.
 
 OK so that means that probably people will prefer to build their own
 DH-1024 to use with 2048 keys.

I hope most users are going to use DH-2048 with RSA-4096 instead, but
yeah, that's the idea :)

 I didn't have enough time to implement the 3 patches I promised yet,
 
 I'm not the one who will blame you for that, if you knew the number of
 things I don't have the time to do!

I can't even begin to imagine!

 I expect to be able to send the ssl-dh-param-file patch tomorrow, as it
 is mostly written (but not well tested yet), as well as the patch to
 move from 1024-bit DH to 2048-bit by default.
 
 Great! Do you think it would make sense to backport the ssl-dh-param-file
 to 1.5 ? I mean, will some users need this in the short term (or said
 differently, may we use this as an incentive to be more careful about
 that ?).

Here it is. Yes, while I am of course a bit reluctant about the idea of
adding a new feature in 1.5, I think it makes sense to backport this one
because it makes it easier to use custom DH parameters, which is the
best option security-wise. Note that if we decide to go the safe way by
not backporting it, it is still possible to work around and do the same
thing by adding custom DH parameters to each cert file.

 Also for 1.5.13 as I understand it, I should regenerate a new dhparam-1024
 to get rid of oakley group 2. I'll need some directions on how to do this
 correctly.

Yes, of course. I am attaching a patch that replace all the hard-coded
DH parameters by new ones, removing the 8192-bit one in the process
because I don't think it will ever be used (it's just too CPU-intensive,
especially now that ECDHE is widely available). Just replace the content
of dh1024_p, dh1024_g, dh2048_p, dh2048_g, dh4096_p and dh4096_g by the
values you get from running those commands on your own host (preferably
with some entropy available):

$ openssl dhparam 1024 -C
$ openssl dhparam 2048 -C
$ openssl dhparam 4096 -C

Please don't hesitate to get back to me if needed, I know I have the bad
habit of skipping crucial steps in my explanations.

-- 
Rémi
From fe610d85b580c8dcf2aa9e07d5ed98abd6604ebd Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgaco...@aquaray.fr
Date: Fri, 29 May 2015 15:53:22 +0200
Subject: [PATCH] MEDIUM: ssl: add the possibility to use a global DH
 parameters file

This patch adds the ssl-dh-param-file global setting. It sets the
default DH parameters that will be used during the SSL/TLS handshake when
ephemeral Diffie-Hellman (DHE) key exchange is used, for all bind lines
which do not explicitely define theirs.
---
 doc/configuration.txt| 17 ++-
 include/proto/ssl_sock.h |  3 +++
 src/cfgparse.c   | 18 +++
 src/ssl_sock.c   | 57 +---
 4 files changed, 81 insertions(+), 14 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 9676643..655ede0 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -766,6 +766,20 @@ ssl-default-server-options [option]...
   default ssl-options to force on all server lines. Please check the server
   keyword to see available options.
 
+ssl-dh-param-file file
+  This setting is only available when support for OpenSSL was built in. It sets
+  the default DH parameters that are used during the SSL/TLS handshake when
+  ephemeral Diffie-Hellman (DHE) key exchange is used, for all bind lines
+  which do not explicitely define theirs. It will be overridden by custom DH
+  parameters found in a bind certificate file if any. If custom DH parameters
+  are not specified either by using ssl-dh-param-file or by setting them directly
+  in the certificate file, pre-generated DH parameters of the size specified
+  by tune.ssl.default-dh-param will be used. Custom parameters are known to be
+  more secure and therefore their use is recommended.
+  Custom DH parameters may be generated by using the OpenSSL command
+  openssl dhparam size, where size should be at least 2048, as 1024-bit DH
+  parameters should not be considered secure anymore.
+
 ssl-server-verify [none|required]
   The default behavior for SSL verify on servers side. If specified to 'none',
   servers certificates are not verified. The default is 'required' except if
@@ -1224,7 +1238,8 @@ tune.ssl.default-dh-param number
   this maximum value. Default value if 1024. Only 1024 or higher values are
   allowed. Higher values will increase the CPU load, and values greater than
   1024 bits are not supported by Java 7 and earlier clients. This value is not
-  used if static Diffie-Hellman parameters are supplied via the certificate file.
+  used if static Diffie-Hellman parameters are supplied either directly
+  in the certificate file or by using the ssl-dh-param-file parameter

Re: A few thoughts on Haproxy and weakdh/logjam

[Remi Gacogne]

Hi Julien,

On 05/27/2015 12:05 PM, Julien Vehent wrote:
 This is by far the best write-up on DHE compatibility issues I've seen.
 Would you mind organizing your research into something we could publish
 on https://wiki.mozilla.org/Security/Server_Side_TLS ?
 I've added some notes about compatibility between DHE and Java, but
 nothing concerning the pre-defined DH groups yet. If you had time to
 submit it as a pull-request on 
 https://github.com/mozilla/server-side-tls/blob/gh-pages/Server_Side_TLS.mediawiki
 that would be absolutely fantastic!

I see that you have already added a new section about DHE and Java,
that's great! I will have at look at whether I can improve the page and
will submit a pull request if it makes sense.

By the way this document is awesome, thank your for writing it!

-- 
Rémi



signature.asc
Description: OpenPGP digital signature

Re: A few thoughts on Haproxy and weakdh/logjam

[Remi Gacogne]

Hi,

On 05/23/2015 08:47 AM, Willy Tarreau wrote:
 Do you have any idea about the ratio of clients (on the net) which don't
 support ECDHE right now but support DHE ?

Basically, by totally removing DHE, we would be losing forward secrecy for:
- Java = 6 ;
- OpenSSL = 1.0.0 ;
- Android = 3.

Note that moving to a DH size of 2048-bit is an issue if you have Java 6
clients anyway (Java 7 does not support DHE  1024-bit either, but does
support ECDHE).

 Then I'd insert a 5th option between 3 and 4 above : use an haproxy-specific
 version, which becomes a target but has less chances of being targetted if
 it only represents a small percentage of the traffic and requires a nation-
 wide grid to pre-compute the values. I mean, this could be an option that
 would even improve the situation for the stable branches. All users of 1.5
 running with dhparam 1024 could use a different one than the known vulnerable
 one. That would not make them safe, but less likely vulnerable. The doc
 should be clear about this though.

Ok, moving from Oakley group 2 by default seems to be a smart move
anyway. We probably should use this opportunity to move to
non-standardized groups for all group sizes. It can't hurt, especially
since it took so long to cryptographers to realize that using the Oakley
group 2 everywhere was a liability.

 And for future versions we'd support loading it from a file, and emit a
 warning whenever a pre-computed 1024 is used without being picked from
 a file.

Yes, I think it's a good idea. I will post 3 proposals for 1.6 later
this week:
- a proper fix for the bug found by Hervé Commowick (thanks for the bug
report and the testing, by the way) ;
- a new configuration option, something like ssl-dh-param-file, allowing
the use of a global DH parameters file (which may still be overridden by
setting the DH parameters directly in a certificate file) ;
- a patch to alter the default DH size from 1024-bit to 2048-bit.

I think it may be a good idea to backport the first one to 1.5, and to
add one replacing default groups with random, non-standardized ones. I
would rather prefer someone from HAProxy generate the new ones, for
transparency's sake, but I can help for the process :)

I was thinking about what Lukas Tribus said in an other thread, and I
agree that the DH configuration is too complicated. Maybe we should try
to deprecate the pre-computed DH parameters in 1.6 by issuing a warning
whenever they are used, ie then at least one ciphersuite uses a DHE key
exchange and no user-specified DH parameters are used (globally or in
the certificate file). But as you pointed out, Willy, it means that we
cannot fix invalid / weak groups that are stored in files (most probably
forever) in the future, so I am not sure what the smartest move is.

-- 
Rémi



signature.asc
Description: OpenPGP digital signature

Re: SSL custom dhparam problem

[Remi Gacogne]

Hi Hervé,

On 05/21/2015 10:11 PM, Hervé Commowick wrote:

 I encounter a problem with dhparam configuration, if i have 2 bind lines, a
 tune.ssl.default-dh-param 2048, and a custom group dhparam in one of the
 pem file, ALL bind lines will use 1024, the one with the custom group will
 work as expected, and the one without will use the default Oakley group 2
 instead of the 2048-bit MODP group 14 (thx Remi for the wording, i'm not
 sure to well understand all of that :))
 
 this is clearly a bug amha, thx anyone who can help (Remi ? :) )

Oh, this is a bug indeed, and it's my fault. In order to prevent the
display of warning messages about default-dh-param not being set when a
static DH group value is used, the value of default-dh-param is
overridden when a static DH group value is found. It does work when you
have only one bind, but it's clearly wrong when more than one is used,
like in your configuration.

Could you try with the attached patch? It's a patch against the 1.6
trunk but it does apply cleanly against 1.5.12.

It will result in false positive messages about default-dh-param not
being set when it's not needed, but to prevent that we will need to
check if each bind has a static DH group value, which I'm not very fond of.

-- 
Rémi



diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 9302869..5317a28 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1347,10 +1347,6 @@ int ssl_sock_load_dh_params(SSL_CTX *ctx, const char *file)
 	if (dh) {
 		ret = 1;
 		SSL_CTX_set_tmp_dh(ctx, dh);
-		/* Setting ssl default dh param to the size of the static DH params
-		   found in the file. This way we know that there is no use
-		   complaining later about ssl-default-dh-param not being set. */
-		global.tune.ssl_default_dh_param = DH_size(dh) * 8;
 	}
 	else {
 		/* Clear openssl global errors stack */


signature.asc
Description: OpenPGP digital signature

A few thoughts on Haproxy and weakdh/logjam

[Remi Gacogne]

Haproxy and weakdh/logjam

Hi,

Everyone has probably heard about the recently disclosed weakdh/logjam
attack [0] already. Here are a few personal thoughts on the impact on
Haproxy.

The weakdh issue is twofold:

- if the HTTPS server is willing to accept a cipher suite using a very
weak Diffie-Hellman (DH) group, like for example
TLS_DHE_RSA_EXPORT_WITH_DES40_CBC_SHA, then it is possible for an
adversary in position of man-in-the-middle (MitM) to downgrade the
security level of a given connection to this cipher suite, thus reducing
the security to 512-bit DH ;

- a nation-state adversary may be able to pre-compute all the possible
values of a commonly-shared 1024-bit DH group, thus being able to
decrypt all exchanges using that group.

The first point depends on the cipher suite specified by the
administrator with the ssl-default-bind-ciphers or ciphers
parameters. I strongly encourage everyone to use the modern cipher suite
described on the Mozilla wiki [1]. Please at least consider using the
Intermediate one.

In the default configuration, Haproxy uses a 1024-bit DH key generated
from the second Oakley group [2] for Diffie-Hellman Ephemeral (DHE) key
exchange. This group is widely used, and is likely to be the first
target for pre-computation by an adversary with large enough computing
capabilities. I would advise using instead a 2048-bit key generated from
the MODP group 14, by setting the tune.ssl.default-dh-param parameter to
2048, or even disabling DHE altogether if you are expecting every client
to support ECDHE key exchange. Note that increasing the
tune.ssl.default-dh-param will increase the CPU load on your server, and
may therefore increase the connection establishment latency.

If you cannot increase the DH key size above 1024-bit, please at least
generate a custom DH group with the openssl dhparam 2048 command, and
add the result of this command to your certificate file.

Best regards,

[0]: https://weakdh.org/
[1]: https://wiki.mozilla.org/Security/Server_Side_TLS
[2]: https://tools.ietf.org/html/rfc2409#section-6.2

-- 
Remi



signature.asc
Description: OpenPGP digital signature

Re: Custom SSL DHparams prime

[Remi Gacogne]

Hi,

 from what I've seen in the sources and documentation a default and
 pre-generated prime will be used as default (unless appended to the
 certificate). HAProxy uses the related functions provided by OpenSSL
 itself (get_rfc3526_prime_2048, ...).  What I miss here is an option to
 specify my own dhparams file to avoid using those pre-generated ones
 and/ore appending some to all certificates. Wouldn't it make sense to
 allow it to be read from a file, globally?

I don't think the 2048-bit MODP group 14 used by Haproxy is at risk
right now, still it can't hurt to use a large number of different groups.
You can use your own dhparam by appending it to the file specified with
the crt command, after your certificate chain and key.

-- 
Rémi




signature.asc
Description: OpenPGP digital signature

Re: Custom SSL DHparams prime

[Remi Gacogne]

 You can use your own dhparam by appending it to the file specified with
 the crt command, after your certificate chain and key.
 
 Well, I meant globally, as default.
 
 global
 tune.ssl.default-dh-param /path/to/custom/dhparams.pem

I don't think it's possible right now, but it should not be too hard to
add this feature.

 2048 was just an example. There is 1024 and IIRC 768 as well. One might
 be forced to use 1024.
 Also, according to the documentation HAProxy wouldn't allow/use anything
 greater than tune.ssl.default-dh-param which is 1024 by default - unless
 I misunderstood something.

If you add a custom group to your certificate file, it will override the
default-dh-param configuration. The default-dh-param has been added to
provide a default group when no custom one has been provided. Maybe the
documentation is not very clear on this point (sorry about that).

- No custom group in the certificate file, tune.ssl.default-dh-param not
specified or set to 1024 = The default Oakley group 2 (1024) is used
- No custom group in the certificate file, tune.ssl.default-dh-param is
set to 2048 = the 2048-bit MODP group 14 will be used, except if the
certificate has a RSA key smaller than 2048-bit, then Oakley group 2
(1024) is used
- No custom group in the certificate file, tune.ssl.default-dh-param is
set to 4096 = the 4096-bit MODP group 16 will be used, except if the
certificate has a RSA key smaller than 4096-bit but larger or equal to
2048, then 2048-bit MODP group 14 is used. If the key is smaller than
2048, then Oakley group 2 (1024) is used.
- No custom group in the certificate file, tune.ssl.default-dh-param is
set to 8192 = the 8192-bit MODP group 18 will be used, with the same
exceptions than previously.
- A custom group is set in the certificate file, no matter what the
tune.ssl.default-dh-param is set to, no matter the size of the RSA key
in your certificate, no matter the size of the custom group, this custom
group is used.


-- 
Rémi



signature.asc
Description: OpenPGP digital signature

Re: Re: Does HAproxy support sending ServerName TLS extension to backend servers?

[Remi Gacogne]

On 03/26/2015 08:34 AM, Baptiste wrote:

 HAProxy does not support SNI on backend yet.
 The biggest problem is not to send the SNI, the problem is what to send :)
 Do you send the Host header sent by the client, do you want to forge
 one, what happens if you do rewritting of the Host header, etc...
 So we could discuss the options here, then we'll be able to code
 something I guess...

Hi,

FWIW, Apache HTTPd's mod_proxy sends a server_name value equals to the
Host header contained in the request sent to the backend. It may be
the request header (possibly rewritten) if the ProxyPreserveHost option
is set to On, otherwise it's the backend's hostname.

Note that:
- rfc6066 section 3 explicitly forbids sending a literal IPv4 or IPv6
address, the server_name value must be a FQDN ;
- connection reuse is tricky when SNI is enabled, as the same connection
cannot be reused if the server_name value differs.






signature.asc
Description: OpenPGP digital signature

Re: [PATCH 0/2] Add support for TLS ticket keys configuration

[Remi Gacogne]

On 02/24/2015 04:42 PM, Nenad Merdanovic wrote:

 TLS_TICKETS_NO is a build time option, so you can set it to whatever
 you want.

Ok, fair enough.

 The idea which I discussed with Willy is to build an interface to be
 able to update the keys via the socket so we don't even have to 
 reload in the future.

That would be awesome!

Thanks again for this great feature!

Best regards,

-- 
Rémi



signature.asc
Description: OpenPGP digital signature

Re: RE: Making TLS go faster

[Remi Gacogne]

On 11/13/2014 05:36 PM, Lukas Tribus wrote:

 Be advisted that OCSP stapling is slowly dying , check [2] and
 [3].

I hope not. OCSP without stapling is dying, yes, but OCSP stapling along
with the X.509 Must Staple extension [1], and mode likely the X.509 TLS
feature extension [2], are a scalable way of solving a real problem.

[1] https://tools.ietf.org/html/draft-hallambaker-muststaple-00
[2] https://tools.ietf.org/html/draft-hallambaker-tlsfeature-05




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi everyone,

 Be that as it may, BoringSSL changed the internal structure because
 of a specific feature, so whether we use the API or access directly
 is irrelevant because the structure is different:

 https://boringssl.googlesource.com/boringssl/+/858a88daf27975f67d9f63e18f95645be2886bfb%5E!/


 Perhaps we should #ifdef the particular code in haproxy out when
 boringssl is used (and be silent or simply assume that DHE is used).
 
 I don't have a working BoringSSL build right now, but it seems that
 SSL_get_ciphers() still returns a valid STACK_OF(SSL_CIPHER) *, so I
 think the previous fix would still work. The new internal structure
 seems to be still using the same STACK_OF(SSL_CIPHER) *, with an
 additional field to handle ciphers groups.

I finally found the time to test with a proper boringssl build. This
minor patch cleans the way haproxy checks for enabled DHE ciphers at
configuration time, replacing a direct access to the cipher_list member
by a call to SSL_get_ciphers(), as suggested by Lukas Tribus.

It enables the check for enabled DHE ciphers when compiled against
boringssl, and may prevent issues later should the SSL_CTX struct be
modified in OpenSSL.



-- 
Rémi
From 22d7f5782a18555f8a19510bd776d47c4521f917 Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Fri, 10 Oct 2014 17:04:26 +0200
Subject: [PATCH] MINOR: ssl: use SSL_get_ciphers() instead of directly
 accessing the cipher list.

---
 src/ssl_sock.c | 36 ++--
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index c35d7ff..4e39c8b 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1478,9 +1478,8 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 		SSL_MODE_ENABLE_PARTIAL_WRITE |
 		SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER |
 		SSL_MODE_RELEASE_BUFFERS;
-#ifndef OPENSSL_IS_BORINGSSL
 	STACK_OF(SSL_CIPHER) * ciphers = NULL;
-	SSL_CIPHER * cipher = NULL;
+	SSL_CIPHER const * cipher = NULL;
 	char cipher_description[128];
 	/* The description of ciphers using an Ephemeral Diffie Hellman key exchange
 	   contains  Kx=DH  or  Kx=DH(. Beware of  Kx=DH/,
@@ -1489,10 +1488,7 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 	const char dhe_export_description[] =  Kx=DH(;
 	int idx = 0;
 	int dhe_found = 0;
-#else /* OPENSSL_IS_BORINGSSL */
-	/* assume dhe_found if boringssl is detected */
-	int dhe_found = 1;
-#endif
+	SSL *ssl = NULL;
 
 	/* Make sure openssl opens /dev/urandom before the chroot */
 	if (!ssl_initialize_random()) {
@@ -1585,22 +1581,26 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 	   no static DH params were in the certificate file. */
 	if (global.tune.ssl_default_dh_param == 0) {
 
-#ifndef OPENSSL_IS_BORINGSSL
-		ciphers = ctx-cipher_list;
-
-		if (ciphers) {
-			for (idx = 0; idx  sk_SSL_CIPHER_num(ciphers); idx++) {
-cipher = sk_SSL_CIPHER_value(ciphers, idx);
-if (SSL_CIPHER_description(cipher, cipher_description, sizeof (cipher_description)) == cipher_description) {
-	if (strstr(cipher_description, dhe_description) != NULL ||
-	strstr(cipher_description, dhe_export_description) != NULL) {
-		dhe_found = 1;
-		break;
+		ssl = SSL_new(ctx);
+
+		if (ssl) {
+			ciphers = SSL_get_ciphers(ssl);
+
+			if (ciphers) {
+for (idx = 0; idx  sk_SSL_CIPHER_num(ciphers); idx++) {
+	cipher = sk_SSL_CIPHER_value(ciphers, idx);
+	if (SSL_CIPHER_description(cipher, cipher_description, sizeof (cipher_description)) == cipher_description) {
+		if (strstr(cipher_description, dhe_description) != NULL ||
+		strstr(cipher_description, dhe_export_description) != NULL) {
+			dhe_found = 1;
+			break;
+		}
 	}
 }
 			}
+			SSL_free(ssl);
+			ssl = NULL;
 		}
-#endif /* OPENSSL_IS_BORINGSSL */
 
 		if (dhe_found) {
 			Warning(Setting tune.ssl.default-dh-param to 1024 by default, if your workload permits it you should set it to at least 2048. Please set a value = 1024 to make this warning disappear.\n);
-- 
2.1.2



signature.asc
Description: OpenPGP digital signature

Re: Segmentation fault with HTTP Basic Auth

[Remi Gacogne]

Hi,

 userlist L1
   # user foo insecure-password foo
   user foo password $apr1$Y/Oslz7K$EqwCC6SqzEn35VilLwh/V0

You can't use this kind of password-encryption format. It's only
supported by the Apache Runtime Framework. Please use an encrypted
password format supported by crypt(3).

 gdb trace:
 - https://gist.github.com/wnkz/e0ae0b0ae60080c41f04

It looks like the encrypted password is passed to crypt() as a salt, and
crypt() returns NULL (setting errno to EINVAL) because it does not
understand the format.
Unfortunately the returned value does not seem to be checked against
NULL and is passed to strcmp(), causing a NULL-pointer dereference.

We could probably replace:

#ifdef CONFIG_HAP_CRYPT
ep = crypt(pass, u-pass);
#else

with:

#ifdef CONFIG_HAP_CRYPT
ep = crypt(pass, u-pass);
if (!ep) {
return 0;
}
#else

Regards,

-- 
Rémi



signature.asc
Description: OpenPGP digital signature

Re: RE: [PATCH] MEDIUM: enable low latency polling on systems which support it

[Remi Gacogne]

Hi Ben, Lukas,

 We need someone to be able to test this, are you aware what nics currently
 support busy polling? What about loopback?

According to this Intel doc [1], it requires an Intel X520 or X540, with
the ixgbe driver.

 As for the docs, I think we should at least mention that:
 - only certain nic drivers support busy polling
 - it will increases power usage
 - while busy_read sysctl is overwritten by the SO_BUSY_POLL socket option,
   busy_poll must be set at sysctl level [1]

Do you happen to know if busy polling work with epoll() ? I stumbled
upon this patch [2] but it does not seem to be included in recent kernels.


[1]:
http://www.intel.fr/content/dam/www/public/us/en/documents/white-papers/open-source-kernel-enhancements-paper.pdf
[2]: https://lkml.org/lkml/2013/8/21/192

-- 
Rémi



signature.asc
Description: OpenPGP digital signature

Re: Re: haproxy segfault

[Remi Gacogne]

Hi,

 (gdb) bt
 #0  http_skip_chunk_crlf (msg=0x388ba50) at src/proto_http.c:2113
 #1  http_request_forward_body (s=s@entry=0x388b9a0, 
 req=req@entry=0x343ddf0, an_bit=an_bit@entry=8192) at src/proto_http.c:5398
 #2  0x00467298 in process_session (t=0x39eb8d0) at 
 src/session.c:1955
 #3  0x00411ff5 in process_runnable_tasks (next=0x7fff5584ca1c) at 
 src/task.c:238
 #4  0x0040a25c in run_poll_loop () at src/haproxy.c:1304
 #5  0x0040799f in main (argc=optimized out, argv=optimized out) 
 at src/haproxy.c:1638

Remind me of [1], do you have response larger than 16G ?
If so, maybe you could try the patch suggested by Willy in [2].

[1]: http://marc.info/?l=haproxym=140727447831648w=2
[2]: http://marc.info/?l=haproxym=140803767115539w=2


-- 
Rémi



signature.asc
Description: OpenPGP digital signature

Re: Re: ssl nbproc 1 and chrome

[Remi Gacogne]

Hi,

 From strace I can see that (16:01:58.776411 - 16:01:58.777349) that
 haproxy reads(from client(chrome) / sends GET /idp/Authn/Kerberos/Login
 with negotiate header(to tomcat) but then closes connection
 (16:01:58.777349, 16:01:58.777613 ...) ?

 16:01:58.776937 epoll_wait(0, {}, 200, 0) = 0
 16:01:58.777083 sendto(12, GET /idp/Authn/Kerberos/Login HTTP/1.1\r\nHost: 
 idp.uef.fi\r\nAuthorization: Negotiate negotheader removed..., 3737, 
 MSG_DONTWAIT|MSG_NOSIGNAL, NULL, 0) = 3737
 16:01:58.777349 epoll_wait(0, {{EPOLLIN|EPOLLRDHUP, {u32=12, u64=12}}}, 200, 
 59) = 1
 16:01:58.777613 recvfrom(12, , 8192, 0, NULL, NULL) = 0
 16:01:58.77 close(12)   = 0

recvfrom() returns 0, meaning that the other end has already closed the
connection:

If no messages are available to be received and the peer has performed
an orderly shutdown, recvfrom() shall return 0.

-- 
Rémi



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi Lukas,

 in src/ssl_sock.c:1582:11:
   ciphers = ctx-cipher_list;

 can we use the API instead of accessing cipher_list directly?
 With [1] perhaps?

Yes, you are right, we could replace this line with something like:

SSL * ssl = NULL;
...
ssl = SSL_new(ctx);
if (ssl) {
   ciphers = SSL_get_ciphers(ssl);
   ...
   SSL_free(ssl);
}

I don't like allocating a new SSL * just to browse the ciphers list, but
it's only done at configuration time and it's cleaner.

 Be that as it may, BoringSSL changed the internal structure because
 of a specific feature, so whether we use the API or access directly
 is irrelevant because the structure is different:
 
 https://boringssl.googlesource.com/boringssl/+/858a88daf27975f67d9f63e18f95645be2886bfb%5E!/
 
 
 Perhaps we should #ifdef the particular code in haproxy out when
 boringssl is used (and be silent or simply assume that DHE is used).

I don't have a working BoringSSL build right now, but it seems that
SSL_get_ciphers() still returns a valid STACK_OF(SSL_CIPHER) *, so I
think the previous fix would still work. The new internal structure
seems to be still using the same STACK_OF(SSL_CIPHER) *, with an
additional field to handle ciphers groups.

-- 
Rémi



signature.asc
Description: OpenPGP digital signature

[PATCH] Fix a memory leak in DHE key exchange

[Remi Gacogne]

Hi,

It was recently reported to the Apache HTTPd dev mailing list [1] that
the DH * value returned by the callback set with
SSL_CTX_set_tmp_dh_callback() is not freed by OpenSSL but must be
tracked by the application and freed when no longer needed, whenever
that is. Unfortunately this behavior is not clearly documented and the
example included in the documentation [2] doesn't do that.

As a result, the patch I submitted earlier in haproxy 1.5 leads haproxy
to leak memory for SSL/TLS connections using Diffie Hellman Ephemeral
key exchange, as the DH * struct holding the DH parameters is not freed
by OpenSSL.

This patch fixes the leak by allocating the DH * structs holding the DH
parameters once, at configuration time, as it has been done in Apache
HTTPd. I am not very satisfied by the fact that it uses 4 file-scope
variables, but I didn't see where else to store the DH parameters.
Willy, Emeric, do you have any comment?

I am sorry I didn't catch that earlier.

Regards,

[1] http://marc.info/?l=apache-httpd-devm=140031858223194w=2
[2] https://www.openssl.org/docs/ssl/SSL_CTX_set_tmp_dh_callback.html

-- 
Rémi


From 1470d3081fea6c6bd5b6103ef04e4fc78c8428ac Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Tue, 15 Jul 2014 11:36:40 +0200
Subject: [PATCH] Fix a memory leak in DHE key exchange

OpenSSL does not free the DH * value returned by the callback specified with SSL_CTX_set_tmp_dh_callback(),
leading to a memory leak for SSL/TLS connections using Diffie Hellman Ephemeral key exchange.
This patch fixes the leak by allocating the DH * structs holding the DH parameters once, at configuration time.
---
 src/ssl_sock.c | 43 ---
 1 file changed, 36 insertions(+), 7 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index 375225d..cf8adc7 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -105,6 +105,13 @@ enum {
 int sslconns = 0;
 int totalsslconns = 0;
 
+#ifndef OPENSSL_NO_DH
+static DH *local_dh_1024 = NULL;
+static DH *local_dh_2048 = NULL;
+static DH *local_dh_4096 = NULL;
+static DH *local_dh_8192 = NULL;
+#endif /* OPENSSL_NO_DH */
+
 #ifdef SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB
 struct certificate_ocsp {
 	struct ebmb_node key;
@@ -1034,16 +1041,16 @@ static DH *ssl_get_tmp_dh(SSL *ssl, int export, int keylen)
 	}
 
 	if (keylen = 8192) {
-		dh = ssl_get_dh_8192();
+		dh = local_dh_8192;
 	}
 	else if (keylen = 4096) {
-		dh = ssl_get_dh_4096();
+		dh = local_dh_4096;
 	}
 	else if (keylen = 2048) {
-		dh = ssl_get_dh_2048();
+		dh = local_dh_2048;
 	}
 	else {
-		dh = ssl_get_dh_1024();
+		dh = local_dh_1024;
 	}
 
 	return dh;
@@ -1079,11 +1086,11 @@ int ssl_sock_load_dh_params(SSL_CTX *ctx, const char *file)
 
 		if (global.tune.ssl_default_dh_param = 1024) {
 			/* we are limited to DH parameter of 1024 bits anyway */
-			dh = ssl_get_dh_1024();
-			if (dh == NULL)
+			local_dh_1024 = ssl_get_dh_1024();
+			if (local_dh_1024 == NULL)
 goto end;
 
-			SSL_CTX_set_tmp_dh(ctx, dh);
+			SSL_CTX_set_tmp_dh(ctx, local_dh_1024);
 		}
 		else {
 			SSL_CTX_set_tmp_dh_callback(ctx, ssl_get_tmp_dh);
@@ -1594,6 +1601,28 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 		global.tune.ssl_default_dh_param = 1024;
 	}
 
+#ifndef OPENSSL_NO_DH
+	if (global.tune.ssl_default_dh_param = 1024) {
+		if (local_dh_1024 == NULL) {
+			local_dh_1024 = ssl_get_dh_1024();
+		}
+		if (global.tune.ssl_default_dh_param = 2048) {
+			if (local_dh_2048 == NULL) {
+local_dh_2048 = ssl_get_dh_2048();
+			}
+			if (global.tune.ssl_default_dh_param = 4096) {
+if (local_dh_4096 == NULL) {
+	local_dh_4096 = ssl_get_dh_4096();
+}
+if (global.tune.ssl_default_dh_param = 8192 
+local_dh_8192 == NULL) {
+	local_dh_8192 = ssl_get_dh_8192();
+}
+			}
+		}
+	}
+#endif /* OPENSSL_NO_DH */
+
 	SSL_CTX_set_info_callback(ctx, ssl_sock_infocbk);
 #if OPENSSL_VERSION_NUMBER = 0x00907000L
 	SSL_CTX_set_msg_callback(ctx, ssl_sock_msgcbk);
-- 
2.0.1



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Fix a memory leak in DHE key exchange

[Remi Gacogne]

Hello Bryan,

On 07/15/2014 09:26 PM, Bryan Talbot wrote:
 Since the patch only concerns DH key parameters = 1024 bits, does that
 mean that exchanges using Elliptic Curve DH (which use much smaller key
 sizes than 1024 bits) are not affected by this issue?

ECDH and ECHDE key exchanges are indeed not affected by this specific
issue. However, for what it is worth, it's not because of the key size
but because DHE and ECDH(E) are handled separately.


Best regards,

-- 
Remi




signature.asc
Description: OpenPGP digital signature

Re: Re: Almost there...

[Remi Gacogne]

On 06/16/2014 03:19 PM, Julien Vehent wrote:
 
 On 2014-06-13 15:19, Willy Tarreau wrote:
 Emeric and Dirkjan at github are
 finishing a basic support for OCSP Stapling which we'd prefer to merge
 before the release in case we figure out that we need a smarter way to
 configure the CA files now.
 
 Did I read that right? Haproxy will have support for OCSP Stapling in 1.5?
 
 If so, this is exciting. I can't wait to test it!

Even if it's only in 1.6, that's very good news indeed!

Any chance to get a sneak peak at it ? :)

-- 
Rémi




signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi everyone,

After good remarks from Willy and Emeric, here is a new version of the
previous patch, including the following changes:

- tune.ssl.default-dh-param does not accept a value of less than 1024
anymore ;
- a comment explaining why we use the certificate key size and not the
keylen value supplied by OpenSSL in the EDH callback has been added ;
- we don't use OpenSSL's private constants but rather the cipher name to
determine if at least one cipher using an ephemeral diffie-hellman key
exchange is in use ;
- the warning indicating that tune.ssl-default-dh-param is not set could
have been displayed even if static DH parameters were supplied in the
certificate file. This has now been fixed.


Best regards,

-- 
Rémi Gacogne

From a4580ca5b34363b589ac1144bb1256cbdd001e67 Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Thu, 12 Jun 2014 14:58:40 +0200
Subject: [PATCH] Add the option to use standardized DH parameters = 1024 bits

When no static DH parameters are specified, this patch makes haproxy
use standardized (rfc 2409 / rfc 3526) DH parameters with prime lenghts
of 1024, 2048, 4096 or 8192 bits for DHE key exchange. The size of the
temporary/ephemeral DH key is computed as the minimum of the RSA/DSA server
key size and the value of a new option named tune.ssl.default-dh-param.
---
 doc/configuration.txt |  11 ++
 include/common/defaults.h |   5 +
 include/types/global.h|   1 +
 src/cfgparse.c|  13 ++
 src/haproxy.c |   1 +
 src/ssl_sock.c| 387 +-
 6 files changed, 380 insertions(+), 38 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index d28c91a..54ccaca 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -497,6 +497,7 @@ The following keywords are supported in the global section :
- tune.ssl.lifetime
- tune.ssl.force-private-cache
- tune.ssl.maxrecord
+   - tune.ssl.default-dh-param
- tune.zlib.memlevel
- tune.zlib.windowsize
 
@@ -1017,6 +1018,16 @@ tune.ssl.maxrecord number
   best value. Haproxy will automatically switch to this setting after an idle
   stream has been detected (see tune.idletimer above).
 
+tune.ssl.default-dh-param number
+  Sets the maximum size of the Diffie-Hellman parameters used for generating
+  the ephemeral/temporary Diffie-Hellman key in case of DHE key exchange. The
+  final size will try to match the size of the server's RSA (or DSA) key (e.g,
+  a 2048 bits temporary DH key for a 2048 bits RSA key), but will not exceed
+  this maximum value. Default value if 1024. Only 1024 or higher values are
+  allowed. Higher values will increase the CPU load, and values greater than
+  1024 bits are not supported by Java 7 and earlier clients. This value is not
+  used if static Diffie-Hellman parameters are supplied via the certificate file.
+
 tune.zlib.memlevel number
   Sets the memLevel parameter in zlib initialization for each session. It
   defines how much memory should be allocated for the internal compression
diff --git a/include/common/defaults.h b/include/common/defaults.h
index f765e90..bdd75cf 100644
--- a/include/common/defaults.h
+++ b/include/common/defaults.h
@@ -214,4 +214,9 @@
 #define SSLCACHESIZE 2
 #endif
 
+/* ssl max dh param size */
+#ifndef SSL_DEFAULT_DH_PARAM
+#define SSL_DEFAULT_DH_PARAM 0
+#endif
+
 #endif /* _COMMON_DEFAULTS_H */
diff --git a/include/types/global.h b/include/types/global.h
index f7942b3..23e3f3d 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -136,6 +136,7 @@ struct global {
 		int sslprivatecache; /* Force to use a private session cache even if nbproc  1 */
 		unsigned int ssllifetime;   /* SSL session lifetime in seconds */
 		unsigned int ssl_max_record; /* SSL max record size */
+		unsigned int ssl_default_dh_param; /* SSL maximum DH parameter size */
 #endif
 #ifdef USE_ZLIB
 		int zlibmemlevel;/* zlib memlevel */
diff --git a/src/cfgparse.c b/src/cfgparse.c
index a3edc32..06ea63e 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -633,6 +633,19 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
 		}
 		global.tune.ssl_max_record = atol(args[1]);
 	}
+	else if (!strcmp(args[0], tune.ssl.default-dh-param)) {
+		if (*(args[1]) == 0) {
+			Alert(parsing [%s:%d] : '%s' expects an integer argument.\n, file, linenum, args[0]);
+			err_code |= ERR_ALERT | ERR_FATAL;
+			goto out;
+		}
+		global.tune.ssl_default_dh_param = atol(args[1]);
+		if (global.tune.ssl_default_dh_param  1024) {
+			Alert(parsing [%s:%d] : '%s' expects a value = 1024.\n, file, linenum, args[0]);
+			err_code |= ERR_ALERT | ERR_FATAL;
+			goto out;
+		}
+	}
 #endif
 	else if (!strcmp(args[0], tune.bufsize)) {
 		if (*(args[1]) == 0) {
diff --git a/src/haproxy.c b/src/haproxy.c
index 6bfab06..c4442de 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -143,6 +143,7 @@ struct global global = {
 		.chksize = BUFSIZE,
 #ifdef

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

 That's really nice, I've just applied it with Emeric's approval.

Thanks Willy, but I just remembered that my patch walks directly into
what I spotted earlier, that in OpenSSL the name of ciphers using
ephemeral diffie-hellman for key exchange can start with EDH, but also
DHE, EXP-EDH or EXP1024-DHE.

Here is a patch to fix that, hopefully it will be the only issue
remaining :)


-- 
Rémi Gacogne


From 04b9e1b7f16c2783d30b04e04f3b0ac5198f Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Thu, 12 Jun 2014 16:47:46 +0200
Subject: [PATCH] Fix detection of ephemeral diffie-hellman key exchange.

In OpenSSL, the name of a cipher using ephemeral diffie-hellman for key
 exchange can start with EDH, but also DHE, EXP-EDH or EXP1024-DHE.
---
 src/ssl_sock.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index bfbb5b8..466a4f5 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1024,8 +1024,11 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 	SSL_CIPHER * cipher = NULL;
 	const char * cipher_name = NULL;
 	/* The name of ciphers using an Ephemeral Diffie Hellman key exchange
-	   starts with EDH. */
+	   starts with EDH, DHE, EXP-EDH or EXP1024-DHE. */
 	const char edh_name[] = EDH;
+	const char dhe_name[] = DHE;
+	const char export_edh_name[] = EXP-EDH;
+	const char export1024_dhe_name[] = EXP1024-DHE;
 	int idx = 0;
 	int dhe_found = 0;
 
@@ -1125,7 +1128,10 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 			for (idx = 0; idx  sk_SSL_CIPHER_num(ciphers); idx++) {
 cipher = sk_SSL_CIPHER_value(ciphers, idx);
 cipher_name = SSL_CIPHER_get_name(cipher);
-if (strncmp(cipher_name, edh_name, sizeof (edh_name)-1) == 0) {
+if (strncmp(cipher_name, edh_name, sizeof (edh_name)-1) == 0 ||
+strncmp(cipher_name, dhe_name, sizeof (dhe_name)-1) == 0 ||
+strncmp(cipher_name, export_edh_name, sizeof (export_edh_name)-1) == 0 ||
+strncmp(cipher_name, export1024_dhe_name, sizeof (export1024_dhe_name)-1) == 0) {
 	dhe_found = 1;
 	break;
 }
-- 
2.0.0



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

 I don't understand, that was precisely the intent of using
 SSL_cipher_description() which always returns Kx=DH in all circumstances.
 Is there any case you're aware where this does not work ? From what I saw
 in the code, it was a direct mapping of your test of the bit mask, so I'm
 a bit confused :-/

Yes, it's my fault. I decided not to use SSL_CIPHER_description()
because it returns a string composed of several values that is computed
for every call, and instead I used SSL_CIPHER_get_name(), which returns
a const hardcoded string. I totally forgot that we had previously
discussed the fact that the ciphers naming convention was, well, fuzzy.

Would you prefer that I submit a patch replacing the use of
SSL_CIPHER_get_name() by SSL_CIPHER_description(), in order to have a
single strstr() instead of possibly 4 strncmp() ?

-- 
Rémi Gacogne





signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

 Yes I think it's better exactly for the reason you reported (inconsistent
 naming over time). I'm having a hard time believing that Kx=DH has any
 reason to change as often as the internal bitfields or cipher names given
 that the output is even documented in the man page.

Well, I really hope you're right. This patch looks in the cipher's
description instead of its name. Sorry about the mess.


-- 
Rémi Gacogne


From e26407eb5dc2ae918e243408a9e2c66b726dd17e Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Thu, 12 Jun 2014 18:20:11 +0200
Subject: [PATCH] Fix detection of ephemeral diffie-hellman key exchange by
 using the cipher description.

In OpenSSL, the name of a cipher using ephemeral diffie-hellman for key
 exchange can start with EDH, but also DHE, EXP-EDH or EXP1024-DHE.
We work around this issue by using the cipher's description instead of
the cipher's name.
Hopefully the description is less likely to change in the future.
---
 src/ssl_sock.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index bfbb5b8..8fb8b5f 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -1022,10 +1022,12 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 		SSL_MODE_RELEASE_BUFFERS;
 	STACK_OF(SSL_CIPHER) * ciphers = NULL;
 	SSL_CIPHER * cipher = NULL;
-	const char * cipher_name = NULL;
-	/* The name of ciphers using an Ephemeral Diffie Hellman key exchange
-	   starts with EDH. */
-	const char edh_name[] = EDH;
+	char cipher_description[128];
+	/* The description of ciphers using an Ephemeral Diffie Hellman key exchange
+	   contains  Kx=DH  or  Kx=DH(. Beware of  Kx=DH/,
+	   which is not ephemeral DH. */
+	const char dhe_description[] =  Kx=DH ;
+	const char dhe_export_description[] =  Kx=DH(;
 	int idx = 0;
 	int dhe_found = 0;
 
@@ -1124,10 +1126,12 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 		if (ciphers) {
 			for (idx = 0; idx  sk_SSL_CIPHER_num(ciphers); idx++) {
 cipher = sk_SSL_CIPHER_value(ciphers, idx);
-cipher_name = SSL_CIPHER_get_name(cipher);
-if (strncmp(cipher_name, edh_name, sizeof (edh_name)-1) == 0) {
-	dhe_found = 1;
-	break;
+if (SSL_CIPHER_description(cipher, cipher_description, sizeof (cipher_description)) == cipher_description) {
+	if (strstr(cipher_description, dhe_description) != NULL ||
+	strstr(cipher_description, dhe_export_description) != NULL) {
+		dhe_found = 1;
+		break;
+	}
 }
 			}
 
-- 
2.0.0



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi,

 Well, maybe your latest patch is still the best way to go then. After all,
 it's very simple to see what can happen : if ciphers allow DHE and the
 admin has not configured its param, then there's a risk that some users
 will run it with too low a parameter and we'd rather warn them. So if
 the warning is emitted only when preparing an SSL context (and we don't
 need to emit it more than once), then only possibly affected users will
 get it, and those not using SSL or not using DHE will not see it.
 
 I'm just seeing SSL_CIPHER_description() which returns DH optionally
 followed by a size, maybe it would be a more portable way of checking the
 presence of DHE in your patch ?

Sorry, I didn't manage to find the time to look at this until now. You
are right, we could look for the Kx=DH string in
SSL_CIPHER_description() output, but I am not sure there is any
guarantee that it will stick, and I don't find it very elegant (one
internal snprintf() then one strcmp() for each ciphersuite).

Therefore I have integrated the latest proposal (iterating over enabled
ciphersuites and looking at the cipher internals) in this new patch.

I think it does what is expected from the user point of view, ie it only
warns if at least one DHE ciphersuite is enabled. Other than that, it
does not increase the CPU load with the default value, and still makes
it easy to increase the ephemeral DH strength.

From a developer point of view, it should add any new OpenSSL version
requirement, and the only issue I foresee is if OpenSSL alters the value
of SSL_kEDH. This is not the case in 1.0.2-beta1, so I don't expect it
will happen soon. Anyhow, it would only cause a build issue or an
unwanted warning message, so I don't think it matters much.

As usual, if anyone sees a better way or has any remarks, please get
back to me :)

Kind regards,

-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42
From 7a1ea32fb8376017901deb22a96aaa362278ff37 Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Mon, 2 Jun 2014 14:13:34 +0200
Subject: [PATCH] Add the option to use standardized DH parameters = 1024 bits

When no static DH parameters are specified, this patch makes haproxy
use standardized (rfc 2409 / rfc 3526) DH parameters with prime lenghts
of 1024, 2048, 4096 or 8192 bits for DHE key exchange. The size of the
temporary/ephemeral DH key is computed as the minimum of the RSA/DSA server
key size and the value of a new option named tune.ssl.default-dh-param.
---
 doc/configuration.txt |  11 ++
 include/common/defaults.h |   5 +
 include/types/global.h|   1 +
 src/cfgparse.c|   8 +
 src/haproxy.c |   1 +
 src/ssl_sock.c| 381 +-
 6 files changed, 369 insertions(+), 38 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index d7ba2f8..ef38044 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -497,6 +497,7 @@ The following keywords are supported in the global section :
- tune.ssl.lifetime
- tune.ssl.force-private-cache
- tune.ssl.maxrecord
+   - tune.ssl.default-dh-param
- tune.zlib.memlevel
- tune.zlib.windowsize
 
@@ -1017,6 +1018,16 @@ tune.ssl.maxrecord number
   best value. Haproxy will automatically switch to this setting after an idle
   stream has been detected (see tune.idletimer above).
 
+tune.ssl.default-dh-param number
+  Sets the maximum size of the Diffie-Hellman parameters used for generating
+  the ephemeral/temporary Diffie-Hellman key in case of DHE key exchange. The
+  final size will try to match the size of the server's RSA (or DSA) key (e.g,
+  a 2048 bits temporary DH key for a 2048 bits RSA key), but will not exceed
+  this maximum value. Default value if 1024. Higher values will increase the
+  CPU load, and values greater than 1024 bits are not supported by Java 7 and
+  earlier clients. This value is not used if static Diffie-Hellman parameters
+  are supplied via the certificate file.
+
 tune.zlib.memlevel number
   Sets the memLevel parameter in zlib initialization for each session. It
   defines how much memory should be allocated for the internal compression
diff --git a/include/common/defaults.h b/include/common/defaults.h
index f765e90..bdd75cf 100644
--- a/include/common/defaults.h
+++ b/include/common/defaults.h
@@ -214,4 +214,9 @@
 #define SSLCACHESIZE 2
 #endif
 
+/* ssl max dh param size */
+#ifndef SSL_DEFAULT_DH_PARAM
+#define SSL_DEFAULT_DH_PARAM 0
+#endif
+
 #endif /* _COMMON_DEFAULTS_H */
diff --git a/include/types/global.h b/include/types/global.h
index f7942b3..23e3f3d 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -136,6 +136,7 @@ struct global {
 		int sslprivatecache; /* Force to use a private session cache even if nbproc  1 */
 		unsigned

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi Willy, Bryan,

 On Mon, May 19, 2014 at 12:49:21PM -0700, Bryan Talbot wrote:
 It seems like the warning would be emitted in cases when DH exchange is
 disabled. ECDH is supported by nearly all new browsers and devices (that we
 care about anyway) and so have DH disabled and only ECDH enabled when PFS
 can be used -- specifically to avoid the large DH overhead especially for
 mobile devices.

 With the patch, it sounds like we would need to include a setting for
 default-dh-param even though it would never actually be used (or include
 a dh-param in our cert) to avoid a warning.

 Is it possible to only generate the dh-param and warnings if a cipher that
 needs it is enabled?
 
 I thought it was the case where the code was placed, but maybe I was
 wrong. Rémi, what do you think ?

Well, the warning won't show up if DHE support has been disabled at
compile time (using OPENSSL_NO_DH for example), but Bryan is right, it
will be displayed even if no DHE cipher suite has been enabled.

Right now, the warning is displayed in ssl_sock_load_dh_params(), which
is called whenever a certificate file is parsed. At that time, I don't
think we can determine whether some cipher suites using DHE are enabled
or not, because the ciphers keyword might not have been parsed yet.

We could probably move the warning to a later time, for example in
ssl_sock_prepare_ctx(), iterate on each enabled ciphers and only display
the warning if we find at least one DHE cipher enabled.
Something like this (not tested):

--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -724,6 +996,15 @@ int ssl_sock_load_cert_list_file(char *file, struct
bind_conf *bind_conf, struct
 #ifndef SSL_MODE_RELEASE_BUFFERS/* needs
OpenSSL = 1.0.0 */
 #define SSL_MODE_RELEASE_BUFFERS 0
 #endif
+
+#ifndef SSL_kEDH
+#if OPENSSL_VERSION_NUMBER  0x100fL
+#define SSL_kEDH 0x0010L
+#else
+#define SSL_kEDH 0x0008L
+#endif
+#endif
+
 int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx,
struct proxy *curproxy)
 {
int cfgerr = 0;
@@ -740,6 +1021,10 @@ int ssl_sock_prepare_ctx(struct bind_conf
*bind_conf, SSL_CTX *ctx, struct proxy
SSL_MODE_ENABLE_PARTIAL_WRITE |
SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER |
SSL_MODE_RELEASE_BUFFERS;
+   STACK_OF(SSL_CIPHER) * ciphers = NULL;
+   SSL_CIPHER * cipher = NULL;
+   int idx = 0;
+   int dhe_found = 0;

/* Make sure openssl opens /dev/urandom before the chroot */
if (!ssl_initialize_random()) {
@@ -828,6 +1113,26 @@ int ssl_sock_prepare_ctx(struct bind_conf
*bind_conf, SSL_CTX *ctx, struct proxy
cfgerr++;
}

+   if (global.tune.ssl_default_dh_param  1024) {
+   ciphers = ctx-cipher_list;
+
+   if (ciphers) {
+   for (idx = 0; idx  sk_SSL_CIPHER_num(ciphers);
idx++) {
+   cipher = sk_SSL_CIPHER_value(ciphers, idx);
+   if (cipher-algorithm_mkey  SSL_kEDH) {
+   dhe_found = 1;
+   break;
+   }
+   }
+   }
+
+   if (dhe_found) {
+   Warning(Setting tune.ssl.default-dh-param to
1024 by default, if your workload permits it you should set it to at
least 2048. Please set a value = 1024 to make this warning disappear.\n);
+   }
+
+   global.tune.ssl_default_dh_param = 1024;
+   }
+

But I think it's kind of ugly, especially because I can't find anything
in the OpenSSL API to check the key exchange algorithm used by a given
cipher suite and therefore have to directly access the SSL_CIPHER struct.

-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi Willy,

 I'd have applied a very simple change to your patch : I'd have initialized
 global.tune.ssl_max_dh_param to zero by default, and emitted a warning here :
 
 +   if (global.tune.ssl_max_dh_param = 1024) {
 +   /* we are limited to DH parameter of 1024 bits anyway 
 */
 +   Warning(Setting global.tune.ssl_max_dh_param to 1024 
 by default, if your workload permits it you should set it to at least 2048. 
 Please set a value = 1024 to make this warning disappear.);
 + global.tune.ssl_max_dh_param = 1024;
 +   dh = ssl_get_dh_1024();
 +   if (dh == NULL)
 +   goto end;
 
 What do you think ? That way it seems like only people really using the 
 default
 value will get the warning.

Yes, I think that's a good idea. You probably want to display this
warning only when global.tune.ssl_max_dh_param is less than 1024 though,
not equal :)

Regards,

-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42



signature.asc
Description: OpenPGP digital signature

RE: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi,


What happens if you also have DH appended to your certificates? You set
global.tune.ssl_max_dh_param to 1024 but you have a 4096bit DH in your
certificate file, which one is used then? An answer could be 'Don't do
that' :-) but I was curious.


The certificate's dh_param is used. To avoid this kind of confusion,
may I suggest that we call this something like ssl_max_dh_param_fallback,
since it is indeed a fallback and only used when openssl doesn't find
dh_params in the certificate?


I think you have a point here, tune.ssl.max-dh-fallback-size maybe?


This also means that we will see this warning when this setting is not
configured, but the certificate actually contains it.


That should not be the case, if I am not mistaken, as 
global.tune.ssl_max_dh_param is only checked when the certificate does not 
contain DH parameters.



Btw, in what condition are we checking and displaying this warning?
When haproxy is compiled with OpenSSL? When we actually use a certificate?


When a X.509 server certificate has been successfully loaded.


Regards,

--
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Creteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : +33 1 84 04 04 05

[PATCH] Minor fix: SSL_CTX_set_options() and SSL_CTX_set_mode() take a long, not, an int

[Remi Gacogne]

Hi,

This is a minor fix, but the SSL_CTX_set_options() and
SSL_CTX_set_mode() functions take a long, not an int parameter. As
SSL_OP_ALL is now (since OpenSSL 1.0.0) defined as 0x8BFFL, I think
it is worth fixing.

Best regards,

-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42
From 1d0a3584b181412e12e993f531b3fd3303fc4600 Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Mon, 19 May 2014 10:29:58 +0200
Subject: [PATCH] SSL_CTX_set_options() and SSL_CTX_set_mode() take a long, not
 an int

---
 src/ssl_sock.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index fd0b41f..880e727 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -728,7 +728,7 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 {
 	int cfgerr = 0;
 	int verify = SSL_VERIFY_NONE;
-	int ssloptions =
+	long ssloptions =
 		SSL_OP_ALL | /* all known workarounds for bugs */
 		SSL_OP_NO_SSLv2 |
 		SSL_OP_NO_COMPRESSION |
@@ -736,7 +736,7 @@ int ssl_sock_prepare_ctx(struct bind_conf *bind_conf, SSL_CTX *ctx, struct proxy
 		SSL_OP_SINGLE_ECDH_USE |
 		SSL_OP_NO_SESSION_RESUMPTION_ON_RENEGOTIATION |
 		SSL_OP_CIPHER_SERVER_PREFERENCE;
-	int sslmode =
+	long sslmode =
 		SSL_MODE_ENABLE_PARTIAL_WRITE |
 		SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER |
 		SSL_MODE_RELEASE_BUFFERS;
@@ -995,11 +995,11 @@ static int ssl_sock_srv_verifycbk(int ok, X509_STORE_CTX *ctx)
 int ssl_sock_prepare_srv_ctx(struct server *srv, struct proxy *curproxy)
 {
 	int cfgerr = 0;
-	int options =
+	long options =
 		SSL_OP_ALL | /* all known workarounds for bugs */
 		SSL_OP_NO_SSLv2 |
 		SSL_OP_NO_COMPRESSION;
-	int mode =
+	long mode =
 		SSL_MODE_ENABLE_PARTIAL_WRITE |
 		SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER |
 		SSL_MODE_RELEASE_BUFFERS;
-- 
1.9.3



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

 Ah yes, you're right. But anyway the purpose was to explain the principle.
 Do you want to update your patch and resubmit then ?

Yes, of course, thank you. What do you think about Lukas idea to change
the name of the setting, in order to clarify the fact that it only
applies as a fallback when there is no DH parameters in the certificate
file?



-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42



signature.asc
Description: OpenPGP digital signature

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

 Yes, of course, thank you. What do you think about Lukas idea to change
 the name of the setting, in order to clarify the fact that it only
 applies as a fallback when there is no DH parameters in the certificate
 file?
 
 I think it's a good idea, it could avoid some confusion in the future.
 Maybe we don't need the max word in it ? Thus it could be :
 
 tune.ssl.dh-param-fallback
 
 Or maybe default instead of fallback since we're using this terminology
 in the rest of the product ? What about default-dh-param ?

Ok, I think default-dh-param will be fine. Here is a new version of the
previous patch, using the new name and adding the warning you suggested.

Thank you, Sander, Lukas and Willy!


Best regards,

-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42
From 9f10e9c7ba111467a21f493fc22eeda9831ea724 Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Mon, 19 May 2014 15:45:10 +0200
Subject: [PATCH] Add the option to use standardized DH parameters = 1024 bits

When no static DH parameters are specified, this patch makes haproxy
use standardized (rfc 2409 / rfc 3526) DH parameters with prime lenghts
of 1024, 2048, 4096 or 8192 bits for DHE key exchange. The size of the
temporary/ephemeral DH key is computed as the minimum of the RSA/DSA server
key size and the value of a new option named tune.ssl.default-dh-param.
---
 doc/configuration.txt |  11 ++
 include/common/defaults.h |   5 +
 include/types/global.h|   1 +
 src/cfgparse.c|   8 ++
 src/haproxy.c |   1 +
 src/ssl_sock.c| 351 +-
 6 files changed, 340 insertions(+), 37 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index a5ec732..661a2b2 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -497,6 +497,7 @@ The following keywords are supported in the global section :
- tune.ssl.lifetime
- tune.ssl.force-private-cache
- tune.ssl.maxrecord
+   - tune.ssl.default-dh-param
- tune.zlib.memlevel
- tune.zlib.windowsize
 
@@ -1017,6 +1018,16 @@ tune.ssl.maxrecord number
   best value. Haproxy will automatically switch to this setting after an idle
   stream has been detected (see tune.idletimer above).
 
+tune.ssl.default-dh-param number
+  Sets the maximum size of the Diffie-Hellman parameters used for generating
+  the ephemeral/temporary Diffie-Hellman key in case of DHE key exchange. The
+  final size will try to match the size of the server's RSA (or DSA) key (e.g,
+  a 2048 bits temporary DH key for a 2048 bits RSA key), but will not exceed
+  this maximum value. Default value if 1024. Higher values will increase the
+  CPU load, and values greater than 1024 bits are not supported by Java 7 and
+  earlier clients. This value is not used if static Diffie-Hellman parameters
+  are supplied via the certificate file.
+
 tune.zlib.memlevel number
   Sets the memLevel parameter in zlib initialization for each session. It
   defines how much memory should be allocated for the internal compression
diff --git a/include/common/defaults.h b/include/common/defaults.h
index f765e90..bdd75cf 100644
--- a/include/common/defaults.h
+++ b/include/common/defaults.h
@@ -214,4 +214,9 @@
 #define SSLCACHESIZE 2
 #endif
 
+/* ssl max dh param size */
+#ifndef SSL_DEFAULT_DH_PARAM
+#define SSL_DEFAULT_DH_PARAM 0
+#endif
+
 #endif /* _COMMON_DEFAULTS_H */
diff --git a/include/types/global.h b/include/types/global.h
index c945f53..82daee9 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -132,6 +132,7 @@ struct global {
 		int sslprivatecache; /* Force to use a private session cache even if nbproc  1 */
 		unsigned int ssllifetime;   /* SSL session lifetime in seconds */
 		unsigned int ssl_max_record; /* SSL max record size */
+		unsigned int ssl_default_dh_param; /* SSL maximum DH parameter size */
 #endif
 #ifdef USE_ZLIB
 		int zlibmemlevel;/* zlib memlevel */
diff --git a/src/cfgparse.c b/src/cfgparse.c
index 5edb773..b85189a 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -633,6 +633,14 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
 		}
 		global.tune.ssl_max_record = atol(args[1]);
 	}
+	else if (!strcmp(args[0], tune.ssl.default-dh-param)) {
+		if (*(args[1]) == 0) {
+			Alert(parsing [%s:%d] : '%s' expects an integer argument.\n, file, linenum, args[0]);
+			err_code |= ERR_ALERT | ERR_FATAL;
+			goto out;
+		}
+		global.tune.ssl_default_dh_param = atol(args[1]);
+	}
 #endif
 	else if (!strcmp(args[0], tune.bufsize)) {
 		if (*(args[1]) == 0) {
diff --git a/src/haproxy.c b/src/haproxy.c
index 060a155..fbdedbc 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -143,6 +143,7 @@ struct global global = {
 		.chksize = BUFSIZE,
 #ifdef USE_OPENSSL
 		.sslcachesize

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi,

On 05/05/2014 12:06 PM, Sander Klein wrote:

 I've added a 2048bit dhparam to my most used certificates and I don't
 see a big jump in resource usage.
 
 This was not a big scientific test, I just added the DH params in my
 production and looked if the haproxy process started eating more CPU. As
 far as I can tell CPU usage went up just a couple percent. Not a very
 big deal.
 
 So, to me using 2048bit doesn't seem like a problem. And. I can
 always switch to nbproc  1 ;-)

Thank you Sander for taking the time to do this test! I am still not
sure it is a good idea to move a default of 2048 bits though.

Here is a new version of the previous patch that should not require
OpenSSL 0.9.8a to build, but instead includes the needed primes from
rfc2409 and rfc3526 if OpenSSL does not provide them. I have to admit I
don't have access to an host with an old enough OpenSSL to test it
correctly. It still defaults to use 1024 bits DHE parameters in order
not to break anything.

Willy, do you have any thoughts about this patch or any other way to
simplify the use of stronger DHE parameters in haproxy 1.5? I know it
can already be done by generating static DH parameters, but I am afraid
most administrators may find it too complicated and therefore not dare
to test it.

Best regards,

-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42
From b616a26f62e7a7eacd30d1ab1ff5a04539146539 Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Mon, 12 May 2014 18:07:15 +0200
Subject: [PATCH] Add the option to use standardized DH parameters = 1024 bits

When no static DH parameters are specified, this patch makes haproxy
use standardized (rfc 2409 / rfc 3526) DH parameters with prime lenghts
of 1024, 2048, 4096 or 8192 bits for DHE key exchange. The size of the
temporary/ephemeral DH key is computed as the minimum of the RSA/DSA server
key size and the value of a new option named tune.ssl.max-dh-param-size.
---
 doc/configuration.txt |  11 ++
 include/common/defaults.h |   5 +
 include/types/global.h|   1 +
 src/cfgparse.c|   8 ++
 src/haproxy.c |   1 +
 src/ssl_sock.c| 348 +-
 6 files changed, 336 insertions(+), 38 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 6da8b3e..7406dea 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -497,6 +497,7 @@ The following keywords are supported in the global section :
- tune.ssl.lifetime
- tune.ssl.force-private-cache
- tune.ssl.maxrecord
+   - tune.ssl.max-dh-param-size
- tune.zlib.memlevel
- tune.zlib.windowsize
 
@@ -1017,6 +1018,16 @@ tune.ssl.maxrecord number
   best value. Haproxy will automatically switch to this setting after an idle
   stream has been detected (see tune.idletimer above).
 
+tune.ssl.max-dh-param-size number
+  Sets the maximum size of the Diffie-Hellman parameters used for generating
+  the ephemeral/temporary Diffie-Hellman key in case of DHE key exchange. The
+  final size will try to match the size of the server's RSA (or DSA) key (e.g,
+  a 2048 bits temporary DH key for a 2048 bits RSA key), but will not exceed
+  this maximum value. Default value if 1024. Higher values will increase the
+  CPU load, and values greater than 1024 bits are not supported by Java 7 and
+  earlier clients. This value is not used if static Diffie-Hellman parameters
+  are supplied via the certificate file.
+
 tune.zlib.memlevel number
   Sets the memLevel parameter in zlib initialization for each session. It
   defines how much memory should be allocated for the internal compression
diff --git a/include/common/defaults.h b/include/common/defaults.h
index f765e90..944f3aa 100644
--- a/include/common/defaults.h
+++ b/include/common/defaults.h
@@ -214,4 +214,9 @@
 #define SSLCACHESIZE 2
 #endif
 
+/* ssl max dh param size */
+#ifndef SSL_MAX_DH_PARAM
+#define SSL_MAX_DH_PARAM 1024
+#endif
+
 #endif /* _COMMON_DEFAULTS_H */
diff --git a/include/types/global.h b/include/types/global.h
index c945f53..8470640 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -132,6 +132,7 @@ struct global {
 		int sslprivatecache; /* Force to use a private session cache even if nbproc  1 */
 		unsigned int ssllifetime;   /* SSL session lifetime in seconds */
 		unsigned int ssl_max_record; /* SSL max record size */
+		unsigned int ssl_max_dh_param; /* SSL maximum DH parameter size */
 #endif
 #ifdef USE_ZLIB
 		int zlibmemlevel;/* zlib memlevel */
diff --git a/src/cfgparse.c b/src/cfgparse.c
index decdcfd..d2e0191 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -633,6 +633,14 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
 		}
 		global.tune.ssl_max_record = atol(args[1]);
 	}
+	else if (!strcmp(args[0

[PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi,

This is an updated version of the previous patch. This version adds a
new configuration option named tune.ssl.max-dh-param-size, which sets
the maximum size of the ephemeral DH key used for DHE key exchange, if
no static DH parameters are found in the certificate file.

The default value for max-dh-param-size is set to 1024, thus keeping
the current behavior by default. Setting a higher value (for example
2048 with a 2048 bits RSA/DSA server key) allows an easy upgrade
to stronger ephemeral DH keys (and back if needed).

Regarding the recent discussions on this matter, I agree with the fact
that ECDHE should be preferred over DHE whenever it is available, but
I think we may want to keep offering a decent forward secrecy option to
clients not supporting elliptic curves yet, for example older versions
of Android or clients using OpenSSL 0.9.8.

This patch is a proposal based on the feedback I got from the previous
one, please feel free to criticize anything from the core idea (an easy
way to use stronger DHE key size) to the new parameter's name, I will
gladly welcome any remarks :)


Regards,

-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42
From f03b547984c513855383b11dc76aaecbbbc65838 Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Fri, 2 May 2014 15:41:13 +0200
Subject: [PATCH] Add a configurable support of standardized DH parameters =
 1024 bits, disabled by default

When no static DH parameters are specified, this patch makes haproxy
use standardized (rfc 2409 / rfc 3526) DH parameters with prime lenghts
of 1024, 2048, 4096 and 8192 bits for DHE key exchange. The size of the
temporary/ephemeral DH key is computed as the minimum of the RSA/DSA server
key size and the value of a new option named tune.ssl.max-dh-param-size.
---
 doc/configuration.txt |  11 
 include/common/defaults.h |   5 ++
 include/types/global.h|   1 +
 src/cfgparse.c|   8 +++
 src/haproxy.c |   1 +
 src/ssl_sock.c| 154 ++
 6 files changed, 142 insertions(+), 38 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 8207067..1c9e4e6 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -496,6 +496,7 @@ The following keywords are supported in the global section :
- tune.ssl.cachesize
- tune.ssl.lifetime
- tune.ssl.maxrecord
+   - tune.ssl.max-dh-param-size
- tune.zlib.memlevel
- tune.zlib.windowsize
 
@@ -1006,6 +1007,16 @@ tune.ssl.maxrecord number
   best value. Haproxy will automatically switch to this setting after an idle
   stream has been detected (see tune.idletimer above).
 
+tune.ssl.max-dh-param-size number
+ Sets the maximum size of the Diffie-Hellman parameters used for generating
+ the ephemeral/temporary Diffie-Hellman key in case of DHE key exchange. The
+ final size will try to match the size of the server's RSA (or DSA) key (e.g,
+ a 2048 bits temporary DH key for a 2048 bits RSA key), but will not exceed
+ this maximum value. Default value if 1024. Higher values will increase the
+ CPU load, and values greater than 1024 bits are not supported by Java 7 and
+ earlier clients. This value is not used if static Diffie-Hellman parameters
+ are supplied via the certificate file.
+
 tune.zlib.memlevel number
   Sets the memLevel parameter in zlib initialization for each session. It
   defines how much memory should be allocated for the internal compression
diff --git a/include/common/defaults.h b/include/common/defaults.h
index f765e90..944f3aa 100644
--- a/include/common/defaults.h
+++ b/include/common/defaults.h
@@ -214,4 +214,9 @@
 #define SSLCACHESIZE 2
 #endif
 
+/* ssl max dh param size */
+#ifndef SSL_MAX_DH_PARAM
+#define SSL_MAX_DH_PARAM 1024
+#endif
+
 #endif /* _COMMON_DEFAULTS_H */
diff --git a/include/types/global.h b/include/types/global.h
index 241afe9..2fba7ca 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -131,6 +131,7 @@ struct global {
 		int sslcachesize;  /* SSL cache size in session, defaults to 2 */
 		unsigned int ssllifetime;   /* SSL session lifetime in seconds */
 		unsigned int ssl_max_record; /* SSL max record size */
+		unsigned int ssl_max_dh_param; /* SSL maximum DH parameter size */
 #endif
 #ifdef USE_ZLIB
 		int zlibmemlevel;/* zlib memlevel */
diff --git a/src/cfgparse.c b/src/cfgparse.c
index c4f092f..9a976ba 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -630,6 +630,14 @@ int cfg_parse_global(const char *file, int linenum, char **args, int kwm)
 		}
 		global.tune.ssl_max_record = atol(args[1]);
 	}
+	else if (!strcmp(args[0], tune.ssl.max-dh-param-size)) {
+		if (*(args[1]) == 0) {
+			Alert(parsing [%s:%d] : '%s' expects an integer argument.\n, file, linenum, args[0]);
+			err_code |= ERR_ALERT

Re: [PATCH] Add a configurable support of standardized DH parameters = 1024 bits, disabled by default

[Remi Gacogne]

Hi Lukas,

 The default value for max-dh-param-size is set to 1024, thus keeping
 the current behavior by default. Setting a higher value (for example
 2048 with a 2048 bits RSA/DSA server key) allows an easy upgrade
 to stronger ephemeral DH keys (and back if needed).
 
 
 Please note that Sander used 4096bit - which is why he saw huge CPE load.
 
 Imho we can default max-dh-param-size to 2048bit.

I am afraid upgrading DH key size from 1024 bits to 2048 bits can divide
performance by 2 for CPU-bound installations doing mostly DHE key
exchanges, based on some quick benchmarks I ran. Of course it depends on
the ratio of new SSL/TLS connections using DHE (without resumption) you
get, but I think it may too big of an impact to change the default
without warnings.

-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42



signature.asc
Description: OpenPGP digital signature

Re: CPU increase between ss-20140329 and ss-20140425

[Remi Gacogne]

Since previously, the DH param was always 1024 bits and usually, key
sizes are 2048, I think that using a 2048 bits DH param adds a
performance impact but I never evaluated it since using a 1024 DHE param
is not unusual (or was not unusual, I am far to be up-to-date on
this). The impact should have been minimal since g is kept to 2 and this
is the important one (DHE is about g^a mod p where g and p are the DH
parameters) but we are nonetheless dealing with far more larger integers
and I suppose that the exponentiation has a performance hit when p gets
larger.

The strength of RSA and DH are equivalent so using the same size
for both is definitively a good practice.


OK thanks for explaining. Can't this be linked to the size of the keys
or anything else ? I'm asking because we can hardly accept to divide
the performance by 4 for users with no solution for them to work around
the issue. We know what they'll do : revert and stay on the previous
version which worked well for them.


In fact, I said that the performance impact should have been low, but
this is false. g is kept to 2, but the a in g^a mod p (a is the secret
number chosen by the server for the DH exchange) is a random number
between 0 and p - 1, so there is a big performance impact to use a 2048
bit DH parameter vs a 1024.


I agree with your explanation and the significative performance impact, 
though I did not expect that users would be CPU-bound with a 2048 bits DH 
parameter, as most of the requests are typically reusing an existing TLS 
session (via TLS session ID or TLS tickets), therefore skipping a new key 
exchange.


I think that we have two options:

- leave the situation as it is now, and let users concerned with security 
use a static 2048 bits (or larger) static DH parameter in the certificate 
file ;
- recommit the patch I submitted as it is, and let users concerned with 
the CPU impact use static DH parameter in the certificate file.


I am in favor of the second option, because I think that the level of 
security expected when using a 2048 bits RSA is a 2048 DH key exchange 
(see rfc3766). Of course this means that users upgrading from a previous 
version may see a significant increase in CPU usage, which I agree is bad.


I should probably send a new patch though, because I am afraid that using 
a DH parameter larger than 2048 bits (which was the case with the previous 
patch for RSA keys larger than 2048) is too much CPU consuming. I wonder 
why mod_ssl users does not seem to complain?


--
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Creteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : +33 1 84 04 04 05

Re: CPU increase between ss-20140329 and ss-20140425

[Remi Gacogne]

With haproxy 1.5-dev23 and no DH parameters in the cert file :
$ ab -n1000 -c100 -Z ECDHE-RSA-AES256-GCM-SHA384,2048,256 https://127.0.0.1/

Requests per second:427.94 [#/sec] (mean)
Time per request:   233.679 [ms] (mean)

[...]

The same test with 1024 bits DH parameters in the cert file :
$ ab -n1000 -c100 -Z DHE-RSA-AES256-GCM-SHA384,2048,256 https://127.0.0.1/

Requests per second:290.67 [#/sec] (mean)
Time per request:   344.027 [ms] (mean)


That's a bit strange, are you using the same 1024 bits DH parameters 
in the cert file that the ones that are hardcoded in 1.5-dev22 and 
1.5-dev24? Because then I would have expected the same results.


I tried to reproduce your tests with siege by comparing dev-22 without DH 
parameters in the cert file and -dev23 with those:


-BEGIN DH PARAMETERS-
MIGHAoGBAJJAJDXDoS5E03MNjnjK36eOL1tRqVa/9NuOVlI+lpXmPjJQbP65EvKn
fSLnG7VMhoCJO4KtG88zf393ltP7loGB2bofcDSr+x+XsxBM8yA/Zj6BmQt+CQ9s
TF7hoOV+wXTT6ErZ5y5qx9pq6hLfKXwTGFT78hrE6HnCO7xgtPdTAgEC
-END DH PARAMETERS-

I get roughly the same CPS (forcing haproxy to provide only DHE key 
exchange with the ciphers keyword) for both versions, which seems logical.


--
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Creteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : +33 1 84 04 04 05

Re: CPU increase between ss-20140329 and ss-20140425

[Remi Gacogne]

That's a bit strange, are you using the same 1024 bits DH parameters in
the cert file that the ones that are hardcoded in 1.5-dev22 and
1.5-dev24? Because then I would have expected the same results.


In the same conditions I get the same results, But look a the ciphers.
One test is for ECDHE, the other one is for DHE.


Oh, I am very sorry. I didn't expect to see ECDHE here, so I totally 
missed it. Thank you!


--
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Creteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : +33 1 84 04 04 05

RE: CPU increase between ss-20140329 and ss-20140425

[Remi Gacogne]

I wonder why mod_ssl users does not seem to complain?


A TLS handshake blocks the event-loop on haproxy or nginx. It also blocks
on Apache, but since there is no event-loop but a lot of threads instead,
the TLS handshake does not impact that much.


Makes sense, thanks.

--
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Creteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : +33 1 84 04 04 05

Re: CPU increase between ss-20140329 and ss-20140425

[Remi Gacogne]

Hi,


I've done a search and it breaks between 20140413 and 20140415.


Are you using static DH parameters? If not, could you try using some? 
Until 20140415 the default was (1024 bits DH parameters):


-BEGIN DH PARAMETERS- 
MIGHAoGBAJJAJDXDoS5E03MNjnjK36eOL1tRqVa/9NuOVlI+lpXmPjJQbP65EvKn

fSLnG7VMhoCJO4KtG88zf393ltP7loGB2bofcDSr+x+XsxBM8yA/Zj6BmQt+CQ9s
TF7hoOV+wXTT6ErZ5y5qx9pq6hLfKXwTGFT78hrE6HnCO7xgtPdTAgEC
-END DH PARAMETERS-

--
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Creteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : +33 1 84 04 04 05

[PATCH] Add standardized DH parameters = 1024 bits

[Remi Gacogne]

Hi,

This patch adds standardized (rfc 2409 / rfc 3526) DH parameters with
prime lengths of 1024, 2048, 3072, 4096, 6144 and 8192 bits, based on
the private key size. As of now, static DH parameters of 1024 bits are
used when no custom DH parameters are provided in the cert file,
effectively reducing the strength of the key exchange to 1024 bits even
when the private key is stronger than that.

The recent CVE-2014-0160 vulnerability (aka heartbleed) and the
massive key regeneration that followed caused a lot of people to upgrade
to 2048 or even 4096 bits RSA keys, so I believe it would be a good time
to upgrade DH parameters altogether.

This patch is based on what Apache HTTPd's mod_ssl does since 2.4.7:

https://httpd.apache.org/docs/2.4/mod/mod_ssl.html#sslcertificatefile.

Please be aware that this patch may cause an issue with some SSL/TLS
clients, mostly Java 7 or earlier, that do not support primes larger
than 1024 bits. mod_ssl advises using custom DH parameters of 1024 bits
for servers encountering this kind of issues:

https://httpd.apache.org/docs/2.4/ssl/ssl_faq.html#javadh


Please feel free to get back to me for any question or remark.


Best regards,

-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42
From 6142ec6b30a0cff26b3e5ea1aaf90ad535c4789a Mon Sep 17 00:00:00 2001
From: Remi Gacogne rgacogne[at]aquaray[dot]fr
Date: Mon, 14 Apr 2014 16:47:31 +0200
Subject: [PATCH] Add standardized DH parameters = 1024 bits

This patch adds standardized (rfc 2409 / rfc 3526)
DH parameters with prime lengths of 1024, 2048, 3072, 4096, 6144 and
8192 bits, based on the private key size.
---
 src/ssl_sock.c | 184 -
 1 file changed, 143 insertions(+), 41 deletions(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index a11aed2..f7c50c5 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -270,36 +270,149 @@ static int ssl_sock_switchctx_cbk(SSL *ssl, int *al, struct bind_conf *s)
 #endif /* SSL_CTRL_SET_TLSEXT_HOSTNAME */
 
 #ifndef OPENSSL_NO_DH
+
+static DH *ssl_get_dh_1024(void)
+{
+	DH *dh = DH_new();
+	if (dh) {
+		dh-p = get_rfc2409_prime_1024(NULL);
+		/* See RFC 2409, Section 6 Oakley Groups
+		   for the reason why we use 2 as a generator.
+		*/
+		BN_dec2bn(dh-g, 2);
+		if (!dh-p || !dh-g) {
+			DH_free(dh);
+			dh = NULL;
+		}
+	}
+	return dh;
+}
+
+static DH *ssl_get_dh_2048(void)
+{
+	DH *dh = DH_new();
+	if (dh) {
+		dh-p = get_rfc3526_prime_2048(NULL);
+		/* See RFC 3526, Section 3 2048-bit MODP Group
+		   for the reason why we use 2 as a generator.
+		 */
+		BN_dec2bn(dh-g, 2);
+		if (!dh-p || !dh-g) {
+			DH_free(dh);
+			dh = NULL;
+		}
+	}
+	return dh;
+}
+
+static DH *ssl_get_dh_3072(void)
+{
+	DH *dh = DH_new();
+	if (dh) {
+		dh-p = get_rfc3526_prime_3072(NULL);
+		/* See RFC 3526, Section 4 3072-bit MODP Group
+		   for the reason why we use 2 as a generator.
+		 */
+		BN_dec2bn(dh-g, 2);
+		if (!dh-p || !dh-g) {
+			DH_free(dh);
+			dh = NULL;
+		}
+	}
+	return dh;
+}
+
+static DH *ssl_get_dh_4096(void)
+{
+	DH *dh = DH_new();
+	if (dh) {
+		dh-p = get_rfc3526_prime_4096(NULL);
+		/* See RFC 3526, Section 5 4096-bit MODP Group
+		   for the reason why we use 2 as a generator.
+		 */
+		BN_dec2bn(dh-g, 2);
+		if (!dh-p || !dh-g) {
+			DH_free(dh);
+			dh = NULL;
+		}
+	}
+	return dh;
+}
+
+static DH *ssl_get_dh_6144(void)
+{
+	DH *dh = DH_new();
+	if (dh) {
+		dh-p = get_rfc3526_prime_6144(NULL);
+		/* See RFC 3526, Section 6 6144-bit MODP Group
+		   for the reason why we use 2 as a generator.
+		 */
+		BN_dec2bn(dh-g, 2);
+		if (!dh-p || !dh-g) {
+			DH_free(dh);
+			dh = NULL;
+		}
+	}
+	return dh;
+}
+
+static DH *ssl_get_dh_8192(void)
+{
+	DH *dh = DH_new();
+	if (dh) {
+		dh-p = get_rfc3526_prime_8192(NULL);
+		/* See RFC 3526, Section 7 8192-bit MODP Group
+		   for the reason why we use 2 as a generator.
+		 */
+		BN_dec2bn(dh-g, 2);
+		if (!dh-p || !dh-g) {
+			DH_free(dh);
+			dh = NULL;
+		}
+	}
+	return dh;
+}
+
+/* Returns Diffie-Hellman parameters matching the private key length */
+static DH *ssl_get_tmp_dh(SSL *ssl, int export, int keylen)
+{
+	DH *dh = NULL;
+	EVP_PKEY *pkey = SSL_get_privatekey(ssl);
+	int type = pkey ? EVP_PKEY_type(pkey-type) : EVP_PKEY_NONE;
+
+	if (type == EVP_PKEY_RSA || type == EVP_PKEY_DSA) {
+		keylen = EVP_PKEY_bits(pkey);
+	}
+
+	if (keylen = 8192) {
+		dh = ssl_get_dh_8192();
+	}
+	else if (keylen = 6144) {
+		dh = ssl_get_dh_6144();
+	}
+	else if (keylen = 4096) {
+		dh = ssl_get_dh_4096();
+	}
+	else if (keylen = 3072) {
+		dh = ssl_get_dh_3072();
+	}
+	else if (keylen = 2048) {
+		dh = ssl_get_dh_2048();
+	}
+	else {
+		dh = ssl_get_dh_1024();
+	}
+
+	return dh;
+}
+
 /* Loads Diffie-Hellman parameter from a file. Returns 1 if loaded, else -1
if an error occured, and 0 if parameter not found. */
-int

Re: [PATCH] Add standardized DH parameters = 1024 bits

[Remi Gacogne]

Hello Willy,

Nice speak at FRnOG 22 :)

 This patch adds standardized (rfc 2409 / rfc 3526) DH parameters with
 prime lengths of 1024, 2048, 3072, 4096, 6144 and 8192 bits, based on
 the private key size. As of now, static DH parameters of 1024 bits are
 used when no custom DH parameters are provided in the cert file,
 effectively reducing the strength of the key exchange to 1024 bits even
 when the private key is stronger than that.
 
 (...)
 
 Great, thank you. I'm just having a question, since I'm seeing a number
 of openssl functions involved, have you tried them with multiple versions
 to ensure that we don't need to add some extra #ifdefs in order not to
 break build on older libs ? Please at least check on openssl-0.9.8 (I
 think it's in RHEL5).

Yes, all functions exists in at least 0.9.8a. The most recently added
ones are get_rfc_prime_(), which have been present since 0.9.8a
(released 11 Oct 2005).

If you think it may be an issue, I will gladly add the missing #ifdefs,
but as even Debian 5 and RHEL 5 have an OpenSSL = 0.9.8a, I am not sure
it is needed.


-- 
Rémi Gacogne

Aqua Ray
SAS au capital de 105.720 Euros
RCS Créteil 447 997 099
www.aquaray.fr

14, rue Jules Vanzuppe
94854 IVRY-SUR-SEINE CEDEX (France)
Tel : (+33) (0)1 84 04 04 05
Fax : (+33) (0)1 77 65 60 42



signature.asc
Description: OpenPGP digital signature