Fix handling of SCRAM-SHA-256's channel binding with RSA-PSS certificates
authorMichael Paquier
Wed, 15 Feb 2023 01:12:33 +0000 (10:12 +0900)
committerMichael Paquier
Wed, 15 Feb 2023 01:12:33 +0000 (10:12 +0900)
OpenSSL 1.1.1 and newer versions have added support for RSA-PSS
certificates, which requires the use of a specific routine in OpenSSL to
determine which hash function to use when compiling it when using
channel binding in SCRAM-SHA-256.  X509_get_signature_nid(), that is the
original routine the channel binding code has relied on, is not able to
determine which hash algorithm to use for such certificates.  However,
X509_get_signature_info(), new to OpenSSL 1.1.1, is able to do it.  This
commit switches the channel binding logic to rely on
X509_get_signature_info() over X509_get_signature_nid(), which would be
the choice when building with 1.1.1 or newer.

The error could have been triggered on the client or the server, hence
libpq and the backend need to have their related code paths patched.
Note that attempting to load an RSA-PSS certificate with OpenSSL 1.1.0
or older leads to a failure due to an unsupported algorithm.

The discovery of relying on X509_get_signature_info() comes from Jacob,
the tests have been written by Heikki (with few tweaks from me), while I
have bundled the whole together while adding the bits needed for MSVC
and meson.

This issue exists since channel binding exists, so backpatch all the way
down.  Some tests are added in 15~, triggered if compiling with OpenSSL
1.1.1 or newer, where the certificate and key files can easily be
generated for RSA-PSS.

Reported-by: Gunnar "Nick" Bluth
Author: Jacob Champion, Heikki Linnakangas
Discussion: https://postgr.es/m/17760-b6c61e752ec07060@postgresql.org
Backpatch-through: 11

configure
configure.ac
src/backend/libpq/be-secure-openssl.c
src/include/libpq/libpq-be.h
src/include/pg_config.h.in
src/interfaces/libpq/fe-secure-openssl.c
src/interfaces/libpq/libpq-int.h
src/tools/msvc/Solution.pm

index abaebc0e3ed954382be122b15f488d8a55f58e17..23322cf84a04ecbcea80c7bd6cb47e2afa51af20 100755 (executable)
--- a/configure
+++ b/configure
@@ -13092,6 +13092,18 @@ if test "x$ac_cv_func_CRYPTO_lock" = xyes; then :
 #define HAVE_CRYPTO_LOCK 1
 _ACEOF
 
+fi
+done
+
+  # Function introduced in OpenSSL 1.1.1.
+  for ac_func in X509_get_signature_info
+do :
+  ac_fn_c_check_func "$LINENO" "X509_get_signature_info" "ac_cv_func_X509_get_signature_info"
+if test "x$ac_cv_func_X509_get_signature_info" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE_X509_GET_SIGNATURE_INFO 1
+_ACEOF
+
 fi
 done
 
index 350a1d4842a1c70e63ae543d38d5b1b145021ec9..63549491b1edc0707298e86dec12270b02fb3799 100644 (file)
@@ -1316,6 +1316,8 @@ if test "$with_ssl" = openssl ; then
   # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
   # function was removed.
   AC_CHECK_FUNCS([CRYPTO_lock])
+  # Function introduced in OpenSSL 1.1.1.
+  AC_CHECK_FUNCS([X509_get_signature_info])
   AC_DEFINE([USE_OPENSSL], 1, [Define to 1 to build with OpenSSL support. (--with-ssl=openssl)])
 elif test "$with_ssl" != no ; then
   AC_MSG_ERROR([--with-ssl must specify openssl])
index e3b02b12f93b8aa0aa4fe7a3899904283f65c741..13ac9614422fd73747e8eebe9a13113907ed796f 100644 (file)
@@ -1313,7 +1313,7 @@ be_tls_get_peer_serial(Port *port, char *ptr, size_t len)
        ptr[0] = '\0';
 }
 
-#ifdef HAVE_X509_GET_SIGNATURE_NID
+#if defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO)
 char *
 be_tls_get_certificate_hash(Port *port, size_t *len)
 {
@@ -1331,10 +1331,15 @@ 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.
+    * algorithm to use for the result.  Prefer X509_get_signature_info(),
+    * introduced in OpenSSL 1.1.1, which can handle RSA-PSS signatures.
     */
+#if HAVE_X509_GET_SIGNATURE_INFO
+   if (!X509_get_signature_info(server_cert, &algo_nid, NULL, NULL, NULL))
+#else
    if (!OBJ_find_sigid_algs(X509_get_signature_nid(server_cert),
                             &algo_nid, NULL))
+#endif
        elog(ERROR, "could not determine server certificate signature algorithm");
 
    /*
index 02015efe13c20afbbf6b74bfb1e589ccf94d6d94..4acb1cda6ea5c2357dec36916f9beee2b7097182 100644 (file)
@@ -301,7 +301,7 @@ extern void be_tls_get_peer_serial(Port *port, char *ptr, size_t len);
  * This is not supported with old versions of OpenSSL that don't have
  * the X509_get_signature_nid() function.
  */
-#if defined(USE_OPENSSL) && defined(HAVE_X509_GET_SIGNATURE_NID)
+#if defined(USE_OPENSSL) && (defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO))
 #define HAVE_BE_TLS_GET_CERTIFICATE_HASH
 extern char *be_tls_get_certificate_hash(Port *port, size_t *len);
 #endif
index 06fcfb87f61453cd63cb66442d543ca9cb3d8cf4..40d513c1288baecc69af6be5a8c5683a33c40638 100644 (file)
 /* Define to 1 if you have the `writev' function. */
 #undef HAVE_WRITEV
 
+/* Define to 1 if you have the `X509_get_signature_info' function. */
+#undef HAVE_X509_GET_SIGNATURE_INFO
+
 /* Define to 1 if you have the `X509_get_signature_nid' function. */
 #undef HAVE_X509_GET_SIGNATURE_NID
 
index 5e59c9cc99731408c34e74280a727683cfd9608f..7f27767da6a015c8711138a63e757be67d00e5e1 100644 (file)
@@ -371,7 +371,7 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
    return n;
 }
 
-#ifdef HAVE_X509_GET_SIGNATURE_NID
+#if defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO)
 char *
 pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
 {
@@ -391,10 +391,15 @@ 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.
+    * algorithm to use for the result.  Prefer X509_get_signature_info(),
+    * introduced in OpenSSL 1.1.1, which can handle RSA-PSS signatures.
     */
+#if HAVE_X509_GET_SIGNATURE_INFO
+   if (!X509_get_signature_info(peer_cert, &algo_nid, NULL, NULL, NULL))
+#else
    if (!OBJ_find_sigid_algs(X509_get_signature_nid(peer_cert),
                             &algo_nid, NULL))
+#endif
    {
        appendPQExpBufferStr(&conn->errorMessage,
                             libpq_gettext("could not determine server certificate signature algorithm\n"));
index df2f17721cc478e051e3867ceab0c35c4156c101..913d4803efea05ce567e36e17fe74a4b68c07ece 100644 (file)
@@ -787,7 +787,7 @@ extern ssize_t pgtls_write(PGconn *conn, const void *ptr, size_t len);
  * This is not supported with old versions of OpenSSL that don't have
  * the X509_get_signature_nid() function.
  */
-#if defined(USE_OPENSSL) && defined(HAVE_X509_GET_SIGNATURE_NID)
+#if defined(USE_OPENSSL) && (defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO))
 #define HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
 extern char *pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len);
 #endif
index ad3d6a9075cfdc663432f9a8216fa71336005ee3..577b5afea72053dacd27dbfadce0aa3ed593cd02 100644 (file)
@@ -434,6 +434,7 @@ sub GenerateFiles
        HAVE_WCTYPE_H                            => 1,
        HAVE_WRITEV                              => undef,
        HAVE_X509_GET_SIGNATURE_NID              => 1,
+       HAVE_X509_GET_SIGNATURE_INFO             => undef,
        HAVE_X86_64_POPCNTQ                      => undef,
        HAVE__BOOL                               => undef,
        HAVE__BUILTIN_BSWAP16                    => undef,
@@ -549,7 +550,14 @@ sub GenerateFiles
 
        my ($digit1, $digit2, $digit3) = $self->GetOpenSSLVersion();
 
-       # More symbols are needed with OpenSSL 1.1.0 and above.
+       # Symbols needed with OpenSSL 1.1.1 and above.
+       if (   ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
+           || ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '1'))
+       {
+           $define{HAVE_X509_GET_SIGNATURE_INFO} = 1;
+       }
+
+       # Symbols needed with OpenSSL 1.1.0 and above.
        if (   ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
            || ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '0'))
        {