Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1joqPV-0006zK-AY for pgsql-hackers@arkaria.postgresql.org; Fri, 26 Jun 2020 15:37:01 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1joqPS-0000ja-RQ for pgsql-hackers@arkaria.postgresql.org; Fri, 26 Jun 2020 15:36:58 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1joqPS-0000jS-KM for pgsql-hackers@lists.postgresql.org; Fri, 26 Jun 2020 15:36:58 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1joqPJ-0000hD-Pt for pgsql-hackers@postgresql.org; Fri, 26 Jun 2020 15:36:57 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 05QFagTu016725; Fri, 26 Jun 2020 11:36:44 -0400 From: Tom Lane To: Daniel Gustafsson cc: Magnus Hagander , Peter Eisentraut , pgsql-hackers Subject: Re: should libpq also require TLSv1.2 by default? In-reply-to: <2339796.1593177583@sss.pgh.pa.us> References: <4FBC50DB-599A-42AA-90C8-3051E3A748BB@yesql.se> <2305871.1593125045@sss.pgh.pa.us> <2339796.1593177583@sss.pgh.pa.us> Comments: In-reply-to Tom Lane message dated "Fri, 26 Jun 2020 09:19:43 -0400" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <16706.1593185779.0@sss.pgh.pa.us> Date: Fri, 26 Jun 2020 11:36:42 -0400 Message-ID: <16724.1593185802@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <16706.1593185779.1@sss.pgh.pa.us> Here is a quick attempt at getting libpq and the server to report suitable hint messages about SSL version problems. The main thing I don't like about this as formulated is that I hard-wired knowledge of the minimum and maximum SSL versions into the hint messages. That's clearly not very maintainable, but it seems really hard to keep the messages readable/useful without giving concrete version numbers. Anybdy have a better idea? Is there a reasonably direct way to ask OpenSSL what its min and max versions are? regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="ssl-protocol-version-hints-1.patch"; charset="us-ascii" Content-ID: <16706.1593185779.2@sss.pgh.pa.us> Content-Description: ssl-protocol-version-hints-1.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-= secure-openssl.c index 8adf64c78e..380268636a 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -72,6 +72,7 @@ static bool dummy_ssl_passwd_cb_called =3D false; static bool ssl_is_server_start; = static int ssl_protocol_version_to_openssl(int v); +static const char *ssl_protocol_version_to_string(int v); = /* ------------------------------------------------------------ */ /* Public interface */ @@ -365,6 +366,7 @@ be_tls_open_server(Port *port) int err; int waitfor; unsigned long ecode; + bool give_proto_hint; = Assert(!port->ssl); Assert(!port->peer); @@ -451,10 +453,44 @@ aloop: errmsg("could not accept SSL connection: EOF detected"))); break; case SSL_ERROR_SSL: + switch (ERR_GET_REASON(ecode)) + { + /* + * NO_PROTOCOLS_AVAILABLE occurs if the client and + * server min/max protocol version limits don't + * overlap. UNSUPPORTED_PROTOCOL and + * WRONG_VERSION_NUMBER have been observed when trying + * to communicate with an old OpenSSL library. It's + * not very clear what would make OpenSSL return the + * other codes listed here, but a hint about protocol + * versions seems like it's appropriate for all. + */ + case SSL_R_NO_PROTOCOLS_AVAILABLE: + case SSL_R_UNSUPPORTED_PROTOCOL: + case SSL_R_BAD_PROTOCOL_VERSION_NUMBER: + case SSL_R_UNKNOWN_SSL_VERSION: + case SSL_R_UNSUPPORTED_SSL_VERSION: + case SSL_R_VERSION_TOO_HIGH: + case SSL_R_VERSION_TOO_LOW: + case SSL_R_WRONG_SSL_VERSION: + case SSL_R_WRONG_VERSION_NUMBER: + give_proto_hint =3D true; + break; + default: + give_proto_hint =3D false; + break; + } ereport(COMMERROR, (errcode(ERRCODE_PROTOCOL_VIOLATION), errmsg("could not accept SSL connection: %s", - SSLerrmessage(ecode)))); + SSLerrmessage(ecode)), + give_proto_hint ? errhint("This may indicate that the client does = not support any SSL protocol version between %s and %s.", + ssl_min_protocol_version ? + ssl_protocol_version_to_string(ssl_min_protocol_version) : + "TLSv1", + ssl_max_protocol_version ? + ssl_protocol_version_to_string(ssl_max_protocol_version) : + "TLSv1.3") : 0)); break; case SSL_ERROR_ZERO_RETURN: ereport(COMMERROR, @@ -1328,6 +1364,29 @@ ssl_protocol_version_to_openssl(int v) return -1; } = +/* + * Likewise provide a mapping to strings. + */ +static const char * +ssl_protocol_version_to_string(int v) +{ + switch (v) + { + case PG_TLS_ANY: + return "any"; + case PG_TLS1_VERSION: + return "TLSv1"; + case PG_TLS1_1_VERSION: + return "TLSv1.1"; + case PG_TLS1_2_VERSION: + return "TLSv1.2"; + case PG_TLS1_3_VERSION: + return "TLSv1.3"; + } + + return "(unrecognized)"; +} + = static void default_openssl_tls_init(SSL_CTX *context, bool isServerStart) diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/lib= pq/fe-secure-openssl.c index 2d813ef5f9..71407bffd4 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -1304,6 +1304,40 @@ open_client_SSL(PGconn *conn) libpq_gettext("SSL error: %s\n"), err); SSLerrfree(err); + switch (ERR_GET_REASON(ecode)) + { + /* + * NO_PROTOCOLS_AVAILABLE occurs if the client and + * server min/max protocol version limits don't + * overlap. UNSUPPORTED_PROTOCOL and + * WRONG_VERSION_NUMBER have been observed when + * trying to communicate with an old OpenSSL + * library. It's not very clear what would make + * OpenSSL return the other codes listed here, but + * a hint about protocol versions seems like it's + * appropriate for all. + */ + case SSL_R_NO_PROTOCOLS_AVAILABLE: + case SSL_R_UNSUPPORTED_PROTOCOL: + case SSL_R_BAD_PROTOCOL_VERSION_NUMBER: + case SSL_R_UNKNOWN_SSL_VERSION: + case SSL_R_UNSUPPORTED_SSL_VERSION: + case SSL_R_VERSION_TOO_HIGH: + case SSL_R_VERSION_TOO_LOW: + case SSL_R_WRONG_SSL_VERSION: + case SSL_R_WRONG_VERSION_NUMBER: + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("This may indicate that the server does not su= pport any SSL protocol version between %s and %s.\n"), + conn->ssl_min_protocol_version ? + conn->ssl_min_protocol_version : + "TLSv1", + conn->ssl_max_protocol_version ? + conn->ssl_max_protocol_version : + "TLSv1.3"); + break; + default: + break; + } pgtls_close(conn); return PGRES_POLLING_FAILED; } ------- =_aaaaaaaaaa0--