Clear auth context correctly when re-connecting after failed auth attempt.

If authentication over an SSL connection fails, with sslmode=prefer,
libpq will reconnect without SSL and retry. However, we did not clear
the variables related to GSS, SSPI, and SASL authentication state, when
reconnecting. Because of that, the second authentication attempt would
always fail with a "duplicate GSS/SASL authentication request" error.
pg_SSPI_startup did not check for duplicate authentication requests like
the corresponding GSS and SASL functions, so with SSPI, you would leak
some memory instead.

Another way this could manifest itself, on version 10, is if you list
multiple hostnames in the "host" parameter. If the first server requests
Kerberos or SCRAM authentication, but it fails, the attempts to connect to
the other servers will also fail with "duplicate authentication request"
errors.

To fix, move the clearing of authentication state from closePGconn to
pgDropConnection, so that it is cleared also when re-connecting.

Patch by Michael Paquier, with some kibitzing by me.

Backpatch down to 9.3. 9.2 has the same bug, but the code around closing
the connection is somewhat different, so that this patch doesn't apply.
To fix this in 9.2, I think we would need to back-port commit 210eb9b743
first, and then apply this patch. However, given that we only bumped into
this in our own testing, we haven't heard any reports from users about
this, and that 9.2 will be end-of-lifed in a couple of months anyway, it
doesn't seem worth the risk and trouble.

Discussion: https://www.postgresql.org/message-id/CAB7nPqRuOUm0MyJaUy9L3eXYJU3AKCZ-0-03=-aDTZJGV4GyWw@mail.gmail.com

diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index f577bc1c432dc0a8290f86c55c0073b0664c688e..a7a4a6245e43bb4d0bd6d36ac87833b9f6156a7e 100644 (file)
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -372,7 +372,12 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate)
    SECURITY_STATUS r;
    TimeStamp   expire;
 
-   conn->sspictx = NULL;
+   if (conn->sspictx)
+   {
+       printfPQExpBuffer(&conn->errorMessage,
+                   libpq_gettext("duplicate SSPI authentication request\n"));
+       return STATUS_ERROR;
+   }
 
    /*
     * Retreive credentials handle

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 0f700909f45950db38bd4860ac50808c40519872..ab55439db2c80e391111f0d463715d44361b5e28 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -401,15 +401,56 @@ pqDropConnection(PGconn *conn, bool flushInput)
 {
    /* Drop any SSL state */
    pqsecure_close(conn);
+
    /* Close the socket itself */
    if (conn->sock != PGINVALID_SOCKET)
        closesocket(conn->sock);
    conn->sock = PGINVALID_SOCKET;
+
    /* Optionally discard any unread data */
    if (flushInput)
        conn->inStart = conn->inCursor = conn->inEnd = 0;
+
    /* Always discard any unsent data */
    conn->outCount = 0;
+
+   /* Free authentication state */
+#ifdef ENABLE_GSS
+   {
+       OM_uint32   min_s;
+
+       if (conn->gctx)
+           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->ginbuf.length)
+           gss_release_buffer(&min_s, &conn->ginbuf);
+       if (conn->goutbuf.length)
+           gss_release_buffer(&min_s, &conn->goutbuf);
+   }
+#endif
+#ifdef ENABLE_SSPI
+   if (conn->ginbuf.length)
+       free(conn->ginbuf.value);
+   conn->ginbuf.length = 0;
+   conn->ginbuf.value = NULL;
+   if (conn->sspitarget)
+       free(conn->sspitarget);
+   conn->sspitarget = NULL;
+   if (conn->sspicred)
+   {
+       FreeCredentialsHandle(conn->sspicred);
+       free(conn->sspicred);
+       conn->sspicred = NULL;
+   }
+   if (conn->sspictx)
+   {
+       DeleteSecurityContext(conn->sspictx);
+       free(conn->sspictx);
+       conn->sspictx = NULL;
+   }
+   conn->usesspi = 0;
+#endif
 }
 
 
@@ -3005,41 +3046,6 @@ closePGconn(PGconn *conn)
    if (conn->lobjfuncs)
        free(conn->lobjfuncs);
    conn->lobjfuncs = NULL;
-#ifdef ENABLE_GSS
-   {
-       OM_uint32   min_s;
-
-       if (conn->gctx)
-           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->ginbuf.length)
-           gss_release_buffer(&min_s, &conn->ginbuf);
-       if (conn->goutbuf.length)
-           gss_release_buffer(&min_s, &conn->goutbuf);
-   }
-#endif
-#ifdef ENABLE_SSPI
-   if (conn->ginbuf.length)
-       free(conn->ginbuf.value);
-   conn->ginbuf.length = 0;
-   conn->ginbuf.value = NULL;
-   if (conn->sspitarget)
-       free(conn->sspitarget);
-   conn->sspitarget = NULL;
-   if (conn->sspicred)
-   {
-       FreeCredentialsHandle(conn->sspicred);
-       free(conn->sspicred);
-       conn->sspicred = NULL;
-   }
-   if (conn->sspictx)
-   {
-       DeleteSecurityContext(conn->sspictx);
-       free(conn->sspictx);
-       conn->sspictx = NULL;
-   }
-#endif
 }
 
 /*