Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge nginx/branches/stable-1.22 and ensure build succeeds #1

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

xstefen
Copy link

@xstefen xstefen commented Jun 7, 2022

It does πŸ€·πŸ»β€β™‚οΈ 🍻

pluknet and others added 30 commits June 7, 2022 01:53
The variable contains a negotiated curve used for the handshake key
exchange process.  Known curves are listed by their names, unknown
ones are shown in hex.

Note that for resumed sessions in TLSv1.2 and older protocols,
$ssl_curve contains the curve used during the initial handshake,
while in TLSv1.3 it contains the curve used during the session
resumption (see the SSL_get_negotiated_group manual page for
details).

The variable is only meaningful when using OpenSSL 3.0 and above.
With older versions the variable is empty.
With sendfile in threads, "task already active" alerts might appear in logs
if a write event happens on the main HTTP/2 connection, triggering a sendfile
in threads while another thread operation is already running.  Observed
with "aio threads; aio_write on; sendfile on;" and with thread event handlers
modified to post a write event to the main HTTP/2 connection (though can
happen without any modifications).

Similarly, sendfile() with AIO preloading on FreeBSD can trigger duplicate
aio operation, resulting in "second aio post" alerts.  This is, however,
harder to reproduce, especially on modern FreeBSD systems, since sendfile()
usually does not return EBUSY.

Fix is to avoid starting a sendfile operation if other thread operation
is active by checking r->aio in the thread handler (and, similarly, in
aio preload handler).  The added check also makes duplicate calls protection
redundant, so it is removed.
With sendfile() in threads ("aio threads; sendfile on;"), client connection
can block on writing, waiting for sendfile() to complete.  In HTTP/2 this
might result in the request hang, since an attempt to continue processing
in thread event handler will call request's write event handler, which
is usually stopped by ngx_http_v2_send_chain(): it does nothing if there
are no additional data and stream->queued is set.  Further, HTTP/2 resets
stream's c->write->ready to 0 if writing blocks, so just fixing
ngx_http_v2_send_chain() is not enough.

Can be reproduced with test suite on Linux with:

TEST_NGINX_GLOBALS_HTTP="aio threads; sendfile on;" prove h2*.t

The following tests currently fail: h2_keepalive.t, h2_priority.t,
h2_proxy_max_temp_file_size.t, h2.t, h2_trailers.t.

Similarly, sendfile() with AIO preloading on FreeBSD can block as well,
with similar results.  This is, however, harder to reproduce, especially
on modern FreeBSD systems, since sendfile() usually does not return EBUSY.

Fix is to modify ngx_http_v2_send_chain() so it actually tries to send
data to the main connection when called, and to make sure that
c->write->ready is set by the relevant event handlers.
ngx_http_v2_huff_decode.c and ngx_http_v2_huff_encode.c are renamed
to ngx_http_huff_decode.c and ngx_http_huff_encode.c.
If a configuration parsing fails for some reason, ngx_regex_module_init()
is not called, and ngx_pcre_studies remained set despite the fact that
the pool it was allocated from is already freed.  This might result in
a segmentation fault during runtime regular expression compilation, such
as in SSI, for example, in the single process mode, or if a worker process
died and was respawned from a master process in such an inconsistent state.

Fix is to clear ngx_pcre_studies from the pool cleanup handler (which is
anyway used to free JIT-compiled patterns).
Notably, ngx_pcre_pool and ngx_pcre_studies are renamed to ngx_regex_pool
and ngx_regex_studies, respectively.
Removed ICC-specific PCRE optimizations which tried to link with PCRE
object files instead of the library.  Made compiler-specific code
minimal.
The PCRE2 library is now used by default if found, instead of the
original PCRE library.  If needed for some reason, this can be disabled
with the --without-pcre2 configure option.

To make it possible to specify paths to the library and include files
via --with-cc-opt / --with-ld-opt, the library is first tested without
any additional paths and options.  If this fails, the pcre2-config script
is used.

Similarly to the original PCRE library, it is now possible to build PCRE2
from sources with nginx configure, by using the --with-pcre= option.
It automatically detects if PCRE or PCRE2 sources are provided.

Note that compiling PCRE2 10.33 and later requires inttypes.h.  When
compiling on Windows with MSVC, inttypes.h is only available starting
with MSVC 2013.  In older versions some replacement needs to be provided
("echo '#include <stdint.h>' > pcre2-10.xx/src/inttypes.h" is good enough
for MSVC 2010).

The interface on nginx side remains unchanged.
With this change, dynamic modules using nginx regex interface can be used
regardless of the variant of the PCRE library nginx was compiled with.

If a module is compiled with different PCRE library variant, in case of
ngx_regex_exec() errors it will report wrong function name in error
messages.  This is believed to be tolerable, given that fixing this will
require interface changes.
Notably, NAXSI is known to misuse ngx_regex_compile() with rc.options set
to PCRE_CASELESS | PCRE_MULTILINE.  With PCRE2 support, and notably binary
compatibility changes, it is no longer possible to set PCRE[2]_MULTILINE
option without using proper interface.  To facilitate correct usage,
this change adds the NGX_REGEX_MULTILINE option.
Starting with FreeBSD 11, there is no need to use AIO operations to preload
data into cache for sendfile(SF_NODISKIO) to work.  Instead, sendfile()
handles non-blocking loading data from disk by itself.  It still can, however,
return EBUSY if a page is already being loaded (for example, by a different
process).  If this happens, we now post an event for the next event loop
iteration, so sendfile() is retried "after a short period", as manpage
recommends.

The limit of the number of EBUSY tolerated without any progress is preserved,
but now it does not result in an alert, since on an idle system event loop
iteration might be very short and EBUSY can happen many times in a row.
Instead, SF_NODISKIO is simply disabled for one call once the limit is
reached.

With this change, sendfile(SF_NODISKIO) is now used automatically as long as
sendfile() is enabled, and no longer requires "aio on;".
The SF_NOCACHE flag, introduced in FreeBSD 11 along with the new non-blocking
sendfile() implementation by glebius@, makes it possible to use sendfile()
along with the "directio" directive.
Linux with EPOLLEXCLUSIVE usually notifies only the process which was first
to add the listening socket to the epoll instance.  As a result most of the
connections are handled by the first worker process (ticket #2285).  To fix
this, we re-add the socket periodically, so other workers will get a chance
to accept connections.
When a worker process is shutting down, keepalive is not used: this is checked
before the ngx_http_set_keepalive() call in ngx_http_finalize_connection().
Yet the "Connection: keep-alive" header was still sent, even if we know that
the worker process is shutting down, potentially resulting in additional
requests being sent to the connection which is going to be closed anyway.
While clients are expected to be able to handle asynchronous close events
(see ticket #1022), it is certainly possible to send the "Connection: close"
header instead, informing the client that the connection is going to be closed
and potentially saving some unneeded work.

With this change, we additionally check for worker process shutdown just
before sending response headers, and disable keepalive accordingly.
Line continuation as used in the syntax file might be broken if "compatible"
is set or "C" is added to cpoptions.  Fix is to set the "cpoptions" option
to vim default value at script start and restore it later, see
":help use-cpo-save".
Chrome only uses TLS session tickets once with TLS 1.3, likely following
RFC 8446 Appendix C.4 recommendation.  With OpenSSL, this works fine with
built-in session tickets, since these are explicitly renewed in case of
TLS 1.3 on each session reuse, but results in only two connections being
reused after an initial handshake when using ssl_session_ticket_key.

Fix is to always renew TLS session tickets in case of TLS 1.3 when using
ssl_session_ticket_key, similarly to how it is done by OpenSSL internally.
@lzlrd
Copy link
Member

lzlrd commented Jun 7, 2022

Thanks for your help on the rebase; I aim to give this a look and get an update out sometime this week however I have quite a lot on my hands this week and can't give an assured ETA.

@xstefen
Copy link
Author

xstefen commented Jun 8, 2022

No worries, Just doing my part, or trying to.

Also, not sure if you were familiar already, just stumbled across https://hg.nginx.org/nginx-quic

@FireMasterK
Copy link

I've done a similar thing, but for nginx 1.23.1 at https://github.com/FireMasterK/zestginx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants