Re: 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: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: pgsql-bugs(at)postgresql(dot)org
Subject: Re: BUG #12799: libpq - SSL pqsecure_read() doesn't clean openssl error queue before reading
Date: 2015-02-27 02:00:29
Message-ID: 319431910.43023.1425002429817.JavaMail.zimbra@4linux.com.br
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs


> <pedantic>
> The OpenSSL manual doesn't directly require you to call
> ERR_clear_error() before every SSL_* call. It just requires that you
> ensure that the error queue is empty. Libpq ensures that by always
> clearing the queue *after* an error happens, in SSLerrmessage().
> </pedantic>
>
Ok, i understand.

> I'm actually not 100% sure we clear the error queue after every error.
> Looking at the OpenSSL sources on SSL_get_error(), it seems possible
> that it returns SSL_ERROR_SYSCALL, with an error in the error queue. The
> libpq code does not call SSLerrmessag() when SSL_get_error() returns
> SSL_ERROR_SYSCALL, so that would leave the error dangling in the queue.
> Also, if there are multiple errors in the queue, we only return and
> remove from the queue the first one.
>
> So should we clear the error queue before each call, just to be sure?
> The risk is that the application has performed some other OpenSSL
> operations, outside libpq, but not checked the error queue yet, and when
> we clear the queue, the original error is gone and isn't reported. An
> application probably shouldn't do that, you should check the error after
> each SSL call, and not leave the error dangling in the error queue as a
> landmine for the next SSL call. OTOH, if the application is built so
> that the error queue is cleared before each SSL call, not after, that's
> incompatible with the way libpq currently works. And that's certainly a
> much more common practice than the hypothetical application that would
> miss the error if it's cleared unexpectedly.

If i understand correctly:
- If libpq clear the error queue before each call this can break current applications that not check error queue immedialty after calls.
- But if libpq not clear the queue, application can leave errors on the queue which lead libpq throws unrelated errors.

On PHP case, the problem is that the queue is only cleared if the developer explicit call "openssl_error_string()" for each openssl function they called before. I think this behavior can be changed on PHP to prevent this kind of problem with libpq or other libs that use openssl.

>
> Overall, I agree we should clear the error queue before each SSL call.
> It makes the application as whole more robust.
>
> > 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)

>
> - Heikki
>
> --
> Message protected by MailGuard: e-mail anti-virus, anti-spam and content
> filtering.http://www.mailguard.com.au/mg
> Click here to report this message as spam:
> https://console.mailguard.com.au/ras/1LubMvNYuN/6iGxWUBDMTyqo5LXNO4F30/0
>
>

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Daniele Posenato 2015-02-27 12:27:11 Re: BUG #12785: server process (PID 2872) was terminated by exception 0xC0000005
Previous Message gowridhar.madu 2015-02-26 18:17:19 BUG #12810: database conection drop