Sessionid cache management is inseparable from managing individual
session lifetimes. E.g. for reference-counted sessions (like those in
SChannel and OpenSSL engines) every session addition and removal
should be accompanied with refcount increment and decrement
respectively. Failing to do so synchronously leads to a race condition
that causes symptoms like use-after-free and memory corruption.
This commit:
- makes existing session cache locking explicit, thus allowing
individual engines to manage lock's scope.
- fixes OpenSSL and SChannel engines by putting refcount management
inside this lock's scope in relevant places.
- adds these explicit locking calls to other engines that use
sessionid cache to accommodate for this change. Note, however,
that it is unknown whether any of these engines could also have
this race.
Bug: https://github.com/curl/curl/issues/815Fixes#815Closes#847
... introduced in curl-7_48_0-293-g2968c83:
Error: COMPILER_WARNING:
lib/vtls/openssl.c: scope_hint: In function ‘Curl_ossl_check_cxn’
lib/vtls/openssl.c:767:15: warning: conversion to ‘int’ from ‘ssize_t’
may alter its value [-Wconversion]
- In the case of recv error, limit returning 'connection still in place'
to EINPROGRESS, EAGAIN and EWOULDBLOCK.
This is an improvement on the parent commit which changed the openssl
connection check to use recv MSG_PEEK instead of SSL_peek.
Ref: https://github.com/curl/curl/commit/856baf5#comments
Calling SSL_peek can cause bytes to be read from the raw socket which in
turn can upset the select machinery that determines whether there's data
available on the socket.
Since Curl_ossl_check_cxn only tries to determine whether the socket is
alive and doesn't actually need to see the bytes SSL_peek seems like
the wrong function to call.
We're able to occasionally reproduce a connect timeout due to this
bug. What happens is that Curl doesn't know to call SSL_connect again
after the peek happens since data is buffered in the SSL buffer and thus
select won't fire for this socket.
Closes#795
Only protocols that actually have a protocol registered for ALPN and NPN
should try to get that negotiated in the TLS handshake. That is only
HTTPS (well, http/1.1 and http/2) right now. Previously ALPN and NPN
would wrongly be used in all handshakes if libcurl was built with it
enabled.
Reported-by: Jay Satiro
Fixes#789
When compiling with OpenSSL 1.1.0 (so that the HAVE_X509_GET0_SIGNATURE
&& HAVE_X509_GET0_EXTENSIONS pre-processor block is active), Visual C++
14 complains:
warning C4701: potentially uninitialized local variable 'palg' used
warning C4701: potentially uninitialized local variable 'psig' used
... to allow users to see which specfic wildcard that matched when such
is used.
Also minor logic cleanup to simplify the code, and I removed all tabs
from verbose strings.
As of https://boringssl-review.googlesource.com/#/c/6980/, almost all of
BoringSSL #ifdefs in cURL should be unnecessary:
- BoringSSL provides no-op stubs for compatibility which replaces most
#ifdefs.
- DES_set_odd_parity has been in BoringSSL for nearly a year now. Remove
the compatibility codepath.
- With a small tweak to an extend_key_56_to_64 call, the NTLM code
builds fine.
- Switch OCSP-related #ifdefs to the more generally useful
OPENSSL_NO_OCSP.
The only #ifdefs which remain are Curl_ossl_version and the #undefs to
work around OpenSSL and wincrypt.h name conflicts. (BoringSSL leaves
that to the consumer. The in-header workaround makes things sensitive to
include order.)
This change errs on the side of removing conditionals despite many of
the restored codepaths being no-ops. (BoringSSL generally adds no-op
compatibility stubs when possible. OPENSSL_VERSION_NUMBER #ifdefs are
bad enough!)
Closes#640
When trying to verify a peer without having any root CA certificates
set, this makes libcurl use the TLS library's built in default as
fallback.
Closes#569
... by extracting the LIB + REASON from the OpenSSL error code. OpenSSL
1.1.0+ returned a new func number of another cerfificate fail so this
required a fix and this is the better way to catch this error anyway.
They tend to never get updated anyway so they're frequently inaccurate
and we never go back to revisit them anyway. We document issues to work
on properly in KNOWN_BUGS and TODO instead.
BoringSSL implements `BIO_get_mem_data` as a function, instead of a
macro, and expects the output pointer to be a `char **`. We have to add
an explicit cast to grab the pointer as a `const char **`.
Closes#524
This reverts commit 370ee919b3.
Issue #509 has all the details but it was confirmed that the crash was
not due to this, so the previous commit was wrong.
sk_X509_pop will decrease the size of the stack which means that the loop would
end after having added only half of the certificates.
Also make sure that the X509 certificate is freed in case
SSL_CTX_add_extra_chain_cert fails.
- Change algorithm init to happen after OpenSSL config load.
Additional algorithms may be available due to the user's config so we
initialize the algorithms after the user's config is loaded.
Bug: https://github.com/bagder/curl/issues/447
Reported-by: Denis Feklushkin
If strict certificate checking is disabled (CURLOPT_SSL_VERIFYPEER
and CURLOPT_SSL_VERIFYHOST are disabled) do not fail if the server
doesn't present a certificate at all.
Closes#392
MSVC 12 complains:
lib\vtls\openssl.c(1554): warning C4701: potentially uninitialized local
variable 'verstr' used It's a false positive, but as it's normally not,
I have enabled warning-as-error for that warning.
setup-vms.h: More symbols for SHA256, hacks for older VAX
openssl.h: Use OpenSSL OPENSSL_NO_SHA256 macro to allow building on VAX.
openssl.c: Use OpenSSL version checks and OPENSSL_NO_SHA256 macro to
allow building on VAX and 64 bit VMS.
Make sure that the error buffer is always initialized and simplify the
use of it to make the logic easier.
Bug: https://github.com/bagder/curl/issues/318
Reported-by: sneis
The symbol SSL3_MT_NEWSESSION_TICKET appears to have been introduced at
around openssl 0.9.8f, and the use of it in lib/vtls/openssl.c breaks
builds with older openssls (certainly with 0.9.8b, which is the latest
older version I have to try with).
- Try building a chain using issuers in the trusted store first to avoid
problems with server-sent legacy intermediates.
Prior to this change server-sent legacy intermediates with missing
legacy issuers would cause verification to fail even if the client's CA
bundle contained a valid replacement for the intermediate and an
alternate chain could be constructed that would verify successfully.
https://rt.openssl.org/Ticket/Display.html?id=3621&user=guest&pass=guest
- Change fopen calls to use FOPEN_READTEXT instead of "r" or "rt"
- Change fopen calls to use FOPEN_WRITETEXT instead of "w" or "wt"
This change is to explicitly specify when we need to read/write text.
Unfortunately 't' is not part of POSIX fopen so we can't specify it
directly. Instead we now have FOPEN_READTEXT, FOPEN_WRITETEXT.
Prior to this change we had an issue on Windows if an application that
uses libcurl overrides the default file mode to binary. The default file
mode in Windows is normally text mode (translation mode) and that's what
libcurl expects.
Bug: https://github.com/bagder/curl/pull/258#issuecomment-107093055
Reported-by: Orgad Shaneh
BoringSSL removed support for direct callers of SSL_CTX_callback_ctrl
and SSL_CTX_ctrl, so move to a way that should work on BoringSSL and
OpenSSL.
re #275
The OpenSSL trace callback is wonderfully undocumented but given a
journey in the source code, it seems the cases were ssl_ver is zero
doesn't follow the same pattern and thus turned out confusing and
misleading. For now, we skip doing any CURLINFO_TEXT logging on those
but keep sending them as CURLINFO_SSL_DATA_OUT/IN.
Also, I added direction to the text info and I edited some functions
slightly.
Bug: https://github.com/bagder/curl/issues/219
Reported-by: Jay Satiro, Ashish Shukla
This header file must be included after all header files except
memdebug.h, as it does similar memory function redefinitions and can be
similarly affected by conflicting definitions in system or dependent
library headers.
SSLeay was the name of the library that was subsequently turned into
OpenSSL many moons ago (1999). curl does not work with the old SSLeay
library since years. This is now reflected by only using USE_OPENSSL in
code that depends on OpenSSL.
For consistency with other conditionally compiled code in openssl.c,
use OPENSSL_IS_BORINGSSL rather than HAVE_BORINGSSL and try to use
HAVE_BORINGSSL outside of openssl.c when the OpenSSL header files are
not included.
The struct went private in 1.0.2 so we cannot read the version number
from there anymore. Use SSL_version() instead!
Reported-by: Gisle Vanem
Bug: http://curl.haxx.se/mail/lib-2015-02/0034.html
Modified the Curl_ossl_cert_status_request() function to return FALSE
when built with BoringSSL or when OpenSSL is missing the necessary TLS
extensions.
