Re: should libpq also require TLSv1.2 by default? - Mailing list pgsql-hackers

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

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 = 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 = true;
+                        break;
+                    default:
+                        give_proto_hint = 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
protocolversion 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/libpq/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 support any
SSLprotocol 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;
                 }