From ff6ce9a3a691a96e8e47ed449bc51c5a178e6931 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 28 Dec 2020 15:43:44 -0500 Subject: Fix bugs in libpq's GSSAPI encryption support. The critical issue fixed here is that if a GSSAPI-encrypted connection is successfully made, pqsecure_open_gss() cleared conn->allow_ssl_try, as an admittedly-hacky way of preventing us from then trying to tunnel SSL encryption over the already-encrypted connection. The problem with that is that if we abandon the GSSAPI connection because of a failure during authentication, we would not attempt SSL encryption in the next try with the same server. This can lead to unexpected connection failure, or silently getting a non-encrypted connection where an encrypted one is expected. Fortunately, we'd only manage to make a GSSAPI-encrypted connection if both client and server hold valid tickets in the same Kerberos infrastructure, which is a relatively uncommon environment. Nonetheless this is a very nasty bug with potential security consequences. To fix, don't reset the flag, instead adding a check for conn->gssenc being already true when deciding whether to try to initiate SSL. While here, fix some lesser issues in libpq's GSSAPI code: * Use the need_new_connection stanza when dropping an attempted GSSAPI connection, instead of partially duplicating that code. The consequences of this are pretty minor: AFAICS it could only lead to auth_req_received or password_needed remaining set when they shouldn't, which is not too harmful. * Fix pg_GSS_error() to not repeat the "mprefix" it's given multiple times, and to notice any failure return from gss_display_status(). * Avoid gratuitous dependency on NI_MAXHOST in pg_GSS_load_servicename(). Per report from Mikael Gustavsson. Back-patch to v12 where this code was introduced. Discussion: https://postgr.es/m/e5b0b6ed05764324a2f3fe7acfc766d5@smhi.se --- src/interfaces/libpq/fe-connect.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'src/interfaces/libpq/fe-connect.c') diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index d2b1c207eb4..ae4866c8e42 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -2909,11 +2909,16 @@ keep_going: /* We will come back to here until there is #ifdef USE_SSL /* - * If SSL is enabled and we haven't already got it running, - * request it instead of sending the startup message. + * If SSL is enabled and we haven't already got encryption of + * some sort running, request SSL instead of sending the + * startup message. */ if (conn->allow_ssl_try && !conn->wait_ssl_try && - !conn->ssl_in_use) + !conn->ssl_in_use +#ifdef ENABLE_GSS + && !conn->gssenc +#endif + ) { ProtocolVersion pv; @@ -3042,6 +3047,7 @@ keep_going: /* We will come back to here until there is } /* Otherwise, proceed with normal startup */ conn->allow_ssl_try = false; + /* We can proceed using this connection */ conn->status = CONNECTION_MADE; return PGRES_POLLING_WRITING; } @@ -3139,8 +3145,7 @@ keep_going: /* We will come back to here until there is * don't hang up the socket, though. */ conn->try_gss = false; - pqDropConnection(conn, true); - conn->status = CONNECTION_NEEDED; + need_new_connection = true; goto keep_going; } @@ -3158,6 +3163,7 @@ keep_going: /* We will come back to here until there is } conn->try_gss = false; + /* We can proceed using this connection */ conn->status = CONNECTION_MADE; return PGRES_POLLING_WRITING; } @@ -3186,8 +3192,7 @@ keep_going: /* We will come back to here until there is * the current connection to do so, though. */ conn->try_gss = false; - pqDropConnection(conn, true); - conn->status = CONNECTION_NEEDED; + need_new_connection = true; goto keep_going; } return pollres; @@ -3354,10 +3359,9 @@ keep_going: /* We will come back to here until there is */ if (conn->gssenc && conn->gssencmode[0] == 'p') { - /* postmaster expects us to drop the connection */ + /* only retry once */ conn->try_gss = false; - pqDropConnection(conn, true); - conn->status = CONNECTION_NEEDED; + need_new_connection = true; goto keep_going; } #endif -- cgit v1.2.3