aboutsummaryrefslogtreecommitdiff
path: root/src
Commit message (Collapse)AuthorAge
...
* Improved logging of invalid headers.Maxim Dounin2021-06-28
| | | | | | | | | | | | | | In 71edd9192f24 logging of invalid headers which were rejected with the NGX_HTTP_PARSE_INVALID_HEADER error was restricted to just the "client sent invalid header line" message, without any attempts to log the header itself. This patch returns logging of the header up to the invalid character and the character itself. The r->header_end pointer is now properly set in all cases to make logging possible. The same logging is also introduced when parsing headers from upstream servers.
* Disabled control characters and space in header names.Maxim Dounin2021-06-28
| | | | | | | | | | | | | | Control characters (0x00-0x1f, 0x7f), space, and colon were never allowed in header names. The only somewhat valid use is header continuation which nginx never supported and which is explicitly obsolete by RFC 7230. Previously, such headers were considered invalid and were ignored by default (as per ignore_invalid_headers directive). With this change, such headers are unconditionally rejected. It is expected to make nginx more resilient to various attacks, in particular, with ignore_invalid_headers switched off (which is inherently unsecure, though nevertheless sometimes used in the wild).
* Disabled control characters in URIs.Maxim Dounin2021-06-28
| | | | | | | | Control characters (0x00-0x1f, 0x7f) were never allowed in URIs, and must be percent-encoded by clients. Further, these are not believed to appear in practice. On the other hand, passing such characters might make various attacks possible or easier, despite the fact that currently allowed control characters are not significant for HTTP request parsing.
* Disabled spaces in URIs (ticket #196).Maxim Dounin2021-06-28
| | | | | | | From now on, requests with spaces in URIs are immediately rejected rather than allowed. Spaces were allowed in 31e9677b15a1 (0.8.41) to handle bad clients. It is believed that now this behaviour causes more harm than good.
* Core: escaping of chars not allowed in URIs per RFC 3986.Maxim Dounin2021-06-28
| | | | | | | | | | | | | | | | | | | | | Per RFC 3986 only the following characters are allowed in URIs unescaped: unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~" gen-delims = ":" / "/" / "?" / "#" / "[" / "]" / "@" sub-delims = "!" / "$" / "&" / "'" / "(" / ")" / "*" / "+" / "," / ";" / "=" And "%" can appear as a part of escaping itself. The following characters are not allowed and need to be escaped: %00-%1F, %7F-%FF, " ", """, "<", ">", "\", "^", "`", "{", "|", "}". Not escaping ">" is known to cause problems at least with MS Exchange (see http://nginx.org/pipermail/nginx-ru/2010-January/031261.html) and in Tomcat (ticket #2191). The patch adds escaping of the following chars in all URI parts: """, "<", ">", "\", "^", "`", "{", "|", "}". Note that comments are mostly preserved to outline important characters being escaped.
* Core: fixed comment about escaping in arguments.Maxim Dounin2021-06-28
| | | | | After 4954530db2af, the ";" character is escaped by ngx_escape_uri(NGX_ESCAPE_ARGS).
* Disabled requests with both Content-Length and Transfer-Encoding.Maxim Dounin2021-06-28
| | | | | | | | HTTP clients are not allowed to generate such requests since Transfer-Encoding introduction in RFC 2068, and they are not expected to appear in practice except in attempts to perform a request smuggling attack. While handling of such requests is strictly defined, the most secure approach seems to reject them.
* Added CONNECT method rejection.Maxim Dounin2021-06-28
| | | | | | | | | No valid CONNECT requests are expected to appear within nginx, since it is not a forward proxy. Further, request line parsing will reject proper CONNECT requests anyway, since we don't allow authority-form of request-target. On the other hand, RFC 7230 specifies separate message length rules for CONNECT which we don't support, so make sure to always reject CONNECTs to avoid potential abuse.
* Moved TRACE method rejection to a better place.Maxim Dounin2021-06-28
| | | | | | | Previously, TRACE requests were rejected before parsing Transfer-Encoding. This is not important since keepalive is not enabled at this point anyway, though rejecting such requests after properly parsing other headers is less likely to cause issues in case of further code changes.
* Core: added the ngx_rbtree_data() macro.Vladimir Homutov2021-06-21
|
* Fixed format strings for ngx_win32_version.Maxim Dounin2021-06-18
|
* gRPC: RST_STREAM(NO_ERROR) handling micro-optimization.Sergey Kandaurov2021-06-17
| | | | | After 2096b21fcd10, a single RST_STREAM(NO_ERROR) may not result in an error. This change removes several unnecessary ctx->type checks for such a case.
* gRPC: handling GOAWAY with a higher last stream identifier.Sergey Kandaurov2021-06-17
| | | | | Previously, once received from upstream, it couldn't limit opening additional streams in a cached keepalive connection.
* Fixed SSL logging with lingering close.Maxim Dounin2021-06-01
| | | | | | | | | | Recent fixes to SSL shutdown with lingering close (554c6ae25ffc, 1.19.5) broke logging of SSL variables. To make sure logging of SSL variables works properly, avoid freeing c->ssl when doing an SSL shutdown before lingering close. Reported by Reinis Rozitis (http://mailman.nginx.org/pipermail/nginx/2021-May/060670.html).
* SSL: ngx_ssl_shutdown() rework.Maxim Dounin2021-06-01
| | | | | | | | Instead of calling SSL_free() with each return point, introduced a single place where cleanup happens. As a positive side effect, this fixes two potential memory leaks on ngx_handle_read_event() and ngx_handle_write_event() errors where there were no SSL_free() calls (though unlikely practical, as errors there are only expected to happen due to bugs or kernel issues).
* Core: disabled SO_REUSEADDR on UDP sockets while testing config.Maxim Dounin2021-05-31
| | | | | | | | | | | On Linux, SO_REUSEADDR allows completely duplicate UDP sockets, so using SO_REUSEADDR when testing configuration results in packets being dropped if there is an existing traffic on the sockets being tested (ticket #2187). While dropped packets are expected with UDP, it is better to avoid this when possible. With this change, SO_REUSEADDR is no longer set on datagram sockets when testing configuration.
* Core: disabled cloning sockets when testing config (ticket #2188).Maxim Dounin2021-05-31
| | | | | | | | | | | | | Since we anyway do not set SO_REUSEPORT when testing configuration (see ecb5cd305b06), trying to open additional sockets does not make much sense, as all these additional sockets are expected to result in EADDRINUSE errors from bind(). On the other hand, there are reports that trying to open these sockets takes significant time under load: total configuration testing time greater than 15s was observed in ticket #2188, compared to less than 1s without load. With this change, no additional sockets are opened during testing configuration.
* Version bump.Maxim Dounin2021-05-31
|
* Resolver: explicit check for compression pointers in question.Maxim Dounin2021-05-25
| | | | | | | | | | | | | | | Since nginx always uses exactly one entry in the question section of a DNS query, and never uses compression pointers in this entry, parsing of a DNS response in ngx_resolver_process_response() does not expect compression pointers to appear in the question section of the DNS response. Indeed, compression pointers in the first name of a DNS response hardly make sense, do not seem to be allowed by RFC 1035 (which says "a pointer to a prior occurance of the same name", note "prior"), and were never observed in practice. Added an explicit check to ngx_resolver_process_response()'s parsing of the question section to properly report an error if compression pointers nevertheless appear in the question section.
* Resolver: simplified ngx_resolver_copy().Maxim Dounin2021-05-25
| | | | | | Instead of checking on each label if we need to place a dot or not, now it always adds a dot after a label, and reduces the resulting length afterwards.
* Resolver: reworked ngx_resolver_copy() copy loop.Maxim Dounin2021-05-25
| | | | | | To make the code easier to read, reworked the ngx_resolver_copy() copy loop to match the one used to calculate length. No functional changes.
* Resolver: fixed label types handling in ngx_resolver_copy().Maxim Dounin2021-05-25
| | | | | | | | | | | Previously, anything with any of the two high bits set were interpreted as compression pointers. This is incorrect, as RFC 1035 clearly states that "The 10 and 01 combinations are reserved for future use". Further, the 01 combination is actually allocated for EDNS extended label type (see RFC 2671 and RFC 6891), not really used though. Fix is to reject unrecognized label types rather than misinterpreting them as compression pointers.
* Resolver: fixed off-by-one read in ngx_resolver_copy().Maxim Dounin2021-05-25
| | | | | | It is believed to be harmless, and in the worst case it uses some uninitialized memory as a part of the compression pointer length, eventually leading to the "name is out of DNS response" error.
* Resolver: fixed off-by-one write in ngx_resolver_copy().Maxim Dounin2021-05-25
| | | | Reported by Luis Merino, Markus Vervier, Eric Sesterhenn, X41 D-Sec GmbH.
* Location header escaping in redirects (ticket #882).Ruslan Ermilov2021-05-24
| | | | | The header is escaped in redirects based on request URI or location name (auto redirect).
* Fixed log action when using SSL certificates with variables.Maxim Dounin2021-05-24
| | | | | | | | | | When variables are used in ssl_certificate or ssl_certificate_key, a request is created in the certificate callback to evaluate the variables, and then freed. Freeing it, however, updates c->log->action to "closing request", resulting in confusing error messages like "client timed out ... while closing request" when a client times out during the SSL handshake. Fix is to restore c->log->action after calling ngx_http_free_request().
* Stream: the "fastopen" parameter of the "listen" directive.Ruslan Ermilov2021-05-20
| | | | Based on a patch by Anbang Wen.
* Core: fixed comment about msie_refresh escaping.Ruslan Ermilov2021-05-19
| | | | | After 12a656452ad1, the "%" character is no longer escaped by ngx_escape_uri(NGX_ESCAPE_REFRESH).
* Mail: max_errors directive.Maxim Dounin2021-05-19
| | | | | Similarly to smtpd_hard_error_limit in Postfix and smtp_max_unknown_commands in Exim, specifies the number of errors after which the connection is closed.
* Mail: IMAP pipelining support.Maxim Dounin2021-05-19
| | | | | | | | | | The change is mostly the same as the SMTP one (04e43d03e153 and 3f5d0af4e40a), and ensures that nginx is able to properly handle or reject multiple IMAP commands. The s->cmd field is not really used and set for consistency. Non-synchronizing literals handling in invalid/unknown commands is limited, so when a non-synchronizing literal is detected at the end of a discarded line, the connection is closed.
* Mail: stricter checking of IMAP tags.Maxim Dounin2021-05-19
| | | | | | Only "A-Za-z0-9-._" characters now allowed (which is stricter than what RFC 3501 requires, but expected to be enough for all known clients), and tags shouldn't be longer than 32 characters.
* Mail: fixed backslash handling in IMAP literals.Maxim Dounin2021-05-19
| | | | | | | | | | | | | | | | | | | Previously, s->backslash was set if any of the arguments was a quoted string with a backslash character. After successful command parsing this resulted in all arguments being filtered to remove backslashes. This is, however, incorrect, as backslashes should not be removed from IMAP literals. For example: S: * OK IMAP4 ready C: a01 login {9} S: + OK C: user\name "pass\"word" S: * BAD internal server error resulted in "Auth-User: username" instead of "Auth-User: user\name" as it should. Fix is to apply backslash filtering on per-argument basis during parsing.
* Mail: removed dead s->arg_start handling.Maxim Dounin2021-05-19
| | | | | | | | As discussed in the previous change, s->arg_start handling in the "done" labels of ngx_mail_pop3_parse_command(), ngx_mail_imap_parse_command(), and ngx_mail_smtp_parse_command() is wrong: s->arg_start cannot be set there, as it is handled and cleared on all code paths where the "done" labels are reached. The relevant code is dead and now removed.
* Mail: fixed s->arg_start clearing on invalid IMAP commands.Maxim Dounin2021-05-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, s->arg_start was left intact after invalid IMAP commands, and this might result in an argument incorrectly added to the following command. Similarly, s->backslash was left intact as well, leading to unneeded backslash removal. For example (LFs from the client are explicitly shown as "<LF>"): S: * OK IMAP4 ready C: a01 login "\<LF> S: a01 BAD invalid command C: a0000000000\2 authenticate <LF> S: a00000000002 aBAD invalid command The backslash followed by LF generates invalid command with s->arg_start and s->backslash set, the following command incorrectly treats anything from the old s->arg_start to the space after the command as an argument, and removes the backslash from the tag. If there is no space, s->arg_end will be NULL. Both things seem to be harmless though. In particular: - This can be used to provide an incorrect argument to a command without arguments. The only command which seems to look at the single argument is AUTHENTICATE, and it checks the argument length before trying to access it. - Backslash removal uses the "end" pointer, and stops due to "src < end" condition instead of scanning all the process memory if s->arg_end is NULL (and arg[0].len is huge). - There should be no backslashes in unquoted strings. An obvious fix is to clear s->arg_start and s->backslash on invalid commands, similarly to how it is done in POP3 parsing (added in 810:e3aa8f305d21) and SMTP parsing. This, however, makes it clear that s->arg_start handling in the "done" label is wrong: s->arg_start cannot be legitimately set there, as it is expected to be cleared in all possible cases when the "done" label is reached. The relevant code is dead and will be removed by the following change.
* Mail: POP3 pipelining support.Maxim Dounin2021-05-19
| | | | | | | The change is mostly the same as the SMTP one (04e43d03e153 and 3f5d0af4e40a), and ensures that nginx is able to properly handle or reject multiple POP3 commands, as required by the PIPELINING capability (RFC 2449). The s->cmd field is not really used and set for consistency.
* Mail: optimized discarding invalid SMTP commands.Maxim Dounin2021-05-19
| | | | | | | | | | There is no need to scan buffer from s->buffer->pos, as we already scanned the buffer till "p" and wasn't able to find an LF. There is no real need for this change in SMTP, since it is at most a microoptimization of a non-common code path. Similar code in IMAP, however, will have to start scanning from "p" to be correct, since there can be newlines in IMAP literals.
* Mail: fixed handling of invalid SMTP commands split between reads.Maxim Dounin2021-05-19
| | | | | | | | | | | | | | | | Previously, if an invalid SMTP command was split between reads, nginx failed to wait for LF before returning an error, and interpreted the rest of the command received later as a separate command. The sw_invalid state in ngx_mail_smtp_parse_command(), introduced in 04e43d03e153, did not work, since ngx_mail_smtp_auth_state() clears s->state when returning an error due to NGX_MAIL_PARSE_INVALID_COMMAND. And not clearing s->state will introduce another problem: the rest of the command would trigger duplicate error when rest of the command is received. Fix is to return NGX_AGAIN from ngx_mail_smtp_parse_command() until full command is received.
* Mail: fixed SMTP pipelining to send the response immediately.Maxim Dounin2021-05-19
| | | | | | | | | | | | | Previously, if there were some pipelined SMTP data in the buffer when a proxied connection with the backend was established, nginx called ngx_mail_proxy_handler() to send these data, and not tried to send the response to the last command. In most cases, this response was later sent along with the response to the pipelined command, but if for some reason client decides to wait for the response before finishing the next command this might result in a connection hang. Fix is to always call ngx_mail_proxy_handler() to send the response, and additionally post an event to send the pipelined data if needed.
* Upstream: variables support in certificates.Maxim Dounin2021-05-06
|
* Auth basic: changed alcf->user_file to be a pointer.Maxim Dounin2021-05-06
| | | | | This saves some memory in typical case when auth_basic_user_file is not explicitly set, and unifies the code with alcf->realm.
* Changed complex value slots to use NGX_CONF_UNSET_PTR.Maxim Dounin2021-05-06
| | | | | | | | | | | With this change, it is now possible to use ngx_conf_merge_ptr_value() to merge complex values. This change follows much earlier changes in ngx_conf_merge_ptr_value() and ngx_conf_set_str_array_slot() in 1452:cd586e963db0 (0.6.10) and 1701:40d004d95d88 (0.6.22), and the change in ngx_conf_set_keyval_slot() (7728:485dba3e2a01, 1.19.4). To preserve compatibility with existing 3rd party modules, both NULL and NGX_CONF_UNSET_PTR are accepted for now.
* Restored zeroing of ngx_channel_t in ngx_pass_open_channel().Ruslan Ermilov2021-04-22
| | | | | | | | | Due to structure's alignment, some uninitialized memory contents may have been passed between processes. Zeroing was removed in 0215ec9aaa8a. Reported by Johnny Wang.
* Mail: fixed reading with fully filled buffer (ticket #2159).Maxim Dounin2021-04-21
| | | | | | | | | | | With SMTP pipelining, ngx_mail_read_command() can be called with s->buffer without any space available, to parse additional commands received to the buffer on previous calls. Previously, this resulted in recv() being called with zero length, resulting in zero being returned, which was interpreted as a connection close by the client, so nginx silently closed connection. Fix is to avoid calling c->recv() if there is no free space in the buffer, but continue parsing of the already received commands.
* Version bump.Maxim Dounin2021-04-21
|
* Changed keepalive_requests default to 1000 (ticket #2155).Maxim Dounin2021-04-08
| | | | | | | | | | | | | | | | | | | | | | It turns out no browsers implement HTTP/2 GOAWAY handling properly, and large enough number of resources on a page results in failures to load some resources. In particular, Chrome seems to experience errors if loading of all resources requires more than 1 connection (while it is usually able to retry requests at least once, even with 2 connections there are occasional failures for some reason), Safari if loading requires more than 3 connections, and Firefox if loading requires more than 10 connections (can be configured with network.http.request.max-attempts, defaults to 10). It does not seem to be possible to resolve this on nginx side, even strict limiting of maximum concurrency does not help, and loading issues seems to be triggered by merely queueing of a request for a particular connection. The only available mitigation seems to use higher keepalive_requests value. The new default is 1000 and matches previously used default for http2_max_requests. It is expected to be enough for 99.98% of the pages (https://httparchive.org/reports/state-of-the-web?start=latest#reqTotal) even in Chrome.
* Added $connection_time variable.Maxim Dounin2021-04-08
|
* Introduced the "keepalive_time" directive.Maxim Dounin2021-04-08
| | | | | | | Similar to lingering_time, it limits total connection lifetime before keepalive is switched off. The default is 1 hour, which is close to the total maximum connection lifetime possible with default keepalive_requests and keepalive_timeout.
* HTTP/2: relaxed PRIORITY frames limit.Maxim Dounin2021-04-07
| | | | | | | | | | | | | | Firefox uses several idle streams for PRIORITY frames[1], and "http2_max_concurrent_streams 1;" results in "client sent too many PRIORITY frames" errors when a connection is established by Firefox. Fix is to relax the PRIORITY frames limit to use at least 100 as the initial value (which is the recommended by the HTTP/2 protocol minimum limit on the number of concurrent streams, so it is not unreasonable for clients to assume that similar number of idle streams can be used for prioritization). [1] https://hg.mozilla.org/mozilla-central/file/32a9e6e145d6e3071c3993a20bb603a2f388722b/netwerk/protocol/http/Http2Stream.cpp#l1270
* Gzip: updated handling of zlib variant from Intel.Maxim Dounin2021-04-05
| | | | | | | | In current versions (all versions based on zlib 1.2.11, at least since 2018) it no longer uses 64K hash and does not force window bits to 13 if it is less than 13. That is, it needs just 16 bytes more memory than normal zlib, so these bytes are simply added to the normal size calculation.
* Gzip: support for zlib-ng.Maxim Dounin2021-04-05
|