Opened 3 months ago

Closed 5 weeks ago

Last modified 4 weeks ago

#1654 closed defect (fixed)

nginx does not honor ssl_protocols for TLSv1.3

Reported by: zizazit@… Owned by:
Priority: minor Milestone:
Component: nginx-core Version: 1.14.x
Keywords: Cc:
uname -a: Linux 4.14.72-1-lts #1 SMP Wed Sep 26 12:31:03 CEST 2018 x86_64 GNU/Linux
nginx -V: nginx version: nginx/1.14.0 built with OpenSSL 1.1.0h 27 Mar 2018 (running with OpenSSL 1.1.1 11 Sep 2018) TLS SNI support enabled configure arguments: --prefix=/etc/nginx --conf-path=/etc/nginx/nginx.conf --sbin-path=/usr/bin/nginx --pid-path=/run/nginx.pid --lock-path=/run/lock/nginx.lock --user=http --group=http --http-log-path=/var/log/nginx/access.log --error-log-path=stderr --http-client-body-temp-path=/var/lib/nginx/client-body --http-proxy-temp-path=/var/lib/nginx/proxy --http-fastcgi-temp-path=/var/lib/nginx/fastcgi --http-scgi-temp-path=/var/lib/nginx/scgi --http-uwsgi-temp-path=/var/lib/nginx/uwsgi --with-cc-opt='-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong -fno-plt -D_FORTIFY_SOURCE=2' --with-ld-opt=-Wl,-O1,--sort-common,--as-needed,-z,relro,-z,now --with-compat --with-debug --with-file-aio --with-http_addition_module --with-http_auth_request_module --with-http_dav_module --with-http_degradation_module --with-http_flv_module --with-http_geoip_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_mp4_module --with-http_realip_module --with-http_secure_link_module --with-http_slice_module --with-http_ssl_module --with-http_stub_status_module --with-http_sub_module --with-http_v2_module --with-mail --with-mail_ssl_module --with-pcre-jit --with-stream --with-stream_geoip_module --with-stream_realip_module --with-stream_ssl_module --with-stream_ssl_preread_module --with-threads

Description

When configured with ssl_protocols TLSv1 TLSv1.1 TLSv1.2; nginx should not support TLSv1.3 but it still does

curl -v --tls-max 1.3 https://example
[...]
* SSL connection using TLSv1.3 / TLS_AES_256_GCM_SHA384
* ALPN, server accepted to use http/1.1

Change History (10)

comment:1 Changed 3 months ago by zizazit@…

Oops... I didn't notice the OpenSSL version that nginx is compiled with does not support TLSv1.3, but the version its running with does. Please close the ticket, sorry.

comment:2 Changed 3 months ago by mdounin

This can be solved by explicitly calling SSL_CTX_set_max_proto_version(TLS1_2_VERSION) if TLSv1.3 is not available, so nginx will be limited to TLSv1.2 when it was compiled with TLSv1.2 as the highest protocol version but is being run with TLSv1.3 in the library. Here is a patch:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1539615549 -10800
#      Mon Oct 15 17:59:09 2018 +0300
# Node ID 5f998d26af64ae048a12d65d078ea152877c9f35
# Parent  64721be763b61bf9075e9f4851a46197df9bac51
SSL: explicitly set max version (ticket #1654).

With maximum version explicitly set, TLSv1.3 will not be unexpectedly
enabled if nginx compiled with OpenSSL 1.1.0 (without TLSv1.3 support)
will be run with OpenSSL 1.1.1 (with TLSv1.3 support).

diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -345,6 +345,11 @@ ngx_ssl_create(ngx_ssl_t *ssl, ngx_uint_
     }
 #endif
 
+#ifdef SSL_CTX_set_min_proto_version
+    SSL_CTX_set_min_proto_version(ssl->ctx, 0);
+    SSL_CTX_set_max_proto_version(ssl->ctx, TLS1_2_VERSION);
+#endif
+
 #ifdef TLS1_3_VERSION
     SSL_CTX_set_min_proto_version(ssl->ctx, 0);
     SSL_CTX_set_max_proto_version(ssl->ctx, TLS1_3_VERSION);

It is not clear if it worth the effort though.

comment:3 Changed 3 months ago by Maxim Dounin <mdounin@…>

In 7372:ed8738b1c7c4/nginx:

SSL: explicitly set maximum version (ticket #1654).

With maximum version explicitly set, TLSv1.3 will not be unexpectedly
enabled if nginx compiled with OpenSSL 1.1.0 (without TLSv1.3 support)
will be run with OpenSSL 1.1.1 (with TLSv1.3 support).

comment:4 Changed 3 months ago by mdounin

  • Resolution set to fixed
  • Status changed from new to closed

Fix committed.

comment:5 Changed 2 months ago by greenreaper@…

FWIW, this seems like more of a feature than a bug if you're running on a platform like Debian stretch which you've upgraded to OpenSSL 1.1.1. Now it seems you can't enable TLS 1.3 in nginx without recompiling it as well. Maybe a cosmic build (#1661) will work, as Ubuntu pkg tend to work on Debian.

Confusingly, adding TLSv1.3 to ssl_protocols gives no warning; it just doesn't work as expected. This compares to ssl_early_data for which there is such a warning. Maybe it should override the minimum?

comment:6 Changed 2 months ago by mdounin

FWIW, this seems like more of a feature than a bug if you're running on a platform like Debian stretch which you've upgraded to OpenSSL 1.1.1.

Certainly this is a bug, especially given that we do not enable TLS 1.3 by default now. Also note that unexpectedly enabling TLS 1.3 might result in problems, see here for an example.

Confusingly, adding TLSv1.3 to ssl_protocols gives no warning; it just doesn't work as expected.

This is mostly because we used to enable by default protocols which are not present in every OpenSSL library available out there, and issuing warnings only for explicitly specified yet unsupported protocols is really cumbersome.

comment:7 Changed 7 weeks ago by Maxim Dounin <mdounin@…>

In 7421:11be3c0723bd/nginx:

SSL: explicitly set maximum version (ticket #1654).

With maximum version explicitly set, TLSv1.3 will not be unexpectedly
enabled if nginx compiled with OpenSSL 1.1.0 (without TLSv1.3 support)
will be run with OpenSSL 1.1.1 (with TLSv1.3 support).

comment:8 follow-up: Changed 5 weeks ago by yura3d@…

  • Resolution fixed deleted
  • Status changed from closed to reopened

I think the solution suggested by @mdounin above is strange.
I have nginx-1.15.7 installed from the nginx.org repository for Debian stretch. Also I have OpenSSL 1.1.1, built with TLS 1.3 support.
nginx -V:

nginx version: nginx/1.15.7
built by gcc 6.3.0 20170516 (Debian 6.3.0-18+deb9u1)
built with OpenSSL 1.1.0f  25 May 2017 (running with OpenSSL 1.1.1  11 Sep 2018)
TLS SNI support enabled

The problem: TLS 1.3 is not available even if I add TLSv1.3 to ssl_protocols.
When I rollback from 1.15.7 to 1.15.5 (the last version before this solution was included to nginx), TLS 1.3 makes available again.

I think you should not rely on TLS1_3_VERSION constant in code if you declare support for external ("running") OpenSSL builts. Also I don't understand, why the packages for the mainline version are still made with OpenSSL 1.1.0f, while stable 1.1.1 was ready 3 months ago.

Last edited 5 weeks ago by yura3d@… (previous) (diff)

comment:9 Changed 5 weeks ago by mdounin

  • Resolution set to fixed
  • Status changed from reopened to closed

We cannot selectively enable or disable TLSv1.3 without relying on the appropriate constant. As such, you have to rebuilt nginx with OpenSSL 1.1.1 for TLSv1.3 to be available.

Since Debian stretch is shipped with OpenSSL 1.1.0f, this is the version our packages for Debian stretch are built with. For OpenSSL different OpenSSL versions, consider rebuilding nginx yourself.

comment:10 in reply to: ↑ 8 Changed 4 weeks ago by GreenReaper@…

Replying to yura3d@…:

I think the solution suggested by @mdounin above is strange.
I have nginx-1.15.7 installed from the nginx.org repository for Debian stretch. Also I have OpenSSL 1.1.1, built with TLS 1.3 support.

You could also switch to the repo for Ubuntu cosmic instead, which is built for OpenSSL 1.1.1.
This is the same thing you could do to get new TCP features on jessie.

This won't work for i386 because cosmic isn't built for that anymore.

Incidentally another way to do this without having to rebuild OpenSSL is to pin libc6 libc-dev-bin libc-bin locales libc6-dev libc-l10n libssl1.1 libssl-dev openssl to Debian sid. Obviously risky, YMMV.

Note: See TracTickets for help on using tickets.