Skip to content

Commit

Permalink
Fixes #7632: Backport the patch to shutdown the socket in case of tim…
Browse files Browse the repository at this point in the history
…eout for 3.0
  • Loading branch information
amousset committed Dec 17, 2015
1 parent 6baa0e0 commit 1333eff
Showing 1 changed file with 161 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
From 51cad214e5ee6bc4cfdba9dcbc5a673472eba0c1 Mon Sep 17 00:00:00 2001
From: Dimitrios Apostolou <dimitrios.apostolou@cfengine.com>
Date: Thu, 22 Oct 2015 21:46:23 +0200
Subject: [PATCH] Redmine#6027: Fix possible corruption in case of socket
recv() timeout

Make sure a timed-out connection gets closed at the lowest level, so
that further calls of recv() do not return the delayed server reply.

(cherry picked from commit a08248f86a63d2863b29b8692136c5c43decc492)

Conflicts:
libcfnet/classic.c
libcfnet/tls_generic.c
---
libcfnet/classic.c | 27 +++++++++++++++++++++++++--
libcfnet/tls_generic.c | 35 +++++++++++++++++++++++++++++++----
libcfnet/tls_generic.h | 2 +-
libutils/platform.h | 3 +++
4 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/libcfnet/classic.c b/libcfnet/classic.c
index 0c0b7cc..bb02dc7 100644
--- a/libcfnet/classic.c
+++ b/libcfnet/classic.c
@@ -53,7 +53,11 @@ static bool LastRecvTimedOut(void)
* @param buffer Buffer into which to read data
* @param toget Number of bytes to read; a '\0' shall be written after
* the data; buffer must have space for that.
- * @return -1 on error; or actual length read.
+ *
+ * @return number of bytes actually received, might be less than #toget
+ * <toget when connection has been gracefully closed while we
+ * were expecting more data.
+ * -1 in case of timeout or error - socket is unusable
*/
int RecvSocketStream(int sd, char buffer[CF_BUFSIZE], int toget)
{
@@ -83,7 +87,26 @@ int RecvSocketStream(int sd, char buffer[CF_BUFSIZE], int toget)
}
else
{
- Log(LOG_LEVEL_ERR, "Couldn't receive. (recv: %s)", GetErrorStr());
+ if (LastRecvTimedOut())
+ {
+ Log(LOG_LEVEL_ERR, "Receive timeout"
+ " (received=%dB, expecting=%dB) (recv: %s)",
+ already, toget, GetErrorStr());
+ Log(LOG_LEVEL_VERBOSE,
+ "Consider increasing body agent control"
+ " \"default_timeout\" setting");
+
+ /* Shutdown() TCP connection despite of EAGAIN error, in
+ * order to avoid receiving this delayed response later on
+ * (Redmine #6027). */
+ shutdown(sd, SHUT_RDWR);
+ }
+ else
+ {
+ Log(LOG_LEVEL_ERR, "Couldn't receive (recv: %s)",
+ GetErrorStr());
+ }
+ return -1;
}
return -1;
}
diff --git a/libcfnet/tls_generic.c b/libcfnet/tls_generic.c
index 6211f23..d3bd047 100644
--- a/libcfnet/tls_generic.c
+++ b/libcfnet/tls_generic.c
@@ -527,7 +527,7 @@ static const char *TLSPrimarySSLError(int code)
* @warning Use only for SSL_connect(), SSL_accept(), SSL_do_handshake(),
* SSL_read(), SSL_peek(), SSL_write(), see SSL_get_error man page.
*/
-void TLSLogError(SSL *ssl, LogLevel level, const char *prepend, int retcode)
+int TLSLogError(SSL *ssl, LogLevel level, const char *prepend, int retcode)
{
assert(prepend != NULL);

@@ -581,6 +581,8 @@ void TLSLogError(SSL *ssl, LogLevel level, const char *prepend, int retcode)
(errstr2 == NULL) ? "" : errstr2, /* most likely empty */
syserr);
}
+
+ return errcode;
}

static void assert_SSLIsBlocking(const SSL *ssl)
@@ -652,9 +654,12 @@ int TLSSend(SSL *ssl, const char *buffer, int length)
* @param ssl SSL information.
* @param buffer Buffer, of size at least CF_BUFSIZE, to store received data.
* @param length Length of the data to receive, must be < CF_BUFSIZE.
+ *
* @return The length of the received data, which could be smaller or equal
- * than the requested or -1 in case of error or 0 if connection was
- * closed.
+ * than the requested amount.
+ * -1 in case of timeout or error - SSL session is unusable
+ * 0 if connection was closed
+ *
* @note Use only for *blocking* sockets. Set
* SSL_CTX_set_mode(SSL_MODE_AUTO_RETRY) to make sure that either
* operation completed or an error occurred.
@@ -668,7 +673,29 @@ int TLSRecv(SSL *ssl, char *buffer, int length)
int received = SSL_read(ssl, buffer, length);
if (received < 0)
{
- TLSLogError(ssl, LOG_LEVEL_ERR, "SSL_read", received);
+ int errcode = TLSLogError(ssl, LOG_LEVEL_ERR, "SSL_read", received);
+
+ /* SSL_read() might get an internal recv() timeout, since we've set
+ * SO_RCVTIMEO. In that case, the internal socket returns EAGAIN or
+ * EWOULDBLOCK and SSL_read() returns SSL_ERROR_WANT_READ. */
+ if (errcode == SSL_ERROR_WANT_READ) /* recv() timeout */
+ {
+ /* Make sure that the peer will send us no more data. */
+ SSL_shutdown(ssl);
+ shutdown(SSL_get_fd(ssl), SHUT_RDWR);
+
+ /* Empty possible SSL_read() buffers. */
+
+ int ret = 1;
+ int bytes_still_buffered = SSL_pending(ssl);
+ while (bytes_still_buffered > 0 && ret > 0)
+ {
+ char tmpbuf[bytes_still_buffered];
+ ret = SSL_read(ssl, tmpbuf, bytes_still_buffered);
+ bytes_still_buffered -= ret;
+ }
+ }
+
return -1;
}
else if (received == 0)
diff --git a/libcfnet/tls_generic.h b/libcfnet/tls_generic.h
index e168d29..8b2ccd9 100644
--- a/libcfnet/tls_generic.h
+++ b/libcfnet/tls_generic.h
@@ -41,7 +41,7 @@ bool TLSGenericInitialize(void);
int TLSVerifyCallback(X509_STORE_CTX *ctx, void *arg);
int TLSVerifyPeer(ConnectionInfo *conn_info, const char *remoteip, const char *username);
X509 *TLSGenerateCertFromPrivKey(RSA *privkey);
-void TLSLogError(SSL *ssl, LogLevel level, const char *prepend, int code);
+int TLSLogError(SSL *ssl, LogLevel level, const char *prepend, int code);
int TLSSend(SSL *ssl, const char *buffer, int length);
int TLSRecv(SSL *ssl, char *buffer, int length);
int TLSRecvLines(SSL *ssl, char *buf, size_t buf_size);
diff --git a/libutils/platform.h b/libutils/platform.h
index 994c4fb..77d71ab 100644
--- a/libutils/platform.h
+++ b/libutils/platform.h
@@ -63,6 +63,9 @@
# include <iphlpapi.h>
# include <ws2tcpip.h>
# include <objbase.h> // for disphelper
+# ifndef SHUT_RDWR // for shutdown()
+# define SHUT_RDWR SD_BOTH
+# endif
#endif

/* Standard C. */

0 comments on commit 1333eff

Please sign in to comment.