Support TLS handshake directly without SSLRequest negotiation

By skipping SSLRequest, you can eliminate one round-trip when
establishing a TLS connection. It is also more friendly to generic TLS
proxies that don't understand the PostgreSQL protocol.

This is disabled by default in libpq, because the direct TLS handshake
will fail with old server versions. It can be enabled with the
sslnegotation=direct option. It will still fall back to the negotiated
TLS handshake if the server rejects the direct attempt, either because
it is an older version or the server doesn't support TLS at all, but
the fallback can be disabled with the sslnegotiation=requiredirect
option.

Author: Greg Stark, Heikki Linnakangas
Reviewed-by: Matthias van de Meent, Jacob Champion

4 files changed, 109 insertions, 10 deletions

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index cf3b5a8fded..4bd523ec6e3 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -129,6 +129,7 @@ static int	ldapServiceLookup(const char *purl, PQconninfoOption *options,
 #define DefaultSSLMode	"disable"
 #define DefaultSSLCertMode "disable"
 #endif
+#define DefaultSSLNegotiation	"postgres"
 #ifdef ENABLE_GSS
 #include "fe-gssapi-common.h"
 #define DefaultGSSMode "prefer"
@@ -272,6 +273,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
 		"SSL-Mode", "", 12,		/* sizeof("verify-full") == 12 */
 	offsetof(struct pg_conn, sslmode)},
 
+	{"sslnegotiation", "PGSSLNEGOTIATION", DefaultSSLNegotiation, NULL,
+		"SSL-Negotiation", "", 14,	/* sizeof("requiredirect") == 14 */
+	offsetof(struct pg_conn, sslnegotiation)},
+
 	{"sslcompression", "PGSSLCOMPRESSION", "0", NULL,
 		"SSL-Compression", "", 1,
 	offsetof(struct pg_conn, sslcompression)},
@@ -1572,6 +1577,39 @@ pqConnectOptions2(PGconn *conn)
 #endif
 	}
 
+	/*
+	 * validate sslnegotiation option, default is "postgres" for the postgres
+	 * style negotiated connection with an extra round trip but more options.
+	 */
+	if (conn->sslnegotiation)
+	{
+		if (strcmp(conn->sslnegotiation, "postgres") != 0
+			&& strcmp(conn->sslnegotiation, "direct") != 0
+			&& strcmp(conn->sslnegotiation, "requiredirect") != 0)
+		{
+			conn->status = CONNECTION_BAD;
+			libpq_append_conn_error(conn, "invalid %s value: \"%s\"",
+									"sslnegotiation", conn->sslnegotiation);
+			return false;
+		}
+
+#ifndef USE_SSL
+		if (conn->sslnegotiation[0] != 'p')
+		{
+			conn->status = CONNECTION_BAD;
+			libpq_append_conn_error(conn, "sslnegotiation value \"%s\" invalid when SSL support is not compiled in",
+									conn->sslnegotiation);
+			return false;
+		}
+#endif
+	}
+	else
+	{
+		conn->sslnegotiation = strdup(DefaultSSLNegotiation);
+		if (!conn->sslnegotiation)
+			goto oom_error;
+	}
+
 #ifdef USE_SSL
 
 	/*
@@ -3294,9 +3332,20 @@ keep_going:						/* We will come back to here until there is
 					goto error_return;
 
 				/*
-				 * If SSL is enabled, request SSL and proceed with SSL
-				 * handshake.  We will come back here after SSL encryption has
-				 * been established, with ssl_in_use set.
+				 * If direct SSL is enabled, jump right into SSL handshake. We
+				 * will come back here after SSL encryption has been
+				 * established, with ssl_in_use set.
+				 */
+				if (conn->current_enc_method == ENC_DIRECT_SSL && !conn->ssl_in_use)
+				{
+					conn->status = CONNECTION_SSL_STARTUP;
+					return PGRES_POLLING_WRITING;
+				}
+
+				/*
+				 * If negotiated SSL is enabled, request SSL and proceed with
+				 * SSL handshake.  We will come back here after SSL encryption
+				 * has been established, with ssl_in_use set.
 				 */
 				if (conn->current_enc_method == ENC_NEGOTIATED_SSL && !conn->ssl_in_use)
 				{
@@ -3487,6 +3536,10 @@ keep_going:						/* We will come back to here until there is
 				}
 				if (pollres == PGRES_POLLING_FAILED)
 				{
+					/*
+					 * Failed direct ssl connection, possibly try a new
+					 * connection with postgres negotiation
+					 */
 					CONNECTION_FAILED();
 				}
 				/* Else, return POLLING_READING or POLLING_WRITING status */
@@ -4202,7 +4255,7 @@ init_allowed_encryption_methods(PGconn *conn)
 	if (conn->raddr.addr.ss_family == AF_UNIX)
 	{
 		/* Don't request SSL or GSSAPI over Unix sockets */
-		conn->allowed_enc_methods &= ~(ENC_NEGOTIATED_SSL | ENC_GSSAPI);
+		conn->allowed_enc_methods &= ~(ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL | ENC_GSSAPI);
 
 		/*
 		 * XXX: we probably should not do this. sslmode=require works
@@ -4228,7 +4281,14 @@ init_allowed_encryption_methods(PGconn *conn)
 #ifdef USE_SSL
 	/* sslmode anything but 'disable', and GSSAPI not required */
 	if (conn->sslmode[0] != 'd' && conn->gssencmode[0] != 'r')
-		conn->allowed_enc_methods |= ENC_NEGOTIATED_SSL;
+	{
+		if (conn->sslnegotiation[0] == 'p')
+			conn->allowed_enc_methods |= ENC_NEGOTIATED_SSL;
+		else if (conn->sslnegotiation[0] == 'd')
+			conn->allowed_enc_methods |= ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL;
+		else if (conn->sslnegotiation[0] == 'r')
+			conn->allowed_enc_methods |= ENC_DIRECT_SSL;
+	}
 #endif
 
 #ifdef ENABLE_GSS
@@ -4266,7 +4326,12 @@ encryption_negotiation_failed(PGconn *conn)
 	conn->failed_enc_methods |= conn->current_enc_method;
 
 	if (select_next_encryption_method(conn, true))
-		return 1;
+	{
+		if (conn->current_enc_method == ENC_DIRECT_SSL)
+			return 2;
+		else
+			return 1;
+	}
 	else
 		return 0;
 }
@@ -4284,6 +4349,18 @@ connection_failed(PGconn *conn)
 	Assert((conn->failed_enc_methods & conn->current_enc_method) == 0);
 	conn->failed_enc_methods |= conn->current_enc_method;
 
+	/*
+	 * If the server reported an error after the SSL handshake, no point in
+	 * retrying with negotiated vs direct SSL.
+	 */
+	if ((conn->current_enc_method & (ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL)) != 0 &&
+		conn->ssl_handshake_started)
+	{
+		conn->failed_enc_methods |= (ENC_DIRECT_SSL | ENC_NEGOTIATED_SSL) & conn->allowed_enc_methods;
+	}
+	else
+		conn->failed_enc_methods |= conn->current_enc_method;
+
 	return select_next_encryption_method(conn, false);
 }
 
@@ -4345,6 +4422,18 @@ select_next_encryption_method(PGconn *conn, bool have_valid_connection)
 	if (conn->sslmode[0] == 'a')
 		SELECT_NEXT_METHOD(ENC_PLAINTEXT);
 
+	/*
+	 * If enabled, try direct SSL. Unless we have a valid TCP connection that
+	 * failed negotiating GSSAPI encryption or a plaintext connection in case
+	 * of sslmode='allow'; in that case we prefer to reuse the connection with
+	 * negotiated SSL, instead of reconnecting to do direct SSL. The point of
+	 * direct SSL is to avoid the roundtrip from the negotiation, but
+	 * reconnecting would also incur a roundtrip.
+	 */
+	if (have_valid_connection)
+		SELECT_NEXT_METHOD(ENC_NEGOTIATED_SSL);
+
+	SELECT_NEXT_METHOD(ENC_DIRECT_SSL);
 	SELECT_NEXT_METHOD(ENC_NEGOTIATED_SSL);
 
 	if (conn->sslmode[0] != 'a')
@@ -4567,6 +4656,7 @@ freePGconn(PGconn *conn)
 	free(conn->keepalives_interval);
 	free(conn->keepalives_count);
 	free(conn->sslmode);
+	free(conn->sslnegotiation);
 	free(conn->sslcert);
 	free(conn->sslkey);
 	if (conn->sslpassword)
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 0c8c9f8dcba..a43e74284f2 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1612,6 +1612,7 @@ pgtls_close(PGconn *conn)
 			SSL_free(conn->ssl);
 			conn->ssl = NULL;
 			conn->ssl_in_use = false;
+			conn->ssl_handshake_started = false;
 
 			destroy_needed = true;
 		}
@@ -1825,9 +1826,10 @@ static BIO_METHOD *my_bio_methods;
 static int
 my_sock_read(BIO *h, char *buf, int size)
 {
+	PGconn	   *conn = (PGconn *) BIO_get_app_data(h);
 	int			res;
 
-	res = pqsecure_raw_read((PGconn *) BIO_get_app_data(h), buf, size);
+	res = pqsecure_raw_read(conn, buf, size);
 	BIO_clear_retry_flags(h);
 	if (res < 0)
 	{
@@ -1849,6 +1851,9 @@ my_sock_read(BIO *h, char *buf, int size)
 		}
 	}
 
+	if (res > 0)
+		conn->ssl_handshake_started = true;
+
 	return res;
 }
 
diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index c0443d68fdc..73f6e65ae55 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -73,7 +73,7 @@ typedef enum
 	CONNECTION_AUTH_OK,			/* Received authentication; waiting for
 								 * backend startup. */
 	CONNECTION_SETENV,			/* This state is no longer used. */
-	CONNECTION_SSL_STARTUP,		/* Negotiating SSL. */
+	CONNECTION_SSL_STARTUP,		/* Performing SSL handshake. */
 	CONNECTION_NEEDED,			/* Internal state: connect() needed. */
 	CONNECTION_CHECK_WRITABLE,	/* Checking if session is read-write. */
 	CONNECTION_CONSUME,			/* Consuming any extra messages. */
@@ -81,7 +81,7 @@ typedef enum
 	CONNECTION_CHECK_TARGET,	/* Internal state: checking target server
 								 * properties. */
 	CONNECTION_CHECK_STANDBY,	/* Checking if server is in standby mode. */
-	CONNECTION_ALLOCATED		/* Waiting for connection attempt to be
+	CONNECTION_ALLOCATED,		/* Waiting for connection attempt to be
 								 * started.  */
 } ConnStatusType;
 
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 0119cb4cfae..3691e5ee969 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -235,7 +235,8 @@ typedef enum
 #define ENC_ERROR			0
 #define ENC_PLAINTEXT		0x01
 #define ENC_GSSAPI			0x02
-#define ENC_NEGOTIATED_SSL	0x04
+#define ENC_DIRECT_SSL		0x04
+#define ENC_NEGOTIATED_SSL	0x08
 
 /* Target server type (decoded value of target_session_attrs) */
 typedef enum
@@ -394,6 +395,8 @@ struct pg_conn
 	char	   *keepalives_count;	/* maximum number of TCP keepalive
 									 * retransmits */
 	char	   *sslmode;		/* SSL mode (require,prefer,allow,disable) */
+	char	   *sslnegotiation; /* SSL initiation style
+								 * (postgres,direct,requiredirect) */
 	char	   *sslcompression; /* SSL compression (0 or 1) */
 	char	   *sslkey;			/* client key filename */
 	char	   *sslcert;		/* client certificate filename */
@@ -563,6 +566,7 @@ struct pg_conn
 
 	/* SSL structures */
 	bool		ssl_in_use;
+	bool		ssl_handshake_started;
 	bool		ssl_cert_requested; /* Did the server ask us for a cert? */
 	bool		ssl_cert_sent;	/* Did we send one in reply? */