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.welter@4linux.com.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