Coverity CID 1202836. If the proxy environment variable returned an empty
string, it would be leaked. While an empty string is not really a proxy, other
logic in this function already allows a blank string to be returned so allow
that here to avoid the leak.
Coverity CID 1215287. There's a potential risk for a memory leak in
here, and moving the free call to be unconditional seems like a cheap
price to remove the risk.
Coverity CID 1215296. There's a potential risk for a memory leak in
here, and moving the free call to be unconditional seems like a cheap
price to remove the risk.
Coverity detected this. CID 1241954. When Curl_poll() returns a negative value
'mcode' was uninitialized. Pretty harmless since this is debug code only and
would at worst cause an error to _not_ be returned...
Mostly because we use C strings and they end at a binary zero so we know
we can't open a file name using an embedded binary zero.
Reported-by: research@g0blin.co.uk
The switch to using Curl_expire_latest() in commit cacdc27f52 was a
mistake and was against the advice even mentioned in that commit. The
comparison in asyn-thread.c:Curl_resolver_is_resolved() makes
Curl_expire() the suitable function to use.
Bug: http://curl.haxx.se/bug/view.cgi?id=1426
Reported-By: graysky
Previously we did not handle EOF from underlying transport socket and
wrongly just returned error code CURL_AGAIN from http2_recv, which
caused busy loop since socket has been closed. This patch adds the
code to handle EOF situation and tells the upper layer that we got
EOF.
Removed ISC_REQ_* flags from calls to InitializeSecurityContext to fix
bug in NTLM handshake for HTTP proxy authentication.
NTLM handshake for HTTP proxy authentication failed with error
SEC_E_INVALID_TOKEN from InitializeSecurityContext for certain proxy
servers on generating the NTLM Type-3 message.
The flag ISC_REQ_CONFIDENTIALITY seems to cause the problem according
to the observations and suggestions made in a bug report for the
QT project (https://bugreports.qt-project.org/browse/QTBUG-17322).
Removing all the flags solved the problem.
Bug: http://curl.haxx.se/mail/lib-2014-08/0273.html
Reported-by: Ulrich Telle
Assisted-by: Steve Holme, Daniel Stenberg
As a sort of step forward, this script will now first try to get the
data from the HTTPS URL using curl, and only if that fails it will
switch back to the HTTP transfer using perl's native LWP functionality.
To reduce the risk of this script being tricked.
Using HTTPS to get a cert bundle introduces a chicken-and-egg problem so
we can't really ever completely disable HTTP, but chances are that most
users already have a ca cert bundle that trusts the mozilla.org site
that this script downloads from.
A future version of this script will probably switch to require a
dedicated "insecure" command line option to allow downloading over HTTP
(or unverified HTTPS).
By not detecting and rejecting domain names for partial literal IP
addresses properly when parsing received HTTP cookies, libcurl can be
fooled to both send cookies to wrong sites and to allow arbitrary sites
to set cookies for others.
CVE-2014-3613
Bug: http://curl.haxx.se/docs/adv_20140910A.html
Historically the default "unknown" value for progress.size_dl and
progress.size_ul has been zero, since these values are initialized
implicitly by the calloc that allocates the curl handle that these
variables are a part of. Users of curl that install progress
callbacks may expect these values to always be >= 0.
Currently it is possible for progress.size_dl and progress.size_ul
to by set to a value of -1, if Curl_pgrsSetDownloadSize() or
Curl_pgrsSetUploadSize() are passed a "size" of -1 (which a few
places currently do, and a following patch will add more). So
lets update Curl_pgrsSetDownloadSize() and Curl_pgrsSetUploadSize()
so they make sure that these variables always contain a value that
is >= 0.
Updates test579 and test599.
Signed-off-by: Brandon Casey <drafnel@gmail.com>
As the current element in the list is free()d by Curl_llist_remove(),
when the associated connection is pending, reworked the loop to avoid
accessing the next element through e->next afterward.
SecCertificateCopyPublicKey() is not available on iPhone. Use
CopyCertSubject() instead to see if the certificate returned by
SecCertificateCreateWithData() is valid.
Reported-by: Toby Peterson
... as the struct is free()d in the end anyway. It was first pointed out
to me that one of the ->msglist assignments were supposed to have been
->pending but was a copy and paste mistake when I realized none of the
clearing of pointers had to be there.
... instead of scanning through all handles, stash only the actual
handles that are in that state in the new ->pending list and scan that
list only. It should be mostly empty or very short. And only used for
pipelining.
This avoids a rather hefty slow-down especially notable if you add many
handles to the same multi handle. Regression introduced in commit
0f147887 (version 7.30.0).
Bug: http://curl.haxx.se/mail/lib-2014-07/0206.html
Reported-by: David Meyer
Forwards the setting as minimum ssl version (if set) to polarssl. If
the server does not support the requested version the SSL Handshake will
fail.
Bug: http://curl.haxx.se/bug/view.cgi?id=1419
SecCertificateCreateWithData() returns a non-NULL SecCertificateRef even
if the buffer holds an invalid or corrupt certificate. Call
SecCertificateCopyPublicKey() to make sure cacert is a valid
certificate.
Introducing Curl_expire_latest(). To be used when we the code flow only
wants to get called at a later time that is "no later than X" so that
something can be checked (and another timeout be added).
The low-speed logic for example could easily be made to set very many
expire timeouts if it would be called faster or sooner than what it had
set its own timer and this goes for a few other timers too that aren't
explictiy checked for timer expiration in the code.
If there's no condition the code that says if(time-passed >= TIME), then
Curl_expire_latest() is preferred to Curl_expire().
If there exists such a condition, it is on the other hand important that
Curl_expire() is used and not the other.
Bug: http://curl.haxx.se/mail/lib-2014-06/0235.html
Reported-by: Florian Weimer
While waiting for a host resolve, check if the host cache may have
gotten the name already (by someone else), for when the same name is
resolved by several simultanoues requests.
The resolver thread occasionally gets stuck in getaddrinfo() when the
DNS or anything else is crappy or slow, so when a host is found in the
DNS cache, leave the thread alone and let itself cleanup the mess.
If the --cacert option is used with a CA certificate bundle that
contains multiple CA certificates, iterate through it, adding each
certificate as a trusted root CA.
This is usually due to failed auth. There's no point in us keeping such
a connection alive since it shouldn't be re-used anyway.
Bug: http://curl.haxx.se/bug/view.cgi?id=1381
Reported-by: Marcel Raad
This was done to make sure NTLM state that is bound to a connection
doesn't survive and gets used for the subsequent request - but
disconnects can also be done to for example make room in the connection
cache and thus that connection is not strictly related to the easy
handle's current operation.
The http authentication state is still kept in the easy handle since all
http auth _except_ NTLM is connection independent and thus survive over
multiple connections.
Bug: http://curl.haxx.se/mail/lib-2014-08/0148.html
Reported-by: Paras S
Problem: if CURLOPT_FORBID_REUSE is set, requests using NTLM failed
since NTLM requires multiple requests that re-use the same connection
for the authentication to work
Solution: Ignore the forbid reuse flag in case the NTLM authentication
handshake is in progress, according to the NTLM state flag.
Fixed known bug #77.
A conditionally compiled block in connect.c references WinSock 2
symbols, but used `#ifdef HAVE_WINSOCK_H` instead of `#ifdef
HAVE_WINSOCK2_H`.
Bug: http://curl.haxx.se/mail/lib-2014-08/0155.html
The URL is not a property of the connection so it should not be freed in
the connection disconnect but in the Curl_close() that frees the easy
handle.
Bug: http://curl.haxx.se/mail/lib-2014-08/0148.html
Reported-by: Paras S
Corrected a number of the error codes that can be returned from the
Curl_sasl_create_gssapi_security_message() function when things go
wrong.
It makes more sense to return CURLE_BAD_CONTENT_ENCODING when the
inbound security challenge can't be decoded correctly or doesn't
contain the KERB_WRAP_NO_ENCRYPT flag and CURLE_OUT_OF_MEMORY when
EncryptMessage() fails. Unfortunately the previous error code of
CURLE_RECV_ERROR was a copy and paste mistakes on my part and should
have been correct in commit 4b491c675f :(
... to handle "*/[total]". Also, removed the strange hack that made
CURLOPT_FAILONERROR on a 416 response after a *RESUME_FROM return
CURLE_OK.
Reported-by: Dimitrios Siganos
Bug: http://curl.haxx.se/mail/lib-2014-06/0221.html
In preparation for the upcoming SSPI implementation of GSSAPI
authentication, moved the definition of KERB_WRAP_NO_ENCRYPT from
socks_sspi.c to curl_sspi.h allowing it to be shared amongst other
SSPI based code.
... as mxr.mozilla.org is due to be retired.
The new host doesn't support If-Modified-Since nor ETags, meaning that
the script will now defer to download and do a post-transfer checksum
check to see if a new output is to be generated. The new output format
will hold the SHA1 checksum of the source file for that purpose.
We call this version 1.22
Reported-by: Ed Morley
Bug: http://curl.haxx.se/bug/view.cgi?id=1409
Bringing back the old functionality that was mistakenly removed when the
connection cache was remade. When creating a new connection, all the
existing ones are checked and those that are known to be dead get
disconnected for real and removed from the connection cache. It helps
the cache from holding on to very many stale connections and aids in
keeping down the number of system sockets in wait states.
Help-by: Jonatan Vela <jonatan.vela@ergon.ch>
Bug: http://curl.haxx.se/mail/lib-2014-06/0189.html
Curl_poll and Curl_wait_ms require the fix applied to Curl_socket_check
in commits b61e8b8 and c771968:
When poll or select are interrupted and coincides with the timeout
elapsing, the functions return -1 indicating an error instead of 0 for
the timeout.
Given the SSPI package info query indicates a token size of 4096 bytes,
updated to use a dynamic buffer for the response message generation
rather than a fixed buffer of 1024 bytes.
Updated to use a dynamic buffer for the SPN generation via the recently
introduced Curl_sasl_build_spn() function rather than a fixed buffer of
1024 characters, which should have been more than enough, but by using
the new function removes the need for another variable sname to do the
wide character conversion in Unicode builds.
Updated Curl_sasl_create_digest_md5_message() to use a dynamic buffer
for the SPN generation via the recently introduced Curl_sasl_build_spn()
function rather than a fixed buffer of 128 characters.
Curl_sasl_create_digest_md5_message() would simply cast the SPN variable
to a TCHAR when calling InitializeSecurityContext(). This meant that,
under Unicode builds, it would not be valid wide character string.
Updated to use the recently introduced Curl_sasl_build_spn() function
which performs the correct conversion for us.
Various parts of the libcurl source code build a SPN for inclusion in
authentication data. This information is either used by our own native
generation routines or passed to authentication functions in third-party
libraries such as SSPI. However, some of these instances use fixed
buffers rather than dynamically allocated ones and not all of those that
should, convert to wide character strings in Unicode builds.
Implemented a common function that generates a SPN and performs the
wide character conversion where necessary.
Following the recent changes and in attempt to align the SSPI based
authentication code performed the following:
* Use NULL and SECBUFFVERSION rather than hard coded constants.
* Avoid comparison of zero in if statements.
* Standardised the buf and desc setup code.
Given the SSPI package info query indicates a token size of 2888 bytes,
and as with the Winbind code and commit 9008f3d56, use a dynamic buffer
for the Type-1 and Type-3 message generation rather than a fixed buffer
of 1024 bytes.
Just as with the SSPI implementations of Digest and Negotiate added a
package info query so that libcurl can a) return a more appropriate
error code when the NTLM package is not supported and b) it can be of
use later to allocate a dynamic buffer for the Type-1 and Type-3
output tokens rather than use a fixed buffer of 1024 bytes.
OPENSSL_config() is "strongly recommended" to use but unfortunately that
function makes an exit() call on wrongly formatted config files which
makes it hard to use in some situations. OPENSSL_config() itself calls
CONF_modules_load_file() and we use that instead and we ignore its
return code!
Reported-by: Jan Ehrhardt
Bug: http://curl.haxx.se/bug/view.cgi?id=1401
If the server rejects our authentication attempt and curl hasn't
called CompleteAuthToken() then the status variable will be
SEC_I_CONTINUE_NEEDED and not SEC_E_OK.
As such the existing detection mechanism for determining whether or not
the authentication process has finished is not sufficient.
However, the WWW-Authenticate: Negotiate header line will not contain
any data when the server has exhausted the negotiation, so we can use
that coupled with the already allocated context pointer.
To prevent infinite loop in readwrite_data() function when stream is
reset before any response body comes, reset closed flag to false once
it is evaluated to true.
"Expect: 100-continue", which was once deprecated in HTTP/2, is now
resurrected in HTTP/2 draft 14. This change adds its support to
HTTP/2 code. This change also includes stricter header field
checking.
Previously it only returned a CURLcode for errors, which is when it
returns a different size than what was passed in to it.
The http2 code only checked the curlcode and thus failed.
Under these circumstances, the connection hasn't been fully established
and smtp_connect hasn't been called, yet smtp_done still calls the state
machine which dereferences the NULL conn pointer in struct pingpong.
This now provides a weak random function since PolarSSL doesn't have a
quick and easy way to provide a good one. It does however provide the
framework to make one so it _can_ and _should_ be done...
To force each backend implementation to really attempt to provide proper
random. If a proper random function is missing, then we can explicitly
make use of the default one we use when TLS support is missing.
This commit makes sure it works for darwinssl, gnutls, nss and openssl.
warning C4267: '=' : conversion from 'size_t' to 'long', possible loss
of data
The member connection_id of struct connectdata is a long (always a
32-bit signed integer on Visual C++) and the member next_connection_id
of struct conncache is a size_t, so one of them should be changed to
match the other.
This patch the size_t in struct conncache to long (the less invasive
change as that variable is only ever used in a single code line).
Bug: http://curl.haxx.se/bug/view.cgi?id=1399
1 - fixes the warnings when built without http2 support
2 - adds CURLE_HTTP2, a new error code for errors detected by nghttp2
basically when they are about http2 specific things.
CyaSSL 3.0.0 returns a unique error code if no CA cert is available,
so translate that into CURLE_SSL_CACERT_BADFILE when peer verification
is requested.
- Replace CURLAUTH_GSSNEGOTIATE with CURLAUTH_NEGOTIATE
- CURL_VERSION_GSSNEGOTIATE is deprecated which
is served by CURL_VERSION_SSPI, CURL_VERSION_GSSAPI and
CURUL_VERSION_SPNEGO now.
- Remove display of feature 'GSS-Negotiate'
This reverts commit cb3e6dfa35 and instead fixes the problem
differently.
The reverted commit addressed a test failure in test 1021 by simplifying
and generalizing the code flow in a way that damaged the
performance. Now we modify the flow so that Curl_proxyCONNECT() again
does as much as possible in one go, yet still do test 1021 with and
without valgrind. It failed due to mistakes in the multi state machine.
Bug: http://curl.haxx.se/bug/view.cgi?id=1397
Reported-by: Paul Saab
It's wrong to assume that we can send a single SPNEGO packet which will
complete the authentication. It's a *negotiation* — the clue is in the
name. So make sure we handle responses from the server.
Curl_input_negotiate() will already handle bailing out if it thinks the
state is GSS_S_COMPLETE (or SEC_E_OK on Windows) and the server keeps
talking to us, so we should avoid endless loops that way.
This is the correct way to do SPNEGO. Just ask for it
Now I correctly see it trying NTLMSSP authentication when a Kerberos ticket
isn't available. Of course, we bail out when the server responds with the
challenge packet, since we don't expect that. But I'll fix that bug next...
This is just fundamentally broken. SPNEGO (RFC4178) is a protocol which
allows client and server to negotiate the underlying mechanism which will
actually be used to authenticate. This is *often* Kerberos, and can also
be NTLM and other things. And to complicate matters, there are various
different OIDs which can be used to specify the Kerberos mechanism too.
A SPNEGO exchange will identify *which* GSSAPI mechanism is being used,
and will exchange GSSAPI tokens which are appropriate for that mechanism.
But this SPNEGO implementation just strips the incoming SPNEGO packet
and extracts the token, if any. And completely discards the information
about *which* mechanism is being used. Then we *assume* it was Kerberos,
and feed the token into gss_init_sec_context() with the default
mechanism (GSS_S_NO_OID for the mech_type argument).
Furthermore... broken as this code is, it was never even *used* for input
tokens anyway, because higher layers of curl would just bail out if the
server actually said anything *back* to us in the negotiation. We assume
that we send a single token to the server, and it accepts it. If the server
wants to continue the exchange (as is required for NTLM and for SPNEGO
to do anything useful), then curl was broken anyway.
So the only bit which actually did anything was the bit in
Curl_output_negotiate(), which always generates an *initial* SPNEGO
token saying "Hey, I support only the Kerberos mechanism and this is its
token".
You could have done that by manually just prefixing the Kerberos token
with the appropriate bytes, if you weren't going to do any proper SPNEGO
handling. There's no need for the FBOpenSSL library at all.
The sane way to do SPNEGO is just to *ask* the GSSAPI library to do
SPNEGO. That's what the 'mech_type' argument to gss_init_sec_context()
is for. And then it should all Just Work™.
That 'sane way' will be added in a subsequent patch, as will bug fixes
for our failure to handle any exchange other than a single outbound
token to the server which results in immediate success.
Before GnuTLS 3.3.6, the gnutls_x509_crt_check_hostname() function
didn't actually check IP addresses in SubjectAltName, even though it was
explicitly documented as doing so. So do it ourselves...
The old way using getpwuid could cause problems in programs that enable
reading from netrc files simultaneously in multiple threads.
Reported-by: David Woodhouse
The AES-GCM ciphers were added to GnuTLS as late as ver. 3.0.1 but
the code path in which they're referenced here is only ever used for
somewhat older GnuTLS versions. This caused undeclared identifier errors
when compiling against those.
This seems to have become necessary for SRP support to work starting
with GnuTLS ver. 2.99.0. Since support for SRP was added to GnuTLS
before the function that takes this priority string, there should be no
issue with backward compatibility.
When an error has been detected, skip the final forced call to the
progress callback by making sure to pass the current return code
variable in the Curl_done() call in the CURLM_STATE_DONE state.
This avoids the "extra" callback that could occur even if you returned
error from the progress callback.
Bug: http://curl.haxx.se/mail/lib-2014-06/0062.html
Reported by: Jonathan Cardoso Machado
The static connection counter caused a race condition. Moving the
connection id counter into conncache solves it, as well as simplifying
the related logic.
They were added because of an older code path that used allocations and
should not have been left in the code. With this change the logic goes
back to how it was.
Curl_rand() will return a dummy and repatable random value for this
case. Makes it possible to write test cases that verify output.
Also, fake timestamp with CURL_FORCETIME set.
Only when built debug enabled of course.
Curl_ssl_random() was not used anymore so it has been
removed. Curl_rand() is enough.
create_digest_md5_message: generate base64 instead of hex string
curl_sasl: also fix memory leaks in some OOM situations
httpproxycode is not reset in Curl_initinfo, so a 407 is not reset even
if curl_easy_reset is called between transfers.
Bug: http://curl.haxx.se/bug/view.cgi?id=1380
The method change is forbidden by the obsolete RFC2616, but libcurl did
it anyway for compatibility reasons. The new RFC7231 allows this
behaviour so there's no need for the scary "Violate RFC 2616/10.3.x"
notice. Also update the comments accordingly.
The SASL/Digest previously used the current time's seconds +
microseconds to add randomness but it is much better to instead get more
data from Curl_rand().
It will also allow us to easier "fake" that for debug builds on demand
in a future.
Rather than use a short 8-byte hex string, extended the cnonce to be
32-bytes long, like Windows SSPI does.
Used a combination of random data as well as the current date and
time for the generation.
"Any two of the parameters, readfds, writefds, or exceptfds, can be
given as null. At least one must be non-null, and any non-null
descriptor set must contain at least one handle to a socket."
http://msdn.microsoft.com/en-ca/library/windows/desktop/ms740141(v=vs.85).aspx
When using select(), cURL doesn't adhere to this (WinSock-specific)
rule, and can ask to monitor empty fd_sets, which leads to select()
returning WSAEINVAL (i.e. EINVAL) and connections failing in mysterious
ways as a result (at least when using the curl_multi_socket_action()
interface).
Bug: http://curl.haxx.se/mail/lib-2014-05/0278.html
OpenSSL passes out and outlen variable uninitialized to
select_next_proto_cb callback function. If the callback function
returns SSL_TLSEXT_ERR_OK, the caller assumes the callback filled
values in out and outlen and processes as such. Previously, if there
is no overlap in protocol lists, curl code does not fill any values in
these variables and returns SSL_TLSEXT_ERR_OK, which means we are
triggering undefined behavior. valgrind warns this.
This patch fixes this issue by fallback to HTTP/1.1 if there is no
overlap.
Make all code use connclose() and connkeep() when changing the "close
state" for a connection. These two macros take a string argument with an
explanation, and debug builds of curl will include that in the debug
output. Helps tracking connection re-use/close issues.
