aboutsummaryrefslogtreecommitdiff
path: root/src/interfaces/libpq/fe-secure-gssapi.c
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2020-12-28 15:43:44 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2020-12-28 15:43:44 -0500
commitb3a5bf719cf7cbc2eb3f15c0e05a8032aa502cf3 (patch)
tree87b3630f04e66d935a4483d54d31c1b71fa5b1fe /src/interfaces/libpq/fe-secure-gssapi.c
parentd37965965ad30aacad4d46cfa5ca42b416331ce6 (diff)
downloadpostgresql-b3a5bf719cf7cbc2eb3f15c0e05a8032aa502cf3.tar.gz
postgresql-b3a5bf719cf7cbc2eb3f15c0e05a8032aa502cf3.zip
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
Diffstat (limited to 'src/interfaces/libpq/fe-secure-gssapi.c')
-rw-r--r--src/interfaces/libpq/fe-secure-gssapi.c9
1 files changed, 3 insertions, 6 deletions
diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
index e8d687c6889..7b5e38396c1 100644
--- a/src/interfaces/libpq/fe-secure-gssapi.c
+++ b/src/interfaces/libpq/fe-secure-gssapi.c
@@ -647,17 +647,14 @@ pqsecure_open_gss(PGconn *conn)
if (output.length == 0)
{
/*
- * We're done - hooray! Kind of gross, but we need to disable SSL
- * here so that we don't accidentally tunnel one over the other.
+ * We're done - hooray! Set flag to tell the low-level I/O routines
+ * to do GSS wrapping/unwrapping.
*/
-#ifdef USE_SSL
- conn->allow_ssl_try = false;
-#endif
+ conn->gssenc = true;
/* Clean up */
gss_release_cred(&minor, &conn->gcred);
conn->gcred = GSS_C_NO_CREDENTIAL;
- conn->gssenc = true;
gss_release_buffer(&minor, &output);
/*