This commit is several drafts squashed together. The changes from each
draft are noted below. If any changes are similar and possibly
contradictory the change in the latest draft takes precedence.
Bug: https://github.com/bagder/curl/issues/244
Reported-by: Chris Araman
%%
%% Draft 1
%%
- return 0 if len == 0. that will have to be documented.
- continue on and process the caches regardless of raw recv
- if decrypted data will be returned then set the error code to CURLE_OK
and return its count
- if decrypted data will not be returned and the connection has closed
(eg nread == 0) then return 0 and CURLE_OK
- if decrypted data will not be returned and the connection *hasn't*
closed then set the error code to CURLE_AGAIN --only if an error code
isn't already set-- and return -1
- narrow the Win2k workaround to only Win2k
%%
%% Draft 2
%%
- Trying out a change in flow to handle corner cases.
%%
%% Draft 3
%%
- Back out the lazier decryption change made in draft2.
%%
%% Draft 4
%%
- Some formatting and branching changes
- Decrypt all encrypted cached data when len == 0
- Save connection closed state
- Change special Win2k check to use connection closed state
%%
%% Draft 5
%%
- Default to CURLE_AGAIN in cleanup if an error code wasn't set and the
connection isn't closed.
%%
%% Draft 6
%%
- Save the last error only if it is an unrecoverable error.
Prior to this I saved the last error state in all cases; unfortunately
the logic to cover that in all cases would lead to some muddle and I'm
concerned that could then lead to a bug in the future so I've replaced
it by only recording an unrecoverable error and that state will persist.
- Do not recurse on renegotiation.
Instead we'll continue on to process any trailing encrypted data
received during the renegotiation only.
- Move the err checks in cleanup after the check for decrypted data.
In either case decrypted data is always returned but I think it's easier
to understand when those err checks come after the decrypted data check.
%%
%% Draft 7
%%
- Regardless of len value go directly to cleanup if there is an
unrecoverable error or a close_notify was already received. Prior to
this change we only acknowledged those two states if len != 0.
- Fix a bug in connection closed behavior: Set the error state in the
cleanup, because we don't know for sure it's an error until that time.
- (Related to above) In the case the connection is closed go "greedy"
with the decryption to make sure all remaining encrypted data has been
decrypted even if it is not needed at that time by the caller. This is
necessary because we can only tell if the connection closed gracefully
(close_notify) once all encrypted data has been decrypted.
- Do not renegotiate when an unrecoverable error is pending.
%%
%% Draft 8
%%
- Don't show 'server closed the connection' info message twice.
- Show an info message if server closed abruptly (missing close_notify).
Some servers will request a client certificate, but not require one.
This change allows libcurl to connect to such servers when using
schannel as its ssl/tls backend. When a server requests a client
certificate, libcurl will now continue the handshake without one,
rather than terminating the handshake. The server can then decide
if that is acceptable or not. Prior to this change, libcurl would
terminate the handshake, reporting a SEC_I_INCOMPLETE_CREDENTIALS
error.
- 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
Stop curl from failing when non-fatal alert is received during
handshake. This e.g. fixes lots of problems when working with https
sites through proxies.
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
Prior to this change libcurl could show multiple 'CyaSSL: Connecting to'
messages since cyassl_connect_step2 is called multiple times, typically.
The message is superfluous even once since libcurl already informs the
user elsewhere in code that it is connecting.
This change is to allow the user's CTX callback to change the minimum
protocol version in the CTX without us later overriding it, as we did
prior to this change.
SSL_CTX_load_verify_locations can return negative values on fail,
therefore to check for failure we check if load is != 1 (success)
instead of if load is == 0 (failure), the latter being incorrect given
that behavior.
(Curl_cyassl_init)
- Return 1 on success, 0 in failure.
Prior to this change the fail path returned an incorrect value and the
evaluation to determine whether CyaSSL_Init had succeeded was incorrect.
Ironically that combined with the way curl_global_init tests SSL library
initialization (!Curl_ssl_init()) meant that CyaSSL having been
successfully initialized would be seen as that even though the code path
and return value in Curl_cyassl_init were wrong.
(cyassl_connect_step1)
- Use TLS 1.0-1.2 by default when available.
CyaSSL/wolfSSL >= v3.3.0 supports setting a minimum protocol downgrade
version.
cyassl/cyassl@322f79f
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.
The function "free" is documented in the way that no action shall occur for
a passed null pointer. It is therefore not needed that a function caller
repeats a corresponding check.
http://stackoverflow.com/questions/18775608/free-a-null-pointer-anyway-or-check-first
This issue was fixed by using the software Coccinelle 1.0.0-rc24.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
This makes curl pick better (stronger) ciphers by default. The strongest
available ciphers are fine according to the HTTP/2 spec so an OpenSSL
built curl is no longer rejected by string HTTP/2 servers.
Bug: http://curl.haxx.se/bug/view.cgi?id=1487
Instead of priting cipher and MAC algorithms names separately, print the
whole cipher suite string which also includes the key exchange algorithm,
along with the negotiated TLS version.
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.
The vtls layer now checks the return value, so it is no longer necessary
to abort if a random number cannot be provided by NSS. This also fixes
the following Coverity report:
Error: FORWARD_NULL (CWE-476):
lib/vtls/nss.c:1918: var_compare_op: Comparing "data" to null implies that "data" might be null.
lib/vtls/nss.c:1923: var_deref_model: Passing null pointer "data" to "Curl_failf", which dereferences it.
lib/sendf.c:154:3: deref_parm: Directly dereferencing parameter "data".
Correctly check for memcmp() return value (it returns 0 if the strings match).
This is not really important, since curl is going to use http/1.1 anyway, but
it's still a bug I guess.
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.
Prior to this change the options for exclusive SSL protocol versions did
not actually set the protocol exclusive.
http://curl.haxx.se/mail/lib-2015-01/0002.html
Reported-by: Dan Fandrich
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.
... of the other cert verification checks so that you can set verifyhost
and verifypeer to FALSE and still check the public key.
Bug: http://curl.haxx.se/bug/view.cgi?id=1471
Reported-by: Kyle J. McKay
Carrying on from commit 037cd0d991, removed the following unimplemented
instances of curlssl_close_all():
Curl_axtls_close_all()
Curl_darwinssl_close_all()
Curl_cyassl_close_all()
Curl_gskit_close_all()
Curl_gtls_close_all()
Curl_nss_close_all()
Curl_polarssl_close_all()
Fixed the following warning and error from commit 3af90a6e19 when SSL
is not being used:
url.c:2004: warning C4013: 'Curl_ssl_cert_status_request' undefined;
assuming extern returning int
error LNK2019: unresolved external symbol Curl_ssl_cert_status_request
referenced in function Curl_setopt
Also known as "status_request" or OCSP stapling, defined in RFC6066 section 8.
This requires GnuTLS 3.1.3 or higher to build, however it's recommended to use
at least GnuTLS 3.3.11 since previous versions had a bug that caused the OCSP
response verfication to fail even on valid responses.
This option can be used to enable/disable certificate status verification using
the "Certificate Status Request" TLS extension defined in RFC6066 section 8.
This also adds the CURLE_SSL_INVALIDCERTSTATUS error, to be used when the
certificate status verification fails, and the Curl_ssl_cert_status_request()
function, used to check whether the SSL backend supports the status_request
extension.
If the session is still used by active SSL/TLS connections, it
cannot be closed yet. Thus we mark the session as not being cached
any longer so that the reference counting mechanism in
Curl_schannel_shutdown is used to close and free the session.
Reported-by: Jean-Francois Durand
...to avoid a session ID getting cached without certificate checking and
then after a subsequent _enabling_ of the check libcurl could still
re-use the session done without cert checks.
Bug: http://curl.haxx.se/docs/adv_20150108A.html
Reported-by: Marc Hesse
Prefer void for unused parameters, rather than assigning an argument to
itself as a) unintelligent compilers won't optimize it out, b) it can't
be used for const parameters, c) it will cause compilation warnings for
clang with -Wself-assign and d) is inconsistent with other areas of the
curl source code.
Moved our Initialize Security Context return attribute definitions to
the SSPI module, as a) these can be used by other SSPI based providers
and b) the ISC required attributes are defined there.
curl_schannel.h:123: warning: right-hand operand of comma expression
has no effect
Some instances of the curlssl_close_all() function were declared with a
void return type whilst others as int. The schannel version returned
CURLE_NOT_BUILT_IN and others simply returned zero, but in all cases the
return code was ignored by the calling function Curl_ssl_close_all().
For the time being and to keep the internal API consistent, changed all
declarations to use a void return type.
To reduce code we might want to consider removing the unimplemented
versions and use a void #define like schannel does.
Otherwise Curl_ssl_init_certinfo() can fail and set the num_of_certs
member variable to the requested count, which could then be used
incorrectly as libcurl closes down.
The return type for this function was 0 on success and 1 on error. This
was then examined by the calling functions and, in most cases, used to
return CURLE_OUT_OF_MEMORY.
Instead use CURLcode for the return type and return the out of memory
error directly, propagating it up the call stack.
The return type of this function is a boolean value, and even uses a
bool internally, so use bool in the function declaration as well as
the variables that store the return value, to avoid any confusion.
- do not grow memory by doubling its size
- do not leak previously allocated memory if reallocation fails
- replace while-loop with a single check to make sure
that the requested amount of data fits into the buffer
Bug: http://curl.haxx.se/bug/view.cgi?id=1450
Reported-by: Warren Menzer
