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
From 93383be85358a1ee03f34f34de45058e238964d1 Mon Sep 17 00:00:00 2001
From: Steven Fackler <sfack...@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

From 169328694b20ae836e2277cd7b9c7f6dce03fb94 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@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

Attachment: signature.asc
Description: PGP signature

Reply via email to