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

From: william(dot)welter(at)4linux(dot)com(dot)br
To: pgsql-bugs(at)postgresql(dot)org
Subject: BUG #12799: libpq - SSL pqsecure_read() doesn't clean openssl error queue before reading
Date: 2015-02-24 03:09:56
Message-ID: 20150224030956.2529.83279@wrigleys.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

The following bug has been logged on the website:

Bug reference: 12799
Logged by: William Felipe Welter
Email address: william(dot)welter(at)4linux(dot)com(dot)br
PostgreSQL version: 9.4.1
Operating system: Ubuntu Linux
Description:

According to OpenSSL manual
(https://www.openssl.org/docs/ssl/SSL_get_error.html#DESCRIPTION) "The
current thread's error queue must be empty before the TLS/SSL I/O operation
is attempted, or SSL_get_error() will not work reliably"

But libpq in pgsecure_read()/pqsecure_write() on branch REL9_4_STABLE or in
pgtls_read()/pgtls_write() on branch MASTER there no calls to
ERR_clear_error() to clear error queue
(https://www.openssl.org/docs/crypto/ERR_clear_error.html) and avoid
unpredictable conditions.

We can reproduce problems with currently implementation on PHP scripts that
use pgsql extension (use libpq) and openssl extension as reported on PHP bug
track on "https://bugs.php.net/bug.php?id=68276" (firstly reported as memory
corruption error) when errors from previous php callings to openssl affect
libpq calls leading to fatal errors.

The solution is simple, see following patches:

Branch master
diff --git a/src/interfaces/libpq/fe-secure-openssl.c
b/src/interfaces/libpq/fe-secure-openssl.c
index 1b9f3a4..8cf0335 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -212,6 +212,7 @@ pgtls_read(PGconn *conn, void *ptr, size_t len)

rloop:
SOCK_ERRNO_SET(0);
+ ERR_clear_error();
n = SSL_read(conn->ssl, ptr, len);
err = SSL_get_error(conn->ssl, n);
switch (err)
@@ -320,6 +321,7 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
int err;

SOCK_ERRNO_SET(0);
+ ERR_clear_error();
n = SSL_write(conn->ssl, ptr, len);
err = SSL_get_error(conn->ssl, n);
switch (err)

Branch REL9_4_STABLE
diff --git a/src/interfaces/libpq/fe-secure.c
b/src/interfaces/libpq/fe-secure.c
index 2752d16..54001c0 100644
--- a/src/interfaces/libpq/fe-secure.c
+++ b/src/interfaces/libpq/fe-secure.c
@@ -350,6 +350,7 @@ pqsecure_read(PGconn *conn, void *ptr, size_t len)

rloop:
SOCK_ERRNO_SET(0);
+ ERR_clear_error();
n = SSL_read(conn->ssl, ptr, len);
err = SSL_get_error(conn->ssl, n);
switch (err)
@@ -511,6 +512,7 @@ pqsecure_write(PGconn *conn, const void *ptr, size_t
len)
DISABLE_SIGPIPE(conn, spinfo, return -1);

SOCK_ERRNO_SET(0);
+ ERR_clear_error();
n = SSL_write(conn->ssl, ptr, len);
err = SSL_get_error(conn->ssl, n);
switch (err)

Similar situation discussed on
stackoverflow:http://stackoverflow.com/questions/18179128/how-to-manage-the-error-queue-in-openssl-ssl-get-error-and-err-get-error

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message David Steele 2015-02-24 05:14:40 Re: View restore error in 9.3-9.4 upgrade
Previous Message Michael Paquier 2015-02-24 01:57:51 Re: View restore error in 9.3-9.4 upgrade