I wrote: > (Still doesn't touch the static-buffer issue, though.) And here's a delta patch for that. This fixes an additional portability hazard, which is that there were various places doing stuff like
input.length = ntohl(*(uint32 *) PqGSSRecvBuffer); That's a SIGBUS hazard on alignment-picky hardware, because there is no guarantee that a variable that's just declared "char ...[...]" will have any particular alignment. But malloc'ing the space will provide maxaligned storage. My FreeBSD testing has now given me enough confidence in these patches that I'm just going to go ahead and push them. But, if you'd like to do some more testing in advance of 12.2 release, please do. regards, tom lane
diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c index 87cb6ea..c25cfda 100644 --- a/src/backend/libpq/be-secure-gssapi.c +++ b/src/backend/libpq/be-secure-gssapi.c @@ -50,27 +50,30 @@ #define PQ_GSS_SEND_BUFFER_SIZE 16384 #define PQ_GSS_RECV_BUFFER_SIZE 16384 -/* PqGSSSendBuffer is for *encrypted* data */ -static char PqGSSSendBuffer[PQ_GSS_SEND_BUFFER_SIZE]; +/* + * Since we manage at most one GSS-encrypted connection per backend, + * we can just keep all this state in static variables. The char * + * variables point to buffers that are allocated once and re-used. + */ +static char *PqGSSSendBuffer; /* Encrypted data waiting to be sent */ static int PqGSSSendLength; /* End of data available in PqGSSSendBuffer */ static int PqGSSSendNext; /* Next index to send a byte from * PqGSSSendBuffer */ static int PqGSSSendConsumed; /* Number of *unencrypted* bytes consumed for * current contents of PqGSSSendBuffer */ -/* PqGSSRecvBuffer is for *encrypted* data */ -static char PqGSSRecvBuffer[PQ_GSS_RECV_BUFFER_SIZE]; +static char *PqGSSRecvBuffer; /* Received, encrypted data */ static int PqGSSRecvLength; /* End of data available in PqGSSRecvBuffer */ -/* PqGSSResultBuffer is for *unencrypted* data */ -static char PqGSSResultBuffer[PQ_GSS_RECV_BUFFER_SIZE]; +static char *PqGSSResultBuffer; /* Decryption of data in gss_RecvBuffer */ static int PqGSSResultLength; /* End of data available in PqGSSResultBuffer */ static int PqGSSResultNext; /* Next index to read a byte from * PqGSSResultBuffer */ -uint32 max_packet_size; /* Maximum size we can encrypt and fit the +static uint32 PqGSSMaxPktSize; /* Maximum size we can encrypt and fit the * results into our output buffer */ + /* * Attempt to write len bytes of data from ptr to a GSSAPI-encrypted connection. * @@ -175,8 +178,8 @@ be_gssapi_write(Port *port, void *ptr, size_t len) * we will have to loop and possibly be called multiple times to get * through all the data. */ - if (bytes_to_encrypt > max_packet_size) - input.length = max_packet_size; + if (bytes_to_encrypt > PqGSSMaxPktSize) + input.length = PqGSSMaxPktSize; else input.length = bytes_to_encrypt; @@ -461,7 +464,20 @@ secure_open_gssapi(Port *port) OM_uint32 major, minor; - /* initialize state variables */ + /* + * 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 + * sufficiently aligned for the length-word accesses that we do in some + * places in this file. + */ + PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) + ereport(FATAL, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"))); PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0; PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0; @@ -608,7 +624,7 @@ secure_open_gssapi(Port *port) */ major = gss_wrap_size_limit(&minor, port->gss->ctx, 1, GSS_C_QOP_DEFAULT, PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32), - &max_packet_size); + &PqGSSMaxPktSize); if (GSS_ERROR(major)) pg_GSS_error(FATAL, gettext_noop("GSSAPI size check error"), diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 89b1346..80b54bc 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -462,7 +462,7 @@ pqDropConnection(PGconn *conn, bool flushInput) /* Always discard any unsent data */ conn->outCount = 0; - /* Free authentication state */ + /* Free authentication/encryption state */ #ifdef ENABLE_GSS { OM_uint32 min_s; @@ -471,6 +471,21 @@ pqDropConnection(PGconn *conn, bool flushInput) gss_delete_sec_context(&min_s, &conn->gctx, GSS_C_NO_BUFFER); if (conn->gtarg_nam) gss_release_name(&min_s, &conn->gtarg_nam); + if (conn->gss_SendBuffer) + { + free(conn->gss_SendBuffer); + conn->gss_SendBuffer = NULL; + } + if (conn->gss_RecvBuffer) + { + free(conn->gss_RecvBuffer); + conn->gss_RecvBuffer = NULL; + } + if (conn->gss_ResultBuffer) + { + free(conn->gss_ResultBuffer); + conn->gss_ResultBuffer = NULL; + } } #endif #ifdef ENABLE_SSPI diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c index 054cf92..9fd06ea 100644 --- a/src/interfaces/libpq/fe-secure-gssapi.c +++ b/src/interfaces/libpq/fe-secure-gssapi.c @@ -53,26 +53,22 @@ #define PQ_GSS_SEND_BUFFER_SIZE 16384 #define PQ_GSS_RECV_BUFFER_SIZE 16384 -/* PqGSSSendBuffer is for *encrypted* data */ -static char PqGSSSendBuffer[PQ_GSS_SEND_BUFFER_SIZE]; -static int PqGSSSendLength; /* End of data available in PqGSSSendBuffer */ -static int PqGSSSendNext; /* Next index to send a byte from - * PqGSSSendBuffer */ -static int PqGSSSendConsumed; /* Number of *unencrypted* bytes consumed for - * current contents of PqGSSSendBuffer */ - -/* PqGSSRecvBuffer is for *encrypted* data */ -static char PqGSSRecvBuffer[PQ_GSS_RECV_BUFFER_SIZE]; -static int PqGSSRecvLength; /* End of data available in PqGSSRecvBuffer */ - -/* PqGSSResultBuffer is for *unencrypted* data */ -static char PqGSSResultBuffer[PQ_GSS_RECV_BUFFER_SIZE]; -static int PqGSSResultLength; /* End of data available in PqGSSResultBuffer */ -static int PqGSSResultNext; /* Next index to read a byte from - * PqGSSResultBuffer */ - -uint32 max_packet_size; /* Maximum size we can encrypt and fit the - * results into our output buffer */ +/* + * We need these state variables per-connection. To allow the functions + * in this file to look mostly like those in be-secure-gssapi.c, set up + * these macros. + */ +#define PqGSSSendBuffer (conn->gss_SendBuffer) +#define PqGSSSendLength (conn->gss_SendLength) +#define PqGSSSendNext (conn->gss_SendNext) +#define PqGSSSendConsumed (conn->gss_SendConsumed) +#define PqGSSRecvBuffer (conn->gss_RecvBuffer) +#define PqGSSRecvLength (conn->gss_RecvLength) +#define PqGSSResultBuffer (conn->gss_ResultBuffer) +#define PqGSSResultLength (conn->gss_ResultLength) +#define PqGSSResultNext (conn->gss_ResultNext) +#define PqGSSMaxPktSize (conn->gss_MaxPktSize) + /* * Attempt to write len bytes of data from ptr to a GSSAPI-encrypted connection. @@ -184,8 +180,8 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len) * we will have to loop and possibly be called multiple times to get * through all the data. */ - if (bytes_to_encrypt > max_packet_size) - input.length = max_packet_size; + if (bytes_to_encrypt > PqGSSMaxPktSize) + input.length = PqGSSMaxPktSize; else input.length = bytes_to_encrypt; @@ -477,7 +473,6 @@ gss_read(PGconn *conn, void *recv_buffer, size_t length, ssize_t *ret) PostgresPollingStatusType pqsecure_open_gss(PGconn *conn) { - static int first = 1; ssize_t ret; OM_uint32 major, minor; @@ -486,12 +481,25 @@ pqsecure_open_gss(PGconn *conn) gss_buffer_desc input = GSS_C_EMPTY_BUFFER, output = GSS_C_EMPTY_BUFFER; - /* Initialize, if first time through */ - if (first) + /* + * If first time through for this connection, allocate buffers and + * initialize state variables. By malloc'ing the buffers separately, we + * ensure that they are sufficiently aligned for the length-word accesses + * that we do in some places in this file. + */ + if (PqGSSSendBuffer == NULL) { + PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE); + PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE); + if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer) + { + printfPQExpBuffer(&conn->errorMessage, + libpq_gettext("out of memory\n")); + return PGRES_POLLING_FAILED; + } PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0; PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0; - first = 0; } /* @@ -651,7 +659,7 @@ pqsecure_open_gss(PGconn *conn) */ major = gss_wrap_size_limit(&minor, conn->gctx, 1, GSS_C_QOP_DEFAULT, PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32), - &max_packet_size); + &PqGSSMaxPktSize); if (GSS_ERROR(major)) { diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 1e36be2..79bc378 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -493,6 +493,23 @@ struct pg_conn bool try_gss; /* GSS attempting permitted */ bool gssenc; /* GSS encryption is usable */ gss_cred_id_t gcred; /* GSS credential temp storage. */ + + /* GSS encryption I/O state --- see fe-secure-gssapi.c */ + char *gss_SendBuffer; /* Encrypted data waiting to be sent */ + int gss_SendLength; /* End of data available in gss_SendBuffer */ + int gss_SendNext; /* Next index to send a byte from + * gss_SendBuffer */ + int gss_SendConsumed; /* Number of *unencrypted* bytes consumed + * for current contents of gss_SendBuffer */ + char *gss_RecvBuffer; /* Received, encrypted data */ + int gss_RecvLength; /* End of data available in gss_RecvBuffer */ + char *gss_ResultBuffer; /* Decryption of data in gss_RecvBuffer */ + int gss_ResultLength; /* End of data available in + * gss_ResultBuffer */ + int gss_ResultNext; /* Next index to read a byte from + * gss_ResultBuffer */ + uint32 gss_MaxPktSize; /* Maximum size we can encrypt and fit the + * results into our output buffer */ #endif #ifdef ENABLE_SSPI