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

[Piotr Sikora]

Hey Maxim,

> 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.

But isn't that the way nginx usually interacts with OpenSSL? i.e.
always calling SSL_CTX_set_cipher_list(), even with the default
"DEFAULT" value?

Also, right now nginx uses NIST P-256 curve. Calling
SSL_CTX_set_ecdh_auto() without limiting list of supported curves
enables all of them, with preference for the strongest ones, which
means that all modern browsers will start using NIST P-521 and clients
compiled against OpenSSL will start using NIST B-571.

This results in rather significant performance loss:

                              op      op/s
 256 bit ecdh (nistp256)   0.0002s   4589.7
 521 bit ecdh (nistp521)   0.0006s   1551.9
 571 bit ecdh (nistb571)   0.0030s    330.9

and I don't think that it's a good idea to silently change used curve
for users using default configuration.

> - 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.

Right, my bad.

> - 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.

Will look into it.

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

Well, it just looks wrong ;)

If you dig into OpenSSL's code, you'll notice that this option is
checked only in 3 places: when setting SSL_CTX_set_tmp_ecdh() /
SSL_set_tmp_ecdh() to see if we need to generate "persistent"
temporary key and during ServerKeyExchange phase to see if we need to
generate ephemeral temporary key. This check consists of 3 conditions
and first two are fulfilled via SSL_CTX_set_ecdh_auto(), so
SSL_OP_SINGLE_ECDH_USE is never check when using it.

Basically,

    SSL_CTX_set_ecdh_auto(ssl->ctx, 1);

is equivalent to:

    SSL_CTX_set_options(ssl->ctx, SSL_OP_SINGLE_ECDH_USE);
    SSL_CTX_set_tmp_ecdh(ssl->ctx, ecdh);

Having said that, I don't mind re-adding it, if it's going to make you happier.

Best regards,
Piotr Sikora