>>>>> "Peter" == Peter Eisentraut <peter.eisentr...@2ndquadrant.com> writes:
>> It seems to me that this is a bug in ProcessStartupPacket, which
>> should accept both GSS or SSL negotiation requests on a connection
>> (in either order). Maybe secure_done should be two flags rather than
>> one?
Peter> I have also seen reports of that. I think your analysis is
Peter> correct.
I figure something along these lines for the fix. Anyone in a position
to test this?
--
Andrew (irc:RhodiumToad)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 9ff2832c00..1b51d4916d 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -404,7 +404,7 @@ static void BackendRun(Port *port) pg_attribute_noreturn();
static void ExitPostmaster(int status) pg_attribute_noreturn();
static int ServerLoop(void);
static int BackendStartup(Port *port);
-static int ProcessStartupPacket(Port *port, bool secure_done);
+static int ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done);
static void SendNegotiateProtocolVersion(List *unrecognized_protocol_options);
static void processCancelRequest(Port *port, void *pkt);
static int initMasks(fd_set *rmask);
@@ -1914,11 +1914,15 @@ initMasks(fd_set *rmask)
* send anything to the client, which would typically be appropriate
* if we detect a communications failure.)
*
- * Set secure_done when negotiation of an encrypted layer (currently, TLS or
- * GSSAPI) is already completed.
+ * Set ssl_done and/or gss_done when negotiation of an encrypted layer
+ * (currently, TLS or GSSAPI) is completed. A successful negotiation of either
+ * encryption layer sets both flags, but a rejected negotiation sets only the
+ * flag for that layer, since the client may wish to try the other one. We
+ * should make no assumption here about the order in which the client may make
+ * requests.
*/
static int
-ProcessStartupPacket(Port *port, bool secure_done)
+ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
{
int32 len;
void *buf;
@@ -1951,7 +1955,7 @@ ProcessStartupPacket(Port *port, bool secure_done)
if (pq_getbytes(((char *) &len) + 1, 3) == EOF)
{
/* Got a partial length word, so bleat about that */
- if (!secure_done)
+ if (!ssl_done && !gss_done)
ereport(COMMERROR,
(errcode(ERRCODE_PROTOCOL_VIOLATION),
errmsg("incomplete startup packet")));
@@ -2003,7 +2007,7 @@ ProcessStartupPacket(Port *port, bool secure_done)
return STATUS_ERROR;
}
- if (proto == NEGOTIATE_SSL_CODE && !secure_done)
+ if (proto == NEGOTIATE_SSL_CODE && !ssl_done)
{
char SSLok;
@@ -2032,11 +2036,14 @@ retry1:
if (SSLok == 'S' && secure_open_server(port) == -1)
return STATUS_ERROR;
#endif
- /* regular startup packet, cancel, etc packet should follow... */
- /* but not another SSL negotiation request */
- return ProcessStartupPacket(port, true);
+ /*
+ * regular startup packet, cancel, etc packet should follow, but not
+ * another SSL negotiation request, and a GSS request should only
+ * follow if SSL was rejected (client may negotiate in either order)
+ */
+ return ProcessStartupPacket(port, true, SSLok == 'S');
}
- else if (proto == NEGOTIATE_GSS_CODE && !secure_done)
+ else if (proto == NEGOTIATE_GSS_CODE && !gss_done)
{
char GSSok = 'N';
#ifdef ENABLE_GSS
@@ -2059,8 +2066,12 @@ retry1:
if (GSSok == 'G' && secure_open_gssapi(port) == -1)
return STATUS_ERROR;
#endif
- /* Won't ever see more than one negotiation request */
- return ProcessStartupPacket(port, true);
+ /*
+ * regular startup packet, cancel, etc packet should follow, but not
+ * another GSS negotiation request, and an SSL request should only
+ * follow if GSS was rejected (client may negotiate in either order)
+ */
+ return ProcessStartupPacket(port, GSSok == 'G', true);
}
/* Could add additional special packet types here */
@@ -4400,7 +4411,7 @@ BackendInitialize(Port *port)
* Receive the startup packet (which might turn out to be a cancel request
* packet).
*/
- status = ProcessStartupPacket(port, false);
+ status = ProcessStartupPacket(port, false, false);
/*
* Stop here if it was bad or a cancel packet. ProcessStartupPacket
