Factor out common I/O loop as bearssl_run_until, which reads/writes TLS
records until the desired engine state is reached. This is now used for
the handshake, read, write, and close.
Match OpenSSL SSL_write behavior, and don't return the number of bytes
written until the corresponding records have been completely flushed
across the socket. This involves keeping track of the length of data
buffered into the TLS engine, and assumes that when CURLE_AGAIN is
returned, the write function will be called again with the same data
and length arguments. This is the same requirement of SSL_write.
Handle TLS close notify as EOF when reading by returning 0.
Closes https://github.com/curl/curl/pull/4748
Have intermediate certificates in the trust store be treated as
trust-anchors, in the same way as self-signed root CA certificates
are. This allows users to verify servers using the intermediate cert
only, instead of needing the whole chain.
Other TLS backends already accept partial chains.
Reported-by: Jeffrey Walton
Bug: https://curl.haxx.se/mail/lib-2019-11/0094.html
- Disable warning C4127 "conditional expression is constant" globally
in curl_setup.h for when building with Microsoft's compiler.
This mainly affects building with the Visual Studio project files found
in the projects dir.
Prior to this change the cmake and winbuild build systems already
disabled 4127 globally for when building with Microsoft's compiler.
Also, 4127 was already disabled for all build systems in the limited
circumstance of the WHILE_FALSE macro which disabled the warning
specifically for while(0). This commit removes the WHILE_FALSE macro and
all other cruft in favor of disabling globally in curl_setup.
Background:
We have various macros that cause 0 or 1 to be evaluated, which would
cause warning C4127 in Visual Studio. For example this causes it:
#define Curl_resolver_asynch() 1
Full behavior is not clearly defined and inconsistent across versions.
However it is documented that since VS 2015 Update 3 Microsoft has
addressed this somewhat but not entirely, not warning on while(true) for
example.
Prior to this change some C4127 warnings occurred when I built with
Visual Studio using the generated projects in the projects dir.
Closes https://github.com/curl/curl/pull/4658
- In all code call Curl_winapi_strerror instead of Curl_strerror when
the error code is known to be from Windows GetLastError.
Curl_strerror prefers CRT error codes (errno) over Windows API error
codes (GetLastError) when the two overlap. When we know the error code
is from GetLastError it is more accurate to prefer the Windows API error
messages.
Reported-by: Richard Alcock
Fixes https://github.com/curl/curl/issues/4550
Closes https://github.com/curl/curl/pull/4581
Prior to this change schannel ignored --tls-max (CURL_SSLVERSION_MAX_
macros) when --tlsv1 (CURL_SSLVERSION_TLSv1) or default TLS
(CURL_SSLVERSION_DEFAULT), using a max of TLS 1.2 always.
Closes https://github.com/curl/curl/pull/4633
- Disable the extra sensitivity except in debug builds (--enable-debug).
- Improve SYSCALL error message logic in ossl_send and ossl_recv so that
"No error" / "Success" socket error text isn't shown on SYSCALL error.
Prior to this change 0ab38f5 (precedes 7.67.0) increased the sensitivity
of OpenSSL's SSL_ERROR_SYSCALL error so that abrupt server closures were
also considered errors. For example, a server that does not send a known
protocol termination point (eg HTTP content length or chunked encoding)
_and_ does not send a TLS termination point (close_notify alert) would
cause an error if it closed the connection.
To be clear that behavior made it into release build 7.67.0
unintentionally. Several users have reported it as an issue.
Ultimately the idea is a good one, since it can help prevent against a
truncation attack. Other SSL backends may already behave similarly (such
as Windows native OS SSL Schannel). However much more of our user base
is using OpenSSL and there is a mass of legacy users in that space, so I
think that behavior should be partially reverted and then rolled out
slowly.
This commit changes the behavior so that the increased sensitivity is
disabled in all curl builds except curl debug builds (DEBUGBUILD). If
after a period of time there are no major issues then it can be enabled
in dev and release builds with the newest OpenSSL (1.1.1+), since users
using the newest OpenSSL are the least likely to have legacy problems.
Bug: https://github.com/curl/curl/issues/4409#issuecomment-555955794
Reported-by: Bjoern Franke
Fixes https://github.com/curl/curl/issues/4624
Closes https://github.com/curl/curl/pull/4623
- Open the CA file using FILE_SHARE_READ mode so that others can read
from it as well.
Prior to this change our schannel code opened the CA file without
sharing which meant concurrent openings (eg an attempt from another
thread or process) would fail during the time it was open without
sharing, which in curl's case would cause error:
"schannel: failed to open CA file".
Bug: https://curl.haxx.se/mail/lib-2019-10/0104.html
Reported-by: Richard Alcock
... as it can make it wait there for a long time for no good purpose.
Patched-by: Jay Satiro
Reported-by: Bylon2 on github
Adviced-by: Nikos Mavrogiannopoulos
Fixes#4487Closes#4541
It was already fixed for BoringSSL in commit a0f8fccb1e.
LibreSSL has had the second argument to SSL_CTX_set_min_proto_version
as uint16_t ever since the function was added in [0].
[0] 56f107201b
Closes https://github.com/curl/curl/pull/4397
For FTPS transfers, curl gets close_notify on the data connection
without that being a signal to close the control connection!
Regression since 3f5da4e59a (7.65.0)
Reported-by: Zenju on github
Reviewed-by: Jay Satiro
Fixes#4329Closes#4340
OpenSSL 1.1.0 adds SSL_CTX_set_<min|max>_proto_version() that we now use
when available. Existing code is preserved for older versions of
OpenSSL.
Closes#4304
SSL_VersionRangeGetDefault returns (TLSv1.0, TLSv1.2) as supported
range in NSS 3.45. It looks like the intention is to raise the minimum
version rather than lowering the maximum, so adjust accordingly. Note
that the caller (nss_setup_connect) initializes the version range to
(TLSv1.0, TLSv1.3), so there is no need to check for >= TLSv1.0 again.
Closes#4187
Reviewed-by: Daniel Stenberg
Reviewed-by: Kamil Dudka
It was used (intended) to pass in the size of the 'socks' array that is
also passed to these functions, but was rarely actually checked/used and
the array is defined to a fixed size of MAX_SOCKSPEREASYHANDLE entries
that should be used instead.
Closes#4169
Several reasons:
- we can't add everyone who's helping out so its unfair to just a few
selected ones.
- we already list all helpers in THANKS and in RELEASE-NOTES for each
release
- we don't want to give the impression that some parts of the code is
"owned" or "controlled" by specific persons
Assisted-by: Daniel Gustafsson
Closes#4129
PK11_IsPresent() checks for the token for the given slot is available,
and sets needlogin flags for the PK11_Authenticate() call. Should it
return false, we should however treat it as an error and bail out.
Closes https://github.com/curl/curl/pull/4110
The file suffix for dynamically loadable objects on macOS is .dylib,
which need to be added for the module definitions in order to get the
NSS TLS backend to work properly on macOS.
Closes https://github.com/curl/curl/pull/4046
The value of the maxPTDs parameter to PR_Init() has since at least
NSPR 2.1, which was released sometime in 1998, been marked ignored
as is accordingly not used in the initialization code. Setting it
to a value when calling PR_Init() is thus benign, but indicates an
intent which may be misleading. Reset the value to zero to improve
clarity.
Closes https://github.com/curl/curl/pull/4054
Change the logic around such that we only keep CRLs that NSS actually
ended up caching around for later deletion. If CERT_CacheCRL() fails
then there is little point in delaying the freeing of the CRL as it
is not used.
Closes https://github.com/curl/curl/pull/4053
... since that needs UI_OpenSSL() which isn't provided when OpenSSL is
built with OPENSSL_NO_UI_CONSOLE which happens when OpenSSL is built for
UWP (with "VC-WIN32-UWP").
Reported-by: Vasily Lobaskin
Fixes#4073Closes#4077
Certinfo gives the same result for all OpenSSL versions.
Also made printing RSA pubkeys consistent with older versions.
Reported-by: Michael Wallner
Fixes#3706Closes#4030
OpenSSL used to call exit(1) on syntax errors in OPENSSL_config(),
which is why we switched to CONF_modules_load_file() and introduced
a comment stating why. This behavior was however changed in OpenSSL
commit abdd677125f3a9e3082f8c5692203590fdb9b860, so remove the now
outdated and incorrect comment. The mentioned commit also declares
OPENSSL_config() deprecated so keep the current coding.
Closes#4033
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
Remove support for, references to and use of "cyaSSL" from the source
and docs. wolfSSL is the current name and there's no point in keeping
references to ancient history.
Assisted-by: Daniel Gustafsson
Closes#3903
They serve very little purpose and mostly just add noise. Most of them
have been around for a very long time. I read them all before removing
or rephrasing them.
Ref: #3876Closes#3883
In Curl_multissl_version() it was possible to overflow the passed in
buffer if the generated version string exceeded the size of the buffer.
Fix by inverting the logic, and also make sure to not exceed the local
buffer during the string generation.
Closes#3863
Reported-by: nevv on HackerOne/curl
Reviewed-by: Jay Satiro
Reviewed-by: Daniel Stenberg
Older versions of OpenSSL report FIPS availabilty via an OPENSSL_FIPS
define. It uses this define to determine whether to publish -fips at
the end of the version displayed. Applications that utilize the version
reported by OpenSSL will see a mismatch if they compare it to what curl
reports, as curl is not modifying the version in the same way. This
change simply adds a check to see if OPENSSL_FIPS is defined, and will
alter the reported version to match what OpenSSL itself provides. This
only appears to be applicable in versions of OpenSSL <1.1.1
Closes#3771
Currently you can do things like --cert <(cat ./cert.crt) with (at least) the
openssl backend, but that doesn't work for nss because is_file rejects fifos.
I don't actually know if this is sufficient, nss might do things internally
(like seeking back) that make this not work, so actual testing is needed.
Closes#3807
As soon as a TLS backend gets ALPN conformation about the specific HTTP
version it can now set the multiplex situation for the "bundle" and
trigger moving potentially queued up transfers to the CONNECT state.
Without this, detecting and avoid reusing a closed TLS connection
(without a previous GOAWAY) when doing HTTP/2 is tricky.
Reported-by: Tom van der Woerdt
Fixes#3750Closes#3763
- remove unused variables
- declare conditionally used variables conditionally
- suppress unused variable warnings in the CMake tests
- remove dead variable stores
- consistently use WIN32 macro to detect Windows
Closes https://github.com/curl/curl/pull/3739
Make functions no-ops if neither both USE_THREADS_POSIX and
HAVE_PTHREAD_H nor both USE_THREADS_WIN32 and HAVE_PROCESS_H are
defined. Previously, if only one of them was defined, there was either
code compiled that did nothing useful or the wrong header included for
the functions used.
Also, move POLARSSL_MUTEX_T define to implementation file as it's not
used externally.
Closes https://github.com/curl/curl/pull/3739
AmiSSL is an Amiga native library which provides a wrapper over OpenSSL.
It also requires all programs using it to use bsdsocket.library
directly, rather than accessing socket functions through clib, which
libcurl was not necessarily doing previously. Configure will now check
for the headers and ensure they are included if found.
Closes#3677
Instead of using a fixed 256 byte buffer in the connectdata struct.
In my build, this reduces the size of the connectdata struct by 11.8%,
from 2160 to 1904 bytes with no functionality or performance loss.
This also fixes a bug in schannel's Curl_verify_certificate where it
called Curl_sspi_strerror when it should have called Curl_strerror for
string from GetLastError. the only effect would have been no text or the
wrong text being shown for the error.
Co-authored-by: Jay Satiro
Closes#3612
Add support for Ephemeral elliptic curve Diffie-Hellman key exchange
algorithm option when selecting ciphers. This became available on the
Win10 SDK.
Closes https://github.com/curl/curl/pull/3608
If mbedtls_ssl_get_session() fails, it may still have allocated
memory that needs to be freed to avoid leaking. Call the library
API function to release session resources on this errorpath as
well as on Curl_ssl_addsessionid() errors.
Closes: #3574
Reported-by: Michał Antoniak <M.Antoniak@posnet.com>
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
Convert numerous infof() calls into debug-build only messages since they
are annoyingly verbose for regular applications. Removed a few.
Bug: https://curl.haxx.se/mail/lib-2019-02/0027.html
Reported-by: Volker Schmid
Closes#3552
Attempt to add support for Secure Channel binding when negotiate
authentication is used. The problem to solve is that by default IIS
accepts channel binding and curl doesn't utilise them. The result was a
401 response. Scope affects only the Schannel(winssl)-SSPI combination.
Fixes https://github.com/curl/curl/issues/3503
Closes https://github.com/curl/curl/pull/3509
Stick to "Schannel" everywhere. The configure option --with-winssl is
kept to allow existing builds to work but --with-schannel is added as an
alias.
Closes#3504
Windows extended potection (aka ssl channel binding) is required
to login to ntlm IIS endpoint, otherwise the server returns 401
responses.
Fixes#3280Closes#3321
.... to not pass in a const in the second argument as that's not how it
is supposed to be used and might cause compiler warnings.
Reported-by: Pavel Pavlov
Fixes#3477Closes#3478
When building with Unicode on MSVC, the compiler warns about freeing a
pointer to const in Curl_unicodefree. Fix this by declaring it as
non-const and casting the argument to Curl_convert_UTF8_to_tchar to
non-const too, like we do in all other places.
Closes https://github.com/curl/curl/pull/3435
This adds a cleanup callback for cyassl. Resolves possible memory leak
when using ECC fixed point cache.
Closes#3395
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
It is allowed to call that function with id set to -1, specifying the
backend by the name instead. We should imitate what is done further down
in that function to allow for that.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Closes#3346
NSS may be built without support for the latest SSL/TLS versions,
leading to "SSL version range is not valid" errors when the library
code supports a recent version (e.g. TLS v1.3) but it has explicitly
been disabled.
This change adjusts the maximum SSL version requested by libcurl to
be the maximum supported version at runtime, as long as that version
is at least as high as the minimum version required by libcurl.
Fixes#3261
The function does not return the same value as snprintf() normally does,
so readers may be mislead into thinking the code works differently than
it actually does. A different function name makes this easier to detect.
Reported-by: Tomas Hoger
Assisted-by: Daniel Gustafsson
Fixes#3296Closes#3297
Session resumption information is not available immediately after a TLS 1.3
handshake. The client must wait until the server has sent a session ticket.
Use OpenSSL's "new session" callback to get the session information and put it
into curl's session cache. For TLS 1.3 sessions, this callback will be invoked
after the server has sent a session ticket.
The "new session" callback is invoked only if OpenSSL's session cache is
enabled, so enable it and use the "external storage" mode which lets curl manage
the contents of the session cache.
A pointer to the connection data and the sockindex are now saved as "SSL extra
data" to make them available to the callback.
This approach also works for old SSL/TLS versions and old OpenSSL versions.
Reviewed-by: Daniel Stenberg <daniel@haxx.se>
Fixes#3202Closes#3271
Since we're close to feature freeze, this change disables this feature
with an #ifdef. Define ALLOW_RENEG at build-time to enable.
This could be converted to a bit for CURLOPT_SSL_OPTIONS to let
applications opt-in this.
Concern-raised-by: David Benjamin
Fixes#3283Closes#3293
