Thread: SSL-mode error reporting in libpq

SSL-mode error reporting in libpq

From
Tom Lane
Date:
In testing the fix for the SSL problem that Martin Pihlak reported, I
realized that libpq doesn't really cope very well with errors reported
by OpenSSL.  In the case at hand, SSL_write returns an SSL_ERROR_SSL
code, which pqsecure_write quite reasonably handles by putting
"SSL error: bad write retry" into conn->errorMessage.  However, it
then sets errno = ECONNRESET, which causes its caller pqSendSome()
to overwrite that potentially-useful message with an outright lie:
"server closed the connection unexpectedly".

I think what we ought to do is adjust the code so that in SSL mode,
pqsecure_write is responsible for constructing all error messages and
pqSendSome should just leave conn->errorMessage alone.

We could perhaps go a bit further and make pqsecure_write responsible
for the error message in non-SSL mode too, but it looks to me like
pqSendSome has to have a switch on the errno anyway to decide whether to
keep trying or not, so moving that responsibility would just lead to
duplicative coding.

Any objections?
        regards, tom lane


Re: SSL-mode error reporting in libpq

From
Magnus Hagander
Date:
On Sun, Jul 24, 2011 at 20:48, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> In testing the fix for the SSL problem that Martin Pihlak reported, I
> realized that libpq doesn't really cope very well with errors reported
> by OpenSSL.  In the case at hand, SSL_write returns an SSL_ERROR_SSL
> code, which pqsecure_write quite reasonably handles by putting
> "SSL error: bad write retry" into conn->errorMessage.  However, it
> then sets errno = ECONNRESET, which causes its caller pqSendSome()
> to overwrite that potentially-useful message with an outright lie:
> "server closed the connection unexpectedly".
>
> I think what we ought to do is adjust the code so that in SSL mode,
> pqsecure_write is responsible for constructing all error messages and
> pqSendSome should just leave conn->errorMessage alone.
>
> We could perhaps go a bit further and make pqsecure_write responsible
> for the error message in non-SSL mode too, but it looks to me like
> pqSendSome has to have a switch on the errno anyway to decide whether to
> keep trying or not, so moving that responsibility would just lead to
> duplicative coding.
>
> Any objections?

I haven't looked at the actual code but - does it make sense to have
them responsible for different parts and append them? If not, then no
objections ;)

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: SSL-mode error reporting in libpq

From
Daniel Farina
Date:
On Sun, Jul 24, 2011 at 11:48 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> We could perhaps go a bit further and make pqsecure_write responsible
> for the error message in non-SSL mode too, but it looks to me like
> pqSendSome has to have a switch on the errno anyway to decide whether to
> keep trying or not, so moving that responsibility would just lead to
> duplicative coding.
>
> Any objections?

No objection.

Some users we have have been confused by this message.  At first I
thought it was an SSL renegotiation problem, but now I realize that
the message is a lie, so anything can be the problem.  A better
message may well decrease support burden for us, and also help us find
problems...

-- 
fdr