diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2020-12-28 15:43:44 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2020-12-28 15:43:44 -0500 |
commit | ff6ce9a3a691a96e8e47ed449bc51c5a178e6931 (patch) | |
tree | 5e6d7459019adc3d8f81128b0eb1289fec178676 /src/interfaces/libpq/fe-secure-gssapi.c | |
parent | cf61b0734c61d93c62827fe4e44fa2162a533b8e (diff) | |
download | postgresql-ff6ce9a3a691a96e8e47ed449bc51c5a178e6931.tar.gz postgresql-ff6ce9a3a691a96e8e47ed449bc51c5a178e6931.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.c | 9 |
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 bfc0f552146..9416306eea0 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); /* |