[PATCH] SSL: support automatic selection of ECDH temporary key parameters

[Maxim Dounin]

Hello!

On Thu, Jan 30, 2014 at 03:23:01PM -0800, Piotr Sikora wrote:

> Hello,
> slightly better patch attached.
> 
> Hopefully, now that ALPN (OpenSSL-1.0.2+ feature) is in, this can be
> also committed :)
> 
> Best regards,
> Piotr Sikora
> 
> 
> # HG changeset patch
> # User Piotr Sikora <piotr at cloudflare.com>
> # Date 1391123946 28800
> #      Thu Jan 30 15:19:06 2014 -0800
> # Node ID 63c549cc2e817772979238f055b72a4f96198a2a
> # Parent  2e40188f83ef5bf1ae5afe0dd445689049f46a5c
> SSL: support automatic selection of ECDH temporary key parameters.
> 
> When compiled against OpenSSL-1.0.2+, the colon separated list of
> supported curves can be provided using either curve NIDs:
> 
>     ssl_ecdh_curve  secp521r1:secp384r1:prime256v1;
> 
> or names:
> 
>     ssl_ecdh_curve  P-521:P-384:P-256;
> 
> Signed-off-by: Piotr Sikora <piotr at cloudflare.com>
> 
> diff -r 2e40188f83ef -r 63c549cc2e81 src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c Thu Jan 30 19:13:12 2014 +0400
> +++ b/src/event/ngx_event_openssl.c Thu Jan 30 15:19:06 2014 -0800
> @@ -681,6 +681,23 @@ ngx_ssl_ecdh_curve(ngx_conf_t *cf, ngx_s
>  {
>  #if OPENSSL_VERSION_NUMBER >= 0x0090800fL
>  #ifndef OPENSSL_NO_ECDH
> +#ifdef SSL_CTRL_SET_ECDH_AUTO
> +
> +    if (SSL_CTX_set1_curves_list(ssl->ctx, name->data) == 0) {
> +        ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> +                      "Unknown curve in \"%s\"", name->data);
> +        return NGX_ERROR;
> +    }
> +
> +    if (SSL_CTX_set_ecdh_auto(ssl->ctx, 1) == 0) {
> +        ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
> +                      "Unable to set automatic curve selection for \"%s\"",
> +                      name->data);
> +        return NGX_ERROR;
> +    }
> +
> +#else
> +
>      int      nid;
>      EC_KEY  *ecdh;
> 
> @@ -710,6 +727,8 @@ ngx_ssl_ecdh_curve(ngx_conf_t *cf, ngx_s
>      SSL_CTX_set_tmp_ecdh(ssl->ctx, ecdh);
> 
>      EC_KEY_free(ecdh);
> +
> +#endif
>  #endif
>  #endif

Sorry for late reponse, but I finally managed to get some time to 
look into the SSL_CTX_set_ecdh_auto() / SSL_CTX_set1_curves_list() 
support in yet-to-be-released OpenSSL 1.0.2.

Comments about the patch below, in no particular order:

- Suggested code doesn't seem to allow to use the default list of 
  curves, as normally available with just a call to
  SSL_CTX_set_ecdh_auto(); this seems to be what OpenSSL 
  recommends to use by default, and we may want to follow.

- Error messages in the ngx_ssl_ecdh_curve() are way off from 
  what's normally used in ngx_event_openssl.c, and probably 
  it's not a good idea to use similar messages in the new code.

- If nginx was compiled with OpenSSL 1.0.2, but used with an 
  older version, things will not work at all; this is not something 
  completely unacceptable, but it's something we may want to 
  avoid.

- SSL_CTX_set_options(SSL_OP_SINGLE_ECDH_USE) is not used 
  with OpenSSL 1.0.2, and this looks just wrong.

-- 
Maxim Dounin
http://nginx.org/