From 4b1c8ef107dc195c0b4a493d72c6ffe97077318c Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Thu, 5 Feb 2015 22:44:58 +0200
Subject: [PATCH 1/1] SSL renegotiation fixes.

Remove the ugly "retry twenty times" from server renegotiation code. In
fact, calling SSL_do_handshake() after SSL_renegotiate() is not necessary
at all. SSL_read() and SSL_write() will perform the handshake as well.

In the client-side, avoid letting SSL_write() to read data. That sometimes
makes OpenSSL throw "unexpected message" and other errors, if renegotiation
is happening. Before calling SSL_write(), set a flag that prevents
pqsecure_raw_read() from reading any more data from the socket, and call
SSL_read() to drain any remaining data from OpenSSL's buffer. That makes
ensures that SSL_read() always handles all reads, which seems to be
necessary to make renegotiations robust with OpenSSL.

Also start renegotiation a little bit earlier than before. We used to start
it 1kB before reaching ssl_renegotiation_limit. That's too little, there can
easily be 1kB of data in-flight from the client, before it gets our
renegotiation request.
---
 src/backend/libpq/be-secure-openssl.c    | 25 ++-----------------------
 src/interfaces/libpq/fe-misc.c           | 16 +++++++++++++++-
 src/interfaces/libpq/fe-secure-openssl.c |  1 -
 src/interfaces/libpq/fe-secure.c         | 15 +++++++++++++++
 src/interfaces/libpq/libpq-int.h         |  3 +++
 5 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 39587e8..e847f41 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -576,11 +576,11 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
 	/*
 	 * If SSL renegotiations are enabled and we're getting close to the
 	 * limit, start one now; but avoid it if there's one already in
-	 * progress.  Request the renegotiation 1kB before the limit has
+	 * progress.  Request the renegotiation 32kB before the limit has
 	 * actually expired.
 	 */
 	if (ssl_renegotiation_limit && !in_ssl_renegotiation &&
-		port->count > (ssl_renegotiation_limit - 1) * 1024L)
+		port->count > (ssl_renegotiation_limit - 32) * 1024L)
 	{
 		in_ssl_renegotiation = true;
 
@@ -601,27 +601,6 @@ be_tls_write(Port *port, void *ptr, size_t len, int *waitfor)
 			ereport(COMMERROR,
 					(errcode(ERRCODE_PROTOCOL_VIOLATION),
 					 errmsg("SSL failure during renegotiation start")));
-		else
-		{
-			int			retries;
-
-			/*
-			 * A handshake can fail, so be prepared to retry it, but only
-			 * a few times.
-			 */
-			for (retries = 0;; retries++)
-			{
-				if (SSL_do_handshake(port->ssl) > 0)
-					break;	/* done */
-				ereport(COMMERROR,
-						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("SSL handshake failure on renegotiation, retrying")));
-				if (retries >= 20)
-					ereport(FATAL,
-							(errcode(ERRCODE_PROTOCOL_VIOLATION),
-							 errmsg("could not complete SSL handshake on renegotiation, too many failures")));
-			}
-		}
 	}
 
 	errno = 0;
diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index 945e283..4c47821 100644
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -834,7 +834,7 @@ pqSendSome(PGconn *conn, int len)
 {
 	char	   *ptr = conn->outBuffer;
 	int			remaining = conn->outCount;
-	int			result = 0;
+	int			result;
 
 	if (conn->sock == PGINVALID_SOCKET)
 	{
@@ -845,11 +845,24 @@ pqSendSome(PGconn *conn, int len)
 		return -1;
 	}
 
+#ifdef USE_OPENSSL
+	if (conn->ssl)
+	{
+		block_reads = true;
+		result = pqReadData(conn);
+		block_reads = false;
+		if (result < 0)
+			return -1;
+	}
+#endif
+
 	/* while there's still data to send */
+	result = 0;
 	while (len > 0)
 	{
 		int			sent;
 
+		block_reads = true;
 #ifndef WIN32
 		sent = pqsecure_write(conn, ptr, len);
 #else
@@ -861,6 +874,7 @@ pqSendSome(PGconn *conn, int len)
 		 */
 		sent = pqsecure_write(conn, ptr, Min(len, 65536));
 #endif
+		block_reads = false;
 
 		if (sent < 0)
 		{
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index a32af34..38e7071 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -100,7 +100,6 @@ static long win32_ssl_create_mutex = 0;
 #endif
 #endif   /* ENABLE_THREAD_SAFETY */
 
-
 /* ------------------------------------------------------------ */
 /*			 Procedures common to all secure sessions			*/
 /* ------------------------------------------------------------ */
diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c
index 57c572b..aac405b 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -124,6 +124,9 @@ struct sigpipe_info
 #define RESTORE_SIGPIPE(conn, spinfo)
 #endif   /* WIN32 */
 
+bool block_reads = false;
+bool block_writes = false;
+
 /* ------------------------------------------------------------ */
 /*			 Procedures common to all secure sessions			*/
 /* ------------------------------------------------------------ */
@@ -227,6 +230,12 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
 	int			result_errno = 0;
 	char		sebuf[256];
 
+	if (block_reads)
+	{
+		errno = EWOULDBLOCK;
+		return -1;
+	}
+
 	n = recv(conn->sock, ptr, len, 0);
 
 	if (n < 0)
@@ -307,6 +316,12 @@ pqsecure_raw_write(PGconn *conn, const void *ptr, size_t len)
 
 	DECLARE_SIGPIPE_INFO(spinfo);
 
+	if (block_writes)
+	{
+		errno = EWOULDBLOCK;
+		return -1;
+	}
+
 #ifdef MSG_NOSIGNAL
 	if (conn->sigpipe_flag)
 		flags |= MSG_NOSIGNAL;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 008fd67..ce55411 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -618,6 +618,9 @@ extern int	pqWriteReady(PGconn *conn);
 
 /* === in fe-secure.c === */
 
+extern bool block_reads;
+extern bool block_writes;
+
 extern int	pqsecure_initialize(PGconn *);
 extern void pqsecure_destroy(void);
 extern PostgresPollingStatusType pqsecure_open_client(PGconn *);
-- 
2.1.4

