Hi all,

During the gssencmode CVE discussion, we noticed that PQconnectPoll()
handles the error cases for TLS and GSS transport encryption slightly
differently. After TLS fails, the connection handle is dead and future
calls to PQconnectPoll() return immediately. But after GSS encryption
fails, the connection handle can still be used to reenter the GSS
handling code.

This doesn't appear to have any security implications today -- and a
client has to actively try to reuse a handle that's already failed --
but it seems undesirable. Michael (cc'd) came up with a patch, which I
have attached here and will register in the CF.

Thanks,
--Jacob
From 75b1be5f484a28e543290e208474d3603a3270bf Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchamp...@timescale.com>
Date: Mon, 13 Feb 2023 10:30:43 -0800
Subject: [PATCH] PQconnectPoll: poison connection on gssenc error

Currently, conn->status isn't updated when gssenc establishment fails,
which allows any (misbehaved) future calls to PQconnectPoll() to reenter
CONNECTION_GSS_STARTUP. Align this code with the CONNECTION_SSL_STARTUP
case, which jumps to error_return and poisons the connection handle.

Patch is by Michael Paquier; I just rebased over HEAD.
---
 src/interfaces/libpq/fe-connect.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 50b5df3490..1070b6db24 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3148,17 +3148,22 @@ keep_going:						/* We will come back to here until there is
 					conn->status = CONNECTION_MADE;
 					return PGRES_POLLING_WRITING;
 				}
-				else if (pollres == PGRES_POLLING_FAILED &&
-						 conn->gssencmode[0] == 'p')
+				else if (pollres == PGRES_POLLING_FAILED)
 				{
-					/*
-					 * We failed, but we can retry on "prefer".  Have to drop
-					 * the current connection to do so, though.
-					 */
-					conn->try_gss = false;
-					need_new_connection = true;
-					goto keep_going;
+					if (conn->gssencmode[0] == 'p')
+					{
+						/*
+						 * We failed, but we can retry on "prefer".  Have to drop
+						 * the current connection to do so, though.
+						 */
+						conn->try_gss = false;
+						need_new_connection = true;
+						goto keep_going;
+					}
+					/* Else it's a hard failure */
+					goto error_return;
 				}
+				/* Else, return POLLING_READING or POLLING_WRITING status */
 				return pollres;
 #else							/* !ENABLE_GSS */
 				/* unreachable */
-- 
2.25.1

Reply via email to