8000 Don't assume that "E" response to NEGOTIATE_SSL_CODE means pre-7.0 se… · postwait/postgres@d3dcf7a · GitHub
[go: up one dir, main page]
Skip to content

Commit d3dcf7a

Browse files
tglsfdctglsfdc
committed
Don't assume that "E" response to NEGOTIATE_SSL_CODE means pre-7.0 server.
These days, such a response is far more likely to signify a server-side problem, such as fork failure. Reporting "server does not support SSL" (in sslmode=require) could be quite misleading. But the results could be even worse in sslmode=prefer: if the problem was transient and the next connection attempt succeeds, we'll have silently fallen back to protocol version 2.0, possibly disabling features the user needs. Hence, it seems best to just eliminate the assumption that backing off to non-SSL/2.0 protocol is the way to recover from an "E" response, and instead treat the server error the same as we would in non-SSL cases. I tested this change against a pre-7.0 server, and found that there was a second logic bug in the "prefer" path: the test to decide whether to make a fallback connection attempt assumed that we must have opened conn->ssl, which in fact does not happen given an "E" response. After fixing that, the code does indeed connect successfully to pre-7.0, as long as you didn't set sslmode=require. (If you did, you get "Unsupported frontend protocol", which isn't completely off base given the server certainly doesn't support SSL.) Since there seems no reason to believe that pre-7.0 servers exist anymore in the wild, back-patch to all supported branches.
1 parent e5d2db5 commit d3dcf7a

File tree

1 file changed

+17
-25
lines changed

1 file changed

+17
-25
lines changed

‎src/interfaces/libpq/fe-connect.c

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,16 +1368,19 @@ PQconnectPoll(PGconn *conn)
13681368
/* should not happen really */
13691369
return PGRES_POLLING_READING;
13701370
}
1371-
/* mark byte consumed */
1372-
conn->inStart = conn->inCursor;
13731371
if (SSLok == 'S')
13741372
{
1373+
/* mark byte consumed */
1374+
conn->inStart = conn->inCursor;
13751375
/* Do one-time setup; this creates conn->ssl */
13761376
if (pqsecure_initialize(conn) == -1)
13771377
goto error_return;
13781378
}
13791379
else if (SSLok == 'N')
13801380
{
1381+
/* mark byte consumed */
1382+
conn->inStart = conn->inCursor;
1383+
/* OK to do without SSL? */
13811384
if (conn->sslmode[0] == 'r') /* "require" */
13821385
{
13831386
/* Require SSL, but server does not want it */
@@ -1392,27 +1395,17 @@ PQconnectPoll(PGconn *conn)
13921395
}
13931396
else if (SSLok == 'E')
13941397
{
1395-
/* Received error - probably protocol mismatch */
1396-
if (conn->Pfdebug)
1397-
fprintf(conn->Pfdebug, "received error from server, attempting fallback to pre-7.0\n");
1398-
if (conn->sslmode[0] == 'r') /* "require" */
1399-
{
1400-
/* Require SSL, but server is too old */
1401-
printfPQExpBuffer(&conn->errorMessage,
1402-
libpq_gettext("server does not support SSL, but SSL was required\n"));
1403-
goto error_return;
1404-
}
1405-
/* Otherwise, try again without SSL */
1406-
conn->allow_ssl_try = false;
1407-
/* Assume it ain't gonna handle protocol 3, either */
1408-
conn->pversion = PG_PROTOCOL(2, 0);
1409-
/* Must drop the old connection */
1410-
closesocket(conn->sock);
1411-
conn->sock = -1;
1412-
conn->status = CONNECTION_NEEDED;
1413-
/* Discard any unread/unsent data */
1414-
conn->inStart = conn->inCursor = conn->inEnd = 0;
1415-
conn->outCount = 0;
1398+
/*
1399+
* Server failure of some sort, such as failure to
1400+
* fork a backend process. We need to process and
1401+
* report the error message, which might be formatted
1402+
* according to either protocol 2 or protocol 3.
1403+
* Rather than duplicate the code for that, we flip
1404+
* into AWAITING_RESPONSE state and let the code there
1405+
* deal with it. Note we have *not* consumed the "E"
1406+
* byte here.
1407+
*/
1408+
conn->status = CONNECTION_AWAITING_RESPONSE;
14161409
goto keep_going;
14171410
}
14181411
else
@@ -1646,8 +1639,7 @@ PQconnectPoll(PGconn *conn)
16461639
* then do a non-SSL retry
16471640
*/
16481641
if (conn->sslmode[0] == 'p' /* "prefer" */
1649-
&& conn->ssl
1650-
&& conn->allow_ssl_try /* redundant? */
1642+
&& conn->allow_ssl_try
16511643
&& !conn->wait_ssl_try) /* redundant? */
16521644
{
16531645
/* only retry once */

0 commit comments

Comments
 (0)
0