]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
libpq: Extend "read pending" check from SSL to GSS
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Tue, 7 Jul 2026 15:45:34 +0000 (18:45 +0300)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Tue, 7 Jul 2026 15:53:51 +0000 (18:53 +0300)
An extra check for pending bytes in the SSL layer has been part of
pqReadReady() for a very long time (79ff2e96d). But when GSS transport
encryption was added, it didn't receive the same treatment. (As
79ff2e96d notes, "The bug that I fixed in this patch is exceptionally
hard to reproduce reliably.")

Without that check, it's possible to hit a hang in gssencmode, if the
server splits a large libpq message such that the final message in a
streamed response is part of the same wrapped token as the split
message:

    DataRowDataRowDataRowDataRowDataRowData
    -- token boundary --
    RowDataRowCommandCompleteReadyForQuery

If the split message takes up enough memory to nearly fill libpq's
receive buffer, libpq may return from pqReadData() before the later
messages are pulled out of the PqGSSRecvBuffer. Without additional
socket activity from the server, pqReadReady() (via pqSocketCheck())
will never again return true, hanging the connection.

Pull the pending-bytes check into the pqsecure API layer, where both
SSL and GSS now implement it.

Note that this does not fix the root problem! Third party clients of
libpq have no way to call pqsecure_read_is_pending() in their own
polling. This just brings the GSS implementation up to par with the
existing SSL workaround; a broader fix is left to a subsequent commit.

In preparation for the broader fix, this patch already changes the
*_read_pending() functions to return the number of bytes in the buffer
rather than just a boolean. The current callers don't need that, but
the subsequent fix will.

Author: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://postgr.es/m/CAOYmi%2BmpymrgZ76Jre2dx_PwRniS9YZojwH0rZnTuiGHCsj0rA%40mail.gmail.com
Backpatch-through: 14

src/interfaces/libpq/fe-misc.c
src/interfaces/libpq/fe-secure-gssapi.c
src/interfaces/libpq/fe-secure-openssl.c
src/interfaces/libpq/fe-secure.c
src/interfaces/libpq/libpq-int.h

index d416760127dab134660fa9d8253b9c6ed7f19e72..1d87cdaa068f65089769a9c63ef69d2238eaa092 100644 (file)
@@ -1080,14 +1080,12 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
                return -1;
        }
 
-#ifdef USE_SSL
-       /* Check for SSL library buffering read bytes */
-       if (forRead && conn->ssl_in_use && pgtls_read_pending(conn))
+       /* Check for SSL/GSS library buffering read bytes */
+       if (forRead && pqsecure_bytes_pending(conn) != 0)
        {
                /* short-circuit the select */
                return 1;
        }
-#endif
 
        /* We will retry as long as we get EINTR */
        do
index 39b59318368952698e37232b383f81f6ca4a8a2b..95717b1a91673603669fe65ba7d959a345864e05 100644 (file)
@@ -475,6 +475,13 @@ gss_read(PGconn *conn, void *recv_buffer, size_t length, ssize_t *ret)
        return PGRES_POLLING_OK;
 }
 
+ssize_t
+pg_GSS_bytes_pending(PGconn *conn)
+{
+       Assert(PqGSSResultLength >= PqGSSResultNext);
+       return (ssize_t) (PqGSSResultLength - PqGSSResultNext);
+}
+
 /*
  * Negotiate GSSAPI transport for a connection.  When complete, returns
  * PGRES_POLLING_OK.  Will return PGRES_POLLING_READING or
index d22b5279b1259ce407b6acf9297898b073c5d6da..d1fbe1faf6e9539bc1494d759f44e4c45fc3afb0 100644 (file)
@@ -268,10 +268,40 @@ rloop:
        return n;
 }
 
-bool
-pgtls_read_pending(PGconn *conn)
+ssize_t
+pgtls_bytes_pending(PGconn *conn)
 {
-       return SSL_pending(conn->ssl) > 0;
+       int                     pending;
+
+       /*
+        * OpenSSL readahead is documented to break SSL_pending().
+        */
+       Assert(!SSL_get_read_ahead(conn->ssl));
+
+       pending = SSL_pending(conn->ssl);
+       if (pending < 0)
+       {
+               /* shouldn't be possible */
+               Assert(false);
+               appendPQExpBufferStr(&conn->errorMessage,
+                                                        "OpenSSL reports negative bytes pending\n");
+               return -1;
+       }
+       else if (pending == INT_MAX)
+       {
+               /*
+                * If we ever found a legitimate way to hit this, we'd need to loop
+                * around in the caller to call pgtls_bytes_pending() again.  Throw an
+                * error rather than complicate the code in that way, because
+                * SSL_read() should be bounded to the size of a single TLS record,
+                * and conn->inBuffer can't currently go past INT_MAX in size anyway.
+                */
+               appendPQExpBufferStr(&conn->errorMessage,
+                                                        "OpenSSL reports INT_MAX bytes pending");
+               return -1;
+       }
+
+       return (ssize_t) pending;
 }
 
 ssize_t
index 4c85b7299cc9fd2ea3b61a3e22b4c4692fa5244c..2fc21e178089eed180c8d3b688c27dd2b9a79949 100644 (file)
@@ -284,6 +284,28 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len)
        return n;
 }
 
+/*
+ *     Return the number of bytes available in the transport buffer.
+ *
+ * If pqsecure_read() is called for this number of bytes, it's guaranteed to
+ * return successfully without reading from the underlying socket.
+ */
+ssize_t
+pqsecure_bytes_pending(PGconn *conn)
+{
+#ifdef USE_SSL
+       if (conn->ssl_in_use)
+               return pgtls_bytes_pending(conn);
+#endif
+#ifdef ENABLE_GSS
+       if (conn->gssenc)
+               return pg_GSS_bytes_pending(conn);
+#endif
+
+       /* Plaintext connections have no transport buffer. */
+       return 0;
+}
+
 /*
  *     Write data to a secure connection.
  *
index 0456c04857eb8a59d34149f46a89afe16c7f57cd..b2d4afda74ca6784f8f4343f6f316b18ce9e92f6 100644 (file)
@@ -743,6 +743,7 @@ extern int  pqsecure_initialize(PGconn *, bool, bool);
 extern PostgresPollingStatusType pqsecure_open_client(PGconn *);
 extern void pqsecure_close(PGconn *);
 extern ssize_t pqsecure_read(PGconn *, void *ptr, size_t len);
+extern ssize_t pqsecure_bytes_pending(PGconn *);
 extern ssize_t pqsecure_write(PGconn *, const void *ptr, size_t len);
 extern ssize_t pqsecure_raw_read(PGconn *, void *ptr, size_t len);
 extern ssize_t pqsecure_raw_write(PGconn *, const void *ptr, size_t len);
@@ -796,9 +797,9 @@ extern void pgtls_close(PGconn *conn);
 extern ssize_t pgtls_read(PGconn *conn, void *ptr, size_t len);
 
 /*
- *     Is there unread data waiting in the SSL read buffer?
+ *     Return the number of bytes available in the transport buffer.
  */
-extern bool pgtls_read_pending(PGconn *conn);
+extern ssize_t pgtls_bytes_pending(PGconn *conn);
 
 /*
  *     Write data to a secure connection.
@@ -852,6 +853,7 @@ extern PostgresPollingStatusType pqsecure_open_gss(PGconn *conn);
  */
 extern ssize_t pg_GSS_write(PGconn *conn, const void *ptr, size_t len);
 extern ssize_t pg_GSS_read(PGconn *conn, void *ptr, size_t len);
+extern ssize_t pg_GSS_bytes_pending(PGconn *conn);
 #endif
 
 /* === in libpq-trace.c === */