database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-27 23:21:58 +03:00
Code Activity

Don't assume that "E" response to NEGOTIATE_SSL_CODE means pre-7.0 server.

Browse Source 
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.
This commit is contained in:
Tom Lane
2011-08-27 16:37:17 -04:00
parent e5d2db5d22
commit d3dcf7ae7b
1 changed files with 17 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
src/interfaces/libpq/fe-connect.c
View File

@ -1368,16 +1368,19 @@ keep_going: /* We will come back to here until there is
/* should not happen really */ /* should not happen really */
return PGRES_POLLING_READING; return PGRES_POLLING_READING;
} }
/* mark byte consumed */
conn->inStart = conn->inCursor;
if (SSLok == 'S') if (SSLok == 'S')
{ {
/* mark byte consumed */
conn->inStart = conn->inCursor;
/* Do one-time setup; this creates conn->ssl */ /* Do one-time setup; this creates conn->ssl */
if (pqsecure_initialize(conn) == -1) if (pqsecure_initialize(conn) == -1)
goto error_return; goto error_return;
} }
else if (SSLok == 'N') else if (SSLok == 'N')
{ {
/* mark byte consumed */
conn->inStart = conn->inCursor;
/* OK to do without SSL? */
if (conn->sslmode[0] == 'r') /* "require" */ if (conn->sslmode[0] == 'r') /* "require" */
{ {
/* Require SSL, but server does not want it */ /* Require SSL, but server does not want it */
@ -1392,27 +1395,17 @@ keep_going: /* We will come back to here until there is
} }
else if (SSLok == 'E') else if (SSLok == 'E')
{ {
/* Received error - probably protocol mismatch */ /*
if (conn->Pfdebug) * Server failure of some sort, such as failure to
fprintf(conn->Pfdebug, "received error from server, attempting fallback to pre-7.0\n"); * fork a backend process. We need to process and
if (conn->sslmode[0] == 'r') /* "require" */ * report the error message, which might be formatted
{ * according to either protocol 2 or protocol 3.
/* Require SSL, but server is too old */ * Rather than duplicate the code for that, we flip
printfPQExpBuffer(&conn->errorMessage, * into AWAITING_RESPONSE state and let the code there
libpq_gettext("server does not support SSL, but SSL was required\n")); * deal with it. Note we have *not* consumed the "E"
goto error_return; * byte here.
} */
/* Otherwise, try again without SSL */ conn->status = CONNECTION_AWAITING_RESPONSE;
conn->allow_ssl_try = false;
/* Assume it ain't gonna handle protocol 3, either */
conn->pversion = PG_PROTOCOL(2, 0);
/* Must drop the old connection */
closesocket(conn->sock);
conn->sock = -1;
conn->status = CONNECTION_NEEDED;
/* Discard any unread/unsent data */
conn->inStart = conn->inCursor = conn->inEnd = 0;
conn->outCount = 0;
goto keep_going; goto keep_going;
} }
else else
@ -1646,8 +1639,7 @@ keep_going: /* We will come back to here until there is
* then do a non-SSL retry * then do a non-SSL retry
*/ */
if (conn->sslmode[0] == 'p' /* "prefer" */ if (conn->sslmode[0] == 'p' /* "prefer" */
&& conn->ssl && conn->allow_ssl_try
&& conn->allow_ssl_try /* redundant? */
&& !conn->wait_ssl_try) /* redundant? */ && !conn->wait_ssl_try) /* redundant? */
{ {
/* only retry once */ /* only retry once */