On Thu, Apr 14, 2016 at 02:22:36PM +0200, Janusz Dziemidowicz wrote: > 2016-04-14 12:05 GMT+02:00 Willy Tarreau <w...@1wt.eu>: > > Hi David, > > > > On Wed, Apr 13, 2016 at 03:19:45PM -0500, David Martin wrote: > >> This is my first attempt at a patch, I'd love to get some feedback on this. > >> > >> Adds support for SSL_CTX_set_ecdh_auto which is available in OpenSSL 1.0.2. > > > >> From 05bee3e95e5969294998fb9e2794ef65ce5a6c1f Mon Sep 17 00:00:00 2001 > >> From: David Martin <dmart...@gmail.com> > >> Date: Wed, 13 Apr 2016 15:09:35 -0500 > >> Subject: [PATCH] use SSL_CTX_set_ecdh_auto() for ecdh curve selection > >> > >> Use SSL_CTX_set_ecdh_auto if the OpenSSL version supports it, this > >> allows the server to negotiate ECDH curves much like it does ciphers. > >> Prefered curves can be specified using the existing ecdhe bind options > >> (ecdhe secp384r1:prime256v1) > > > > Could it have a performance impact ? I mean, may this allow a client to > > force the server to use curves that imply harder computations for example ? > > I'm asking because some people got seriously hit by the move from dhparm > > 1024 to 2048, so if this can come with a performance impact we possibly want > > to let the user configure it. > > Switching ECDHE curves can have performance impact, for example result > of openssl speed on my laptop: > 256 bit ecdh (nistp256) 0.0003s 2935.3 > 384 bit ecdh (nistp384) 0.0027s 364.9 > 521 bit ecdh (nistp521) 0.0016s 623.2 > The difference is so high for nistp256 because OpenSSL has heavily > optimized implementation > (https://www.imperialviolet.org/2010/12/04/ecc.html).
Wow, and despite this you want to let the client force the server to switch to 384 ? Looks like a hue DoS to me. > Apart from calling SSL_CTX_set_ecdh_auto() this patch also takes into > account user supplied curve list, so users can customize this as > needed (currently haproxy only allows to select one curve, which is a > limitation of older OpenSSL versions). OK. > However, this patch reuses bind option 'ecdhe'. Currently it is > documented to accept only one curve. I believe it should be at least > updated to state that multiple curves can be used with sufficiently > new OpenSSL. That makes sense. > Also, I'm not sure what will happen when SSL_CTX_set1_curves_list() is > called with NULL (no ecdhe bind option). Even if it is accepted by > OpenSSL it will silently change haproxy default, before this patch it > was only prime256v1 (as defined in ECDHE_DEFAULT_CURVE), afterward it > will default to all curves supported by OpenSSL. Probably the best > would be to keep current default, so it all works consistently in > default configuration, regardless of version of haproxy and OpenSSL. Agreed, we don't want it to change in the back of users. I'm not opposed to adding more tuning options but we have to ensure that users will remain safe after upgrading and will not be exposed to nistp384 without explicitly asking for it. Regards, Willy
