Re: Protocol problem with GSSAPI encryption? - Mailing list pgsql-hackers

>>>>> "Peter" == Peter Eisentraut <peter.eisentraut@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