From c46c85d4594d52fb34d36d4761bb9cfc5626f20b Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Thu, 4 Apr 2019 22:52:42 -0400 Subject: [PATCH] Handle errors during GSSAPI startup better There was some confusion over the format of the error message returned from the server during GSSAPI startup; specifically, it was expected that a length would be returned when, in reality, at this early stage in the startup sequence, no length is returned from the server as part of an error message. Correct the client-side code for dealing with error messages sent by the server during startup by simply reading what's available into our buffer, after we've discovered it's an error message, and then reporting back what was returned. In passing, also add in documentation of the environment variable PGGSSENCMODE which was missed previously, and adjust the code to look for the PGGSSENCMODE variable (the environment variable change was missed in the prior GSSMODE -> GSSENCMODE commit). Error-handling issue discovered by Peter Eisentraut, the rest were items discovered during testing of the error handling. --- doc/src/sgml/libpq.sgml | 10 ++++++++ src/interfaces/libpq/fe-connect.c | 4 +-- src/interfaces/libpq/fe-secure-gssapi.c | 34 ++++++------------------- 3 files changed, 20 insertions(+), 28 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index a97af98979..924b7ce50e 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -7495,6 +7495,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) + + + + PGGSSENCMODE + + PGGSSENCMODE behaves the same as the connection parameter. + + + diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 68cf422457..5dcc7d7692 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -308,7 +308,7 @@ static const internalPQconninfoOption PQconninfoOptions[] = { * Expose gssencmode similarly to sslmode - we can still handle "disable" * and "prefer". */ - {"gssencmode", "PGGSSMODE", DefaultGSSMode, NULL, + {"gssencmode", "PGGSSENCMODE", DefaultGSSMode, NULL, "GSS-Mode", "", 7, /* sizeof("disable") == 7 */ offsetof(struct pg_conn, gssencmode)}, @@ -1875,7 +1875,7 @@ connectDBStart(PGconn *conn) resetPQExpBuffer(&conn->errorMessage); #ifdef ENABLE_GSS - if (conn->gssencmode[0] == 'd') /* "disable" */ + if (conn->gssencmode[0] == 'd') /* "disable" */ conn->try_gss = false; #endif diff --git a/src/interfaces/libpq/fe-secure-gssapi.c b/src/interfaces/libpq/fe-secure-gssapi.c index ea1c1cd7b7..ec2a4c6478 100644 --- a/src/interfaces/libpq/fe-secure-gssapi.c +++ b/src/interfaces/libpq/fe-secure-gssapi.c @@ -459,42 +459,24 @@ pqsecure_open_gss(PGconn *conn) * * This is safe to do because we shouldn't ever get a packet over 8192 * and therefore the actual length bytes, being that they are in - * network byte order, for any real packet will be two zero bytes. + * network byte order, for any real packet will start with two zero + * bytes. */ if (PqGSSRecvBuffer[0] == 'E') { /* - * For an error message, the length is after the E, so read one - * more byte to get the full length + * For an error packet during startup, we don't get a length, so + * simply read as much as we can fit into our buffer (as a string, + * so leave a spot at the end for a NULL byte too) and report that + * back to the caller. */ - result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, 1, &ret); + result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, PQ_GSS_RECV_BUFFER_SIZE - PqGSSRecvLength - 1, &ret); if (result != PGRES_POLLING_OK) return result; PqGSSRecvLength += ret; - if (PqGSSRecvLength < 1 + sizeof(uint32)) - return PGRES_POLLING_READING; - - input.length = ntohl(*(uint32 *) PqGSSRecvBuffer + 1); - if (input.length > PQ_GSS_RECV_BUFFER_SIZE - sizeof(uint32) - 1) - { - printfPQExpBuffer(&conn->errorMessage, libpq_gettext("Over-size error packet sent by the server.")); - return PGRES_POLLING_FAILED; - } - - result = gss_read(conn, PqGSSRecvBuffer + PqGSSRecvLength, input.length - PqGSSRecvLength - 1 - sizeof(uint32), &ret); - if (result != PGRES_POLLING_OK) - return result; - - PqGSSRecvLength += ret; - - if (PqGSSRecvLength < 1 + sizeof(uint32) + input.length) - return PGRES_POLLING_READING; - - printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("Server error: %s"), - PqGSSRecvBuffer + 1 + sizeof(int32)); + printfPQExpBuffer(&conn->errorMessage, "%s", PqGSSRecvBuffer + 1); return PGRES_POLLING_FAILED; }