Bug#828453: nginx: FTBFS with openssl 1.1.0
On Fri, Nov 11, 2016 at 12:46:49PM +0100, Moritz Muehlenhoff wrote: B0;115;0cOn Fri, Nov 04, 2016 at 10:03:02AM +0200, Christos Trochalakis wrote: On Wed, Nov 02, 2016 at 05:22:21PM +0100, Kurt Roeckx wrote: >On Wed, Nov 02, 2016 at 10:39:29AM +0100, Moritz Muehlenhoff wrote: >> >>The issue hasn't been diagnosed upstream, but this will likely also affect nginx >>once rebuilt against openssl 1.1. > >It seems it was fixed in OpenSSL in the mean time. > I have pushed all the needed changes to master. I believe that we should wait for the bug to be fixed in Debian before uploading. Kurt, do you plan to backport that change or wait for the next OpenSSL release? That patch is part of the openssl 1.1.0c release which was uploaded yesterday. Cheers, Moritz It is uploaded for sid but it hit the NEW queue (we introduced a new binary package). https://ftp-master.debian.org/new/nginx_1.10.2-2.html
Bug#828453: nginx: FTBFS with openssl 1.1.0
B0;115;0cOn Fri, Nov 04, 2016 at 10:03:02AM +0200, Christos Trochalakis wrote: > On Wed, Nov 02, 2016 at 05:22:21PM +0100, Kurt Roeckx wrote: > >On Wed, Nov 02, 2016 at 10:39:29AM +0100, Moritz Muehlenhoff wrote: > >> > >>The issue hasn't been diagnosed upstream, but this will likely also affect > >>nginx > >>once rebuilt against openssl 1.1. > > > >It seems it was fixed in OpenSSL in the mean time. > > > > I have pushed all the needed changes to master. > > I believe that we should wait for the bug to be fixed in Debian before > uploading. Kurt, do you plan to backport that change or wait for the > next OpenSSL release? That patch is part of the openssl 1.1.0c release which was uploaded yesterday. Cheers, Moritz
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Fri, Nov 04, 2016 at 10:03:02AM +0200, Christos Trochalakis wrote: > On Wed, Nov 02, 2016 at 05:22:21PM +0100, Kurt Roeckx wrote: > > On Wed, Nov 02, 2016 at 10:39:29AM +0100, Moritz Muehlenhoff wrote: > > > > > > The issue hasn't been diagnosed upstream, but this will likely also > > > affect nginx > > > once rebuilt against openssl 1.1. > > > > It seems it was fixed in OpenSSL in the mean time. > > > > I have pushed all the needed changes to master. > > I believe that we should wait for the bug to be fixed in Debian before > uploading. Kurt, do you plan to backport that change or wait for the > next OpenSSL release? I will make an upload that will have that fixed soon. Kurt
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Wed, Nov 02, 2016 at 05:22:21PM +0100, Kurt Roeckx wrote: On Wed, Nov 02, 2016 at 10:39:29AM +0100, Moritz Muehlenhoff wrote: The issue hasn't been diagnosed upstream, but this will likely also affect nginx once rebuilt against openssl 1.1. It seems it was fixed in OpenSSL in the mean time. I have pushed all the needed changes to master. I believe that we should wait for the bug to be fixed in Debian before uploading. Kurt, do you plan to backport that change or wait for the next OpenSSL release?
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Wed, Nov 02, 2016 at 05:22:21PM +0100, Kurt Roeckx wrote: > On Wed, Nov 02, 2016 at 10:39:29AM +0100, Moritz Muehlenhoff wrote: > > > > The issue hasn't been diagnosed upstream, but this will likely also affect > > nginx > > once rebuilt against openssl 1.1. > > It seems it was fixed in OpenSSL in the mean time. Ack, fixed with https://github.com/openssl/openssl/commit/a7faa6da317887e14e8e28254a83555983ed6ca7 Cheers, Moritz
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Wed, Nov 02, 2016 at 10:39:29AM +0100, Moritz Muehlenhoff wrote: > > The issue hasn't been diagnosed upstream, but this will likely also affect > nginx > once rebuilt against openssl 1.1. It seems it was fixed in OpenSSL in the mean time. Kurt
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Sat, Oct 29, 2016 at 12:53:54PM +0200, Kurt Roeckx wrote: > On Sat, Oct 29, 2016 at 12:34:51PM +0300, Christos Trochalakis wrote: > > On Sat, Oct 29, 2016 at 11:29:12AM +0200, Kurt Roeckx wrote: > > > On Sat, Oct 29, 2016 at 11:04:33AM +0300, Christos Trochalakis wrote: > > > > > > > > I am not sure if the first lua patch is safe (regarding the > > > > "ssl_conn->tlsext_status_expected = 1;" removal). > > > > > > I'm not sure which patch you're talking about. I remember > > > something about something doing weird things with the state > > > machine for renegiotation that they never should have done, is > > > that that? > > > > > > > > > Kurt > > > > > > > I am talking about (src/ngx_http_lua_ssl_ocsp.c): > > https://github.com/openresty/lua-nginx-module/pull/761/files#diff-50267b7dd63c740bc5c1d29c7387e789L493 > > So I already commented on that before it seems, but I added a new > comment saying I think it's correct. FTR, the Wikimedia Foundation started to use nginx 1.11 with openssl 1.1 and ran into this problem: https://github.com/openssl/openssl/issues/1799 The issue hasn't been diagnosed upstream, but this will likely also affect nginx once rebuilt against openssl 1.1. Cheers, Moritz
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Sat, Oct 29, 2016 at 12:34:51PM +0300, Christos Trochalakis wrote: > On Sat, Oct 29, 2016 at 11:29:12AM +0200, Kurt Roeckx wrote: > > On Sat, Oct 29, 2016 at 11:04:33AM +0300, Christos Trochalakis wrote: > > > > > > I am not sure if the first lua patch is safe (regarding the > > > "ssl_conn->tlsext_status_expected = 1;" removal). > > > > I'm not sure which patch you're talking about. I remember > > something about something doing weird things with the state > > machine for renegiotation that they never should have done, is > > that that? > > > > > > Kurt > > > > I am talking about (src/ngx_http_lua_ssl_ocsp.c): > https://github.com/openresty/lua-nginx-module/pull/761/files#diff-50267b7dd63c740bc5c1d29c7387e789L493 So I already commented on that before it seems, but I added a new comment saying I think it's correct. Kurt
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Sat, Oct 29, 2016 at 11:29:12AM +0200, Kurt Roeckx wrote: On Sat, Oct 29, 2016 at 11:04:33AM +0300, Christos Trochalakis wrote: I am not sure if the first lua patch is safe (regarding the "ssl_conn->tlsext_status_expected = 1;" removal). I'm not sure which patch you're talking about. I remember something about something doing weird things with the state machine for renegiotation that they never should have done, is that that? Kurt I am talking about (src/ngx_http_lua_ssl_ocsp.c): https://github.com/openresty/lua-nginx-module/pull/761/files#diff-50267b7dd63c740bc5c1d29c7387e789L493
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Sat, Oct 29, 2016 at 11:21:05AM +0200, Kurt Roeckx wrote: On Sat, Oct 29, 2016 at 11:04:33AM +0300, Christos Trochalakis wrote: On Tue, Oct 11, 2016 at 10:41:01AM +0300, Christos Trochalakis wrote: > On Fri, Sep 02, 2016 at 10:52:15PM +0200, Kurt Roeckx wrote: > > Hi, > > > > It seems the version in experimental needs this patch to build > > nginx itself: > > http://hg.nginx.org/nginx/rev/1891b2892b68 > > > > You might also want this one: > > http://hg.nginx.org/nginx/rev/3eb1a92a2f05 > > > > But then there some files in debian/modules that have minor > > problems. > > > > For nginx-lua see: > > https://github.com/openresty/lua-nginx-module/pull/761 > > > > nginx-upstream-fair also has a problem with the reference > > counters. > > > > > > Kurt > > > > To recap, the following patches are needed to compile nginx stable (1.10.1) against > OpenSSL 1.1.0, note that the situation is a bit different than experimental, we build > 1.11.x releases there.: > > nginx: backport "SSL: adopted session ticket handling for OpenSSL 1.1.0." (3eb1a92a2f05) > nginx: backport "SSL: guarded SSL_R_NO_CIPHERS_PASSED not present in OpenSSL 1.1.0." (1891b2892b68) > upstream-fair: https://github.com/gnosek/nginx-upstream-fair/pull/22 (not merged upstream) > nginx-lua: https://github.com/openresty/lua-nginx-module/pull/761 (not merged upstream) > > We should also fix ngx_ssl_dhparam() by either: > > nginx: backport "SSL: removed default DH parameters" (1aa9650a8154) > or > by applying the user patch > https://trac.nginx.org/nginx/attachment/ticket/860/nginx-openssl110pre5.patch > which is less intrusive and is what a user expects from nginx 1.10 (1.11 > dropped default DH params). See also my latest comment (#14) & reply on > https://trac.nginx.org/nginx/attachment/ticket/860. > > Pending > === > > Lua v0.10.6 introduces a new regression as reported in: > https://github.com/openresty/lua-nginx-module/issues/757#issuecomment-247567447 > > Kurt, can you evaluate the patch regarding ngx_ssl_dhparam and help us with the > lua v0.10.6 issue? We have some good news, nginx 1.10.2 includes all the changes needed for building against OpenSSL 1.1.0. Modules: upstream-fair: https://github.com/gnosek/nginx-upstream-fair/pull/22 nginx-lua: https://github.com/openresty/lua-nginx-module/pull/761 + https://github.com/wikimedia/operations-software-nginx/commit/e6785d912c992cae676593a8bd266e8c486b098d I am not sure if the first lua patch is safe (regarding the "ssl_conn->tlsext_status_expected = 1;" removal). I have forced-pushed a new stretch-openssl-1.1 that builds successfully. I had a quick look at the patch from https://trac.nginx.org/nginx/attachment/ticket/860/nginx-openssl110pre5.patch Not having seen the full source, I think this is wrong: DH_free(dh); +#if OPENSSL_VERSION_NUMBER >= 0x1015L +BN_free(p); +BN_free(g); +#endif If DH_set0_pqg() has been succesfully called with p and g, dh is now the owner of those pointers. Calling DH_free(dh) will free them, and so you'll have a double free. Kurt Nginx 1.10.2 included all the patches needed to build against openssl 1.1.0 so we don't need to apply that patch ourselves. But indeed you are right, upstream dropped this hunk: http://hg.nginx.org/nginx/rev/131bc715ce87 We now only have to deal with nginx modules patches (lua, upstream-fair).
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Sat, Oct 29, 2016 at 11:04:33AM +0300, Christos Trochalakis wrote: > > I am not sure if the first lua patch is safe (regarding the > "ssl_conn->tlsext_status_expected = 1;" removal). I'm not sure which patch you're talking about. I remember something about something doing weird things with the state machine for renegiotation that they never should have done, is that that? Kurt
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Sat, Oct 29, 2016 at 11:04:33AM +0300, Christos Trochalakis wrote: > On Tue, Oct 11, 2016 at 10:41:01AM +0300, Christos Trochalakis wrote: > > On Fri, Sep 02, 2016 at 10:52:15PM +0200, Kurt Roeckx wrote: > > > Hi, > > > > > > It seems the version in experimental needs this patch to build > > > nginx itself: > > > http://hg.nginx.org/nginx/rev/1891b2892b68 > > > > > > You might also want this one: > > > http://hg.nginx.org/nginx/rev/3eb1a92a2f05 > > > > > > But then there some files in debian/modules that have minor > > > problems. > > > > > > For nginx-lua see: > > > https://github.com/openresty/lua-nginx-module/pull/761 > > > > > > nginx-upstream-fair also has a problem with the reference > > > counters. > > > > > > > > > Kurt > > > > > > > To recap, the following patches are needed to compile nginx stable (1.10.1) > > against > > OpenSSL 1.1.0, note that the situation is a bit different than > > experimental, we build > > 1.11.x releases there.: > > > > nginx: backport "SSL: adopted session ticket handling for OpenSSL 1.1.0." > > (3eb1a92a2f05) > > nginx: backport "SSL: guarded SSL_R_NO_CIPHERS_PASSED not present in > > OpenSSL 1.1.0." (1891b2892b68) > > upstream-fair: https://github.com/gnosek/nginx-upstream-fair/pull/22 (not > > merged upstream) > > nginx-lua: https://github.com/openresty/lua-nginx-module/pull/761 (not > > merged upstream) > > > > We should also fix ngx_ssl_dhparam() by either: > > > > nginx: backport "SSL: removed default DH parameters" (1aa9650a8154) > > or > > by applying the user patch > > https://trac.nginx.org/nginx/attachment/ticket/860/nginx-openssl110pre5.patch > > which is less intrusive and is what a user expects from nginx 1.10 (1.11 > > dropped default DH params). See also my latest comment (#14) & reply on > > https://trac.nginx.org/nginx/attachment/ticket/860. > > > > Pending > > === > > > > Lua v0.10.6 introduces a new regression as reported in: > > https://github.com/openresty/lua-nginx-module/issues/757#issuecomment-247567447 > > > > Kurt, can you evaluate the patch regarding ngx_ssl_dhparam and help us with > > the > > lua v0.10.6 issue? > > We have some good news, nginx 1.10.2 includes all the changes needed for > building against OpenSSL 1.1.0. > > Modules: > upstream-fair: https://github.com/gnosek/nginx-upstream-fair/pull/22 > nginx-lua: https://github.com/openresty/lua-nginx-module/pull/761 + > https://github.com/wikimedia/operations-software-nginx/commit/e6785d912c992cae676593a8bd266e8c486b098d > > I am not sure if the first lua patch is safe (regarding the > "ssl_conn->tlsext_status_expected = 1;" removal). > > I have forced-pushed a new stretch-openssl-1.1 that builds successfully. > I had a quick look at the patch from https://trac.nginx.org/nginx/attachment/ticket/860/nginx-openssl110pre5.patch Not having seen the full source, I think this is wrong: DH_free(dh); +#if OPENSSL_VERSION_NUMBER >= 0x1015L +BN_free(p); +BN_free(g); +#endif If DH_set0_pqg() has been succesfully called with p and g, dh is now the owner of those pointers. Calling DH_free(dh) will free them, and so you'll have a double free. Kurt
Bug#828453: nginx: FTBFS with openssl 1.1.0
On Tue, Oct 11, 2016 at 10:41:01AM +0300, Christos Trochalakis wrote: On Fri, Sep 02, 2016 at 10:52:15PM +0200, Kurt Roeckx wrote: Hi, It seems the version in experimental needs this patch to build nginx itself: http://hg.nginx.org/nginx/rev/1891b2892b68 You might also want this one: http://hg.nginx.org/nginx/rev/3eb1a92a2f05 But then there some files in debian/modules that have minor problems. For nginx-lua see: https://github.com/openresty/lua-nginx-module/pull/761 nginx-upstream-fair also has a problem with the reference counters. Kurt To recap, the following patches are needed to compile nginx stable (1.10.1) against OpenSSL 1.1.0, note that the situation is a bit different than experimental, we build 1.11.x releases there.: nginx: backport "SSL: adopted session ticket handling for OpenSSL 1.1.0." (3eb1a92a2f05) nginx: backport "SSL: guarded SSL_R_NO_CIPHERS_PASSED not present in OpenSSL 1.1.0." (1891b2892b68) upstream-fair: https://github.com/gnosek/nginx-upstream-fair/pull/22 (not merged upstream) nginx-lua: https://github.com/openresty/lua-nginx-module/pull/761 (not merged upstream) We should also fix ngx_ssl_dhparam() by either: nginx: backport "SSL: removed default DH parameters" (1aa9650a8154) or by applying the user patch https://trac.nginx.org/nginx/attachment/ticket/860/nginx-openssl110pre5.patch which is less intrusive and is what a user expects from nginx 1.10 (1.11 dropped default DH params). See also my latest comment (#14) & reply on https://trac.nginx.org/nginx/attachment/ticket/860. Pending === Lua v0.10.6 introduces a new regression as reported in: https://github.com/openresty/lua-nginx-module/issues/757#issuecomment-247567447 Kurt, can you evaluate the patch regarding ngx_ssl_dhparam and help us with the lua v0.10.6 issue? We have some good news, nginx 1.10.2 includes all the changes needed for building against OpenSSL 1.1.0. Modules: upstream-fair: https://github.com/gnosek/nginx-upstream-fair/pull/22 nginx-lua: https://github.com/openresty/lua-nginx-module/pull/761 + https://github.com/wikimedia/operations-software-nginx/commit/e6785d912c992cae676593a8bd266e8c486b098d I am not sure if the first lua patch is safe (regarding the "ssl_conn->tlsext_status_expected = 1;" removal). I have forced-pushed a new stretch-openssl-1.1 that builds successfully.