The name length of a configured parameter was truncated to one byte
before the per-parameter length field was written, so that a
uwsgi_param name of 256 bytes or longer was encoded with a wrong
length while the name itself was copied in full. The resulting request
was malformed, and upstream servers responded by closing the connection.
While long names are barely used in practice, it is easy to support
them. The length is now properly encoded up to the allowed length.
The name length of a configured parameter was truncated to one byte
before the per-parameter length field was written, so that a
fastcgi_param name of 128 bytes or longer was encoded with a wrong
length while the name itself was copied in full. The resulting request
was malformed, and upstream servers responded by closing the connection.
While long names are barely used in practice, it is easy to support
them. The length is now properly encoded up to the allowed length.
Add ngx_proxy_protocol_v2_write() that writes the fixed 16-byte
PPv2 header and address block, and appends a list of TLVs.
An address block is written using IPv4/IPv6 mixed-family promotion:
the highest family present on either side (IPv6 > IPv4) is used for
the header, and the other address is promoted into it. Both v4-mapped
addresses are demoted to IPv4.
When PROXY protocol v2 is configured and the downstream connection
is SSL, standard PROXY protocol v2 TLVs are populated:
PP2_TYPE_ALPN from SSL_get0_alpn_selected()
PP2_TYPE_AUTHORITY from SSL_get_servername()
PP2_SUBTYPE_SSL_VERSION from SSL_get_version()
PP2_SUBTYPE_SSL_CIPHER from SSL_get_cipher_name()
PP2_SUBTYPE_SSL_CN from client cert subject CN
PP2_SUBTYPE_SSL_SIG_ALG from server certificate
PP2_SUBTYPE_SSL_KEY_ALG from server certificate
The CRC32c TLV is written last and covers the entire header in place.
Co-authored-by: Vadim Zhestikov <v.zhestikov@f5.com>
This change implements the Castagnoli polynomial code 0x11EDC6F41,
as described in RFC4960, Appendix B. It will be used to calculate
PROXY protocol v2 CRC32c checksums.
The HTTP/2 proxy and gRPC upstream clients accepted WINDOW_UPDATE
frames with a zero flow-control window increment. RFC 9113 requires
such frames to be handled as PROTOCOL_ERROR at stream or connection
scope.
Reject these frames as invalid upstream responses.
HTTP/2 and gRPC requests are now always sent with the ":authority"
pseudo-header, while the "Host" header is no longer passed.
Where a host value is set explicitly with "proxy_set_header Host"
or "grpc_set_header Host", that value is conveyed in ":authority".
As per RFC 9113, Section 8.3.1, clients and intermediaries MUST
generate an ":authority" pseudo-header to convey authority information.
For HTTP/1.1, HTTP/2 and gRPC, if the explicit value is empty, then
the value of $proxy_host is used instead. As per RFC 9110, Sections
4.2.1 and 4.2.2, the "http" and "https" URI schemes MUST contain a
non-empty host identifier. The change enforces this rule by
introducing a default.
Closes: https://github.com/nginx/nginx/issues/1426
Previously, the response length was explicitly set to zero if the
initial HEADERS frame was seen with the END_STREAM flag. This was
meant to properly pass responses in the Trailers-Only format as
introduced in 6559a42013. This handling was too broad though,
passing incomplete responses that carry a non-zero Content-Length
to the client as if they were complete and empty.
Now such responses are caught by the existing checks later on and
are passed as incomplete, similarly to how this is handled elsewhere.
For instance, on HTTP/2 client connections this now results in proper
signalling, an incomplete stream followed by stream reset.
In a8289aa69c, we introduced a new code to finalize the evaluation of
a complex value, deferring the stack update to this new code. However,
the change inadvertently broke compatibility with several third-party
modules that were reusing ngx_http_script_complex_value_code.
This change relegates omitted ngx_http_script_complex_value_end_code
from crash to a potential read of uninitialized bytes at the end of the
allocated buffer.
As per RFC 9000, Section 4.5:
Once a final size for a stream is known, it cannot change. If a
RESET_STREAM or STREAM frame is received indicating a change in the
final size for the stream, an endpoint SHOULD respond with an error of
type FINAL_SIZE_ERROR. A receiver SHOULD treat receipt of data at or
beyond the final size as an error of type FINAL_SIZE_ERROR, even after
a stream is closed.
Previously, the stream final size was validated only after other checks
that could return earlier and skip validation: the receive state check
and, for STREAM frames, the check for data below the current receive
offset. As a result, a STREAM or RESET_STREAM frame carrying a final
size error was silently ignored once the receiving part of the stream
had reached a state in which the frame would otherwise be discarded, or
when a STREAM frame did not advance the receive offset. Now the final
size is validated first, so a FINAL_SIZE_ERROR is generated in these
cases as well.
Previously, stream flow control was not applied to the RESET_STREAM
final_size, which allowed a client to exceed it. The excess had to be
within the connection flow control limits anyway.
Reported by Tony Wang.
The check in ngx_select_init_conf() can be insufficient because it
doesn't cover descriptors for local files, which are not accounted
for in cycle->connection_n. For instance, to reach this condition
with new connections, no reusable connections must be available for
recycling in ngx_get_connection().
The fix adopts a similar code from the win32 select module.
Since d9b7669666, the single-SV zero copy path in $r->print() no
longer required the SV to be read-only. Reference counting introduced
by that change keeps the SV alive, but not its string buffer: modifying
a mutable scalar after $r->print() overwrites the buffer in place or
reallocates it, while nginx might still have it queued in r->out, since
small responses are postponed by ngx_http_write_filter(). This resulted
in corrupted responses, and in freed memory being sent to the client
when the scalar was grown.
The SV is now required to be read-only again. Reference counting is
still needed here, as a read-only SV in an eval() might be freed with
the eval's op tree.
Directory listing entries (file names, metadata) were allocated
from the request pool and lived until request teardown. Use a
temporary pool instead, destroying it immediately after the
response buffer is built.
The size parser and the image decoders used ctx->length, which is the
allocation size (set to image_filter_buffer when the upstream response
omits Content-Length), as the amount of valid data, so a truncated
response without Content-Length could be parsed or decoded past the
received bytes into uninitialized buffer memory.
The length is now adjusted to the actually-read size once the response
body has been read.
Reported by dukesp69 and YLChen-007.
Previously, if a subrequest was posted twice, it could be finalized in
both calls, excessively reducing r->main->count and potentially leading
to a use-after-free.
The fix is to avoid posting a request if it's already posted. Also,
as a hardening measure, r->write_event_handler is now reset to a no-op
handler during active subrequest finalization.
The problem manifests itself in ngx_http_ssi_filter_module during
unbuffered proxying. If a subrequest is created for an SSI include
statement while the main request has some data postponed by another
include, this subrequest becomes double-posted when the main request
data is flushed. The first post comes from ngx_http_subrequest() and
the second one comes from ngx_http_postpone_filter(). In case of a
quick subrequest finalization, the above mentioned problem happens.
Reported by P4P3R-HAK.
When ngx_http_regex_exec() reallocates r->captures array, it doesn't update
r->ncaptures value, if regex didn't match. So the next use of unnamed regex
capture triggers uninitialized read and potential buffer overrun.
This config demonstrates the issue:
map test $my_map {
volatile;
~mismatch(.*) 1; # reallocates r->captures in subrequests
default "";
}
server {
location ~(.*) { # sets r->ncaptures
slice 50;
# $1 will read from uninitialized memory in slice subrequests
proxy_set_header Test $my_map$1;
proxy_set_header Range $slice_range;
proxy_pass http://backend;
}
}
The issue was introduced by 746fba0d79.
If the script result turned out to be shorter than its predicted
length, the result string contained uninitialized bytes at the end.
The fix is to cut the result string by its actual size.
The following locations returned trailing garbage to the client with
URI "/1234abcd".
map $uri $foo {
~^/(?<bar>[0-9]).*$ $bar;
}
location ~(?<bar>[0-9]*)[a-z]*$ {
return 200 $1:$foo;
}
location ~(?<bar>[0-9]*)[a-z]*$ {
set $qux $1:$foo;
return 200 $qux;
}
Following the previous change, this change adds script overrun
protection to direct script evaluation in the proxy, fastcgi, scgi,
uwsgi, grpc proxy, index, and try_files modules.
This change is a modified version of a patch by Maxim Dounin. The
modifications include ngx_http_proxy_v2_module and hardened size checks.
Signed-off-by: Roman Arutyunyan <arut@nginx.com>
Origin: <https://freenginx.org/hg/nginx/rev/7e4d7feb4c77>
With this change, all script copy operations now check if there is
enough room in the buffer. To do so, the script engine now provides the
e->end pointer, which specifies expected buffer end, and each copy
operation is checked against it with the ngx_http_script_check_length()
function.
The e->end pointer is optional and only checked when set, thus
introducing no incompatible API changes. All standard functions were
updated to use it, notably ngx_http_complex_value(), ngx_http_script_run(),
ngx_http_script_regex_start_code(), ngx_http_script_complex_value_code().
Direct script evaluation in the proxy, fastcgi, scgi, uwsgi, grpc proxy,
index, and try_files modules will be updated by a separate patch.
In particular, this catches issues as observed when evaluating variables
with side effects, such as in the following configuration:
map $uri $map {
~(?<capture>.*) $capture;
}
set $capture "";
set $temp "$capture $map";
As well as when evaluating non-cacheable variables, where length of a
variable might change between length and copy codes, such as in the
following configuration:
map prefix:$capture $map_volatile {
volatile;
~(?<capture>.*) $capture;
}
set $capture "";
set $temp "$map_volatile";
Similar changes were made in the stream module.
Signed-off-by: Roman Arutyunyan <arut@nginx.com>
Origin: <https://freenginx.org/hg/nginx/rev/d172f506ab5f>
As per RFC 2817 Section 5.2:
Like any other pipelined HTTP/1.1 request, data to be tunneled may be
sent immediately after the blank line. The usual caveats also apply:
data may be discarded if the eventual response is negative, and the
connection may be reset with no response if more than one TCP segment
is outstanding.
As per RFC 9110, Section 9.3.6:
A CONNECT request message does not have content.
Also, as per Section 8.6:
A user agent SHOULD NOT send a Content-Length header field when the
request message does not contain content and the method semantics do
not anticipate such data.
Similarly to "listen ... sndbuf/rcvbuf", proxy_socket_sndbuf and
proxy_socket_rcvbuf can be used to set the SO_SNDBUF and SO_RCVBUF
socket options, respectively, on upstream connections. By default,
the values are left unchanged.
Closes: https://github.com/nginx/nginx/issues/1297
Similarly to "listen ... sndbuf/rcvbuf", proxy_socket_sndbuf and
proxy_socket_rcvbuf can be used to set the SO_SNDBUF and SO_RCVBUF
socket options, respectively, for upstream connections. By default,
the values are left unchanged.
Similar changes made in fastcgi, grpc, scgi, uwsgi, tunnel.
ngx_event_connect_peer() honors SO_RCVBUF via pc->rcvbuf but had
no equivalent for SO_SNDBUF. This adds pc->sndbuf and the matching
setsockopt() call, mirroring the existing SO_RCVBUF path.
Both setsockopt() calls are made non-fatal.
Existing callers leaving sndbuf at 0 retain prior behavior, since
setsockopt() is only invoked when the field is non-zero. These
fields are consumed by various proxying modules in follow-up commits.
Handling of pseudo-headers is refactored to be more in line with
HTTP/3 implementation, that is, a request line is now constructed
as soon as pseudo-headers are followed by regular fields. Here
this plugs a missing handling for absent mandatory or out-of-order
pseudo-headers.
Such requests are now rejected immediately as malformed.
Closes: https://github.com/nginx/nginx/issues/1526
Previously, using stale request objects resulted in accesses to already
freed memory, causing segmentation faults:
location /stale {
perl 'sub {
my $r = shift;
$prev->log_error(0, "next request arrived") if $prev;
$prev = $r;
$r->send_http_header;
return OK;
}';
}
Similarly, incorrectly blessed objects might cause segmentation faults,
such as in the following configuration:
location /bless {
perl 'sub {
my $v = 10;
my $r = bless \$v, "nginx";
$r->send_http_header;
return OK;
}';
}
With this change, active request object is recorded in the
ngx_http_perl_call_handler() function, and checked by
ngx_http_perl_set_request() to prevent use of unexpected request
objects.
Reported by Axel Mierczuk, Keith Hoodlet, 1Password’s Off-by-1 Labs.
Signed-off-by: Sergey Kandaurov <pluknet@nginx.com>
Origin: https://freenginx.org/hg/nginx/rev/86a2685756ae
Perl scalars might have a limited lifetime, and using them without
appropriate reference counting is incorrect. In particular, heap
use-after-free was observed in the following configuration (note the
"eval", which limits lifetime of the string being printed), which
demonstrates that the previously used SvREADONLY() optimizations are
incorrect:
location / {
perl 'sub {
my $r = shift;
$r->send_http_header;
eval q!$r->print("it works")!;
return OK;
}';
}
Similarly, errors were observed with handlers in $r->sleep() and
$r->has_request_body() when a handler comes from an eval, such as in
the following configuration:
location / {
perl 'sub {
my $r = shift;
$r->sleep(100, eval q!sub {
my $r = shift;
$r->send_http_header;
$r->print("it works");
return OK;
}!);
return OK;
}';
}
Accordingly, the SvREADONLY() optimization was removed in
ngx_http_perl_sv2str(), since it is expected to be used for small
strings, and using proper reference counting likely will be more costly
than just copying the string. In $r->print(), $r->sleep(), and
$r->has_request_body() proper reference counting was implemented, with
decrement operations being performed by pool cleanup handlers.
As a positive side effect, $r->print() can now avoid copying any single
scalar, not just read-only scalars.
Reported by Evan Hellman,
https://github.com/freenginx/nginx/issues/26
Signed-off-by: Sergey Kandaurov <pluknet@nginx.com>
Origin: https://freenginx.org/hg/nginx/rev/7a3dbb7905ad
Previously, a revalidated cached response could fail on a cached
keepalive connection with "upstream sent frame for unknown stream"
error followed by "cache file contains invalid header".
This happened because after a 304 response, nginx parsed the cached
response while the upstream peer connection was still attached to the
request. The HTTP/2 proxy code treated cached frames as frames from
that live connection, and could assign a real stream id instead of
treating the cached response as having no real stream.
The fix is to treat cached response parsing as cache-only regardless
of the current upstream peer connection: set the stream id to 0 and
skip live upstream control-frame handling while r->cached is set.
Closes: https://github.com/nginx/nginx/issues/1318
HTTP/2 and gRPC upstream response header parsing used the HPACK string
length to allocate header name and value buffers. The length was not
checked against the configured upstream buffer size before allocation.
A malicious upstream could force nginx to allocate excessive
request-pool memory before the header was rejected.
Reject oversized HTTP/2 header name and value lengths before allocation.
Also keep a per-header-block limit based on the upstream buffer size, so
a header block cannot consume unbounded memory across fields.
Vadim Zhestikov
Sergey Kandaurov
Dmitry Plotnikov
felix h
Roman Arutyunyan
Vinay Kumar Tokala
Aleksei Bavshin
Amin Vakil
Sourav Bhowmik
David Carlier
Srujan Rai
Pavel Pautov
Maxim Dounin
Patrik Wall
Zhidao HONG