From 53cd555f73d91150db7bc316bc3dca831ee98ce3 Mon Sep 17 00:00:00 2001 From: Greg Stark 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