On Thu, Jan 20, 2022 at 09:05:35AM +1300, Thomas Munro wrote: > Good news, I'm glad they nailed that down. I recall that this > behaviour was a bit of a moving target in earlier versions: > > https://www.postgresql.org/message-id/CAEepm%3D3cc5wYv%3DX4Nzy7VOUkdHBiJs9bpLzqtqJWxdDUp5DiPQ%40mail.gmail.com Ahh.. So you saw the same problem a couple of years back. This thread did not catch much attention. I don't think that it makes much sense to leave this unchecked as the message is confusing as it stands. Perhaps we could do something like the attached by adding a note about OpenSSL 3.0 to revisit this code once we unplug support for 1.1.1 and avoiding the errno==0 case? The frontend has its own ideas of socket failures as it requires thread support, making things different with the backend, but it seems to me that we could see cases where SOCK_ERRNO is also 0. That's mostly what you suggested on the other thread. The error handling changes are really cosmetic, so I'd rather leave the back-branches out of that. Thoughts? -- Michael
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 3d0168a369..12ddc6f82f 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -493,7 +493,13 @@ aloop: WAIT_EVENT_SSL_OPEN_SERVER); goto aloop; case SSL_ERROR_SYSCALL: - if (r < 0) + /* + * OpenSSL 1.1.1 and older versions return nothing on + * an unexpected EOF, and errno may not be set. Note that + * in OpenSSL 3.0, an unexpected EOF would result in + * SSL_ERROR_SSL with a meaningful error set on the stack. + */ + if (r < 0 && errno != 0) ereport(COMMERROR, (errcode_for_socket_access(), errmsg("could not accept SSL connection: %m"))); diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index 9f735ba437..b47ea87b43 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -314,6 +314,12 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len) n = 0; break; case SSL_ERROR_SYSCALL: + /* + * OpenSSL 1.1.1 and older versions return nothing on + * an unexpected EOF, and errno may not be set. Note that + * in OpenSSL 3.0, an unexpected EOF would result in + * SSL_ERROR_SSL with a meaningful error set on the stack. + */ if (n < 0) { result_errno = SOCK_ERRNO; @@ -322,11 +328,14 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len) libpq_gettext("server closed the connection unexpectedly\n" "\tThis probably means the server terminated abnormally\n" "\tbefore or while processing the request.\n")); - else + else if (result_errno != 0) appendPQExpBuffer(&conn->errorMessage, libpq_gettext("SSL SYSCALL error: %s\n"), SOCK_STRERROR(result_errno, sebuf, sizeof(sebuf))); + else + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("SSL SYSCALL error: internal failure\n")); } else {
Attachment:
signature.asc
Description: PGP signature