From 17ee70be1279807b3cff0f496127f6fc64894f73 Mon Sep 17 00:00:00 2001 From: Greg Stark Date: Wed, 15 Mar 2023 15:51:02 -0400 Subject: [PATCH v7 03/12] Direct SSL connections postmaster support --- src/backend/libpq/be-secure.c | 13 +++ src/backend/libpq/pqcomm.c | 9 +- src/backend/postmaster/postmaster.c | 135 ++++++++++++++++++++++------ src/include/libpq/libpq-be.h | 10 +++ src/include/libpq/libpq.h | 2 +- 5 files changed, 136 insertions(+), 33 deletions(-) diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index 6923c241b99..0e4786cb2b6 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 c606bf34473..ff3a03a2428 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -1136,13 +1136,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 feb471dd1df..085c940aec5 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -423,6 +423,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,99 @@ 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(); + + pq_startmsgread(); + firstbyte = pq_peekbyte(); + pq_endmsgread(); + 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 +2048,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 +2112,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 +2124,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 +2168,7 @@ retry1: GSSok = 'G'; #endif - while (send(port->sock, &GSSok, 1, 0) != 1) + while (secure_write(port, &GSSok, 1) != 1) { if (errno == EINTR) continue; @@ -4347,7 +4422,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); /* * If we're going to reject the connection due to database state, say so diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 47d66d55241..3bb80ae073d 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -227,6 +227,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 6171a0d17a5..79c5f6b754b 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