Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit f44c609

Browse files
committed
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 210eb9b 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
1 parent b8bd32a commit f44c609

File tree

2 files changed

+47
-36
lines changed

2 files changed

+47
-36
lines changed

src/interfaces/libpq/fe-auth.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,12 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate)
372372
SECURITY_STATUS r;
373373
TimeStamp expire;
374374

375-
conn->sspictx = NULL;
375+
if (conn->sspictx)
376+
{
377+
printfPQExpBuffer(&conn->errorMessage,
378+
libpq_gettext("duplicate SSPI authentication request\n"));
379+
return STATUS_ERROR;
380+
}
376381

377382
/*
378383
* Retrieve credentials handle

src/interfaces/libpq/fe-connect.c

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -401,15 +401,56 @@ pqDropConnection(PGconn *conn, bool flushInput)
401401
{
402402
/* Drop any SSL state */
403403
pqsecure_close(conn);
404+
404405
/* Close the socket itself */
405406
if (conn->sock != PGINVALID_SOCKET)
406407
closesocket(conn->sock);
407408
conn->sock = PGINVALID_SOCKET;
409+
408410
/* Optionally discard any unread data */
409411
if (flushInput)
410412
conn->inStart = conn->inCursor = conn->inEnd = 0;
413+
411414
/* Always discard any unsent data */
412415
conn->outCount = 0;
416+
417+
/* Free authentication state */
418+
#ifdef ENABLE_GSS
419+
{
420+
OM_uint32 min_s;
421+
422+
if (conn->gctx)
423+
gss_delete_sec_context(&min_s, &conn->gctx, GSS_C_NO_BUFFER);
424+
if (conn->gtarg_nam)
425+
gss_release_name(&min_s, &conn->gtarg_nam);
426+
if (conn->ginbuf.length)
427+
gss_release_buffer(&min_s, &conn->ginbuf);
428+
if (conn->goutbuf.length)
429+
gss_release_buffer(&min_s, &conn->goutbuf);
430+
}
431+
#endif
432+
#ifdef ENABLE_SSPI
433+
if (conn->ginbuf.length)
434+
free(conn->ginbuf.value);
435+
conn->ginbuf.length = 0;
436+
conn->ginbuf.value = NULL;
437+
if (conn->sspitarget)
438+
free(conn->sspitarget);
439+
conn->sspitarget = NULL;
440+
if (conn->sspicred)
441+
{
442+
FreeCredentialsHandle(conn->sspicred);
443+
free(conn->sspicred);
444+
conn->sspicred = NULL;
445+
}
446+
if (conn->sspictx)
447+
{
448+
DeleteSecurityContext(conn->sspictx);
449+
free(conn->sspictx);
450+
conn->sspictx = NULL;
451+
}
452+
conn->usesspi = 0;
453+
#endif
413454
}
414455

415456

@@ -3007,41 +3048,6 @@ closePGconn(PGconn *conn)
30073048
if (conn->lobjfuncs)
30083049
free(conn->lobjfuncs);
30093050
conn->lobjfuncs = NULL;
3010-
#ifdef ENABLE_GSS
3011-
{
3012-
OM_uint32 min_s;
3013-
3014-
if (conn->gctx)
3015-
gss_delete_sec_context(&min_s, &conn->gctx, GSS_C_NO_BUFFER);
3016-
if (conn->gtarg_nam)
3017-
gss_release_name(&min_s, &conn->gtarg_nam);
3018-
if (conn->ginbuf.length)
3019-
gss_release_buffer(&min_s, &conn->ginbuf);
3020-
if (conn->goutbuf.length)
3021-
gss_release_buffer(&min_s, &conn->goutbuf);
3022-
}
3023-
#endif
3024-
#ifdef ENABLE_SSPI
3025-
if (conn->ginbuf.length)
3026-
free(conn->ginbuf.value);
3027-
conn->ginbuf.length = 0;
3028-
conn->ginbuf.value = NULL;
3029-
if (conn->sspitarget)
3030-
free(conn->sspitarget);
3031-
conn->sspitarget = NULL;
3032-
if (conn->sspicred)
3033-
{
3034-
FreeCredentialsHandle(conn->sspicred);
3035-
free(conn->sspicred);
3036-
conn->sspicred = NULL;
3037-
}
3038-
if (conn->sspictx)
3039-
{
3040-
DeleteSecurityContext(conn->sspictx);
3041-
free(conn->sspictx);
3042-
conn->sspictx = NULL;
3043-
}
3044-
#endif
30453051
}
30463052

30473053
/*

0 commit comments

Comments
 (0)