Re: BUG #12799: libpq - SSL pqsecure_read() doesn't clean openssl error queue before reading

On 3/1/2015 3:32 PM, Heikki Linnakangas wrote:
> On 02/27/2015 04:00 AM, william(dot)welter(at)4linux(dot)com(dot)br wrote:
>>>> The solution is simple, see following patches:
>>>
>>> Yeah. Should also do the same in the backend, and also before
>>> SSL_connect/SSL_accept() calls. I.e. any time SSL_get_error is used.
>>
>> Question: Is possible to fix this to next major release ? (im
>> volunteering)
>
> Yeah. If you could write a complete patch, per above, I can commit it.
>
> - Heikki
>
>
We independently ran into this, diagnosed it, and fixed it. Here is the
complete patch covering every use of SSL_get_error.

diff -ur src/backend/libpq/be-secure-openssl.c
src/backend/libpq/be-secure-openssl.c
--- src/backend/libpq/be-secure-openssl.c 2016-02-18
16:40:38.540898100 -0500
+++ src/backend/libpq/be-secure-openssl.c 2016-02-18
16:59:07.331898100 -0500
@@ -353,6 +353,10 @@
port->ssl_in_use = true;

aloop:
+ /* make sure SSL_get_error_doesn't fetch an
+ * old error that predates the SSL_accept below.
+ */
+ ERR_clear_error();
r = SSL_accept(port->ssl);
if (r <= 0)
{
@@ -501,6 +505,9 @@
int err;

errno = 0;
+ /* make sure SSL_get_error_doesn't fetch an
+ * old error that predates the SSL_read below.
+ */
n = SSL_read(port->ssl, ptr, len);
err = SSL_get_error(port->ssl, n);
switch (err)
@@ -558,6 +565,9 @@
int err;

errno = 0;
+ /* make sure SSL_get_error_doesn't fetch an
+ * old error that predates the SSL_write below.
+ */
n = SSL_write(port->ssl, ptr, len);
err = SSL_get_error(port->ssl, n);
switch (err)
diff -ur src/interfaces/libpq/fe-secure-openssl.c
src/interfaces/libpq/fe-secure-openssl.c
--- src/interfaces/libpq/fe-secure-openssl.c 2016-02-18
16:40:03.575898100 -0500
+++ src/interfaces/libpq/fe-secure-openssl.c 2016-02-18
16:58:01.711898100 -0500
@@ -212,6 +212,10 @@

rloop:
SOCK_ERRNO_SET(0);
+ /* make sure SSL_get_error_doesn't fetch an
+ * old error that predates the SSL_read below.
+ */
+ ERR_clear_error();
n = SSL_read(conn->ssl, ptr, len);
err = SSL_get_error(conn->ssl, n);
switch (err)
@@ -320,6 +324,10 @@
int err;

SOCK_ERRNO_SET(0);
+ /* make sure SSL_get_error_doesn't fetch an
+ * old error that predates the SSL_write below.
+ */
+ ERR_clear_error();
n = SSL_write(conn->ssl, ptr, len);
err = SSL_get_error(conn->ssl, n);
switch (err)
@@ -1327,6 +1335,10 @@
{
int r;

+ /* make sure SSL_get_error_doesn't fetch an
+ * old error that predates the SSL_connect below.
+ */
+ ERR_clear_error();
r = SSL_connect(conn->ssl);
if (r <= 0)
{

- Dave