Fix GSS client to non-GSS server connection
authorStephen Frost
Sat, 2 May 2020 15:39:26 +0000 (11:39 -0400)
committerStephen Frost
Sat, 2 May 2020 15:39:26 +0000 (11:39 -0400)
If the client is compiled with GSSAPI support and tries to start up GSS
with the server, but the server is not compiled with GSSAPI support, we
would mistakenly end up falling through to call ProcessStartupPacket
with secure_done = true, but the client might then try to perform SSL,
which the backend wouldn't understand and we'd end up failing the
connection with:

FATAL:  unsupported frontend protocol 1234.5679: server supports 2.0 to 3.0

Fix by arranging to track ssl_done independently from gss_done, instead
of trying to use the same boolean for both.

Author: Andrew Gierth
Discussion: https://postgr.es/m/[email protected]
Backpatch: 12-, where GSSAPI encryption was added.

src/backend/postmaster/postmaster.c

index 0578e92ba9c416d74693753e6083886db3227566..e19d5dc1a64a3f3ad05243a9fe46322a01212508 100644 (file)
@@ -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 */
@@ -4412,7 +4423,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