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

On Tue, May 23, 2017 at 6:26 AM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
> 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.

It seems to me that it could be the case, the man page of SSL_write
tells me that:
0 The write operation was not successful. Probably the
underlying connection was closed. Call SSL_get_error() with the return
value ret to find out, whether an error occurred or the connection was
shut down cleanly (SSL_ERROR_ZERO_RETURN).

> 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.

errno maps to ECONNRESET in this case. So send() can return 0 only if
the caller has specified 0 as the number of bytes to send?

> 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.

Yep. Agreed. Hopefully improved in the attached.

> In libpq, do we want to set the "SSL connection has been closed
> unexpectedly" message?

Perhaps not.

> 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.

Yes.

Attached is an updated patch.
--
Michael