Allow larger packets during GSSAPI authentication exchange.

tglsfdc · tglsfdc · commit 39b1d190714a · 2025-05-30T12:55:15.000-04:00
Our GSSAPI code only allows packet sizes up to 16kB. However it emerges that during authentication, larger packets might be needed; various authorities suggest 48kB or 64kB as the maximum packet size. This limitation caused login failure for AD users who belong to many AD groups. To add insult to injury, we gave an unintelligible error message, typically "GSSAPI context establishment error: The routine must be called again to complete its function: Unknown error". As noted in code comments, the 16kB packet limit is effectively a protocol constant once we are doing normal data transmission: the GSSAPI code splits the data stream at those points, and if we change the limit then we will have cross-version compatibility problems due to the receiver's buffer being too small in some combinations. However, during the authentication exchange the packet sizes are not determined by us, but by the underlying GSSAPI library. So we might as well just try to send what the library tells us to. An unpatched recipient will fail on a packet larger than 16kB, but that's not worse than the sender failing without even trying. So this doesn't introduce any meaningful compatibility problem. We still need a buffer size limit, but we can easily make it be 64kB rather than 16kB until transport negotiation is complete. (Larger values were discussed, but don't seem likely to add anything.) Reported-by: Chris Gooch <cgooch@bamfunds.com> Fix-suggested-by: Jacob Champion <jacob.champion@enterprisedb.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: Jacob Champion <jacob.champion@enterprisedb.com> Discussion: https://postgr.es/m/DS0PR22MB5971A9C8A3F44BCC6293C4DABE99A@DS0PR22MB5971.namprd22.prod.outlook.com Backpatch-through: 13
diff --git a/src/backend/libpq/be-secure-gssapi.c b/src/backend/libpq/be-secure-gssapi.c
@@ -45,11 +45,18 @@
  * don't want the other side to send arbitrarily huge packets as we
  * would have to allocate memory for them to then pass them to GSSAPI.
  *
- * Therefore, these two #define's are effectively part of the protocol
+ * Therefore, this #define is effectively part of the protocol
  * spec and can't ever be changed.
  */
-#define PQ_GSS_SEND_BUFFER_SIZE 16384
-#define PQ_GSS_RECV_BUFFER_SIZE 16384
+#define PQ_GSS_MAX_PACKET_SIZE 16384	/* includes uint32 header word */
+
+/*
+ * However, during the authentication exchange we must cope with whatever
+ * message size the GSSAPI library wants to send (because our protocol
+ * doesn't support splitting those messages).  Depending on configuration
+ * those messages might be as much as 64kB.
+ */
+#define PQ_GSS_AUTH_BUFFER_SIZE 65536	/* includes uint32 header word */
 
 /*
  * Since we manage at most one GSS-encrypted connection per backend,
@@ -209,12 +216,12 @@ be_gssapi_write(Port *port, void *ptr, size_t len)
 			errno = ECONNRESET;
 			return -1;
 		}
-		if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32))
+		if (output.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32))
 		{
 			ereport(COMMERROR,
 					(errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)",
 							(size_t) output.length,
-							PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32))));
+							PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32))));
 			errno = ECONNRESET;
 			return -1;
 		}
@@ -345,12 +352,12 @@ be_gssapi_read(Port *port, void *ptr, size_t len)
 		/* Decode the packet length and check for overlength packet */
 		input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer);
 
-		if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))
+		if (input.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32))
 		{
 			ereport(COMMERROR,
 					(errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)",
 							(size_t) input.length,
-							PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))));
+							PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32))));
 			errno = ECONNRESET;
 			return -1;
 		}
@@ -510,10 +517,13 @@ secure_open_gssapi(Port *port)
 	 * 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.
+	 *
+	 * We'll use PQ_GSS_AUTH_BUFFER_SIZE-sized buffers until transport
+	 * negotiation is complete, then switch to PQ_GSS_MAX_PACKET_SIZE.
 	 */
-	PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE);
-	PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE);
-	PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE);
+	PqGSSSendBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE);
+	PqGSSRecvBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE);
+	PqGSSResultBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE);
 	if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer)
 		ereport(FATAL,
 				(errcode(ERRCODE_OUT_OF_MEMORY),
@@ -560,16 +570,16 @@ secure_open_gssapi(Port *port)
 
 		/*
 		 * During initialization, packets are always fully consumed and
-		 * shouldn't ever be over PQ_GSS_RECV_BUFFER_SIZE in length.
+		 * shouldn't ever be over PQ_GSS_AUTH_BUFFER_SIZE in total length.
 		 *
 		 * Verify on our side that the client doesn't do something funny.
 		 */
-		if (input.length > PQ_GSS_RECV_BUFFER_SIZE)
+		if (input.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32))
 		{
 			ereport(COMMERROR,
-					(errmsg("oversize GSSAPI packet sent by the client (%zu > %d)",
+					(errmsg("oversize GSSAPI packet sent by the client (%zu > %zu)",
 							(size_t) input.length,
-							PQ_GSS_RECV_BUFFER_SIZE)));
+							PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32))));
 			return -1;
 		}
 
@@ -616,12 +626,12 @@ secure_open_gssapi(Port *port)
 		{
 			uint32		netlen = pg_hton32(output.length);
 
-			if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32))
+			if (output.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32))
 			{
 				ereport(COMMERROR,
 						(errmsg("server tried to send oversize GSSAPI packet (%zu > %zu)",
 								(size_t) output.length,
-								PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32))));
+								PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32))));
 				gss_release_buffer(&minor, &output);
 				return -1;
 			}
@@ -676,12 +686,29 @@ secure_open_gssapi(Port *port)
 			break;
 	}
 
+	/*
+	 * Release the large authentication buffers and allocate the ones we want
+	 * for normal operation.
+	 */
+	free(PqGSSSendBuffer);
+	free(PqGSSRecvBuffer);
+	free(PqGSSResultBuffer);
+	PqGSSSendBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE);
+	PqGSSRecvBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE);
+	PqGSSResultBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE);
+	if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer)
+		ereport(FATAL,
+				(errcode(ERRCODE_OUT_OF_MEMORY),
+				 errmsg("out of memory")));
+	PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0;
+	PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0;
+
 	/*
 	 * Determine the max packet size which will fit in our buffer, after
 	 * accounting for the length.  be_gssapi_write will need this.
 	 */
 	major = gss_wrap_size_limit(&minor, port->gss->ctx, 1, GSS_C_QOP_DEFAULT,
-								PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32),
+								PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32),
 								&PqGSSMaxPktSize);
 
 	if (GSS_ERROR(major))
diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c
@@ -47,11 +47,18 @@
  * don't want the other side to send arbitrarily huge packets as we
  * would have to allocate memory for them to then pass them to GSSAPI.
  *
- * Therefore, these two #define's are effectively part of the protocol
+ * Therefore, this #define is effectively part of the protocol
  * spec and can't ever be changed.
  */
-#define PQ_GSS_SEND_BUFFER_SIZE 16384
-#define PQ_GSS_RECV_BUFFER_SIZE 16384
+#define PQ_GSS_MAX_PACKET_SIZE 16384	/* includes uint32 header word */
+
+/*
+ * However, during the authentication exchange we must cope with whatever
+ * message size the GSSAPI library wants to send (because our protocol
+ * doesn't support splitting those messages).  Depending on configuration
+ * those messages might be as much as 64kB.
+ */
+#define PQ_GSS_AUTH_BUFFER_SIZE 65536	/* includes uint32 header word */
 
 /*
  * We need these state variables per-connection.  To allow the functions
@@ -204,12 +211,12 @@ pg_GSS_write(PGconn *conn, const void *ptr, size_t len)
 			goto cleanup;
 		}
 
-		if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32))
+		if (output.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32))
 		{
 			appendPQExpBuffer(&conn->errorMessage,
 							  libpq_gettext("client tried to send oversize GSSAPI packet (%zu > %zu)\n"),
 							  (size_t) output.length,
-							  PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32));
+							  PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32));
 			errno = EIO;		/* for lack of a better idea */
 			goto cleanup;
 		}
@@ -344,12 +351,12 @@ pg_GSS_read(PGconn *conn, void *ptr, size_t len)
 		/* Decode the packet length and check for overlength packet */
 		input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer);
 
-		if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))
+		if (input.length > PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32))
 		{
 			appendPQExpBuffer(&conn->errorMessage,
 							  libpq_gettext("oversize GSSAPI packet sent by the server (%zu > %zu)\n"),
 							  (size_t) input.length,
-							  PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
+							  PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32));
 			errno = EIO;		/* for lack of a better idea */
 			return -1;
 		}
@@ -488,12 +495,15 @@ pqsecure_open_gss(PGconn *conn)
 	 * 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.
+	 *
+	 * We'll use PQ_GSS_AUTH_BUFFER_SIZE-sized buffers until transport
+	 * negotiation is complete, then switch to PQ_GSS_MAX_PACKET_SIZE.
 	 */
 	if (PqGSSSendBuffer == NULL)
 	{
-		PqGSSSendBuffer = malloc(PQ_GSS_SEND_BUFFER_SIZE);
-		PqGSSRecvBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE);
-		PqGSSResultBuffer = malloc(PQ_GSS_RECV_BUFFER_SIZE);
+		PqGSSSendBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE);
+		PqGSSRecvBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE);
+		PqGSSResultBuffer = malloc(PQ_GSS_AUTH_BUFFER_SIZE);
 		if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer)
 		{
 			appendPQExpBufferStr(&conn->errorMessage,
@@ -568,13 +578,13 @@ pqsecure_open_gss(PGconn *conn)
 			 * so leave a spot at the end for a NULL byte too) and report that
 			 * back to the caller.
 			 */
-			result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, PQ_GSS_RECV_BUFFER_SIZE - PqGSSRecvLength - 1, &ret);
+			result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, PQ_GSS_AUTH_BUFFER_SIZE - PqGSSRecvLength - 1, &ret);
 			if (result != PGRES_POLLING_OK)
 				return result;
 
 			PqGSSRecvLength += ret;
 
-			Assert(PqGSSRecvLength < PQ_GSS_RECV_BUFFER_SIZE);
+			Assert(PqGSSRecvLength < PQ_GSS_AUTH_BUFFER_SIZE);
 			PqGSSRecvBuffer[PqGSSRecvLength] = '\0';
 			appendPQExpBuffer(&conn->errorMessage, "%s\n", PqGSSRecvBuffer + 1);
 
@@ -588,12 +598,12 @@ pqsecure_open_gss(PGconn *conn)
 
 		/* Get the length and check for over-length packet */
 		input.length = pg_ntoh32(*(uint32 *) PqGSSRecvBuffer);
-		if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32))
+		if (input.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32))
 		{
 			appendPQExpBuffer(&conn->errorMessage,
 							  libpq_gettext("oversize GSSAPI packet sent by the server (%zu > %zu)\n"),
 							  (size_t) input.length,
-							  PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32));
+							  PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32));
 			return PGRES_POLLING_FAILED;
 		}
 
@@ -655,12 +665,34 @@ pqsecure_open_gss(PGconn *conn)
 		conn->gcred = GSS_C_NO_CREDENTIAL;
 		gss_release_buffer(&minor, &output);
 
+		/*
+		 * Release the large authentication buffers and allocate the ones we
+		 * want for normal operation.  (This maneuver is safe only because
+		 * pqDropConnection will drop the buffers; otherwise, during a
+		 * reconnection we'd be at risk of using undersized buffers during
+		 * negotiation.)
+		 */
+		free(PqGSSSendBuffer);
+		free(PqGSSRecvBuffer);
+		free(PqGSSResultBuffer);
+		PqGSSSendBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE);
+		PqGSSRecvBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE);
+		PqGSSResultBuffer = malloc(PQ_GSS_MAX_PACKET_SIZE);
+		if (!PqGSSSendBuffer || !PqGSSRecvBuffer || !PqGSSResultBuffer)
+		{
+			appendPQExpBufferStr(&conn->errorMessage,
+								 libpq_gettext("out of memory\n"));
+			return PGRES_POLLING_FAILED;
+		}
+		PqGSSSendLength = PqGSSSendNext = PqGSSSendConsumed = 0;
+		PqGSSRecvLength = PqGSSResultLength = PqGSSResultNext = 0;
+
 		/*
 		 * Determine the max packet size which will fit in our buffer, after
 		 * accounting for the length.  pg_GSS_write will need this.
 		 */
 		major = gss_wrap_size_limit(&minor, conn->gctx, 1, GSS_C_QOP_DEFAULT,
-									PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32),
+									PQ_GSS_MAX_PACKET_SIZE - sizeof(uint32),
 									&PqGSSMaxPktSize);
 
 		if (GSS_ERROR(major))
@@ -674,10 +706,12 @@ pqsecure_open_gss(PGconn *conn)
 	}
 
 	/* Must have output.length > 0 */
-	if (output.length > PQ_GSS_SEND_BUFFER_SIZE - sizeof(uint32))
+	if (output.length > PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32))
 	{
-		pg_GSS_error(libpq_gettext("GSSAPI context establishment error"),
-					 conn, major, minor);
+		appendPQExpBuffer(&conn->errorMessage,
+						  libpq_gettext("client tried to send oversize GSSAPI packet (%zu > %zu)\n"),
+						  (size_t) output.length,
+						  PQ_GSS_AUTH_BUFFER_SIZE - sizeof(uint32));
 		gss_release_buffer(&minor, &output);
 		return PGRES_POLLING_FAILED;
 	}
