Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

Started by Michael Paquierover 7 years ago18 messages
#1Michael Paquier
michael@paquier.xyz
2 attachment(s)

Hi all,

Currently, the SCRAM channel binding tls-server-end-point is supported
only with OpenSSL 1.0.2 and newer versions as we rely on
X509_get_signature_nid to get the certificate signature ID, which is the
official way of upstream to get this information as all the contents of
X509 are shadowed since this version.

I think that this matters for users of JDBC with environments shipping
and maintaining longer than upstream versions of OpenSSL like 1.0.1 and
1.0.0 (Redhat is one of them).

Stephen Fackler, who is in CC, has reported to me off-list that it is
possible to get easily this information by looking at
X509->sig_alg->algorithm, which to easily get tls-server-end-point
support for OpenSSL 1.0.0 and 1.0.1.

The official way to get the hashing algorithm is then to apply
OBJ_find_sigid_algs, which is only available since 1.0.0. Stephen has
also pointed me out that we could use EVP_get_digestbynid, but it is
surprising to see that working correctly if you particularly look at
upstream commit ee1d9ec which introduced a sort of hack to fix links
between the digest and signature algorithms in OpenSSL 1.0.0 and above.
Anyway, it seems to me that it would not be a good bet to rely on an
upstream hack as we may get trapped later on if the handlings for
EVP_get_digestbynid get reworked by upstream again. So I propose to
rely on OBJ_find_sigid_algs which is the official interface to get the
signature algorithm as far as I understand.

While hacking on that, I also noticed that OBJ_find_sigid_algs can crash
when using OpenSSL 1.0.0 if a NULL input is used with its 2nd and 3rd
arguments, which is an issue fixed by upstream in commit 177f27d,
present since 1.0.1 but bit 1.0.0, so we need an idle field to get that
to work properly with OpenSSL 1.0.0.

Attached is the result of this investigation, which I have tested using
OpenSSL from 0.9.8 to 1.0.2 compiled manually. 0.9.8 fails any
connection attempt with tls-server-end-point, while 1.0.0, 1.0.1 and
1.0.2 succeed properly.

An open item is added as well.

One of the versions I discussed with Stephen is here by the way, and I
am attaching it to this thread as well for transparency (see
end-point-support-stephen.patch):
https://github.com/sfackler/postgres/commit/93383be85358a1ee03f34f34de45058e238964d1

Thanks,
--
Michael

Attachments:

end-point-support-stephen.patchtext/x-diff; charset=us-asciiDownload
From 93383be85358a1ee03f34f34de45058e238964d1 Mon Sep 17 00:00:00 2001
From: Steven Fackler <sfackler@gmail.com>
Date: Mon, 28 May 2018 14:53:38 -0700
Subject: [PATCH] Support tls-server-end-point on old OpenSSL

X509_get_signature_nid was added in 1.0.2, but the fields required to
get to the signare algorithm are public on versions before that, so we
can just shim out the function when missing.

OBJ_find_sigid_algs was added in 1.0.0, but (somewhat surprisingly)
EVP_get_digestbynid works when given the NID for a signature algorithm
so we can use that instead.
---
 src/backend/libpq/be-secure-openssl.c    | 34 ++++++++++----------
 src/interfaces/libpq/fe-secure-openssl.c | 40 +++++++++++-------------
 src/test/ssl/t/002_scram.pl              | 24 +++-----------
 3 files changed, 39 insertions(+), 59 deletions(-)

diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 48b468f62f..53660105ec 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -1123,16 +1123,26 @@ be_tls_get_peer_finished(Port *port, size_t *len)
 	return result;
 }
 
+#ifndef HAVE_X509_GET_SIGNATURE_NID
+/*
+ * This function was added in OpenSSL 1.0.2, but all of the fields it accesses
+ * are public in older versions, so we can just redefine it when missing.
+ */
+static int
+X509_get_signature_nid(const X509 *x)
+{
+	return OBJ_obj2nid(x->sig_alg->algorithm);
+}
+#endif
+
 char *
 be_tls_get_certificate_hash(Port *port, size_t *len)
 {
-#ifdef HAVE_X509_GET_SIGNATURE_NID
 	X509	   *server_cert;
 	char	   *cert_hash;
-	const EVP_MD *algo_type = NULL;
+	const EVP_MD *algo_type;
 	unsigned char hash[EVP_MAX_MD_SIZE];	/* size for SHA-512 */
 	unsigned int hash_size;
-	int			algo_nid;
 
 	*len = 0;
 	server_cert = SSL_get_certificate(port->ssl);
@@ -1143,8 +1153,8 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
 	 * Get the signature algorithm of the certificate to determine the hash
 	 * algorithm to use for the result.
 	 */
-	if (!OBJ_find_sigid_algs(X509_get_signature_nid(server_cert),
-							 &algo_nid, NULL))
+	algo_type = EVP_get_digestbynid(X509_get_signature_nid(server_cert));
+	if (!algo_type)
 		elog(ERROR, "could not determine server certificate signature algorithm");
 
 	/*
@@ -1153,18 +1163,12 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
 	 * (https://tools.ietf.org/html/rfc5929#section-4.1).  If something else
 	 * is used, the same hash as the signature algorithm is used.
 	 */
-	switch (algo_nid)
+	switch (EVP_MD_type(algo_type))
 	{
 		case NID_md5:
 		case NID_sha1:
 			algo_type = EVP_sha256();
 			break;
-		default:
-			algo_type = EVP_get_digestbynid(algo_nid);
-			if (algo_type == NULL)
-				elog(ERROR, "could not find digest for NID %s",
-					 OBJ_nid2sn(algo_nid));
-			break;
 	}
 
 	/* generate and save the certificate hash */
@@ -1176,12 +1180,6 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
 	*len = hash_size;
 
 	return cert_hash;
-#else
-	ereport(ERROR,
-			(errcode(ERRCODE_PROTOCOL_VIOLATION),
-			 errmsg("channel binding type \"tls-server-end-point\" is not supported by this build")));
-	return NULL;
-#endif
 }
 
 /*
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 43640e3799..e975115525 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -389,15 +389,26 @@ pgtls_get_finished(PGconn *conn, size_t *len)
 	return result;
 }
 
+#ifndef HAVE_X509_GET_SIGNATURE_NID
+/*
+ * This function was added in OpenSSL 1.0.2, but all of the fields it accesses
+ * are public in older versions, so we can just redefine it when missing.
+ */
+static int
+X509_get_signature_nid(const X509 *x)
+{
+	return OBJ_obj2nid(x->sig_alg->algorithm);
+}
+#endif
+
 char *
 pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 {
-#ifdef HAVE_X509_GET_SIGNATURE_NID
 	X509	   *peer_cert;
 	const EVP_MD *algo_type;
 	unsigned char hash[EVP_MAX_MD_SIZE];	/* size for SHA-512 */
 	unsigned int hash_size;
-	int			algo_nid;
+	int			signature_nid;
 	char	   *cert_hash;
 
 	*len = 0;
@@ -411,11 +422,13 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 	 * Get the signature algorithm of the certificate to determine the hash
 	 * algorithm to use for the result.
 	 */
-	if (!OBJ_find_sigid_algs(X509_get_signature_nid(peer_cert),
-							 &algo_nid, NULL))
+	signature_nid = X509_get_signature_nid(peer_cert);
+	algo_type = EVP_get_digestbynid(signature_nid);
+	if (!algo_type)
 	{
 		printfPQExpBuffer(&conn->errorMessage,
-						  libpq_gettext("could not determine server certificate signature algorithm\n"));
+						  libpq_gettext("could not find digest for NID %s\n"),
+						  OBJ_nid2sn(signature_nid));
 		return NULL;
 	}
 
@@ -425,22 +438,12 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 	 * (https://tools.ietf.org/html/rfc5929#section-4.1).  If something else
 	 * is used, the same hash as the signature algorithm is used.
 	 */
-	switch (algo_nid)
+	switch (EVP_MD_type(algo_type))
 	{
 		case NID_md5:
 		case NID_sha1:
 			algo_type = EVP_sha256();
 			break;
-		default:
-			algo_type = EVP_get_digestbynid(algo_nid);
-			if (algo_type == NULL)
-			{
-				printfPQExpBuffer(&conn->errorMessage,
-								  libpq_gettext("could not find digest for NID %s\n"),
-								  OBJ_nid2sn(algo_nid));
-				return NULL;
-			}
-			break;
 	}
 
 	if (!X509_digest(peer_cert, algo_type, hash, &hash_size))
@@ -462,11 +465,6 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 	*len = hash_size;
 
 	return cert_hash;
-#else
-	printfPQExpBuffer(&conn->errorMessage,
-					  libpq_gettext("channel binding type \"tls-server-end-point\" is not supported by this build\n"));
-	return NULL;
-#endif
 }
 
 /* ------------------------------------------------------------ */
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 52a8f458cb..682add4fb7 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -18,10 +18,6 @@ my $number_of_tests = 6;
 # This is the hostname used to connect to the server.
 my $SERVERHOSTADDR = '127.0.0.1';
 
-# Determine whether build supports tls-server-end-point.
-my $supports_tls_server_end_point =
-  check_pg_config("#define HAVE_X509_GET_SIGNATURE_NID 1");
-
 # Allocation of base connection string shared among multiple tests.
 my $common_connstr;
 
@@ -56,22 +52,10 @@ test_connect_ok(
 	"SCRAM authentication with tls-unique as channel binding");
 test_connect_ok($common_connstr, "scram_channel_binding=''",
 	"SCRAM authentication without channel binding");
-if ($supports_tls_server_end_point)
-{
-	test_connect_ok(
-		$common_connstr,
-		"scram_channel_binding=tls-server-end-point",
-		"SCRAM authentication with tls-server-end-point as channel binding");
-}
-else
-{
-	test_connect_fails(
-		$common_connstr,
-		"scram_channel_binding=tls-server-end-point",
-		qr/channel binding type "tls-server-end-point" is not supported by this build/,
-		"SCRAM authentication with tls-server-end-point as channel binding");
-	$number_of_tests++;
-}
+test_connect_ok(
+	$common_connstr,
+	"scram_channel_binding=tls-server-end-point",
+	"SCRAM authentication with tls-server-end-point as channel binding");
 test_connect_fails(
 	$common_connstr,
 	"scram_channel_binding=not-exists",
-- 
2.17.0

0001-Allow-tls-server-end-point-with-OpenSSL-1.0.0-and-1..patchtext/x-diff; charset=us-asciiDownload
From 169328694b20ae836e2277cd7b9c7f6dce03fb94 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 29 May 2018 16:25:14 -0400
Subject: [PATCH] Allow tls-server-end-point with OpenSSL 1.0.0 and 1.0.1

Since OpenSSL 1.0.2, the contents of X509 are shadowed and only
accessible using dedicated upstream APIs.  In order to get the
certificate hash, X509_get_signature_nid is a central part of it.

However, it happens that getting the hashing algorithm for the
certificate is actually possible by looking at the contents of
X509->sig_alg->algorithm.  Another part of the puzzle is
OBJ_find_sigid_algs, which has been introduced in OpenSSL 1.0.0, and is
the reliable way the base signature algorithm from upstream structures.

Down to OpenSSL 0.9.8, we could try to rely on EVP_get_digestbynid
to get this data however seeing this work is rather surprising if one
looks at the upstream commit ee1d9ec0 which re-established the links
between signature algorithms and digests using a hack, so it looks like
a bad bet for the future to rely on this hack to be supported in future
versions of OpenSSL.  Hence it is not considered and only OpenSSL 0.9.8
does not get support for tls-server-end-point which is the oldest
version supported on HEAD.

Another thing to note is that OBJ_find_sigid_alg can crash if a NULL
input is provided as second or third argument, which is an issue fixed
by upstream in 177f27d, included in 1.0.1 and above but *not* 1.0.0.

Patch by Michael Paquier and Steven Fackler, which is based on an
off-list discussion between the two.
---
 configure.in                             |  2 +-
 src/backend/libpq/be-secure-openssl.c    | 21 ++++++++++++++++-----
 src/include/pg_config.h.in               |  3 +++
 src/interfaces/libpq/fe-secure-openssl.c | 21 ++++++++++++++++-----
 src/test/ssl/t/002_scram.pl              |  2 +-
 5 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/configure.in b/configure.in
index 862d8b128d..0f3a352f20 100644
--- a/configure.in
+++ b/configure.in
@@ -1160,7 +1160,7 @@ if test "$with_openssl" = yes ; then
      AC_SEARCH_LIBS(CRYPTO_new_ex_data, [eay32 crypto], [], [AC_MSG_ERROR([library 'eay32' or 'crypto' is required for OpenSSL])])
      AC_SEARCH_LIBS(SSL_new, [ssleay32 ssl], [], [AC_MSG_ERROR([library 'ssleay32' or 'ssl' is required for OpenSSL])])
   fi
-  AC_CHECK_FUNCS([SSL_clear_options SSL_get_current_compression X509_get_signature_nid])
+  AC_CHECK_FUNCS([SSL_clear_options SSL_get_current_compression X509_get_signature_nid OBJ_find_sigid_algs])
   # Functions introduced in OpenSSL 1.1.0. We used to check for
   # OPENSSL_VERSION_NUMBER, but that didn't work with 1.1.0, because LibreSSL
   # defines OPENSSL_VERSION_NUMBER to claim version 2.0.0, even though it
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 48b468f62f..5b03558613 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -1126,13 +1126,15 @@ be_tls_get_peer_finished(Port *port, size_t *len)
 char *
 be_tls_get_certificate_hash(Port *port, size_t *len)
 {
-#ifdef HAVE_X509_GET_SIGNATURE_NID
+#ifdef HAVE_OBJ_FIND_SIGID_ALGS
 	X509	   *server_cert;
 	char	   *cert_hash;
 	const EVP_MD *algo_type = NULL;
 	unsigned char hash[EVP_MAX_MD_SIZE];	/* size for SHA-512 */
 	unsigned int hash_size;
 	int			algo_nid;
+	int			key_nid;
+	int			digest_alg;
 
 	*len = 0;
 	server_cert = SSL_get_certificate(port->ssl);
@@ -1140,11 +1142,20 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
 		return NULL;
 
 	/*
-	 * Get the signature algorithm of the certificate to determine the hash
-	 * algorithm to use for the result.
+	 * Get the signature algorithm of the certificate to determine the
+	 * hash algorithm to use for the result.  X509_get_signature_nid has
+	 * been introduced in 1.0.2.  Down to OpenSSL 0.9.8 the signature
+	 * algorithm is available directly through X509, however
+	 * OBJ_find_sigid_alg(), which is the stable way of getting the hash
+	 * algorithm has been only introduced in 1.0.0.
 	 */
-	if (!OBJ_find_sigid_algs(X509_get_signature_nid(server_cert),
-							 &algo_nid, NULL))
+#ifdef HAVE_X509_GET_SIGNATURE_NID
+	digest_alg = X509_get_signature_nid(server_cert);
+#else
+	digest_alg = OBJ_obj2nid(server_cert->sig_alg->algorithm);
+#endif
+
+	if (!OBJ_find_sigid_algs(digest_alg, &algo_nid, &key_nid))
 		elog(ERROR, "could not determine server certificate signature algorithm");
 
 	/*
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 89b8804251..733617a6c1 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -409,6 +409,9 @@
 /* Define to 1 if you have the <net/if.h> header file. */
 #undef HAVE_NET_IF_H
 
+/* Define to 1 if you have the `OBJ_find_sigid_algs' function. */
+#undef HAVE_OBJ_FIND_SIGID_ALGS
+
 /* Define to 1 if you have the `OPENSSL_init_ssl' function. */
 #undef HAVE_OPENSSL_INIT_SSL
 
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 43640e3799..e4eeeef2e5 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -392,13 +392,15 @@ pgtls_get_finished(PGconn *conn, size_t *len)
 char *
 pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 {
-#ifdef HAVE_X509_GET_SIGNATURE_NID
+#ifdef HAVE_OBJ_FIND_SIGID_ALGS
 	X509	   *peer_cert;
 	const EVP_MD *algo_type;
 	unsigned char hash[EVP_MAX_MD_SIZE];	/* size for SHA-512 */
 	unsigned int hash_size;
 	int			algo_nid;
+	int			key_nid;
 	char	   *cert_hash;
+	int			digest_alg;
 
 	*len = 0;
 
@@ -408,11 +410,20 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 	peer_cert = conn->peer;
 
 	/*
-	 * Get the signature algorithm of the certificate to determine the hash
-	 * algorithm to use for the result.
+	 * Get the signature algorithm of the certificate to determine the
+	 * hash algorithm to use for the result.  X509_get_signature_nid has
+	 * been introduced in 1.0.2.  Down to OpenSSL 0.9.8 the signature
+	 * algorithm is available directly through X509, however
+	 * OBJ_find_sigid_alg(), which is the stable way of getting the hash
+	 * algorithm has been only introduced in 1.0.0.
 	 */
-	if (!OBJ_find_sigid_algs(X509_get_signature_nid(peer_cert),
-							 &algo_nid, NULL))
+#ifdef HAVE_X509_GET_SIGNATURE_NID
+	digest_alg = X509_get_signature_nid(peer_cert);
+#else
+	digest_alg = OBJ_obj2nid(peer_cert->sig_alg->algorithm);
+#endif
+
+	if (!OBJ_find_sigid_algs(digest_alg, &algo_nid, &key_nid))
 	{
 		printfPQExpBuffer(&conn->errorMessage,
 						  libpq_gettext("could not determine server certificate signature algorithm\n"));
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 52a8f458cb..a660615d82 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -20,7 +20,7 @@ my $SERVERHOSTADDR = '127.0.0.1';
 
 # Determine whether build supports tls-server-end-point.
 my $supports_tls_server_end_point =
-  check_pg_config("#define HAVE_X509_GET_SIGNATURE_NID 1");
+  check_pg_config("#define HAVE_OBJ_FIND_SIGID_ALGS 1");
 
 # Allocation of base connection string shared among multiple tests.
 my $common_connstr;
-- 
2.17.0

#2Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Michael Paquier (#1)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On 29/05/18 17:02, Michael Paquier wrote:

Currently, the SCRAM channel binding tls-server-end-point is supported
only with OpenSSL 1.0.2 and newer versions as we rely on
X509_get_signature_nid to get the certificate signature ID, which is the
official way of upstream to get this information as all the contents of
X509 are shadowed since this version.

Hmm. I think Peter went through this in commits ac3ff8b1d8 and
054e8c6cdb. If you got that working now, I suppose we could do that, but
I'm actually inclined to just stick to the current, more straightforward
code, and require OpenSSL 1.0.2 for this feature. OpenSSL 1.0.2 has been
around for several years now. It's not available on all the popular
platforms and distributions yet, but I don't want to bend over backwards
to support those.

[1]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ac3ff8b1d8f98da38c53a701e6397931080a39cf
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=ac3ff8b1d8f98da38c53a701e6397931080a39cf
[2]: https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=054e8c6cdb7f4261869e49d3ed7705cca475182e
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=054e8c6cdb7f4261869e49d3ed7705cca475182e

- Heikki

#3Michael Paquier
michael@paquier.xyz
In reply to: Heikki Linnakangas (#2)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On Tue, May 29, 2018 at 10:33:03PM -0400, Heikki Linnakangas wrote:

Hmm. I think Peter went through this in commits ac3ff8b1d8 and 054e8c6cdb.
If you got that working now, I suppose we could do that, but I'm actually
inclined to just stick to the current, more straightforward code, and
require OpenSSL 1.0.2 for this feature. OpenSSL 1.0.2 has been around for
several years now. It's not available on all the popular platforms and
distributions yet, but I don't want to bend over backwards to support those.

I think that this mainly boils down to how much Postgres JDBC wants to
get support here as some vendors can maintain oldest versions of OpenSSL
for a long time. The extra code is not that much complicated by the
way, still it is true that HEAD is cleaner with its simplicity.

Steven, as the initial reporter, what's your take?
--
Michael

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Michael Paquier (#3)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On 2018-May-29, Michael Paquier wrote:

On Tue, May 29, 2018 at 10:33:03PM -0400, Heikki Linnakangas wrote:

Hmm. I think Peter went through this in commits ac3ff8b1d8 and 054e8c6cdb.
If you got that working now, I suppose we could do that, but I'm actually
inclined to just stick to the current, more straightforward code, and
require OpenSSL 1.0.2 for this feature. OpenSSL 1.0.2 has been around for
several years now. It's not available on all the popular platforms and
distributions yet, but I don't want to bend over backwards to support those.

I think that this mainly boils down to how much Postgres JDBC wants to
get support here as some vendors can maintain oldest versions of OpenSSL
for a long time. The extra code is not that much complicated by the
way, still it is true that HEAD is cleaner with its simplicity.

Steven, as the initial reporter, what's your take?

If SCRAM channel binding is an important aspect to security, and the
older OpenSSL versions will still be around in servers for some time
yet, it seems like it behooves us to go the extra mile and provide an
implementation that works with such existing servers. Looking at
yum.postgresql.org, we seem to offer Postgres 11 packages for RHEL 6,
which appears to have openssl 1.0.0.

Anyway, even if I'm wrong, this thread has stalled. I hereby sprinkle
this thread with magic RMT dust for it to get fixed soon.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#4)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

If SCRAM channel binding is an important aspect to security, and the
older OpenSSL versions will still be around in servers for some time
yet, it seems like it behooves us to go the extra mile and provide an
implementation that works with such existing servers. Looking at
yum.postgresql.org, we seem to offer Postgres 11 packages for RHEL 6,
which appears to have openssl 1.0.0.

Not sure if the difference is relevant here, but an up-to-date RHEL6
installation contains 1.0.1e:

$ rpm -q openssl
openssl-1.0.1e-57.el6.x86_64
openssl-1.0.1e-57.el6.i686

regards, tom lane

#6Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Alvaro Herrera (#4)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On 6/6/18 12:37, Alvaro Herrera wrote:

If SCRAM channel binding is an important aspect to security, and the
older OpenSSL versions will still be around in servers for some time
yet, it seems like it behooves us to go the extra mile and provide an
implementation that works with such existing servers. Looking at
yum.postgresql.org, we seem to offer Postgres 11 packages for RHEL 6,
which appears to have openssl 1.0.0.

There are two channel binding types: tls-unique and
tls-server-end-point. Of the two, tls-unique is the "better" one. We
do support that without a problem. tls-server-end-point is for SSL
implementations that cannot support tls-unique, because the SSL library
does not expose the required information. Most prominently, this is for
JDBC.

So currently, we support channel binding using tls-unique just fine
between libpq and a server. And we support tls-server-end-point between
JDBC and a server using new-ish OpenSSL. We don't support any channel
binding between for example JDBC and a server on CentOS 6. But that's
not a regression, it's just not there.

As Heikki was saying, the proposed patch seems to tread into the
portability problem territory that caused the previous attempt to fail
and had to be reverted. I am not that interested in trying that again
without new insights. I don't think we are going to do ourselves a
favor if we start meddling with that again. There are dozens of OpenSSL
variants out there, and the version history is nonlinear.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#7Steven Fackler
sfackler@gmail.com
In reply to: Peter Eisentraut (#6)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

TLS 1.3, (which is currently in a draft state, but is theoretically being
finalized soon) does not support the TLS channel binding algorithms [1]https://tools.ietf.org/html/draft-ietf-tls-tls13-28#appendix-C.5.
From talking with one of the people working on the TLS 1.3 standard,
tls-unique is seen as particularly problematic. There's some discussion on
the IETF mailing lists from a couple of years ago [2]https://www.ietf.org/mail-archive/web/tls/current/msg18257.html.

Ignoring that line of the draft, the current tls-unique implementation in
Postgres is currently incorrect for TLS 1.3 handshakes anyway since the
server sends the first Finished message rather than the client [3]https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-2. This is
also the case for TLS 1.2 handshakes with session resumption [4]https://tools.ietf.org/html/rfc5246#section-7.3.

Steven

[1]: https://tools.ietf.org/html/draft-ietf-tls-tls13-28#appendix-C.5
[2]: https://www.ietf.org/mail-archive/web/tls/current/msg18257.html
[3]: https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-2
[4]: https://tools.ietf.org/html/rfc5246#section-7.3

On Wed, Jun 6, 2018 at 12:37 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

Show quoted text

On 6/6/18 12:37, Alvaro Herrera wrote:

If SCRAM channel binding is an important aspect to security, and the
older OpenSSL versions will still be around in servers for some time
yet, it seems like it behooves us to go the extra mile and provide an
implementation that works with such existing servers. Looking at
yum.postgresql.org, we seem to offer Postgres 11 packages for RHEL 6,
which appears to have openssl 1.0.0.

There are two channel binding types: tls-unique and
tls-server-end-point. Of the two, tls-unique is the "better" one. We
do support that without a problem. tls-server-end-point is for SSL
implementations that cannot support tls-unique, because the SSL library
does not expose the required information. Most prominently, this is for
JDBC.

So currently, we support channel binding using tls-unique just fine
between libpq and a server. And we support tls-server-end-point between
JDBC and a server using new-ish OpenSSL. We don't support any channel
binding between for example JDBC and a server on CentOS 6. But that's
not a regression, it's just not there.

As Heikki was saying, the proposed patch seems to tread into the
portability problem territory that caused the previous attempt to fail
and had to be reverted. I am not that interested in trying that again
without new insights. I don't think we are going to do ourselves a
favor if we start meddling with that again. There are dozens of OpenSSL
variants out there, and the version history is nonlinear.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#8Michael Paquier
michael@paquier.xyz
In reply to: Steven Fackler (#7)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On Wed, Jun 06, 2018 at 01:16:11PM -0700, Steven Fackler wrote:

Thanks for the pointers, Steven. You should avoid top-posting on this
list, this is not the style used on the Postgres lists.

TLS 1.3, (which is currently in a draft state, but is theoretically being
finalized soon) does not support the TLS channel binding algorithms [1].
From talking with one of the people working on the TLS 1.3 standard,
tls-unique is seen as particularly problematic. There's some discussion on
the IETF mailing lists from a couple of years ago [2].

Well, it seems that we are going this API to decide if an SSL
implementation supports channel binding or not then:
/messages/by-id/20180122072936.GB1772@paquier.xyz
And for TLS 1.3 it would need to use SSL_get_version() or such with the
connection context.

Ignoring that line of the draft, the current tls-unique implementation in
Postgres is currently incorrect for TLS 1.3 handshakes anyway since the
server sends the first Finished message rather than the client [3].

Does this mean that tls-server-end-point goes into unsupported mode?
The emails you mention (thanks!), talk about only tls-unique while the
RFCs mention all channel binding types.

This is also the case for TLS 1.2 handshakes with session resumption [4].

Please let me think about this one, I am not completely sure yet what
that would mean for libpq and the backend code.
--
Michael

#9Steven Fackler
sfackler@gmail.com
In reply to: Michael Paquier (#8)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On Wed, Jun 6, 2018 at 2:21 PM Michael Paquier <michael@paquier.xyz> wrote:

Thanks for the pointers, Steven. You should avoid top-posting on this

list, this is not the style used on the Postgres lists.

Ah sorry about that! Hopefully this looks better.

Does this mean that tls-server-end-point goes into unsupported mode?
The emails you mention (thanks!), talk about only tls-unique while the
RFCs mention all channel binding types.

That's the part that I'm unsure about - tls-server-end-point doesn't seem
particularly objectionable. I asked for some clarification from the person
that I was talking to earlier.

Please let me think about this one, I am not completely sure yet what
that would mean for libpq and the backend code.

On the backend, you can use SSL_session_reused to check if a session was
resumed, and then use SSL_get_peer_finished if it wasn't and
SSL_get_finished if it was. The libpq frontend library doesn't need to
worry about it since it never attempts to reuse sessions.

Steven

#10Michael Paquier
michael@paquier.xyz
In reply to: Alvaro Herrera (#4)
1 attachment(s)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On Wed, Jun 06, 2018 at 12:37:00PM -0400, Alvaro Herrera wrote:

On 2018-May-29, Michael Paquier wrote:
If SCRAM channel binding is an important aspect to security, and the
older OpenSSL versions will still be around in servers for some time
yet, it seems like it behooves us to go the extra mile and provide an
implementation that works with such existing servers. Looking at
yum.postgresql.org, we seem to offer Postgres 11 packages for RHEL 6,
which appears to have openssl 1.0.0.

Or 1.0.1 as Tom has mentioned.

Anyway, even if I'm wrong, this thread has stalled. I hereby sprinkle
this thread with magic RMT dust for it to get fixed soon.

I am still not completely sure what is the correct course of action
here. Heikki and Peter and not much in favor of adding more complexity
here as OpenSSL has a long history of having a non-linear history across
platforms. On the other side, PGDG provides packages down to RHEL6, and
there are surely servers which use it as backend.

I also had a look at how to report the version of OpenSSL when compiling
with MSVC or ./configure. For MSVC, it could be possible to do
something like the attached as openssl is a command available and
Windows installers of OpenSSL usually have the command. One potential
problem is that dll can be either installed in the generic Windows path
where all DLLs are or into the specific path defined at installation as
a choice. Hence this should be more defensive and only trigger if the
executable can be found. Please consider this as just a draft patch
(this generates a warning as well if OPENSSL_CONF is not defined).

For *nix platforms, we could have an m4 macro which calls
OpenSSL_version_num() to get the version number and then
OpenSSL_version(int t) which provides a nice version string. My m4-foo
is not that advanced, but that looks doable.
--
Michael

Attachments:

msvc-openssl-version.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 4543d87d83..ff5c1f9c54 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -92,6 +92,14 @@ sub mkvcbuild
 
 	$solution = CreateSolution($vsVersion, $config);
 
+	# Extract library-specific versions this build is linking to.
+	if ($solution->{options}->{openssl})
+	{
+		my $openssl_bin = "$solution->{options}->{openssl}/bin/openssl.exe";
+		printf("Version of OpenSSL used: ");
+		system("$openssl_bin version");
+	}
+
 	our @pgportfiles = qw(
 	  chklocale.c crypt.c fls.c fseeko.c getrusage.c inet_aton.c random.c
 	  srandom.c getaddrinfo.c gettimeofday.c inet_net_ntop.c kill.c open.c
#11Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Steven Fackler (#7)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On 6/6/18 16:16, Steven Fackler wrote:

TLS 1.3, (which is currently in a draft state, but is theoretically
being finalized soon) does not support the TLS channel binding
algorithms [1]. From talking with one of the people working on the TLS
1.3 standard, tls-unique is seen as particularly problematic. There's
some discussion on the IETF mailing lists from a couple of years ago [2].

I think we'll just have to wait for an updated RFC on channel bindings
for TLS 1.3.

Perhaps we should change PostgreSQL 11 to not advertise channel binding
when TLS 1.3 is used?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#12Michael Paquier
michael@paquier.xyz
In reply to: Peter Eisentraut (#11)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On Mon, Jun 11, 2018 at 10:47:23AM -0400, Peter Eisentraut wrote:

I think we'll just have to wait for an updated RFC on channel bindings
for TLS 1.3.

Perhaps we should change PostgreSQL 11 to not advertise channel binding
when TLS 1.3 is used?

Yeah, that's what we should do and I would vote for doing nothing as
long as we are not sure how the TLS is shaped at the end, as we could as
well be able to use only be-tls-end-point so -PLUS can be advertised.

From a technical point of view, the decision-making can happen with
Port->ssl->version by looking for TLS1_3_VERSION which is new as of
OpenSSL 1.1.1 so that's very fresh (1.1.1 beta 5 is out as of today).
--
Michael

#13Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#10)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On Sat, Jun 09, 2018 at 09:28:17AM +0900, Michael Paquier wrote:

I am still not completely sure what is the correct course of action
here. Heikki and Peter and not much in favor of adding more complexity
here as OpenSSL has a long history of having a non-linear history across
platforms. On the other side, PGDG provides packages down to RHEL6, and
there are surely servers which use it as backend.

As Peter and Heikki have worked as well on all those features with me,
are there any objection to discard this open item? I looked again at
the patch this morning and it is true that OpenSSL's history makes
things harder, so keeping code consistent and simple with their last LTS
version is appealing.
--
Michael

#14Bruce Momjian
bruce@momjian.us
In reply to: Steven Fackler (#7)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On Wed, Jun 6, 2018 at 01:16:11PM -0700, Steven Fackler wrote:

TLS 1.3, (which is currently in a draft state, but is theoretically being
finalized soon) does not support the TLS channel binding algorithms [1]. From

Uh, according to this article, TLS 1.3 was finalized in March:

https://www.theregister.co.uk/2018/03/27/with_tls_13_signed_off_its_implementation_time/

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +
#15Dave Cramer
pg@fastcrypt.com
In reply to: Michael Paquier (#3)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On 29 May 2018 at 22:48, Michael Paquier <michael@paquier.xyz> wrote:

On Tue, May 29, 2018 at 10:33:03PM -0400, Heikki Linnakangas wrote:

Hmm. I think Peter went through this in commits ac3ff8b1d8 and

054e8c6cdb.

If you got that working now, I suppose we could do that, but I'm actually
inclined to just stick to the current, more straightforward code, and
require OpenSSL 1.0.2 for this feature. OpenSSL 1.0.2 has been around for
several years now. It's not available on all the popular platforms and
distributions yet, but I don't want to bend over backwards to support

those.

I think that this mainly boils down to how much Postgres JDBC wants to
get support here as some vendors can maintain oldest versions of OpenSSL
for a long time. The extra code is not that much complicated by the
way, still it is true that HEAD is cleaner with its simplicity.

I'm unclear what this has to do with JDBC ? JDBC doesn't use OpenSSL

Alvaro ?

Dave Cramer

davec@postgresintl.com
www.postgresintl.com

#16Alvaro Hernandez
aht@ongres.com
In reply to: Dave Cramer (#15)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On 24/06/18 18:49, Dave Cramer wrote:

On 29 May 2018 at 22:48, Michael Paquier <michael@paquier.xyz
<mailto:michael@paquier.xyz>> wrote:

On Tue, May 29, 2018 at 10:33:03PM -0400, Heikki Linnakangas wrote:

Hmm. I think Peter went through this in commits ac3ff8b1d8 and

054e8c6cdb.

If you got that working now, I suppose we could do that, but I'm

actually

inclined to just stick to the current, more straightforward

code, and

require OpenSSL 1.0.2 for this feature. OpenSSL 1.0.2 has been

around for

several years now. It's not available on all the popular

platforms and

distributions yet, but I don't want to bend over backwards to

support those.

I think that this mainly boils down to how much Postgres JDBC wants to
get support here as some vendors can maintain oldest versions of
OpenSSL
for a long time.  The extra code is not that much complicated by the
way, still it is true that HEAD is cleaner with its simplicity.

I'm unclear what this has to do with JDBC ? JDBC doesn't use OpenSSL

Alvaro ?

    It's only indirectly related. It does matter on what servers JDBC
would be able to connect to (using SCRAM + channel binding). Only those
with tls-server-end-point will be able to use CB with JDBC, and that is,
as of today, only OpenSSL 1.0.2 or higher, which is not available on
some older distributions.

    Álvaro

--

Alvaro Hernandez

-----------
OnGres

#17Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#13)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On Tue, Jun 19, 2018 at 09:19:40AM +0900, Michael Paquier wrote:

As Peter and Heikki have worked as well on all those features with me,
are there any objection to discard this open item? I looked again at
the patch this morning and it is true that OpenSSL's history makes
things harder, so keeping code consistent and simple with their last LTS
version is appealing.

Okay, in consequence, I have moved this open item to the non-bugs
section.
--
Michael

#18Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Bruce Momjian (#14)
Re: Supporting tls-server-end-point as SCRAM channel binding for OpenSSL 1.0.0 and 1.0.1

On 6/23/18 17:09, Bruce Momjian wrote:

On Wed, Jun 6, 2018 at 01:16:11PM -0700, Steven Fackler wrote:

TLS 1.3, (which is currently in a draft state, but is theoretically being
finalized soon) does not support the TLS channel binding algorithms [1]. From

Uh, according to this article, TLS 1.3 was finalized in March:

https://www.theregister.co.uk/2018/03/27/with_tls_13_signed_off_its_implementation_time/

More generally, is our TLS 1.3 support sound? For instance, I've read
about new cipher suites, so one question is, do the existing
configuration settings that control such things even still work?

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services