In pytests test_08 with the Caddy server, the new rate-limiting in
ngtcp2 did close the connection because it found "too many" stream data
packet repeats.
It is unclear if this is some Caddy issue or if the ngtcp2 implementaton
is wrong. Or if curl can do anything here.
Reported as https://github.com/ngtcp2/ngtcp2/issues/1850
This PR overwrites the ratelimit defaults in ngtcp2 with ten times
increased values. This makes the errors disappear on macOS.
Enable test_08_04/05 in CI again to see if there are any issues
to be found there. (We had those disabled before having parallel
pytests.)
Closes#19274
Replace the check if a ssl session cache is configured with
a function checking if it is configured *and* if an ssl session
cache is available.
During normal operations, a session cache is always there, however
for "connect-only" transfers this might not be the case. When such
transfers receive new sessions/tickets, they need to silently
discard those and not fail.
Reported-by: Marc Aldorasi
Fixes https://github.com/curl/curl/issues/18983
Closes https://github.com/curl/curl/pull/19251
Change the test certificate to carry a altname 'dns:127.0.0.1' which
should *not* match in test_17_05_bad_ip_addr.
wolfSSL: since `wolfSSL_check_domain_name()` does not differentiate
between DNS and IP names, use if only for DNS names. For IP addresses,
get the peer certificate after the handshake and check that using
wolfSSL_X509_check_ip_asc().
Unfortunately, this succeeds where it should not, as wolfSSL internally
used the same check code for both cases. So, skip the test case until
wolfSSL fixes that.
Reported-by: Joshua Rogers
Closes#19252
The parameter `max_pkts` was not checked in the recvmsg() implementation
of vquic_recv_packets() as the packter counter was never increased. This
led to the loop running until an EAGAIN was encountered. Which, in any
real case scenario, does no harm as long as libcurl is ingesting packets
faster than a server is able to send them.
However on a slow device and a fast network this could happen and allow
a denial of serice.
Not a real regression as the vulnerable code has never been released.
libcurl 8.16.0 does not have this bug.
Closes#19186
Adopt ngtcp2_conn_get_stream_user_data which has been available since
ngtcp2 v1.17.0. This improves the time complexity of searching
h3_stream_ctx from O(n) to O(1) where n is the number of stream.
Closes#19132
The choice to continue processing incoming data although the
writeout of the headers/data failed is not obvious. Add a comment
explaining why this is done.
Closes#19093
The transfer loop used to check the socket and if no poll events
were seen, triggered a "DATA_IDLE" event into the filters to let
them schedule times/do things anyway.
Since we no longer check the socket, the filters have been called
already and the DATA_IDLE event is unnecessary work. Remove it.
Closes#19060
quiche receive may report a TLS failure after a verified handshake. That
needs to lead to a transfer receive error.
Reported-by: Joshua Rogers
Closes#18934
The stream blocking might not be the one of the current easy handle.
Look up the stream to be marked as blocking via its stream_id in the
internal hash. Theoretically, this does not have to be one of the h3
streams, so not finding it is not an error.
Fixes#18905
Reported-by: Joshua Rogers
Closes#18906
Similar to our ngtcp2 backend, set our idle timeout for the connection
to 0, meaning we have no such timeout from our side. The effective idle
timeout is then the one announced by the peer.
Closes#18907
In both send/recv functions of the ngtcp2 filter, when TLS verification
has failed, jump out by skipping ingress/egress handling.
Reported-by: Joshua Rogers
Closes#18881
After this patch, the codebase no longer overrides system printf
functions. Instead it explicitly calls either the curl printf functions
`curl_m*printf()` or the system ones using their original names.
Also:
- drop unused `curl_printf.h` includes.
- checksrc: ban system printf functions, allow where necessary.
Follow-up to db98daab05#18844
Follow-up to 4deea9396b#18814Closes#18866
When the close of the quiche filter was never called, the destroy function
did not release all allicated resources.
When closing a quiche filter, set the connected flag to FALSE.
Reported-by: Joshua Rogers
Closes#18880
Also:
- tests/server: replace local `sstrerror()` with `curlx_strerror()`.
- tests/server: show the error code next to the string, where missing.
- curlx: use `curl_msnprintf()` when building for src and tests.
(units was already using it.)
- lib: drop unused includes found along the way.
- curlx_strerror(): avoid compiler warning (and another similar one):
```
In file included from servers.c:14:
../../lib/../../lib/curlx/strerr.c: In function ‘curlx_strerror’:
../../lib/../../lib/curlx/strerr.c:328:32: error: ‘snprintf’ output may be truncated before the last format character [-Werror=format-truncation=]
328 | SNPRINTF(buf, buflen, "%s", msg);
| ^
../../lib/../../lib/curlx/strerr.c:47:18: note: ‘snprintf’ output 1 or more bytes (assuming 2) into a destination of size 1
47 | #define SNPRINTF snprintf
| ^
../../lib/../../lib/curlx/strerr.c:328:7: note: in expansion of macro ‘SNPRINTF’
328 | SNPRINTF(buf, buflen, "%s", msg);
| ^~~~~~~~
```
Follow-up to 45438c8d6f#18823Closes#18840
By making them defaults, then fixing and/or reshuffling remaining
exceptions as necessary.
- checksrc: ban by default: `snprintf`, `vsnprintf`, `sscanf`, `strtol`.
- examples: replace `strtol` with `atoi` to avoid a checksrc exception.
- tests/libtest: replace `strtol` with `atol`.
- tests/server: replace most `strtol` with `atol`.
- tests/server: replace most `strtoul` with `atol`/`atoi`.
- tests/server: drop no longer used `util_ultous`.
- fix typo in checksrc rules: `vsnprint` -> `vsnprintf`.
- update local exceptions.
Also:
- examples: ban curl printf functions. They're discouraged in user code.
- examples: replace curl printf with system printf.
Add `snprintf` workaround for <VS2015.
- examples/synctime: fix `-Wfloat-equal`.
- examples/synctime: exclude for non-Windows and non-UWP Windows.
- examples/synctime: build by default.
Closes#18823
- better tracing of what system call is used and how often
- ngtcp2: combine vquic_send into larger chunks
- ngtcp2: define own PMTU values and enable MTU probing
- ngtcp2: trace interesting remote transport parameters
Closes#18812
- configure/cmake support for enabling the option
- supported in OpenSSL and GnuTLS backends
- when configured, Apple SecTrust is the default trust store
for peer verification. When one of the CURLOPT_* for adding
certificates is used, that default does not apply.
- add documentation of build options and SSL use
Closes#18703
HTTP/3 defines "reserved stream types" that are intended to be ignored
by a receiver. This is part of the "greasing" effort that flexes parts
of the protocol that are needed for future extensions.
curl's OpenSSL-QUIC implementation treated all unexpected streams as
an error. Which seems the right thing to do *but* for these reserved types.
However OpenSSL does not expose this type and thus, curl needs to silently
discard all unexpected streams opened by the server to allow interop
with servers that flex the GREASE parts.
Fixes#18780
Reported-by: Pocs Norbert
Closes#18791
Replace them by `curlx_open()` and `curlx_stat()`.
To make it obvious in the source code what is being executed.
Also:
- tests/server: stop overriding `open()` for test servers.
This is critical for the call made from the signal handler.
For other calls, it's an option to use `curlx_open()`, but
doesn't look important enough to do it, following the path
taken with `fopen()`.
Follow-up to 10bac43b87#18774
Follow-up to 20142f5d06#18634
Follow-up to bf7375ecc5#18503Closes#18776
The return code of SSL_get_stream_read_error_code() was not checked
in one location, but others. Make that consistent.
Reported in Joshua's sarif data
Closes#18725
Fail on errors from SSL_handle_events().
Force quit Caddy test instance that is left hanging longer with
openssl-quic tests for unknown reasons.
Reported in Joshua's sarif data
Closes#18720
Improvements around HTTP Upgrade: and multiplex hanndling:
* add `Curl_conn_set_multiplex()` to set connection's multiplex
bit and trigger "connchanged" events
* call `Curl_conn_set_multiplex()` in filters' `CF_CTRL_CONN_INFO_UPDATE`
implementation where other connection properties are updated.
This prevents connection updates before the final filter chain
is chosen.
* rename enum `UPGR101_INIT` to `UPGR101_NONE`
* rename connection bit `asks_multiplex` to `upgrade_in_progress`
* trigger "connchanged" when `upgrade_in_progress` clears
* rename `WebSockets` to `WebSocket` as it is the common term
used in documentation
Closes#18227
Before this patch `accept4()`, `socket()`, `socketpair()`, `send()` and
`recv()` system symbols were remapped via macros, using the same name,
to local curl debug wrappers. This patch replaces these overrides by
introducing curl-namespaced macros that map either to the system symbols
or to their curl debug wrappers in `CURLDEBUG` (TrackMemory) builds.
This follows a patch that implemented the same for `accept()`.
The old method required tricks to make these redefines work in unity
builds, and avoid them interfering with system headers. These tricks
did not work for system symbols implemented as macros.
The new method allows to setup these mappings once, without interfering
with system headers, upstream macros, or unity builds. It makes builds
more robust.
Also:
- checksrc: ban all mapped functions.
- docs/examples: tidy up checksrc rules.
Follow-up to 9863599d69#18502
Follow-up to 3bb5e58c10#17827Closes#18503
`-Weverything` is not enabled by curl, and not recommended by LLVM,
because it may enable experimental options, and will result in new
fallouts after toolchain upgrades. This patch aims to fix/silence as much
as possible as found with llvm/clang 21.1.0. It also permanently enables
warnings that were fixed in source and deemed manageable in the future.
`-Wformat` warnings are addressed separately via #18343.
Fix/silence warnings in the source:
- typecheck-gcc.h: fix `-Wreserved-identifier`.
- lib: silence `-Wcast-function-type-strict`.
For llvm 16+ or Apple clang 16+.
- asyn-ares: limit `HAPPY_EYEBALLS_DNS_TIMEOUT` to old c-ares versions.
- curl_trc: fix `-Wc++-hidden-decl`.
- doh: fix `-Wc++-keyword`.
- ftp: fix `-Wreserved-identifier`.
- ldap: fix `-Wreserved-identifier`.
- mqtt: comment unused macro to avoid warning.
- multi_ev: drop unused macros to avoid warnings.
- setopt: fix useless `break;` after `return;`.
- gtls, mbedtls, rustls: silence `-Wconditional-uninitialized`.
- socks_sspi, schannel, x509asn1: fix `-Wimplicit-int-enum-cast`.
- x509asn1: fix `-Wc++-keyword`.
- openssl: scope `OSSL_UI_METHOD_CAST` to avoid unused macro warning.
- libssh2, wolfssl: drop unused macros.
- curl_ngtcp2, curl_quiche, httpsrr, urlapi: drop/limit unused macros.
- tool_getparam: fix useless `break;` after `return;` or `break;`.
Not normally enabled because it doesn't work with unity.
https://github.com/llvm/llvm-project/issues/71046
- tool_operate: fix `-Wc++-keyword`.
- curlinfo: fix a `-Wunsafe-buffer-usage`.
- tests: silence `-Wformat-non-iso`.
- lib557: fix `-Wreserved-identifier`.
- lib1565: silence `-Wconditional-uninitialized`.
Enable the above clang warnings permanently in picky mode:
- `-Wc++-hidden-decl`
- `-Wc++-keyword` (except for Windows, where it collides with `wchar_t`)
- `-Wcast-function-type-strict`
- `-Wcast-function-type`
- `-Wconditional-uninitialized`
- `-Wformat-non-iso` (except for clang-cl)
- `-Wreserved-identifier`
- `-Wtentative-definition-compat`
Silence problematic `-Weverything` warnings globally (in picky mode):
- `-Wused-but-marked-unused` (88000+ hits) and
`-Wdisabled-macro-expansion` (2600+ hits).
Triggered by `typecheck-gcc.h` when building with clang 14+.
Maybe there exists a way to fix within that header?
Ref: https://discourse.llvm.org/t/removing-wused-but-marked-unused/55310
- `-Wunsafe-buffer-usage`. clang 16+. 7000+ hits.
May be useful in theory, but such high volume of hits makes it
impractical to review and possibly address. Meant for C++.
Ref: https://clang.llvm.org/docs/SafeBuffers.html
Ref: https://stackoverflow.com/questions/77017567/how-to-fix-code-to-avoid-warning-wunsafe-buffer-usage
Ref: https://discourse.llvm.org/t/rfc-c-buffer-hardening/65734
Ref: https://github.com/llvm/llvm-project/pull/111624
- `-Wimplicit-void-ptr-cast`. clang 21+. 1700+ hits.
C++ warning, deemed pure noise.
Ref: https://github.com/curl/curl/issues/18470#issuecomment-3253506266
- `-Wswitch-default` (180+ hits), `-Wswitch-enum` (190+ hits),
`-Wcovered-switch-default` (20+ hits).
Next to impossible to fix cleanly, esp. when the covered `case`
branches depend on compile-time options.
- `-Wdocumentation-unknown-command` (8+ hits).
Triggered in a few sources. Seems arbitrary and bogus.
- `-Wpadded` (550+ hits).
- `-Wc++-keyword` on Windows, where it collides with `wchar_t`.
(100+ hits)
Ref: https://github.com/llvm/llvm-project/issues/155988
- `-Wreserved-macro-identifier`. clang 13+. 5+ hits.
Sometimes it's necessary to set external macros that use
the reserved namespace. E.g. `_CRT_NONSTDC_NO_DEPRECATE`,
`__ENVIRONMENT_MAC_OS_X_VERSION_MIN_REQUIRED__`, `__NO_NET_API`,
possibly `_REENTRANT`, and more.
It's not worth trying to silence them individually.
- `-Wnonportable-system-include-path` with `clang-cl`.
It'd be broken by doing what the warning suggests.
- `-Wformat-non-iso` for clang-cl.
CMake `PICKY_COMPILER=ON` (the default) or `./configure`
`--enable-warnings` (not the default) is required to enable these
silencing rules.
Also:
- autotools, cmake: fix Apple clang and mainline llvm version translations.
Ref: https://en.wikipedia.org/wiki/Xcode#Toolchain_versions
- autotools, cmake: enable `-Warray-compare` for clang 20+.
Follow-up to 4b7accda5a#17196
- cmake: fix to enable `-Wmissing-variable-declarations` at an earlier
clang version.
- cmake: update internal logic to handle warning options with `+` in
them.
- cmake: fix internal logic to match the whole option when looking
into `CMAKE_C_FLAGS` for custom-disabled warnings.
Follow-up to b85cb8cb4e#18485Closes#18477
Rewrite the code that removes a filter from the connection and discards
it. Always look at the connection, otherwise it will not work of the
filter is at the top of the chain.
Change QUIC filter setup code to always tear down the chain in
construction when an error occured.
HTTP proxy, do not remove the h1/h2 sub filter on close. Leave it to be
discarded with the connection. Avoids keeping an additional pointer that
might become dangling.
Triggered by a reported on a code bug in discard method.
Reported-by: Joshua Rogers
Closes#18596
Access the error codes of ngtcp2 when a connect attempt failes. Trace
the information for analysis. Treat errors as permanent failure by
default, trigger retrying only when the server refused without
indicating an error.
Closes#18521
Some OSes (Linux, macOS, more?) will generate an EMSGSIZE socket error
on the next recv all after receiving an ICMP Packet Too Big on an
unconnected UDP socket.
These can be safely ignored as QUIC's DPLPMTUD uses MTU probes that do
not rely on receiving ICMP packets.
Closes#18505
Default timeout is hardcoded (10 seconds) and doesn't respect
--connect-timeout parameter. In some cases 10 seconds can be not enough
or too long to "establish a connection". Moreover the non-working
--connect-timeout parameter for http3 is confusing. This change makes
the handshake timeout equal to --connect-timeout, if it's set.
Discussion is here https://github.com/curl/curl/discussions/18427Closes#18431
- assume:
- `BIO_CTRL_EOF`
- `SSL_CTRL_SET_MSG_CALLBACK`
- `SSL_CTRL_SET_MSG_CALLBACK`
- `SSL_CTRL_SET_TLSEXT_HOSTNAME`
- `SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER`
- `SSL_OP_DONT_INSERT_EMPTY_FRAGMENTS`
- `SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG`
- `SSL_OP_NO_COMPRESSION`
- `SSL_OP_NO_TICKET`
- `X509_V_FLAG_PARTIAL_CHAIN`
- `X509_V_FLAG_TRUSTED_FIRST`
They are present in all supported OpenSSL (and fork) versions.
- replace `SSL_ERROR_WANT_EARLY` with `SSL_ERROR_WANT_CLIENT_HELLO_CB`.
The former appeared in OpenSSL 1.1.1-dev, but renamed before
the stable release.
- document support for macros:
- `ENGINE_CTRL_GET_CMD_FROM_NAME`
- `SSL_ERROR_WANT_ASYNC_JOB`
- `SSL_ERROR_WANT_ASYNC`
- `SSL2_VERSION_MAJOR`
- `TLS1_3_VERSION`
- drop legacy fallback for `CONF_MFLAGS_DEFAULT_SECTION`.
It was there for OpenSSL 0.9.8 support.
- fix `SSL_CTRL_SET_MSG_CALLBACK` accidentally serving as a guard for
OpenSSL (and forks) as a whole.
Tested OK with OpenSSL 1.0.2 and 1.1.0 in CI.
Closes#18351
