Skip to content

Commit

Permalink
OpenSSL: fix spurious SSL connection aborts
Browse files Browse the repository at this point in the history
Was seeing spurious SSL connection aborts using libcurl and
OpenSSL. I tracked it down to uncleared error state on the
OpenSSL error stack - patch attached deals with that.

Rough idea of problem:

Code that uses libcurl calls some library that uses OpenSSL but
don't clear the OpenSSL error stack after an error.

ssluse.c calls SSL_read which eventually gets an EWOULDBLOCK from
the OS. Returns -1 to indicate an error

ssluse.c calls SSL_get_error. First thing, SSL_get_error calls
ERR_get_error to check the OpenSSL error stack, finds an old
error and returns SSL_ERROR_SSL instead of SSL_ERROR_WANT_READ or
SSL_ERROR_WANT_WRITE.

ssluse.c returns an error and aborts the connection

Solution:

Clear the openssl error stack before calling SSL_* operation if
we're going to call SSL_get_error afterwards.

Notes:

This is much more likely to happen with multi because it's easier
to intersperse other calls to the OpenSSL library in the same
thread.
  • Loading branch information
csapuntz authored and bagder committed Jun 5, 2010
1 parent 4724b9d commit a0dd9df
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 1 deletion.
5 changes: 5 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@

Changelog

Daniel Stenberg (5 June 2010)
- Constantine Sapuntzakis fixed a case of spurious SSL connection aborts using
libcurl and OpenSSL. "I tracked it down to uncleared error state on the
OpenSSL error stack - patch attached deals with that."

Daniel Stenberg (5 June 2010)
- Frank Meier added CURLINFO_PRIMARY_PORT, CURLINFO_LOCAL_IP and
CURLINFO_LOCAL_PORT to curl_easy_getinfo().
Expand Down
3 changes: 2 additions & 1 deletion RELEASE-NOTES
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ This release includes the following bugfixes:
o TFTP block id wrap
o curl_multi_socket_action() timeout handles inaccuracy in timers better
o SCP/SFTP failure to respect the timeout
o spurious SSL connection aborts with OpenSSL

This release includes the following known bugs:

Expand All @@ -49,7 +50,7 @@ advice from friends like these:
Kamil Dudka, Alex Bligh, Ben Greear, Hoi-Ho Chan, Howard Chu, Dirk Manske,
Pavel Raiskup, John-Mark Bell, Eric Mertens, Tor Arntsen, Douglas Kilpatrick,
Igor Novoseltsev, Jason McDonald, Dan Fandrich, Tanguy Fautre, Guenter Knauf,
Julien Chaffraix, Kalle Vahlman, Frank Meier
Julien Chaffraix, Kalle Vahlman, Frank Meier, Constantine Sapuntzakis


Thanks! (and sorry if I forgot to mention someone)
9 changes: 9 additions & 0 deletions lib/ssluse.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
#include <openssl/x509v3.h>
#include <openssl/dsa.h>
#include <openssl/dh.h>
#include <openssl/err.h>
#else
#include <rand.h>
#include <x509v3.h>
Expand Down Expand Up @@ -882,6 +883,8 @@ int Curl_ossl_shutdown(struct connectdata *conn, int sockindex)
int what = Curl_socket_ready(conn->sock[sockindex],
CURL_SOCKET_BAD, SSL_SHUTDOWN_TIMEOUT);
if(what > 0) {
ERR_clear_error();

/* Something to read, let's do it and hope that it is the close
notify alert from the server */
nread = (ssize_t)SSL_read(conn->ssl[sockindex].handle, buf,
Expand Down Expand Up @@ -1684,6 +1687,8 @@ ossl_connect_step2(struct connectdata *conn, int sockindex)
|| ssl_connect_2_reading == connssl->connecting_state
|| ssl_connect_2_writing == connssl->connecting_state);

ERR_clear_error();

err = SSL_connect(connssl->handle);

/* 1 is fine
Expand Down Expand Up @@ -2512,6 +2517,8 @@ static ssize_t ossl_send(struct connectdata *conn,
int memlen;
int rc;

ERR_clear_error();

memlen = (len > (size_t)INT_MAX) ? INT_MAX : (int)len;
rc = SSL_write(conn->ssl[sockindex].handle, mem, memlen);

Expand Down Expand Up @@ -2560,6 +2567,8 @@ static ssize_t ossl_recv(struct connectdata *conn, /* connection data */
ssize_t nread;
int buffsize;

ERR_clear_error();

buffsize = (buffersize > (size_t)INT_MAX) ? INT_MAX : (int)buffersize;
nread = (ssize_t)SSL_read(conn->ssl[num].handle, buf, buffsize);
if(nread < 0) {
Expand Down

0 comments on commit a0dd9df

Please sign in to comment.