Re: [HACKERS] Error-like LOG when connecting with SSL for password authentication - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [HACKERS] Error-like LOG when connecting with SSL for password authentication
Date
Msg-id CAB7nPqTiHZkHDkxGeU5_6NbOZqRfZ7goj+xAf9-+b+1KMVkb5g@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Error-like LOG when connecting with SSL for passwordauthentication  (Heikki Linnakangas <hlinnaka@iki.fi>)
Responses Re: [HACKERS] Error-like LOG when connecting with SSL for passwordauthentication  (Heikki Linnakangas <hlinnaka@iki.fi>)
List pgsql-hackers
On Tue, May 23, 2017 at 6:26 AM, Heikki Linnakangas <hlinnaka@iki.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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression
Next
From: Andres Freund
Date:
Subject: Re: [HACKERS] [BUGS] Concurrent ALTER SEQUENCE RESTART Regression