Re: Error-like LOG when connecting with SSL for password authentication

On 05/22/2017 03:10 AM, Michael Paquier wrote:
> Hi all,
>
> When attempting to connect using password authentication through SSL,
> the backend will complain in its log with the following entry before
> calling sendAuthRequest(), which asks the client for a password:
> LOG: could not receive data from client: Connection reset by peer
>
> After a short talk with Heikki, it seems that be_tls_read() complains
> on SSL_ERROR_ZERO_RETURN, which is documented here:
> https://wiki.openssl.org/index.php/Manual:SSL_get_error(3)
> The TLS/SSL connection has been closed. If the protocol version is SSL
> 3.0 or TLS 1.0, this result code is returned only if a closure alert
> has occurred in the protocol, i.e. if the connection has been closed
> cleanly. Note that in this case SSL_ERROR_ZERO_RETURN does not
> necessarily indicate that the underlying transport has been closed.
>
> As this is a clean shutdown of the SSL connection, shouldn't
> be_tls_read() return 0 to the caller instead of -1? This would map
> with what the non-SSL code path does for raw reads.

Yeah. The be_tls_read() change looks OK to me.

Can SSL_ERROR_ZERO_RETURN also happen in a write? I suppose it can, but
returning 0 from secure_write() seems iffy. My man page for send(2)
doesn't say that it would return 0 if the connection has been closed by
the peer, so that would be different from the non-SSL codepath. Looking
at the only caller to secure_write(), it does check for 0, and would
treat it correctly as EOF, so it would work. But it makes me a bit
nervous, a comment in secure_write() at least would be in order, to
point out that it can return 0 to indicate EOF, unlike the raw write(2)
or send(2) system functions.

In libpq, do we want to set the "SSL connection has been closed
unexpectedly" message? In the non-SSL case, pqsecure_raw_read() doesn't
set any error message on EOF. Also, even though the comment in
pgtls_read() says "... we should not report it as a server crash",
looking at pqReadData, ISTM that returning 0 will in fact lead to the
"server closed the connection unexpectedly" message. And it seems like a
good assumption anyway, that the server did in fact terminate
abnormally, if that happens. We don't expect the server to disconnect
the client without notice, even though it's not unusual for the client
to disconnect without notifying the server.

- Heikki