Tested with ngtcp2 built against the OpenSSL library. Additionally
tested with MultiSSL (NSS for TLS and ngtcp2+OpenSSL for QUIC).
The TLS backend (independent of QUIC) may or may not already have opened
the keylog file before. Therefore Curl_tls_keylog_open is always called
to ensure the file is open.
Tested following the same curl and tshark commands as in commit
"vtls: Extract and simplify key log file handling from OpenSSL" using
WolfSSL v4.4.0-stable-128-g5179503e8 from git master built with
`./configure --enable-all --enable-debug CFLAGS=-DHAVE_SECRET_CALLBACK`.
Full support for this feature requires certain wolfSSL build options,
see "Availability note" in lib/vtls/wolfssl.c for details.
Closes#5327
Create a set of routines for TLS key log file handling to enable reuse
with other TLS backends. Simplify the OpenSSL backend as follows:
- Drop the ENABLE_SSLKEYLOGFILE macro as it is unconditionally enabled.
- Do not perform dynamic memory allocation when preparing a log entry.
Unless the TLS specifications change we can suffice with a reasonable
fixed-size buffer.
- Simplify state tracking when SSL_CTX_set_keylog_callback is
unavailable. My original sslkeylog.c code included this tracking in
order to handle multiple calls to SSL_connect and detect new keys
after renegotiation (via SSL_read/SSL_write). For curl however we can
be sure that a single master secret eventually becomes available
after SSL_connect, so a simple flag is sufficient. An alternative to
the flag is examining SSL_state(), but this seems more complex and is
not pursued. Capturing keys after server renegotiation was already
unsupported in curl and remains unsupported.
Tested with curl built against OpenSSL 0.9.8zh, 1.0.2u, and 1.1.1f
(`SSLKEYLOGFILE=keys.txt curl -vkso /dev/null https://localhost:4433`)
against an OpenSSL 1.1.1f server configured with:
# Force non-TLSv1.3, use TLSv1.0 since 0.9.8 fails with 1.1 or 1.2
openssl s_server -www -tls1
# Likewise, but fail the server handshake.
openssl s_server -www -tls1 -Verify 2
# TLS 1.3 test. No need to test the failing server handshake.
openssl s_server -www -tls1_3
Verify that all secrets (1 for TLS 1.0, 4 for TLS 1.3) are correctly
written using Wireshark. For the first and third case, expect four
matches per connection (decrypted Server Finished, Client Finished, HTTP
Request, HTTP Response). For the second case where the handshake fails,
expect a decrypted Server Finished only.
tshark -i lo -pf tcp -otls.keylog_file:keys.txt -Tfields \
-eframe.number -eframe.time -etcp.stream -e_ws.col.Info \
-dtls.port==4433,http -ohttp.desegment_body:FALSE \
-Y 'tls.handshake.verify_data or http'
A single connection can easily be identified via the `tcp.stream` field.
For HTTP 1.x, it's a protocol error when the server sends more bytes
than announced. If this happens, don't reuse the connection, because the
start position of the next response is undefined.
Closes#5440
When USE_RESOLVE_ON_IPS is set (defined on macOS), it means that
numerical IP addresses still need to get "resolved" - but not with DoH.
Reported-by: Viktor Szakats
Fixes#5454Closes#5459
They're only limited to the maximum string input restrictions, not to
256 bytes.
Added test 1178 to verify
Reported-by: Will Roberts
Fixes#5448Closes#5449
Fixed the alt-svc parser to treat a newline as end of line.
The unit tests in test 1654 were done without CRLF and thus didn't quite
match the real world. Now they use CRLF as well.
Reported-by: Peter Wu
Assisted-by: Peter Wu
Assisted-by: Jay Satiro
Fixes#5445Closes#5446
This reverts commit 74623551f3.
Instead mark the function call with (void). Getting the return code and
using it instead triggered Coverity warning CID 1463596 because
snprintf() can return a negative value...
Closes#5441
... as returning a "" is not a good idea as the string is supposed to be
allocated and returning a const string will cause issues.
Reported-by: Brian Carpenter
Follow-up to ed35d6590eCloses#5405
This change introduces a generic way to provide binary data in setopt
options, called BLOBs.
This change introduces these new setopts:
CURLOPT_ISSUERCERT_BLOB, CURLOPT_PROXY_SSLCERT_BLOB,
CURLOPT_PROXY_SSLKEY_BLOB, CURLOPT_SSLCERT_BLOB and CURLOPT_SSLKEY_BLOB.
Reviewed-by: Daniel Stenberg
Closes#5357
- Stick to a single unified way to use structs
- Make checksrc complain on 'typedef struct {'
- Allow them in tests, public headers and examples
- Let MD4_CTX, MD5_CTX, and SHA256_CTX typedefs remain as they actually
typedef different types/structs depending on build conditions.
Closes#5338
Previously, after PASV and immediately after the data connection has
connected, the function would only return the control socket to wait for
which then made the data connection simply timeout and not get polled
correctly. This become obvious when running test 1631 and 1632 event-
based.
Use them only if `_UNICODE` is defined, in which case command-line
arguments have been converted to UTF-8.
Closes https://github.com/curl/curl/pull/3784
- use `wmain` instead of `main` when `_UNICODE` is defined [0]
- define `argv_item_t` as `wchar_t *` in this case
- use the curl_multibyte gear to convert the command-line arguments to
UTF-8
This makes it possible to pass parameters with characters outside of
the current locale on Windows, which is required for some tests, e.g.
the IDN tests. Out of the box, this currently only works with the
Visual Studio project files, which default to Unicode, and winbuild
with the `ENABLE_UNICODE` option.
[0] https://devblogs.microsoft.com/oldnewthing/?p=40643
Ref: https://github.com/curl/curl/issues/3747
Closes https://github.com/curl/curl/pull/3784
Fix theoretical integer overflow in Curl_auth_create_plain_message.
The security impact of the overflow was discussed on hackerone. We
agreed this is more of a theoretical vulnerability, as the integer
overflow would only be triggerable on systems using 32-bits size_t with
over 4GB of available memory space for the process.
Closes#5391
Since input passed to libcurl with CURLOPT_USERPWD and
CURLOPT_PROXYUSERPWD circumvents the regular string length check we have
in Curl_setstropt(), the input length limit is enforced in
Curl_parse_login_details too, separately.
Reported-by: Thomas Bouzerar
Closes#5383
When looking for a protocol match among supported schemes, check the
most "popular" schemes first. It has zero functionality difference and
for all practical purposes a speed difference will not be measureable
but it still think it makes sense to put the least likely matches last.
"Popularity" based on the 2019 user survey.
Closes#5377
Add three new CMake Find modules (using the curl license, but I grant
others the right to apply the CMake BSD license instead).
This CMake config is simpler than the autotools one because it assumes
ngtcp2 and nghttp3 to be used together. Another difference is that this
CMake config checks whether QUIC is actually supported by the TLS
library (patched OpenSSL or boringssl) since this can be a common
configuration mistake that could result in build errors later.
Unlike autotools, CMake does not warn you that the features are
experimental. The user is supposed to already know that and read the
documentation. It requires a very special build environment anyway.
Tested with ngtcp2+OpenSSL+nghttp3 and quiche+boringssl, both built from
current git master. Use `LD_DEBUG=files src/curl |& grep need` to figure
out which features (libldap-2.4, libssh2) to disable due to conflicts
with boringssl.
Closes#5359
If the QLOGDIR environment variable is set, enable qlogging.
... and create Curl_qlogdir() in the new generic vquic/vquic.c file for
QUIC functions that are backend independent.
Closes#5353
That return code is reserved for build-time conditional code not being
present while this was a regular run-time error from a Windows API.
Reported-by: wangp on github
Fixes#5349Closes#5350
Triggered by a crash detected by OSS-Fuzz after the dynbuf introduction in
ed35d6590e. This should make the trailer handling more straight forward and
hopefully less error-prone.
Deliver the trailer header to the callback already at receive-time. No
longer caches the trailers to get delivered at end of stream.
Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=22030Closes#5348
In my very basic test that lists sftp://127.0.0.1/tmp/, this patched
code makes 161 allocations compared to 194 in git master. A 17%
reduction.
Closes#5336
quiche has the potential to log qlog files. To enable this, you must
build quiche with the qlog feature enabled `cargo build --features
qlog`. curl then passes a file descriptor to quiche, which takes
ownership of the file. The FD transfer only works on UNIX.
The convention is to enable logging when the QLOGDIR environment is
set. This should be a path to a folder where files are written with the
naming template <SCID>.qlog.
Co-authored-by: Lucas Pardue
Replaces #5337Closes#5341
A common set of functions instead of many separate implementations for
creating buffers that can grow when appending data to them. Existing
functionality has been ported over.
In my early basic testing, the total number of allocations seem at
roughly the same amount as before, possibly a few less.
See docs/DYNBUF.md for a description of the API.
Closes#5300
- Check for NULL entry parameter before attempting to deref entry in
Curl_resolver_is_resolved, like is already done in asyn-ares.
This is to silence cppcheck which does not seem to understand that
asyn-ares and asyn-thread have separate Curl_resolver_is_resolved
and those units are mutually exclusive. Prior to this change it warned
of a scenario where asyn-thread's Curl_resolver_is_resolved is called
with a NULL entry from asyn-ares, but that couldn't happen.
Reported-by: rl1987@users.noreply.github.com
Fixes https://github.com/curl/curl/issues/5326
More connection cache accesses are protected by locks.
CONNCACHE_* is a beter prefix for the connection cache lock macros.
Curl_attach_connnection: now called as soon as there's a connection
struct available and before the connection is added to the connection
cache.
Curl_disconnect: now assumes that the connection is already removed from
the connection cache.
Ref: #4915Closes#5009
Regression since 7.69.0 and 68fb25fa3f.
The code wrongly assigned 'from' instead of 'auth' which probably was a
copy and paste mistake from other code, leading to that auth could
remain NULL and later cause an error to be returned.
Assisted-by: Eric Sauvageau
Fixes#5294Closes#5295
Previously, options set explicitly through command line options could be
overridden by the configuration files parsed automatically when
ssh_connect() was called.
By calling ssh_options_parse_config() explicitly, the configuration
files are parsed before setting the options, avoiding the options
override. Once the configuration files are parsed, the automatic
configuration parsing is not executed.
Fixes#4972Closes#5283
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Coverity found CID 1461718:
Integer handling issues (CONSTANT_EXPRESSION_RESULT) "timeout_ms >
9223372036854775807L" is always false regardless of the values of its
operands. This occurs as the logical second operand of "||".
Closes#5240
Prior to this change if there was a 303 reply to a PUT request then
the subsequent request to respond to that redirect would also be a PUT.
It was determined that was most likely incorrect based on the language
of the RFCs. Basically 303 means "see other" resource, which implies it
is most likely not the same resource, therefore we should not try to PUT
to that different resource.
Refer to the discussions in #5237 and #5248 for more information.
Fixes https://github.com/curl/curl/issues/5237
Closes https://github.com/curl/curl/pull/5248
GnuTLS 3.1.10 added new functions we want to use. That version was
released on Mar 22, 2013. Removing support for older versions also
greatly simplifies the code.
Ref: #5271Closes#5276
Detected by Coverity. CID 1462319.
"The same code is executed when the condition result is true or false,
because the code in the if-then branch and after the if statement is
identical."
Closes#5275
When cURL is compiled with support for multiple SSL backends, it is
possible to configure an SSL backend via `curl_global_sslset()`, but
only *before* `curl_global_init()` was called.
If another SSL backend should be used after that, a user might be
tempted to call `curl_global_cleanup()` to start over. However, we did
not foresee that use case and forgot to reset the SSL backend in that
cleanup.
Let's allow that use case.
Fixes#5255Closes#5257
Reported-by: davidedec on github
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
From libssh 0.9.0, ssh_key_type() returns different key types for ECDSA
keys depending on the curve.
Signed-off-by: Anderson Toshiyuki Sasaki <ansasaki@redhat.com>
Fixes#5252Closes#5253
- Fix schannel_send for the case when no timeout was set.
Prior to this change schannel would error if the socket was not ready
to send data and no timeout was set.
This commit is similar to parent commit 89dc6e0 which recently made the
same change for SOCKS, for the same reason. Basically it was not well
understood that when Curl_timeleft returns 0 it is not a timeout of 0 ms
but actually means no timeout.
Fixes https://github.com/curl/curl/issues/5177
Closes https://github.com/curl/curl/pull/5221
- Document in Curl_timeleft's comment block that returning 0 signals no
timeout (ie there's infinite time left).
- Fix SOCKS' Curl_blockread_all for the case when no timeout was set.
Prior to this change if the timeout had a value of 0 and that was passed
to SOCKET_READABLE it would return right away instead of blocking. That
was likely because it was not well understood that when Curl_timeleft
returns 0 it is not a timeout of 0 ms but actually means no timeout.
Ref: https://github.com/curl/curl/pull/5214#issuecomment-612512360
Closes https://github.com/curl/curl/pull/5220
Prior to this change gopher's blocking code would block forever,
ignoring any set timeout value.
Assisted-by: Jay Satiro
Reviewed-by: Daniel Stenberg
Similar to #5220 and #5221Closes#5214
When SRP is requested in the priority string, GnuTLS will disable
support for TLS 1.3. Before this change, curl would always add +SRP to
the priority list, effectively always disabling TLS 1.3 support.
With this change, +SRP is only added to the priority list when SRP
authentication is also requested. This also allows updating the error
handling here to not have to retry without SRP. This is because SRP is
only added when requested and in that case a retry is not needed.
Closes#5223
- If loss of data may occur converting a timediff_t to time_t and
the time value is > TIME_T_MAX then treat it as TIME_T_MAX.
This is a follow-up to 8843678 which removed the (time_t) typecast
from the macros so that conversion warnings could be identified.
Closes https://github.com/curl/curl/pull/5199
In a debug build, settting the environment variable "CURL_SMALLREQSEND"
will make the first HTTP request send not send more bytes than the set
amount, thus ending up verifying that the logic for handling a split
HTTP request send works correctly.
Restores the --head functionality to the curl utility which extracts
'protocol' that is stored that way.
Reported-by: James Fuller
Fixes#5196Closes#5198
