On Apr 15, 2016 4:24 AM, "Janusz Dziemidowicz" <rrapt...@nails.eu.org> wrote: > > 2016-04-14 17:39 GMT+02:00 David Martin <dmart...@gmail.com>: > > Here's a revised patch, it throws a fatal config error if > > SSL_CTX_set1_curves_list() fails. The default echde option is used so > > current configurations should not be impacted. > > > > Sorry Janusz, forgot the list on my reply. > > I believe that now it is wrong as SSL_CTX_set_ecdh_auto works > differently than this code implies. > From what I was able to tell from OpenSSL code (always a pleasure > looking at) it works as follows: > - SSL_CTX_set_ecdh_auto turns on negotiation of curves, without this > no curves will be negotiated (and only one configured curve will be > used, "the old way") > - the list of curves that are considered during negotiation contain > all of the OpenSSL supported curves > - unless you also call SSL_CTX_set1_curves_list() and narrow it down > to the list you prefer > > Right now you patch either calls SSL_CTX_set_ecdh_auto or > SSL_CTX_set1_curves_list, but not both. Unless I'm mistaken, this > kinda is not how it is supposed to be used. > Have you tested behavior of the server with any command line client?
I have tested the current patch with the HAProxy default, a list of curves, a single curve and also an incorrect curve. All seem to behave correctly. The conditional should only skip calling ecdh_auto() if curves_list() returns 0 in which case HAProxy exits anyway. Maybe I'm missing something obvious, this has been a learning experience for me. > > I believe this should be something like: > #if new OpenSSL > SSL_CTX_set_ecdh_auto(... 1) > SSL_CTX_set1_curves_list() with user supplied ecdhe or > ECDHE_DEFAULT_CURVE by default > #elif ... > SSL_CTX_set_tmp_ecdh() with user supplied ecdhe or > ECDHE_DEFAULT_CURVE by default > #endif > > This way haproxy behaves exactly the same with default configuration > and any version of OpenSSL. User can configure multiple curves if > there is sufficiently new OpenSSL. > > Changes to the documentation would also be nice in the patch :) > > -- > Janusz Dziemidowicz Just to be clear I have no intention of running anything other than prime256v1 on my systems nor do I think anyone else should. I had x25519 in mind when looking over the code. Perhaps something like this should wait until haproxy is ready for openssl 1.1.0 as it has little value without an alternative curve. Love the project by the way, thanks for all the awesome work.
