Here's a first cut at ALPN support.
Currently it's using a hard coded "Postgres/3.0" protocol (hard coded
both in the client and the server...). And it's hard coded to be
required for direct connections and supported but not required for
regular connections.
IIRC I put a variable labeled a "GUC" but forgot to actually make it a
GUC. But I'm thinking of maybe removing that variable since I don't
see much of a use case for controlling this manually. I *think* ALPN
is supported by all the versions of OpenSSL we support.
The other patches are unchanged (modulo a free() that I missed in the
client before). They still have the semi-open issues I mentioned in
the previous email.
--
greg
From 32185020927824c4b57af900100a37f92ae6a040 Mon Sep 17 00:00:00 2001
From: Greg Stark <st...@mit.edu>
Date: Mon, 20 Mar 2023 14:09:30 -0400
Subject: [PATCH v3 4/4] alpn support
---
src/backend/libpq/be-secure-openssl.c | 65 ++++++++++++++++++++++++
src/backend/libpq/be-secure.c | 3 ++
src/backend/postmaster/postmaster.c | 23 +++++++++
src/bin/psql/command.c | 7 ++-
src/include/libpq/libpq-be.h | 1 +
src/include/libpq/libpq.h | 1 +
src/interfaces/libpq/fe-connect.c | 5 ++
src/interfaces/libpq/fe-secure-openssl.c | 25 +++++++++
src/interfaces/libpq/libpq-int.h | 1 +
9 files changed, 129 insertions(+), 2 deletions(-)
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 685aa2ed69..034e1cf2ec 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -67,6 +67,12 @@ static int ssl_external_passwd_cb(char *buf, int size, int rwflag, void *userdat
static int dummy_ssl_passwd_cb(char *buf, int size, int rwflag, void *userdata);
static int verify_cb(int ok, X509_STORE_CTX *ctx);
static void info_cb(const SSL *ssl, int type, int args);
+static int alpn_cb(SSL *ssl,
+ const unsigned char **out,
+ unsigned char *outlen,
+ const unsigned char *in,
+ unsigned int inlen,
+ void *userdata);
static bool initialize_dh(SSL_CTX *context, bool isServerStart);
static bool initialize_ecdh(SSL_CTX *context, bool isServerStart);
static const char *SSLerrmessage(unsigned long ecode);
@@ -432,6 +438,11 @@ be_tls_open_server(Port *port)
/* set up debugging/info callback */
SSL_CTX_set_info_callback(SSL_context, info_cb);
+ if (ssl_enable_alpn) {
+ elog(WARNING, "Enabling ALPN Callback");
+ SSL_CTX_set_alpn_select_cb(SSL_context, alpn_cb, port);
+ }
+
if (!(port->ssl = SSL_new(SSL_context)))
{
ereport(COMMERROR,
@@ -1250,6 +1261,60 @@ info_cb(const SSL *ssl, int type, int args)
}
}
+static unsigned char alpn_protos[] = {
+ 12, 'P','o','s','t','g','r','e','s','/','3','.','0'
+};
+static unsigned int alpn_protos_len = sizeof(alpn_protos);
+
+/*
+ * Server callback for ALPN negotiation. We use use the standard "helper"
+ * function even though currently we only accept one value. We store the
+ * negotiated protocol in Port->ssl_alpn_protocol and rely on higher level
+ * logic (in postmaster.c) to decide what to do with that info.
+ *
+ * XXX Not sure if it's kosher to use palloc and elog() inside OpenSSL
+ * callbacks. If we throw an error from here is there a risk of messing up
+ * OpenSSL data structures? It's possible it's ok becuase this is only called
+ * during connection negotiation which we don't try to recover from.
+ */
+static int alpn_cb(SSL *ssl,
+ const unsigned char **out,
+ unsigned char *outlen,
+ const unsigned char *in,
+ unsigned int inlen,
+ void *userdata)
+{
+ /* why does OpenSSL provide a helper function that requires a nonconst
+ * vector when the callback is declared to take a const vector? What are
+ * we to do with that?*/
+ int retval;
+ Assert(userdata != NULL);
+ Assert(out != NULL);
+ Assert(outlen != NULL);
+ Assert(in != NULL);
+
+ retval = SSL_select_next_proto((unsigned char **)out, outlen,
+ alpn_protos, alpn_protos_len,
+ in, inlen);
+ if (*out == NULL || *outlen > alpn_protos_len || outlen <= 0)
+ elog(ERROR, "SSL_select_next_proto returned nonsensical results");
+
+ if (retval == OPENSSL_NPN_NEGOTIATED)
+ {
+ struct Port *port = (struct Port *)userdata;
+
+ port->ssl_alpn_protocol = palloc0(*outlen+1);
+ /* SSL uses unsigned chars but Postgres uses host signedness of chars */
+ strncpy(port->ssl_alpn_protocol, (char*)*out, *outlen);
+ return SSL_TLSEXT_ERR_OK;
+ } else if (retval == OPENSSL_NPN_NO_OVERLAP) {
+ return SSL_TLSEXT_ERR_NOACK;
+ } else {
+ return SSL_TLSEXT_ERR_NOACK;
+ }
+}
+
+
/*
* Set DH parameters for generating ephemeral DH keys. The
* DH parameters can take a long time to compute, so they must be
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index 1020b3adb0..79a61900ba 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -61,6 +61,9 @@ bool SSLPreferServerCiphers;
int ssl_min_protocol_version = PG_TLS1_2_VERSION;
int ssl_max_protocol_version = PG_TLS_ANY;
+/* GUC variable: if false ignore ALPN negotiation */
+bool ssl_enable_alpn = true;
+
/* ------------------------------------------------------------ */
/* Procedures common to all secure sessions */
/* ------------------------------------------------------------ */
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index ec1d895a23..2640b69fed 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -1934,6 +1934,8 @@ ServerLoop(void)
* any bytes from the stream if it's not a direct SSL connection.
*/
+static const char *expected_alpn_protocol = "Postgres/3.0";
+
static int
ProcessSSLStartup(Port *port)
{
@@ -1970,6 +1972,10 @@ ProcessSSLStartup(Port *port)
char *buf = NULL;
elog(LOG, "Detected direct SSL handshake");
+ if (!ssl_enable_alpn) {
+ elog(WARNING, "Received direct SSL connection without ssl_enable_alpn enabled");
+ }
+
/* push unencrypted buffered data back through SSL setup */
len = pq_buffer_has_data();
if (len > 0)
@@ -2000,6 +2006,23 @@ ProcessSSLStartup(Port *port)
return STATUS_ERROR;
}
pfree(port->raw_buf);
+
+ if (port->ssl_alpn_protocol == NULL)
+ {
+ ereport(COMMERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("Received direct SSL connection request without required ALPN protocol negotiation extension")));
+ return STATUS_ERROR;
+ }
+ if (strcmp(port->ssl_alpn_protocol, expected_alpn_protocol) != 0)
+ {
+ ereport(COMMERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("Received direct SSL connection request with unexpected ALPN protocol \"%s\" expected \"%s\"",
+ port->ssl_alpn_protocol,
+ expected_alpn_protocol)));
+ return STATUS_ERROR;
+ }
#else
ereport(COMMERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 61ec049f05..a219962ea1 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -3715,6 +3715,7 @@ printSSLInfo(void)
const char *protocol;
const char *cipher;
const char *compression;
+ const char *alpn;
if (!PQsslInUse(pset.db))
return; /* no SSL */
@@ -3722,11 +3723,13 @@ printSSLInfo(void)
protocol = PQsslAttribute(pset.db, "protocol");
cipher = PQsslAttribute(pset.db, "cipher");
compression = PQsslAttribute(pset.db, "compression");
+ alpn = PQsslAttribute(pset.db, "alpn");
- printf(_("SSL connection (protocol: %s, cipher: %s, compression: %s)\n"),
+ printf(_("SSL connection (protocol: %s, cipher: %s, compression: %s, ALPN: %s)\n"),
protocol ? protocol : _("unknown"),
cipher ? cipher : _("unknown"),
- (compression && strcmp(compression, "off") != 0) ? _("on") : _("off"));
+ (compression && strcmp(compression, "off") != 0) ? _("on") : _("off"),
+ alpn ? alpn : _("none"));
}
/*
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 824b28e824..2258241770 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -217,6 +217,7 @@ typedef struct Port
char *peer_cn;
char *peer_dn;
bool peer_cert_valid;
+ char *ssl_alpn_protocol;
/*
* OpenSSL structures. (Keep these last so that the locations of other
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 2b02f67257..e7adf4a989 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -123,6 +123,7 @@ extern PGDLLIMPORT char *SSLECDHCurve;
extern PGDLLIMPORT bool SSLPreferServerCiphers;
extern PGDLLIMPORT int ssl_min_protocol_version;
extern PGDLLIMPORT int ssl_max_protocol_version;
+extern PGDLLIMPORT bool ssl_enable_alpn;
enum ssl_protocol_versions
{
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 80d3e9169b..d2aa0f3941 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -307,6 +307,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
"SSL-SNI", "", 1,
offsetof(struct pg_conn, sslsni)},
+ {"sslalpn", "PGSSLALPN", "1", NULL,
+ "SSL-ALPN", "", 1,
+ offsetof(struct pg_conn, sslalpn)},
+
{"requirepeer", "PGREQUIREPEER", NULL, NULL,
"Require-Peer", "", 10,
offsetof(struct pg_conn, requirepeer)},
@@ -4322,6 +4326,7 @@ freePGconn(PGconn *conn)
free(conn->sslcrldir);
free(conn->sslcompression);
free(conn->sslsni);
+ free(conn->sslalpn);
free(conn->requirepeer);
free(conn->require_auth);
free(conn->ssl_min_protocol_version);
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 6a4431ddfe..1b15542894 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -884,6 +884,13 @@ destroy_ssl_system(void)
#endif
}
+/* XXX This should move somewhere I guess */
+static unsigned char alpn_protos[] = {
+ 12, 'P','o','s','t','g','r','e','s','/','3','.','0'
+};
+static unsigned int alpn_protos_len = sizeof(alpn_protos);
+
+
/*
* Create per-connection SSL object, and load the client certificate,
* private key, and trusted CA certs.
@@ -1202,6 +1209,11 @@ initialize_SSL(PGconn *conn)
}
}
+ if (conn->sslalpn && conn->sslalpn[0] == '1')
+ {
+ SSL_set_alpn_protos(conn->ssl, alpn_protos, alpn_protos_len);
+ }
+
/*
* Read the SSL key. If a key is specified, treat it as an engine:key
* combination if there is colon present - we don't support files with
@@ -1739,6 +1751,19 @@ PQsslAttribute(PGconn *conn, const char *attribute_name)
if (strcmp(attribute_name, "protocol") == 0)
return SSL_get_version(conn->ssl);
+ if (strcmp(attribute_name, "alpn") == 0)
+ {
+ const unsigned char *data;
+ unsigned int len;
+ static char alpn_str[256]; /* alpn doesn't support longer than 255 bytes */
+ SSL_get0_alpn_selected(conn->ssl, &data, &len);
+ if (data == NULL || len==0 || len > sizeof(alpn_str)-1)
+ return NULL;
+ memcpy(alpn_str, data, len);
+ alpn_str[len] = 0;
+ return alpn_str;
+ }
+
return NULL; /* unknown attribute */
}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 8d8964d835..6cdb5d22b3 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -389,6 +389,7 @@ struct pg_conn
char *sslcrl; /* certificate revocation list filename */
char *sslcrldir; /* certificate revocation list directory name */
char *sslsni; /* use SSL SNI extension (0 or 1) */
+ char *sslalpn; /* use SSL ALPN extension (0 or 1) */
char *requirepeer; /* required peer credentials for local sockets */
char *gssencmode; /* GSS mode (require,prefer,disable) */
char *krbsrvname; /* Kerberos service name */
--
2.39.2
From 53cd555f73d91150db7bc316bc3dca831ee98ce3 Mon Sep 17 00:00:00 2001
From: Greg Stark <st...@mit.edu>
Date: Wed, 15 Mar 2023 15:51:02 -0400
Subject: [PATCH v3 1/4] Direct SSL connections postmaster support
---
src/backend/libpq/be-secure.c | 13 +++
src/backend/libpq/pqcomm.c | 9 +-
src/backend/postmaster/postmaster.c | 133 ++++++++++++++++++++++------
src/include/libpq/libpq-be.h | 10 +++
src/include/libpq/libpq.h | 2 +-
5 files changed, 134 insertions(+), 33 deletions(-)
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index a0f7084018..1020b3adb0 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -235,6 +235,19 @@ secure_raw_read(Port *port, void *ptr, size_t len)
{
ssize_t n;
+ /* Read from the "unread" buffered data first. c.f. libpq-be.h */
+ if (port->raw_buf_remaining > 0)
+ {
+ /* consume up to len bytes from the raw_buf */
+ if (len > port->raw_buf_remaining)
+ len = port->raw_buf_remaining;
+ Assert(port->raw_buf);
+ memcpy(ptr, port->raw_buf + port->raw_buf_consumed, len);
+ port->raw_buf_consumed += len;
+ port->raw_buf_remaining -= len;
+ return len;
+ }
+
/*
* Try to read from the socket without blocking. If it succeeds we're
* done, otherwise we'll wait for the socket using the latch mechanism.
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index da5bb5fc5d..17d025bb8e 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -1126,13 +1126,16 @@ pq_discardbytes(size_t len)
/* --------------------------------
* pq_buffer_has_data - is any buffered data available to read?
*
- * This will *not* attempt to read more data.
+ * Actually returns the number of bytes in the buffer...
+ *
+ * This will *not* attempt to read more data. And reading up to that number of
+ * bytes should not cause reading any more data either.
* --------------------------------
*/
-bool
+size_t
pq_buffer_has_data(void)
{
- return (PqRecvPointer < PqRecvLength);
+ return (PqRecvLength - PqRecvPointer);
}
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 4c49393fc5..ec1d895a23 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -422,6 +422,7 @@ static void BackendRun(Port *port) pg_attribute_noreturn();
static void ExitPostmaster(int status) pg_attribute_noreturn();
static int ServerLoop(void);
static int BackendStartup(Port *port);
+static int ProcessSSLStartup(Port *port);
static int ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done);
static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options);
static void processCancelRequest(Port *port, void *pkt);
@@ -1926,6 +1927,97 @@ ServerLoop(void)
}
}
+/*
+ * Check for a native direct SSL connection.
+ *
+ * This happens before startup packets so we are careful not to actual read
+ * any bytes from the stream if it's not a direct SSL connection.
+ */
+
+static int
+ProcessSSLStartup(Port *port)
+{
+ int firstbyte;
+
+ pq_startmsgread();
+
+ firstbyte = pq_peekbyte();
+ if (firstbyte == EOF)
+ {
+ /*
+ * If we get no data at all, don't clutter the log with a complaint;
+ * such cases often occur for legitimate reasons. An example is that
+ * we might be here after responding to NEGOTIATE_SSL_CODE, and if the
+ * client didn't like our response, it'll probably just drop the
+ * connection. Service-monitoring software also often just opens and
+ * closes a connection without sending anything. (So do port
+ * scanners, which may be less benign, but it's not really our job to
+ * notice those.)
+ */
+ return STATUS_ERROR;
+ }
+
+ /*
+ * First byte indicates standard SSL handshake message
+ *
+ * (It can't be a Postgres startup length because in network byte order
+ * that would be a startup packet hundreds of megabytes long)
+ */
+ if (firstbyte == 0x16)
+ {
+#ifdef USE_SSL
+ ssize_t len;
+ char *buf = NULL;
+ elog(LOG, "Detected direct SSL handshake");
+
+ /* push unencrypted buffered data back through SSL setup */
+ len = pq_buffer_has_data();
+ if (len > 0)
+ {
+ buf = palloc(len);
+ if (pq_getbytes(buf, len) == EOF)
+ return STATUS_ERROR; /* shouldn't be possible */
+ port->raw_buf = buf;
+ port->raw_buf_remaining = len;
+ port->raw_buf_consumed = 0;
+ }
+
+ Assert(pq_buffer_has_data() == 0);
+ if (secure_open_server(port) == -1)
+ {
+ ereport(COMMERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("SSL Protocol Error during direct SSL connection initiation")));
+ return STATUS_ERROR;
+ }
+
+ if (port->raw_buf_remaining > 0)
+ {
+ /* This shouldn't be possible -- it would mean the client sent
+ * encrypted data before we established a session key...
+ */
+ elog(LOG, "Buffered unencrypted data remains after negotiating native SSL connection");
+ return STATUS_ERROR;
+ }
+ pfree(port->raw_buf);
+#else
+ ereport(COMMERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("Received direct SSL connection request with no SSL support")));
+ return STATUS_ERROR;
+#endif
+ }
+
+ pq_endmsgread();
+
+ if (port->ssl_in_use)
+ ereport(DEBUG2,
+ (errmsg_internal("Direct SSL connection established")));
+
+ return STATUS_OK;
+}
+
+
/*
* Read a client's startup packet and do something according to it.
*
@@ -1954,28 +2046,7 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
pq_startmsgread();
- /*
- * Grab the first byte of the length word separately, so that we can tell
- * whether we have no data at all or an incomplete packet. (This might
- * sound inefficient, but it's not really, because of buffering in
- * pqcomm.c.)
- */
- if (pq_getbytes((char *) &len, 1) == EOF)
- {
- /*
- * If we get no data at all, don't clutter the log with a complaint;
- * such cases often occur for legitimate reasons. An example is that
- * we might be here after responding to NEGOTIATE_SSL_CODE, and if the
- * client didn't like our response, it'll probably just drop the
- * connection. Service-monitoring software also often just opens and
- * closes a connection without sending anything. (So do port
- * scanners, which may be less benign, but it's not really our job to
- * notice those.)
- */
- return STATUS_ERROR;
- }
-
- if (pq_getbytes(((char *) &len) + 1, 3) == EOF)
+ if (pq_getbytes(((char *) &len), 4) == EOF)
{
/* Got a partial length word, so bleat about that */
if (!ssl_done && !gss_done)
@@ -2039,8 +2110,11 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
char SSLok;
#ifdef USE_SSL
- /* No SSL when disabled or on Unix sockets */
- if (!LoadedSSL || port->laddr.addr.ss_family == AF_UNIX)
+ /* No SSL when disabled or on Unix sockets.
+ *
+ * Also no SSL negotiation if we already have a direct SSL connection
+ */
+ if (!LoadedSSL || port->laddr.addr.ss_family == AF_UNIX || port->ssl_in_use)
SSLok = 'N';
else
SSLok = 'S'; /* Support for SSL */
@@ -2048,11 +2122,10 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
SSLok = 'N'; /* No support for SSL */
#endif
-retry1:
- if (send(port->sock, &SSLok, 1, 0) != 1)
+ while (secure_write(port, &SSLok, 1) != 1)
{
if (errno == EINTR)
- goto retry1; /* if interrupted, just retry */
+ continue; /* if interrupted, just retry */
ereport(COMMERROR,
(errcode_for_socket_access(),
errmsg("failed to send SSL negotiation response: %m")));
@@ -2093,7 +2166,7 @@ retry1:
GSSok = 'G';
#endif
- while (send(port->sock, &GSSok, 1, 0) != 1)
+ while (secure_write(port, &GSSok, 1) != 1)
{
if (errno == EINTR)
continue;
@@ -4396,7 +4469,9 @@ BackendInitialize(Port *port)
* Receive the startup packet (which might turn out to be a cancel request
* packet).
*/
- status = ProcessStartupPacket(port, false, false);
+ status = ProcessSSLStartup(port);
+ if (status == STATUS_OK)
+ status = ProcessStartupPacket(port, false, false);
/*
* Disable the timeout, and prevent SIGTERM again.
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index ac6407e9f6..824b28e824 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -226,6 +226,16 @@ typedef struct Port
SSL *ssl;
X509 *peer;
#endif
+ /* This is a bit of a hack. The raw_buf is data that was previously read
+ * and buffered in a higher layer but then "unread" and needs to be read
+ * again while establishing an SSL connection via the SSL library layer.
+ *
+ * There's no API to "unread", the upper layer just places the data in the
+ * Port structure in raw_buf and sets raw_buf_remaining to the amount of
+ * bytes unread and raw_buf_consumed to 0.
+ */
+ char *raw_buf;
+ ssize_t raw_buf_consumed, raw_buf_remaining;
} Port;
#ifdef USE_SSL
diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h
index 50fc781f47..2b02f67257 100644
--- a/src/include/libpq/libpq.h
+++ b/src/include/libpq/libpq.h
@@ -80,7 +80,7 @@ extern int pq_getmessage(StringInfo s, int maxlen);
extern int pq_getbyte(void);
extern int pq_peekbyte(void);
extern int pq_getbyte_if_available(unsigned char *c);
-extern bool pq_buffer_has_data(void);
+extern size_t pq_buffer_has_data(void);
extern int pq_putmessage_v2(char msgtype, const char *s, size_t len);
extern bool pq_check_connection(void);
--
2.39.2
From 058e215f801b3e7ade5cacfc7e1889966430624c Mon Sep 17 00:00:00 2001
From: Greg Stark <st...@mit.edu>
Date: Thu, 16 Mar 2023 11:55:16 -0400
Subject: [PATCH v3 2/4] Direct SSL connections client support
---
src/interfaces/libpq/fe-connect.c | 91 +++++++++++++++++++++++++++++--
src/interfaces/libpq/libpq-fe.h | 1 +
src/interfaces/libpq/libpq-int.h | 3 +
3 files changed, 89 insertions(+), 6 deletions(-)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index b9f899c552..80d3e9169b 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -271,6 +271,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = {
"SSL-Mode", "", 12, /* sizeof("verify-full") == 12 */
offsetof(struct pg_conn, sslmode)},
+ {"sslnegotiation", "PGSSLNEGOTIATION", "postgres", NULL,
+ "SSL-Negotiation", "", 14, /* strlen("requiredirect") == 14 */
+ offsetof(struct pg_conn, sslnegotiation)},
+
{"sslcompression", "PGSSLCOMPRESSION", "0", NULL,
"SSL-Compression", "", 1,
offsetof(struct pg_conn, sslcompression)},
@@ -1463,11 +1467,36 @@ connectOptions2(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->negotiation[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->sslmode = strdup(DefaultSSLMode);
- if (!conn->sslmode)
- goto oom_error;
+ libpq_append_conn_error(conn, "sslnegotiation missing?");
+ return false;
}
/*
@@ -1527,6 +1556,18 @@ connectOptions2(PGconn *conn)
conn->gssencmode);
return false;
}
+#endif
+#ifdef USE_SSL
+ /* GSS is incompatible with direct SSL connections so it requires the
+ * default postgres style connection ssl negotiation */
+ if (strcmp(conn->gssencmode, "require") == 0 &&
+ strcmp(conn->sslnegotiation, "postgres") != 0)
+ {
+ conn->status = CONNECTION_BAD;
+ libpq_append_conn_error(conn, "gssencmode value \"%s\" invalid when Direct SSL negotiation is enabled",
+ conn->gssencmode);
+ return false;
+ }
#endif
}
else
@@ -2583,11 +2624,12 @@ keep_going: /* We will come back to here until there is
/* initialize these values based on SSL mode */
conn->allow_ssl_try = (conn->sslmode[0] != 'd'); /* "disable" */
conn->wait_ssl_try = (conn->sslmode[0] == 'a'); /* "allow" */
+ /* direct ssl is incompatible with "allow" or "disabled" ssl */
+ conn->allow_direct_ssl_try = conn->allow_ssl_try && !conn->wait_ssl_try && (conn->sslnegotiation[0] != 'p');
#endif
#ifdef ENABLE_GSS
conn->try_gss = (conn->gssencmode[0] != 'd'); /* "disable" */
#endif
-
reset_connection_state_machine = false;
need_new_connection = true;
}
@@ -3046,6 +3088,28 @@ keep_going: /* We will come back to here until there is
if (pqsecure_initialize(conn, false, true) < 0)
goto error_return;
+ /* If SSL is enabled and direct SSL connections are enabled
+ * and we haven't already established an SSL connection (or
+ * already tried a direct connection and failed or succeeded)
+ * then try just enabling SSL directly.
+ *
+ * If we fail then we'll either fail the connection (if
+ * sslnegotiation is set to requiredirect or turn
+ * allow_direct_ssl_try to false
+ */
+ if (conn->allow_ssl_try
+ && !conn->wait_ssl_try
+ && conn->allow_direct_ssl_try
+ && !conn->ssl_in_use
+#ifdef ENABLE_GSS
+ && !conn->gssenc
+#endif
+ )
+ {
+ conn->status = CONNECTION_SSL_STARTUP;
+ return PGRES_POLLING_WRITING;
+ }
+
/*
* If SSL is enabled and we haven't already got encryption of
* some sort running, request SSL instead of sending the
@@ -3122,9 +3186,11 @@ keep_going: /* We will come back to here until there is
/*
* On first time through, get the postmaster's response to our
- * SSL negotiation packet.
+ * SSL negotiation packet. If we are trying a direct ssl
+ * connection skip reading the negotiation packet and go
+ * straight to initiating an ssl connection.
*/
- if (!conn->ssl_in_use)
+ if (!conn->ssl_in_use && !conn->allow_direct_ssl_try)
{
/*
* We use pqReadData here since it has the logic to
@@ -3230,6 +3296,18 @@ 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 */
+ if (conn->allow_direct_ssl_try)
+ {
+ /* if it's requiredirect then it's a hard failure */
+ if (conn->sslnegotiation[0] == 'r')
+ goto error_return;
+ /* otherwise only retry using postgres connection */
+ conn->allow_direct_ssl_try = false;
+ need_new_connection = true;
+ goto keep_going;
+ }
+
/*
* Failed ... if sslmode is "prefer" then do a non-SSL
* retry
@@ -4231,6 +4309,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/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h
index f3d9220496..05821b8473 100644
--- a/src/interfaces/libpq/libpq-fe.h
+++ b/src/interfaces/libpq/libpq-fe.h
@@ -72,6 +72,7 @@ typedef enum
CONNECTION_AUTH_OK, /* Received authentication; waiting for
* backend startup. */
CONNECTION_SETENV, /* This state is no longer used. */
+ CONNECTION_DIRECT_SSL_STARTUP, /* Starting SSL without PG Negotiation. */
CONNECTION_SSL_STARTUP, /* Negotiating SSL. */
CONNECTION_NEEDED, /* Internal state: connect() needed */
CONNECTION_CHECK_WRITABLE, /* Checking if session is read-write. */
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 1dc264fe54..8d8964d835 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -380,6 +380,7 @@ 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 */
@@ -529,6 +530,8 @@ struct pg_conn
bool ssl_in_use;
#ifdef USE_SSL
+ bool allow_direct_ssl_try; /* Try to make a direct SSL connection
+ * without an "SSL negotiation packet" */
bool allow_ssl_try; /* Allowed to try SSL negotiation */
bool wait_ssl_try; /* Delay SSL negotiation until after
* attempting normal connection */
--
2.39.2
From 15b5d799d013d6e8a242b1e13f425b6e02025cde Mon Sep 17 00:00:00 2001
From: Greg Stark <st...@mit.edu>
Date: Thu, 16 Mar 2023 15:10:15 -0400
Subject: [PATCH v3 3/4] Direct SSL connections documentation
---
doc/src/sgml/libpq.sgml | 102 ++++++++++++++++++++++++++++++++++++----
1 file changed, 93 insertions(+), 9 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 9ee5532c07..0b89b224ba 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1691,10 +1691,13 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
Note that if <acronym>GSSAPI</acronym> encryption is possible,
that will be used in preference to <acronym>SSL</acronym>
encryption, regardless of the value of <literal>sslmode</literal>.
- To force use of <acronym>SSL</acronym> encryption in an
- environment that has working <acronym>GSSAPI</acronym>
- infrastructure (such as a Kerberos server), also
- set <literal>gssencmode</literal> to <literal>disable</literal>.
+ To negotiate <acronym>SSL</acronym> encryption in an environment that
+ has working <acronym>GSSAPI</acronym> infrastructure (such as a
+ Kerberos server), also set <literal>gssencmode</literal>
+ to <literal>disable</literal>.
+ Use of non-default values of <literal>sslnegotiation</literal> can
+ also cause <acronym>SSL</acronym> to be used instead of
+ negotiating <acronym>GSSAPI</acronym> encryption.
</para>
</listitem>
</varlistentry>
@@ -1721,6 +1724,75 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
</listitem>
</varlistentry>
+ <varlistentry id="libpq-connect-sslnegotiation" xreflabel="sslnegotiation">
+ <term><literal>sslnegotiation</literal></term>
+ <listitem>
+ <para>
+ This option controls whether <productname>PostgreSQL</productname>
+ will perform its protocol negotiation to request encryption from the
+ server or will just directly make a standard <acronym>SSL</acronym>
+ connection. Traditional <productname>PostgreSQL</productname>
+ protocol negotiation is the default and the most flexible with
+ different server configurations. If the server is known to support
+ direct <acronym>SSL</acronym> connections then the latter requires one
+ fewer round trip reducing connection latency and also allows the use
+ of protocol agnostic SSL network tools.
+ </para>
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>postgres</literal></term>
+ <listitem>
+ <para>
+ perform <productname>PostgreSQL</productname> protocol
+ negotiation. This is the default if the option is not provided.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>direct</literal></term>
+ <listitem>
+ <para>
+ first attempt to establish a standard SSL connection and if that
+ fails reconnect and perform the negotiation. This fallback
+ process adds significant latency if the initial SSL connection
+ fails.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>requiredirect</literal></term>
+ <listitem>
+ <para>
+ attempt to establish a standard SSL connection and if that fails
+ return a connection failure immediately.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+
+ <para>
+ If <literal>sslmode</literal> set to <literal>disable</literal>
+ or <literal>allow</literal> then <literal>sslnegotiation</literal> is
+ ignored. If <literal>gssencmode</literal> is set
+ to <literal>require</literal> then <literal>sslnegotiation</literal>
+ must be the default <literal>postgres</literal> value.
+ </para>
+
+ <para>
+ Moreover, note if <literal>gssencmode</literal> is set
+ to <literal>prefer</literal> and <literal>sslnegotiation</literal>
+ to <literal>direct</literal> then the effective preference will be
+ direct <acronym>SSL</acronym> connections, followed by
+ negotiated <acronym>GSS</acronym> connections, followed by
+ negotiated <acronym>SSL</acronym> connections, possibly followed
+ lastly by unencrypted connections.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="libpq-connect-sslcompression" xreflabel="sslcompression">
<term><literal>sslcompression</literal></term>
<listitem>
@@ -1874,11 +1946,13 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
<para>
The Server Name Indication can be used by SSL-aware proxies to route
- connections without having to decrypt the SSL stream. (Note that this
- requires a proxy that is aware of the PostgreSQL protocol handshake,
- not just any SSL proxy.) However, <acronym>SNI</acronym> makes the
- destination host name appear in cleartext in the network traffic, so
- it might be undesirable in some cases.
+ connections without having to decrypt the SSL stream. (Note that
+ unless the proxy is aware of the PostgreSQL protocol handshake this
+ would require setting <literal>sslnegotiation</literal>
+ to <literal>direct</literal> or <literal>requiredirect</literal>.)
+ However, <acronym>SNI</acronym> makes the destination host name appear
+ in cleartext in the network traffic, so it might be undesirable in
+ some cases.
</para>
</listitem>
</varlistentry>
@@ -7956,6 +8030,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough)
</para>
</listitem>
+ <listitem>
+ <para>
+ <indexterm>
+ <primary><envar>PGSSLNEGOTIATION</envar></primary>
+ </indexterm>
+ <envar>PGSSLNEGOTIATION</envar> behaves the same as the <xref
+ linkend="libpq-connect-sslnegotiation"/> connection parameter.
+ </para>
+ </listitem>
+
<listitem>
<para>
<indexterm>
--
2.39.2
