From: | Michael Paquier <michael(dot)paquier(at)gmail(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
Cc: | PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Error-like LOG when connecting with SSL for password authentication |
Date: | 2017-05-24 14:29:46 |
Message-ID: | CAB7nPqTiHZkHDkxGeU5_6NbOZqRfZ7goj+xAf9-+b+1KMVkb5g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
ssl-read-commerr-v2.patch | application/octet-stream | 3.1 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2017-05-24 14:32:40 | Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression |
Previous Message | Robert Haas | 2017-05-24 14:24:19 | Re: [HACKERS] Concurrent ALTER SEQUENCE RESTART Regression |