diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2020-12-28 17:44:17 -0500 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2020-12-28 17:44:17 -0500 |
commit | 622ae4621ece72a9f64b5602c74d7aaf373c1631 (patch) | |
tree | 467f643f181e5784f01ef02694a4452e69a611a0 /src/backend/libpq/be-secure-gssapi.c | |
parent | ff6ce9a3a691a96e8e47ed449bc51c5a178e6931 (diff) | |
download | postgresql-622ae4621ece72a9f64b5602c74d7aaf373c1631.tar.gz postgresql-622ae4621ece72a9f64b5602c74d7aaf373c1631.zip |
Fix assorted issues in backend's GSSAPI encryption support.
Unrecoverable errors detected by GSSAPI encryption can't just be
reported with elog(ERROR) or elog(FATAL), because attempting to
send the error report to the client is likely to lead to infinite
recursion or loss of protocol sync. Instead make this code do what
the SSL encryption code has long done, which is to just report any
such failure to the server log (with elevel COMMERROR), then pretend
we've lost the connection by returning errno = ECONNRESET.
Along the way, fix confusion about whether message translation is done
by pg_GSS_error() or its callers (the latter should do it), and make
the backend version of that function work more like the frontend
version.
Avoid allocating the port->gss struct until it's needed; we surely
don't need to allocate it in the postmaster.
Improve logging of "connection authorized" messages with GSS enabled.
(As part of this, I back-patched the code changes from dc11f31a1.)
Make BackendStatusShmemSize() account for the GSS-related space that
will be allocated by CreateSharedBackendStatus(). This omission
could possibly cause out-of-shared-memory problems with very high
max_connections settings.
Remove arbitrary, pointless restriction that only GSS authentication
can be used on a GSS-encrypted connection.
Improve documentation; notably, document the fact that libpq now
prefers GSS encryption over SSL encryption if both are possible.
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/backend/libpq/be-secure-gssapi.c')
-rw-r--r-- | src/backend/libpq/be-secure-gssapi.c | 98 |
1 files changed, 72 insertions, 26 deletions
diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c index 5a73302b7b9..1747fccb143 100644 --- a/src/backend/libpq/be-secure-gssapi.c +++ b/src/backend/libpq/be-secure-gssapi.c @@ -21,6 +21,7 @@ #include "libpq/pqformat.h" #include "miscadmin.h" #include "pgstat.h" +#include "utils/memutils.h" /* @@ -81,10 +82,14 @@ static uint32 PqGSSMaxPktSize; /* Maximum size we can encrypt and fit the * transport negotiation is complete). * * On success, returns the number of data bytes consumed (possibly less than - * len). On failure, returns -1 with errno set appropriately. (For fatal - * errors, we may just elog and exit, if errno wouldn't be sufficient to - * describe the error.) For retryable errors, caller should call again - * (passing the same data) once the socket is ready. + * len). On failure, returns -1 with errno set appropriately. For retryable + * errors, caller should call again (passing the same data) once the socket + * is ready. + * + * Dealing with fatal errors here is a bit tricky: we can't invoke elog(FATAL) + * since it would try to write to the client, probably resulting in infinite + * recursion. Instead, use elog(COMMERROR) to log extra info about the + * failure if necessary, and then return an errno indicating connection loss. */ ssize_t be_gssapi_write(Port *port, void *ptr, size_t len) @@ -108,8 +113,11 @@ be_gssapi_write(Port *port, void *ptr, size_t len) * again, so if it offers a len less than that, something is wrong. */ if (len < PqGSSSendConsumed) - elog(FATAL, "GSSAPI caller failed to retransmit all data needing to be retried"); - + { + elog(COMMERROR, "GSSAPI caller failed to retransmit all data needing to be retried"); + errno = ECONNRESET; + return -1; + } /* Discount whatever source data we already encrypted. */ bytes_to_encrypt = len - PqGSSSendConsumed; bytes_encrypted = PqGSSSendConsumed; @@ -192,17 +200,27 @@ be_gssapi_write(Port *port, void *ptr, size_t len) major = gss_wrap(&minor, gctx, 1, GSS_C_QOP_DEFAULT, &input, &conf_state, &output); if (major != GSS_S_COMPLETE) - pg_GSS_error(FATAL, gettext_noop("GSSAPI wrap error"), major, minor); - + { + pg_GSS_error(_("GSSAPI wrap error"), major, minor); + errno = ECONNRESET; + return -1; + } if (conf_state == 0) - ereport(FATAL, + { + ereport(COMMERROR, (errmsg("outgoing GSSAPI message would not use confidentiality"))); - + errno = ECONNRESET; + return -1; + } if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) - ereport(FATAL, + { + ereport(COMMERROR, (errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)", (size_t) output.length, PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)))); + errno = ECONNRESET; + return -1; + } bytes_encrypted += input.length; bytes_to_encrypt -= input.length; @@ -234,9 +252,11 @@ be_gssapi_write(Port *port, void *ptr, size_t len) * transport negotiation is complete). * * Returns the number of data bytes read, or on failure, returns -1 - * with errno set appropriately. (For fatal errors, we may just elog and - * exit, if errno wouldn't be sufficient to describe the error.) For - * retryable errors, caller should call again once the socket is ready. + * with errno set appropriately. For retryable errors, caller should call + * again once the socket is ready. + * + * We treat fatal errors the same as in be_gssapi_write(), even though the + * argument about infinite recursion doesn't apply here. */ ssize_t be_gssapi_read(Port *port, void *ptr, size_t len) @@ -326,10 +346,14 @@ be_gssapi_read(Port *port, void *ptr, size_t len) input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer); if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)) - ereport(FATAL, + { + ereport(COMMERROR, (errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)", (size_t) input.length, PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32)))); + errno = ECONNRESET; + return -1; + } /* * Read as much of the packet as we are able to on this call into @@ -361,12 +385,18 @@ be_gssapi_read(Port *port, void *ptr, size_t len) major = gss_unwrap(&minor, gctx, &input, &output, &conf_state, NULL); if (major != GSS_S_COMPLETE) - pg_GSS_error(FATAL, gettext_noop("GSSAPI unwrap error"), - major, minor); - + { + pg_GSS_error(_("GSSAPI unwrap error"), major, minor); + errno = ECONNRESET; + return -1; + } if (conf_state == 0) - ereport(FATAL, + { + ereport(COMMERROR, (errmsg("incoming GSSAPI message did not use confidentiality"))); + errno = ECONNRESET; + return -1; + } memcpy(PqGSSResultBuffer, output.value, output.length); PqGSSResultLength = output.length; @@ -469,6 +499,12 @@ secure_open_gssapi(Port *port) minor; /* + * Allocate subsidiary Port data for GSSAPI operations. + */ + port->gss = (pg_gssinfo *) + MemoryContextAllocZero(TopMemoryContext, sizeof(pg_gssinfo)); + + /* * Allocate buffers and initialize state variables. By malloc'ing the * buffers at this point, we avoid wasting static data space in processes * that will never use them, and we ensure that the buffers are @@ -521,10 +557,13 @@ secure_open_gssapi(Port *port) * Verify on our side that the client doesn't do something funny. */ if (input.length > PQ_GSS_RECV_BUFFER_SIZE) - ereport(FATAL, + { + ereport(COMMERROR, (errmsg("oversize GSSAPI packet sent by the client (%zu > %d)", (size_t) input.length, PQ_GSS_RECV_BUFFER_SIZE))); + return -1; + } /* * Get the rest of the packet so we can pass it to GSSAPI to accept @@ -544,7 +583,7 @@ secure_open_gssapi(Port *port) NULL, NULL); if (GSS_ERROR(major)) { - pg_GSS_error(ERROR, gettext_noop("could not accept GSSAPI security context"), + pg_GSS_error(_("could not accept GSSAPI security context"), major, minor); gss_release_buffer(&minor, &output); return -1; @@ -570,10 +609,14 @@ secure_open_gssapi(Port *port) uint32 netlen = pg_hton32(output.length); if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)) - ereport(FATAL, + { + ereport(COMMERROR, (errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)", (size_t) output.length, PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32)))); + gss_release_buffer(&minor, &output); + return -1; + } memcpy(PqGSSSendBuffer, (char *) &netlen, sizeof(uint32)); PqGSSSendLength += sizeof(uint32); @@ -634,8 +677,10 @@ secure_open_gssapi(Port *port) &PqGSSMaxPktSize); if (GSS_ERROR(major)) - pg_GSS_error(FATAL, gettext_noop("GSSAPI size check error"), - major, minor); + { + pg_GSS_error(_("GSSAPI size check error"), major, minor); + return -1; + } port->gss->enc = true; @@ -667,12 +712,13 @@ be_gssapi_get_enc(Port *port) } /* - * Return the GSSAPI principal used for authentication on this connection. + * Return the GSSAPI principal used for authentication on this connection + * (NULL if we did not perform GSSAPI authentication). */ const char * be_gssapi_get_princ(Port *port) { - if (!port || !port->gss->auth) + if (!port || !port->gss) return NULL; return port->gss->princ; |