On Tue, Nov 29, 2016 at 07:02:51PM +0100, Kurt Roeckx wrote:
On Tue, Nov 29, 2016 at 12:56:39PM +0200, Christos Trochalakis wrote:
Hello Piotr,
I am not really familiar with EC, and before digging deeper I am CCing
Kurt, one of the OpenSSL maintainers, who can shed some light into the
issue.
On Mon, Nov 28, 2016 at 12:38:37PM +0100, Piotr Engelking wrote:
> Package: nginx-light
> Version: 1.10.2-2
> Severity: normal
> Tags: security
>
> Using:
>
> ssl_ecdh_curve X25519;
>
> in /etc/nginx/sites-available/<host> results in nginx refusing to start with
> the following error:
>
> Unable to create curve "X25519" (SSL: error:100AE081:elliptic curve
> routines:EC_GROUP _new_by_curve_name:unknown group)
>
> Using:
>
> ssl_ecdh_curve x25519;
>
> results in nginx refusing to start with the following error:
>
> Unknown curve name "x25519" (SSL:)
>
> The bug is probably caused by nginx not accounting for OpenSSL using a
> different API for x25519 and for other elliptic curves.
>
> In absence of specific choice, nginx uses the default OpenSSL elliptic curve
> list, which as of OpenSSL 1.1.0c includes the secp256r1, secp384r1, and
> secp521r1 curves, known to be possibly backdoored.
The default list of 1.1.0 is:
static const unsigned char eccurves_default[] = {
0, 29, /* X25519 (29) */
0, 23, /* secp256r1 (23) */
0, 25, /* secp521r1 (25) */
0, 24, /* secp384r1 (24) */
};
As far as I know, the interfaces to set that are:
SSL(_CTX)_set_tmp_ecdh
SSL(_CTX)_set1_groups
SSL(_CTX)_set1_groups_list
Only the last one supports strings, so if it's using that openssl
should support it. Otherwise nginx needs to do it's own
translation.
The EC_GROUP_new_by_curve_name() takes an NID, which should be
NID_X25519.
EC_GROUP_new_by_curve_name() no longer supports NID_X25519, see:
https://mta.openssl.org/pipermail/openssl-dev/2016-October/008582.html
and https://github.com/openssl/openssl/commit/bc7bfb83b7
This is fixed in nginx mainline (1.11.x) by using
`SSL_CTX_set1_curves_list()` with a string. The three commits (two of
them are style fixes) can be easily backported [0]. I am not sure though
if we should include such a patch for stretch, Kurt what do you think?
I noticed that EC_GROUP_new_by_curve_name() is also used by other
packages, like haproxy, where X25519 curve is also not supported.
[0] See the ec-x25519 branch in our packaging repo:
https://anonscm.debian.org/cgit/collab-maint/nginx.git/log/?h=ec-x25519
