Back-patch 9.4-era SSL renegotiation code into 9.3 and 9.2.

tglsfdc · tglsfdc · commit fbfeceb25362 · 2017-04-17T12:51:40.000-04:00
This back-patches 9.4 commits 31cf1a1, 86029b3, and 36a3be6 into the prior branches, along with relevant bits of b1aebbb and 7ce2a45. We had foreseen doing this once the code was proven, but that never did happen, probably because we got sufficiently fed up with renegotiation to disable it by default. However, we have to do something now because the prior code doesn't even compile against OpenSSL 1.1. Per discussion, the best solution seems to be to make the older branches look like 9.4. Discussion: https://postgr.es/m/20047.1492305247@sss.pgh.pa.us
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
@@ -102,6 +102,9 @@ char	   *ssl_crl_file;
 int			ssl_renegotiation_limit;
 
 #ifdef USE_SSL
+/* are we in the middle of a renegotiation? */
+static bool in_ssl_renegotiation = false;
+
 static SSL_CTX *SSL_context = NULL;
 static bool ssl_loaded_verify_locations = false;
 #endif
@@ -295,6 +298,7 @@ secure_read(Port *port, void *ptr, size_t len)
 						(errcode(ERRCODE_PROTOCOL_VIOLATION),
 						 errmsg("unrecognized SSL error code: %d",
 								err)));
+				errno = ECONNRESET;
 				n = -1;
 				break;
 		}
@@ -326,29 +330,55 @@ secure_write(Port *port, void *ptr, size_t len)
 		int			err;
 		unsigned long ecode;
 
-		if (ssl_renegotiation_limit && port->count > ssl_renegotiation_limit * 1024L)
+		/*
+		 * 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
+		 * actually expired.
+		 */
+		if (ssl_renegotiation_limit && !in_ssl_renegotiation &&
+			port->count > (ssl_renegotiation_limit - 1) * 1024L)
 		{
+			in_ssl_renegotiation = true;
+
+			/*
+			 * The way we determine that a renegotiation has completed is by
+			 * observing OpenSSL's internal renegotiation counter.  Make sure
+			 * we start out at zero, and assume that the renegotiation is
+			 * complete when the counter advances.
+			 *
+			 * OpenSSL provides SSL_renegotiation_pending(), but this doesn't
+			 * seem to work in testing.
+			 */
+			SSL_clear_num_renegotiations(port->ssl);
+
 			SSL_set_session_id_context(port->ssl, (void *) &SSL_context,
 									   sizeof(SSL_context));
 			if (SSL_renegotiate(port->ssl) <= 0)
 				ereport(COMMERROR,
 						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("SSL renegotiation failure")));
-			if (SSL_do_handshake(port->ssl) <= 0)
-				ereport(COMMERROR,
-						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("SSL renegotiation failure")));
-			if (port->ssl->state != SSL_ST_OK)
-				ereport(COMMERROR,
-						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("SSL failed to send renegotiation request")));
-			port->ssl->state |= SSL_ST_ACCEPT;
-			SSL_do_handshake(port->ssl);
-			if (port->ssl->state != SSL_ST_OK)
-				ereport(COMMERROR,
-						(errcode(ERRCODE_PROTOCOL_VIOLATION),
-						 errmsg("SSL renegotiation failure")));
-			port->count = 0;
+						 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")));
+				}
+			}
 		}
 
 wloop:
@@ -393,9 +423,32 @@ secure_write(Port *port, void *ptr, size_t len)
 						(errcode(ERRCODE_PROTOCOL_VIOLATION),
 						 errmsg("unrecognized SSL error code: %d",
 								err)));
+				errno = ECONNRESET;
 				n = -1;
 				break;
 		}
+
+		if (n >= 0)
+		{
+			/* is renegotiation complete? */
+			if (in_ssl_renegotiation &&
+				SSL_num_renegotiations(port->ssl) >= 1)
+			{
+				in_ssl_renegotiation = false;
+				port->count = 0;
+			}
+
+			/*
+			 * if renegotiation is still ongoing, and we've gone beyond the
+			 * limit, kill the connection now -- continuing to use it can be
+			 * considered a security problem.
+			 */
+			if (in_ssl_renegotiation &&
+				port->count > ssl_renegotiation_limit * 1024L)
+				ereport(FATAL,
+						(errcode(ERRCODE_PROTOCOL_VIOLATION),
+						 errmsg("SSL failed to renegotiate connection before limit expired")));
+		}
 	}
 	else
 #endif
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
@@ -553,16 +553,22 @@ prepare_for_client_read(void)
 
 /*
  * client_read_ended -- get out of the client-input state
+ *
+ * This is called just after low-level reads.  It must preserve errno!
  */
 void
 client_read_ended(void)
 {
 	if (DoingCommandRead)
 	{
+		int			save_errno = errno;
+
 		ImmediateInterruptOK = false;
 
 		DisableNotifyInterrupt();
 		DisableCatchupInterrupt();
+
+		errno = save_errno;
 	}
 }
 

Original file line number	Diff line number	Diff line change
`@@ -553,16 +553,22 @@ prepare_for_client_read(void)`
`553`	`553`
`554`	`554`	`/*`
`555`	`555`	`* client_read_ended -- get out of the client-input state`
	`556`	`+ *`
	`557`	`+ * This is called just after low-level reads. It must preserve errno!`
`556`	`558`	`*/`
`557`	`559`	`void`
`558`	`560`	`client_read_ended(void)`
`559`	`561`	`{`
`560`	`562`	`if (DoingCommandRead)`
`561`	`563`	`{`
	`564`	`+ int save_errno = errno;`
	`565`	`+`
`562`	`566`	`ImmediateInterruptOK = false;`
`563`	`567`
`564`	`568`	`DisableNotifyInterrupt();`
`565`	`569`	`DisableCatchupInterrupt();`
	`570`	`+`
	`571`	`+ errno = save_errno;`
`566`	`572`	`}`
`567`	`573`	`}`
`568`	`574`