Fix handling of SCRAM-SHA-256's channel binding with RSA-PSS certificates
authorMichael Paquier <michael@paquier.xyz>
Wed, 15 Feb 2023 01:12:16 +0000 (10:12 +0900)
committerMichael Paquier <michael@paquier.xyz>
Wed, 15 Feb 2023 01:12:16 +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

15 files changed:
configure
configure.ac
meson.build
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/test/ssl/README
src/test/ssl/conf/server-rsapss.config [new file with mode: 0644]
src/test/ssl/ssl/server-rsapss.crt [new file with mode: 0644]
src/test/ssl/ssl/server-rsapss.key [new file with mode: 0644]
src/test/ssl/sslfiles.mk
src/test/ssl/t/002_scram.pl
src/tools/msvc/Solution.pm

index 5d07fd0bb914df43de2992ddcd3760200743e4e3..7bb829ddf43d00a5e1c475031901c21252eecd0c 100755 (executable)
--- a/configure
+++ b/configure
@@ -13013,6 +13013,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 e9b74ced6cae7c5617b832abcc882ff8fb700221..137a40a9425db0756af7c80b1c1d1166bcbcf6f0 100644 (file)
@@ -1385,6 +1385,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 e379a252a519c39244ccbbc3f56de4685aa71f0d..b5daed9f38cadb16a39a3d36e6544625fd78ddba 100644 (file)
@@ -1223,6 +1223,9 @@ if get_option('ssl') == 'openssl'
     # thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
     # function was removed.
     ['CRYPTO_lock'],
+
+    # Function introduced in OpenSSL 1.1.1
+    ['X509_get_signature_info'],
   ]
 
   foreach c : check_funcs
index e3c7c12aa0efea43f7dda8359fa7dc014ddff0cc..b3747f4fd8713e458e70563a6ee7cced94666f7e 100644 (file)
@@ -1429,7 +1429,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)
 {
@@ -1447,10 +1447,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 8c70b2fd5be3bccecdd16c78277ca4e2455e8f38..ac6407e9f61ec15e1797c84573e6cdd1c8220c58 100644 (file)
@@ -308,7 +308,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 c5a80b829e70395951b633752c479bea9dad49f9..2490bf8ace92f333e2bf703f0cbb4c377bc72389 100644 (file)
 /* Define to 1 if you have the `wcstombs_l' function. */
 #undef HAVE_WCSTOMBS_L
 
+/* 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 983536de2516f94b1aaee2c225e968dd6df142e4..6a4431ddfe99395a4771c26187a388b5e67bf40a 100644 (file)
@@ -364,7 +364,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)
 {
@@ -384,10 +384,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
        {
                libpq_append_conn_error(conn, "could not determine server certificate signature algorithm");
                return NULL;
index d94b648ea5b957413acd06040d18137c0522cd5b..d7ec5ed4293e3f366537a197e8fbb628e2206d06 100644 (file)
@@ -806,7 +806,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 b328203c7c25b1a962a675cb4d7d148df7e83d21..2101a466d22b7330916b6153dfaefe0a1baa8f74 100644 (file)
@@ -92,6 +92,7 @@ ssl/ subdirectory. The Makefile also contains a rule, "make sslfiles", to
 recreate them if you need to make changes. "make sslfiles-clean" is required
 in order to recreate the full set of keypairs and certificates. To rebuild
 separate files, touch (or remove) the files in question and run "make sslfiles".
+This step requires at least OpenSSL 1.1.1.
 
 Note
 ====
diff --git a/src/test/ssl/conf/server-rsapss.config b/src/test/ssl/conf/server-rsapss.config
new file mode 100644 (file)
index 0000000..391f9b8
--- /dev/null
@@ -0,0 +1,14 @@
+# An OpenSSL format CSR config file for creating a server certificate.
+#
+# This is identical to server-cn-only certificate, but we specify
+# RSA-PSS as the algorithm on the command line.
+
+[ req ]
+distinguished_name     = req_distinguished_name
+prompt                 = no
+
+[ req_distinguished_name ]
+CN = common-name.pg-ssltest.test
+OU = PostgreSQL test suite
+
+# No Subject Alternative Names
\ No newline at end of file
diff --git a/src/test/ssl/ssl/server-rsapss.crt b/src/test/ssl/ssl/server-rsapss.crt
new file mode 100644 (file)
index 0000000..1c35956
--- /dev/null
@@ -0,0 +1,21 @@
+-----BEGIN CERTIFICATE-----
+MIIDezCCAi4CFCrZutHsw0Vl3OCgOmvtL0I/XAZyMEIGCSqGSIb3DQEBCjA1oA8w
+DQYJYIZIAWUDBAIBBQChHDAaBgkqhkiG9w0BAQgwDQYJYIZIAWUDBAIBBQCiBAIC
+AN4wRjEkMCIGA1UEAwwbY29tbW9uLW5hbWUucGctc3NsdGVzdC50ZXN0MR4wHAYD
+VQQLDBVQb3N0Z3JlU1FMIHRlc3Qgc3VpdGUwHhcNMjMwMjEzMDEyMjA2WhcNMjMw
+MzE1MDEyMjA2WjBGMSQwIgYDVQQDDBtjb21tb24tbmFtZS5wZy1zc2x0ZXN0LnRl
+c3QxHjAcBgNVBAsMFVBvc3RncmVTUUwgdGVzdCBzdWl0ZTCCASAwCwYJKoZIhvcN
+AQEKA4IBDwAwggEKAoIBAQC6YtrZZukJ4n31gKpcIOl65D9roe2jzcIBX1AZq1fR
+I6qmt7aR0iFCKEy9D2fs6lM+NVQSurg7b0gKL+XoOadySAxALIrUwcCQM7rZvUR0
+aKo3Qm0U00ir4x0i73/sTpY25zBSFoqGldmlqiIIWxpe8hqZEc6Sc78Bs2FaAa9A
+5sTLaX5nG6jyreJweLcmv+TYFVqxNq7Y7tC67zWXr6r49JBkSHSibzBr/uFxOGsP
+B9hwGo4/foACjeDNAT0vjwMLnV19Sd2zf9daBo+sd9bCj2C5CpOyXxFtO7cMh0tP
+U3ZqcYPViFxcPObmhnJgqlBbgZD/WLxm1aFgUYjqMQ47AgMBAAEwQgYJKoZIhvcN
+AQEKMDWgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQME
+AgEFAKIEAgIA3gOCAQEAQpYu7fz9iz8CplCOp4SJ1eO9UjbtdxzvuaVR751TfYrX
+OO19jq7YyWgqJDwROnDJBFEy9B+HaXTfscEHpGIHAIpx7S7az/gLnO90HshXcK+/
+CbjW9axRB9TrD2zOrISl9NSuEZ5tbd5/Ml2yzY85CCjYPuNy+euH5XgcXcwF3Q49
+G5eDJnaCCYzwdEOZY8ris9o9go8aL6zNAfhUKToRUfeoBCStOLZSgb6d/IKRB9eg
+M0FImsMI3j5zHCiH0HhMwCRFRuZqTp1EMBHANIJncTZSGWQyKQ71zO/l/3YzwNfm
+c2gyeh0DJWFkEZD3spWs8K6UEoTESP6Ivj47LmnWjg==
+-----END CERTIFICATE-----
diff --git a/src/test/ssl/ssl/server-rsapss.key b/src/test/ssl/ssl/server-rsapss.key
new file mode 100644 (file)
index 0000000..a5bc297
--- /dev/null
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvQIBADALBgkqhkiG9w0BAQoEggSpMIIEpQIBAAKCAQEAumLa2WbpCeJ99YCq
+XCDpeuQ/a6Hto83CAV9QGatX0SOqpre2kdIhQihMvQ9n7OpTPjVUErq4O29ICi/l
+6DmnckgMQCyK1MHAkDO62b1EdGiqN0JtFNNIq+MdIu9/7E6WNucwUhaKhpXZpaoi
+CFsaXvIamRHOknO/AbNhWgGvQObEy2l+Zxuo8q3icHi3Jr/k2BVasTau2O7Quu81
+l6+q+PSQZEh0om8wa/7hcThrDwfYcBqOP36AAo3gzQE9L48DC51dfUnds3/XWgaP
+rHfWwo9guQqTsl8RbTu3DIdLT1N2anGD1YhcXDzm5oZyYKpQW4GQ/1i8ZtWhYFGI
+6jEOOwIDAQABAoIBAAPXZpi55PdieTXUQpxPxDJpx01p4IdAKoRzS3EwkP99d/sR
+qNCekaUyIW9UqT2Hx2Tb1MzCBUZQ40I1614fehK5C2sFdtnls8/gdaIe7FqwIYxA
+lcxhpvjHX2Ht8gLc8OvpC5vDOJkZymZsHM8qa8zcTD/AzzNBOpdHqwdES58YoqEb
+5LOVLBRIoLli2eAWrrnoYl7MQuh3CHHtWGjn3drTzg6Tl2umfNhTMFANZssNexl4
+6npPHBASdevWWsqB8GXD56PaqWxxnjtwzk06lRbloSQYJOicI8OK7eaySpRuHpZV
+3vJKhY3bcRN6joxveXA7jaAPSBvNXp2w5fQ1b2ECgYEA1mzqOCln87aaLzZ1KlWL
+QfxcXmcke1lJgbhW+iEh6iht2OmBlntAlIVv/D3yBDhNrHdrNlUcWvm+VSrbVyxn
+6e1RWHAGPzZNhpcg4odxdI6Oton/OBtsEQ7A6UJ6S7bPTVGVwi9fA4fI0Pfne0wV
+IeJHvjDZboOBi6TF2thcJ2sCgYEA3oYzAt4tEiA+nQyNnP4nWZ17XONA6H8yVeUY
+Sk6eczg8eGAQz9afVtbSI3uRIfQbQ1+mjaUl4pVej2UDXcROpYHgwCLJRBBDbzzB
+4IcPh2woFGZOScQu9Q64C8g6MH4zm3WkFvXyJF3j3dHGFZGq8nmwEARJgAsQ6Yig
+kYL8+HECgYEAtuKUbqxaPlL7dNNU4XOu3+v3eIkuY4qHGH36qUKDI62x6zVWUtvy
++/pHxnOrLRA8p6H/LosvMSUbwpZYGCUGyE2iePSrT1TokKfr42o0SX6hmG1g4iD5
+bh8QSKNrnZJhg4fXXJV8y40PqbQXmmENESZnnH8bpJfDcTBrlLm+99sCgYEA3F1f
+xPZLAglGmHZnA1K5m0iWc01l6RiVu3RNksC6r3XAhKD15S0wzGme3p6vAkXgfd8K
+bHlgxDuR0kWBiOkvzT2KWhvY3vuQHGe5w+VcnoqgQltyKiELM4mo/5oA7ib8anac
+0lQrwJHuZ6wnExMXjFqv3ZyxQQk0bWDtSkzCwjECgYEAusqqCAmryRFWdOif2z+Z
+3vfseSvBdQMj2FO7weqCVPV4Gnae0TO7A1bUpVX/pfkDEPitt5oUgS2KTozW5vwz
+yaQTSB8RO8EG66GURZvPs3Cerkyrgk/OMmbCv3B0ALwhPMBqpemJqeBOuyaAjY8W
+Tqb6E2ofRlYND0xH83gCTig=
+-----END PRIVATE KEY-----
index 5d9dc09a4b00c16e4be7003d53c5daf04aff3e6c..e63342469d3a5c4bbde63ce3ce0372175a0322c9 100644 (file)
@@ -37,13 +37,17 @@ CLIENTS := client client-dn client-revoked client_ext client-long \
        client-revoked-utf8
 
 #
-# To add a new non-standard key, add it to SPECIAL_KEYS and then add a recipe
-# for creating it to the "Special-case keys" section below.
+# To add a new non-standard certificate, add it to SPECIAL_CERTS and then add
+# a recipe for creating it to the "Special-case certificates" section below.
 #
+SPECIAL_CERTS := ssl/server-rsapss.crt
+
+# Likewise for non-standard keys
 SPECIAL_KEYS := ssl/server-password.key \
        ssl/client-der.key \
        ssl/client-encrypted-pem.key \
-       ssl/client-encrypted-der.key
+       ssl/client-encrypted-der.key \
+       ssl/server-rsapss.key
 
 #
 # These files are just concatenations of other files. You can add new ones to
@@ -66,7 +70,13 @@ CRLS := ssl/root.crl \
        ssl/client.crl \
        ssl/server.crl
 
-SSLFILES := $(STANDARD_CERTS) $(STANDARD_KEYS) $(SPECIAL_KEYS) $(COMBINATIONS) $(CRLS)
+SSLFILES := \
+       $(STANDARD_CERTS) \
+       $(STANDARD_KEYS) \
+       $(SPECIAL_CERTS) \
+       $(SPECIAL_KEYS) \
+       $(COMBINATIONS) \
+       $(CRLS)
 SSLDIRS := ssl/client-crldir \
        ssl/server-crldir \
        ssl/root+client-crldir \
@@ -86,6 +96,10 @@ sslfiles: $(SSLFILES) $(SSLDIRS)
 ssl/root_ca.crt: ssl/root_ca.key conf/root_ca.config
        $(OPENSSL) req -new -x509 -config conf/root_ca.config -days 10000 -key $< -out $@
 
+# Certificate using RSA-PSS algorithm. Also self-signed.
+ssl/server-rsapss.crt: ssl/server-rsapss.key conf/server-rsapss.config
+       $(OPENSSL) req -new -x509 -config conf/server-rsapss.config -key $< -out $@
+
 #
 # Special-case keys
 #
@@ -96,6 +110,10 @@ ssl/root_ca.crt: ssl/root_ca.key conf/root_ca.config
 ssl/server-password.key: ssl/server-cn-only.key
        $(OPENSSL) rsa -aes256 -in $< -out $@ -passout 'pass:secret1'
 
+# Key that uses the RSA-PSS algorithm
+ssl/server-rsapss.key:
+       $(OPENSSL) genpkey -algorithm rsa-pss -out $@
+
 # DER-encoded version of client.key
 ssl/client-der.key: ssl/client.key
        $(OPENSSL) rsa -in $< -outform DER -out $@
index 0f3d180cfa994edece3af5b6e7257ad1fa8b665d..1d3905d3a1eb67d1f5ca42bf721171b697f77809 100644 (file)
@@ -46,6 +46,10 @@ my $SERVERHOSTCIDR = '127.0.0.1/32';
 # Determine whether build supports tls-server-end-point.
 my $supports_tls_server_end_point =
   check_pg_config("#define HAVE_X509_GET_SIGNATURE_NID 1");
+# Determine whether build supports detection of hash algorithms for
+# RSA-PSS certificates.
+my $supports_rsapss_certs =
+  check_pg_config("#define HAVE_X509_GET_SIGNATURE_INFO 1");
 
 # Allocation of base connection string shared among multiple tests.
 my $common_connstr;
@@ -136,4 +140,17 @@ $node->connect_ok(
                qr/connection authenticated: identity="ssltestuser" method=scram-sha-256/
        ]);
 
+# Now test with a server certificate that uses the RSA-PSS algorithm.
+# This checks that the certificate can be loaded and that channel binding
+# works. (see bug #17760)
+if ($supports_rsapss_certs)
+{
+       switch_server_cert($node, certfile => 'server-rsapss');
+       $node->connect_ok(
+               "$common_connstr user=ssltestuser channel_binding=require",
+               "SCRAM with SSL and channel_binding=require, server certificate uses 'rsassaPss'",
+               log_like => [
+                       qr/connection authenticated: identity="ssltestuser" method=scram-sha-256/
+               ]);
+}
 done_testing();
index 355ea0f9e5c69a872819594f1fba00f2f1033c38..5399050492464ad0843a4441aec9b852694809be 100644 (file)
@@ -371,6 +371,7 @@ sub GenerateFiles
                HAVE_WCSTOMBS_L                          => 1,
                HAVE_VISIBILITY_ATTRIBUTE                => 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,
@@ -489,7 +490,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'))
                {