From 5fd61055eacf3d0c45be20b90402a87c9848db43 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Wed, 15 Feb 2023 10:12:31 +0900 Subject: [PATCH] Fix handling of SCRAM-SHA-256's channel binding with RSA-PSS certificates 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 | 12 ++++++++++ configure.ac | 2 ++ src/backend/libpq/be-secure-openssl.c | 9 ++++++-- src/include/libpq/libpq-be.h | 2 +- src/include/pg_config.h.in | 3 +++ src/interfaces/libpq/fe-secure-openssl.c | 9 ++++++-- src/interfaces/libpq/libpq-int.h | 2 +- src/test/ssl/README | 1 + src/test/ssl/conf/server-rsapss.config | 14 ++++++++++++ src/test/ssl/ssl/server-rsapss.crt | 21 ++++++++++++++++++ src/test/ssl/ssl/server-rsapss.key | 28 ++++++++++++++++++++++++ src/test/ssl/sslfiles.mk | 26 ++++++++++++++++++---- src/test/ssl/t/002_scram.pl | 17 ++++++++++++++ src/tools/msvc/Solution.pm | 10 ++++++++- 14 files changed, 145 insertions(+), 11 deletions(-) create mode 100644 src/test/ssl/conf/server-rsapss.config create mode 100644 src/test/ssl/ssl/server-rsapss.crt create mode 100644 src/test/ssl/ssl/server-rsapss.key diff --git a/configure b/configure index 02a6add8e0..6c87ca479a 100755 --- a/configure +++ b/configure @@ -13260,6 +13260,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 diff --git a/configure.ac b/configure.ac index 9a73f50c42..01968035f3 100644 --- a/configure.ac +++ b/configure.ac @@ -1352,6 +1352,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]) diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 3d0168a369..8f9b81c71a 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -1320,7 +1320,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) { @@ -1338,10 +1338,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"); /* diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 90c20da22b..351b4f4d97 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -300,7 +300,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 diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index cdd742cb55..d09e9f9a1c 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -718,6 +718,9 @@ /* 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 diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index fe1e98a3b0..af59ff49f7 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -380,7 +380,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) { @@ -400,10 +400,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")); diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index a27dd3785e..ac9042ffdc 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -804,7 +804,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 diff --git a/src/test/ssl/README b/src/test/ssl/README index 7e60700652..ff55697c0a 100644 --- a/src/test/ssl/README +++ b/src/test/ssl/README @@ -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. TODO ==== diff --git a/src/test/ssl/conf/server-rsapss.config b/src/test/ssl/conf/server-rsapss.config new file mode 100644 index 0000000000..391f9b8d89 --- /dev/null +++ b/src/test/ssl/conf/server-rsapss.config @@ -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 index 0000000000..1c35956d57 --- /dev/null +++ b/src/test/ssl/ssl/server-rsapss.crt @@ -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 index 0000000000..a5bc297f1d --- /dev/null +++ b/src/test/ssl/ssl/server-rsapss.key @@ -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----- diff --git a/src/test/ssl/sslfiles.mk b/src/test/ssl/sslfiles.mk index cc023667af..5d5e1376ad 100644 --- a/src/test/ssl/sslfiles.mk +++ b/src/test/ssl/sslfiles.mk @@ -36,13 +36,17 @@ SERVERS := server-cn-and-alt-names \ CLIENTS := client client-dn client-revoked client_ext # -# 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 @@ -65,7 +69,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 \ @@ -85,6 +95,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 # @@ -95,6 +109,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 $@ diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl index 588f47a39b..566cb1256b 100644 --- a/src/test/ssl/t/002_scram.pl +++ b/src/test/ssl/t/002_scram.pl @@ -42,6 +42,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; @@ -132,4 +136,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(); diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm index d30e8fcb11..790f03b05e 100644 --- a/src/tools/msvc/Solution.pm +++ b/src/tools/msvc/Solution.pm @@ -433,6 +433,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, @@ -553,7 +554,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')) {