Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9244c11

Browse files
committed
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
1 parent 87f21d2 commit 9244c11

15 files changed

+148
-11
lines changed

configure

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13013,6 +13013,18 @@ if test "x$ac_cv_func_CRYPTO_lock" = xyes; then :
1301313013
#define HAVE_CRYPTO_LOCK 1
1301413014
_ACEOF
1301513015

13016+
fi
13017+
done
13018+
13019+
# Function introduced in OpenSSL 1.1.1.
13020+
for ac_func in X509_get_signature_info
13021+
do :
13022+
ac_fn_c_check_func "$LINENO" "X509_get_signature_info" "ac_cv_func_X509_get_signature_info"
13023+
if test "x$ac_cv_func_X509_get_signature_info" = xyes; then :
13024+
cat >>confdefs.h <<_ACEOF
13025+
#define HAVE_X509_GET_SIGNATURE_INFO 1
13026+
_ACEOF
13027+
1301613028
fi
1301713029
done
1301813030

configure.ac

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,6 +1385,8 @@ if test "$with_ssl" = openssl ; then
13851385
# thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
13861386
# function was removed.
13871387
AC_CHECK_FUNCS([CRYPTO_lock])
1388+
# Function introduced in OpenSSL 1.1.1.
1389+
AC_CHECK_FUNCS([X509_get_signature_info])
13881390
AC_DEFINE([USE_OPENSSL], 1, [Define to 1 to build with OpenSSL support. (--with-ssl=openssl)])
13891391
elif test "$with_ssl" != no ; then
13901392
AC_MSG_ERROR([--with-ssl must specify openssl])

meson.build

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,6 +1223,9 @@ if get_option('ssl') == 'openssl'
12231223
# thread-safety. In 1.1.0, it's no longer required, and CRYPTO_lock()
12241224
# function was removed.
12251225
['CRYPTO_lock'],
1226+
1227+
# Function introduced in OpenSSL 1.1.1
1228+
['X509_get_signature_info'],
12261229
]
12271230

12281231
foreach c : check_funcs

src/backend/libpq/be-secure-openssl.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1429,7 +1429,7 @@ be_tls_get_peer_serial(Port *port, char *ptr, size_t len)
14291429
ptr[0] = '\0';
14301430
}
14311431

1432-
#ifdef HAVE_X509_GET_SIGNATURE_NID
1432+
#if defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO)
14331433
char *
14341434
be_tls_get_certificate_hash(Port *port, size_t *len)
14351435
{
@@ -1447,10 +1447,15 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
14471447

14481448
/*
14491449
* Get the signature algorithm of the certificate to determine the hash
1450-
* algorithm to use for the result.
1450+
* algorithm to use for the result. Prefer X509_get_signature_info(),
1451+
* introduced in OpenSSL 1.1.1, which can handle RSA-PSS signatures.
14511452
*/
1453+
#if HAVE_X509_GET_SIGNATURE_INFO
1454+
if (!X509_get_signature_info(server_cert, &algo_nid, NULL, NULL, NULL))
1455+
#else
14521456
if (!OBJ_find_sigid_algs(X509_get_signature_nid(server_cert),
14531457
&algo_nid, NULL))
1458+
#endif
14541459
elog(ERROR, "could not determine server certificate signature algorithm");
14551460

14561461
/*

src/include/libpq/libpq-be.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -308,7 +308,7 @@ extern void be_tls_get_peer_serial(Port *port, char *ptr, size_t len);
308308
* This is not supported with old versions of OpenSSL that don't have
309309
* the X509_get_signature_nid() function.
310310
*/
311-
#if defined(USE_OPENSSL) && defined(HAVE_X509_GET_SIGNATURE_NID)
311+
#if defined(USE_OPENSSL) && (defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO))
312312
#define HAVE_BE_TLS_GET_CERTIFICATE_HASH
313313
extern char *be_tls_get_certificate_hash(Port *port, size_t *len);
314314
#endif

src/include/pg_config.h.in

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,9 @@
523523
/* Define to 1 if you have the `wcstombs_l' function. */
524524
#undef HAVE_WCSTOMBS_L
525525

526+
/* Define to 1 if you have the `X509_get_signature_info' function. */
527+
#undef HAVE_X509_GET_SIGNATURE_INFO
528+
526529
/* Define to 1 if you have the `X509_get_signature_nid' function. */
527530
#undef HAVE_X509_GET_SIGNATURE_NID
528531

src/interfaces/libpq/fe-secure-openssl.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len)
364364
return n;
365365
}
366366

367-
#ifdef HAVE_X509_GET_SIGNATURE_NID
367+
#if defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO)
368368
char *
369369
pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
370370
{
@@ -384,10 +384,15 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
384384

385385
/*
386386
* Get the signature algorithm of the certificate to determine the hash
387-
* algorithm to use for the result.
387+
* algorithm to use for the result. Prefer X509_get_signature_info(),
388+
* introduced in OpenSSL 1.1.1, which can handle RSA-PSS signatures.
388389
*/
390+
#if HAVE_X509_GET_SIGNATURE_INFO
391+
if (!X509_get_signature_info(peer_cert, &algo_nid, NULL, NULL, NULL))
392+
#else
389393
if (!OBJ_find_sigid_algs(X509_get_signature_nid(peer_cert),
390394
&algo_nid, NULL))
395+
#endif
391396
{
392397
libpq_append_conn_error(conn, "could not determine server certificate signature algorithm");
393398
return NULL;

src/interfaces/libpq/libpq-int.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,7 @@ extern ssize_t pgtls_write(PGconn *conn, const void *ptr, size_t len);
806806
* This is not supported with old versions of OpenSSL that don't have
807807
* the X509_get_signature_nid() function.
808808
*/
809-
#if defined(USE_OPENSSL) && defined(HAVE_X509_GET_SIGNATURE_NID)
809+
#if defined(USE_OPENSSL) && (defined(HAVE_X509_GET_SIGNATURE_NID) || defined(HAVE_X509_GET_SIGNATURE_INFO))
810810
#define HAVE_PGTLS_GET_PEER_CERTIFICATE_HASH
811811
extern char *pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len);
812812
#endif

src/test/ssl/README

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ ssl/ subdirectory. The Makefile also contains a rule, "make sslfiles", to
9292
recreate them if you need to make changes. "make sslfiles-clean" is required
9393
in order to recreate the full set of keypairs and certificates. To rebuild
9494
separate files, touch (or remove) the files in question and run "make sslfiles".
95+
This step requires at least OpenSSL 1.1.1.
9596

9697
Note
9798
====
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# An OpenSSL format CSR config file for creating a server certificate.
2+
#
3+
# This is identical to server-cn-only certificate, but we specify
4+
# RSA-PSS as the algorithm on the command line.
5+
6+
[ req ]
7+
distinguished_name = req_distinguished_name
8+
prompt = no
9+
10+
[ req_distinguished_name ]
11+
CN = common-name.pg-ssltest.test
12+
OU = PostgreSQL test suite
13+
14+
# No Subject Alternative Names

src/test/ssl/ssl/server-rsapss.crt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
-----BEGIN CERTIFICATE-----
2+
MIIDezCCAi4CFCrZutHsw0Vl3OCgOmvtL0I/XAZyMEIGCSqGSIb3DQEBCjA1oA8w
3+
DQYJYIZIAWUDBAIBBQChHDAaBgkqhkiG9w0BAQgwDQYJYIZIAWUDBAIBBQCiBAIC
4+
AN4wRjEkMCIGA1UEAwwbY29tbW9uLW5hbWUucGctc3NsdGVzdC50ZXN0MR4wHAYD
5+
VQQLDBVQb3N0Z3JlU1FMIHRlc3Qgc3VpdGUwHhcNMjMwMjEzMDEyMjA2WhcNMjMw
6+
MzE1MDEyMjA2WjBGMSQwIgYDVQQDDBtjb21tb24tbmFtZS5wZy1zc2x0ZXN0LnRl
7+
c3QxHjAcBgNVBAsMFVBvc3RncmVTUUwgdGVzdCBzdWl0ZTCCASAwCwYJKoZIhvcN
8+
AQEKA4IBDwAwggEKAoIBAQC6YtrZZukJ4n31gKpcIOl65D9roe2jzcIBX1AZq1fR
9+
I6qmt7aR0iFCKEy9D2fs6lM+NVQSurg7b0gKL+XoOadySAxALIrUwcCQM7rZvUR0
10+
aKo3Qm0U00ir4x0i73/sTpY25zBSFoqGldmlqiIIWxpe8hqZEc6Sc78Bs2FaAa9A
11+
5sTLaX5nG6jyreJweLcmv+TYFVqxNq7Y7tC67zWXr6r49JBkSHSibzBr/uFxOGsP
12+
B9hwGo4/foACjeDNAT0vjwMLnV19Sd2zf9daBo+sd9bCj2C5CpOyXxFtO7cMh0tP
13+
U3ZqcYPViFxcPObmhnJgqlBbgZD/WLxm1aFgUYjqMQ47AgMBAAEwQgYJKoZIhvcN
14+
AQEKMDWgDzANBglghkgBZQMEAgEFAKEcMBoGCSqGSIb3DQEBCDANBglghkgBZQME
15+
AgEFAKIEAgIA3gOCAQEAQpYu7fz9iz8CplCOp4SJ1eO9UjbtdxzvuaVR751TfYrX
16+
OO19jq7YyWgqJDwROnDJBFEy9B+HaXTfscEHpGIHAIpx7S7az/gLnO90HshXcK+/
17+
CbjW9axRB9TrD2zOrISl9NSuEZ5tbd5/Ml2yzY85CCjYPuNy+euH5XgcXcwF3Q49
18+
G5eDJnaCCYzwdEOZY8ris9o9go8aL6zNAfhUKToRUfeoBCStOLZSgb6d/IKRB9eg
19+
M0FImsMI3j5zHCiH0HhMwCRFRuZqTp1EMBHANIJncTZSGWQyKQ71zO/l/3YzwNfm
20+
c2gyeh0DJWFkEZD3spWs8K6UEoTESP6Ivj47LmnWjg==
21+
-----END CERTIFICATE-----

src/test/ssl/ssl/server-rsapss.key

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
-----BEGIN PRIVATE KEY-----
2+
MIIEvQIBADALBgkqhkiG9w0BAQoEggSpMIIEpQIBAAKCAQEAumLa2WbpCeJ99YCq
3+
XCDpeuQ/a6Hto83CAV9QGatX0SOqpre2kdIhQihMvQ9n7OpTPjVUErq4O29ICi/l
4+
6DmnckgMQCyK1MHAkDO62b1EdGiqN0JtFNNIq+MdIu9/7E6WNucwUhaKhpXZpaoi
5+
CFsaXvIamRHOknO/AbNhWgGvQObEy2l+Zxuo8q3icHi3Jr/k2BVasTau2O7Quu81
6+
l6+q+PSQZEh0om8wa/7hcThrDwfYcBqOP36AAo3gzQE9L48DC51dfUnds3/XWgaP
7+
rHfWwo9guQqTsl8RbTu3DIdLT1N2anGD1YhcXDzm5oZyYKpQW4GQ/1i8ZtWhYFGI
8+
6jEOOwIDAQABAoIBAAPXZpi55PdieTXUQpxPxDJpx01p4IdAKoRzS3EwkP99d/sR
9+
qNCekaUyIW9UqT2Hx2Tb1MzCBUZQ40I1614fehK5C2sFdtnls8/gdaIe7FqwIYxA
10+
lcxhpvjHX2Ht8gLc8OvpC5vDOJkZymZsHM8qa8zcTD/AzzNBOpdHqwdES58YoqEb
11+
5LOVLBRIoLli2eAWrrnoYl7MQuh3CHHtWGjn3drTzg6Tl2umfNhTMFANZssNexl4
12+
6npPHBASdevWWsqB8GXD56PaqWxxnjtwzk06lRbloSQYJOicI8OK7eaySpRuHpZV
13+
3vJKhY3bcRN6joxveXA7jaAPSBvNXp2w5fQ1b2ECgYEA1mzqOCln87aaLzZ1KlWL
14+
QfxcXmcke1lJgbhW+iEh6iht2OmBlntAlIVv/D3yBDhNrHdrNlUcWvm+VSrbVyxn
15+
6e1RWHAGPzZNhpcg4odxdI6Oton/OBtsEQ7A6UJ6S7bPTVGVwi9fA4fI0Pfne0wV
16+
IeJHvjDZboOBi6TF2thcJ2sCgYEA3oYzAt4tEiA+nQyNnP4nWZ17XONA6H8yVeUY
17+
Sk6eczg8eGAQz9afVtbSI3uRIfQbQ1+mjaUl4pVej2UDXcROpYHgwCLJRBBDbzzB
18+
4IcPh2woFGZOScQu9Q64C8g6MH4zm3WkFvXyJF3j3dHGFZGq8nmwEARJgAsQ6Yig
19+
kYL8+HECgYEAtuKUbqxaPlL7dNNU4XOu3+v3eIkuY4qHGH36qUKDI62x6zVWUtvy
20+
+/pHxnOrLRA8p6H/LosvMSUbwpZYGCUGyE2iePSrT1TokKfr42o0SX6hmG1g4iD5
21+
bh8QSKNrnZJhg4fXXJV8y40PqbQXmmENESZnnH8bpJfDcTBrlLm+99sCgYEA3F1f
22+
xPZLAglGmHZnA1K5m0iWc01l6RiVu3RNksC6r3XAhKD15S0wzGme3p6vAkXgfd8K
23+
bHlgxDuR0kWBiOkvzT2KWhvY3vuQHGe5w+VcnoqgQltyKiELM4mo/5oA7ib8anac
24+
0lQrwJHuZ6wnExMXjFqv3ZyxQQk0bWDtSkzCwjECgYEAusqqCAmryRFWdOif2z+Z
25+
3vfseSvBdQMj2FO7weqCVPV4Gnae0TO7A1bUpVX/pfkDEPitt5oUgS2KTozW5vwz
26+
yaQTSB8RO8EG66GURZvPs3Cerkyrgk/OMmbCv3B0ALwhPMBqpemJqeBOuyaAjY8W
27+
Tqb6E2ofRlYND0xH83gCTig=
28+
-----END PRIVATE KEY-----

src/test/ssl/sslfiles.mk

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,17 @@ CLIENTS := client client-dn client-revoked client_ext client-long \
3737
client-revoked-utf8
3838

3939
#
40-
# To add a new non-standard key, add it to SPECIAL_KEYS and then add a recipe
41-
# for creating it to the "Special-case keys" section below.
40+
# To add a new non-standard certificate, add it to SPECIAL_CERTS and then add
41+
# a recipe for creating it to the "Special-case certificates" section below.
4242
#
43+
SPECIAL_CERTS := ssl/server-rsapss.crt
44+
45+
# Likewise for non-standard keys
4346
SPECIAL_KEYS := ssl/server-password.key \
4447
ssl/client-der.key \
4548
ssl/client-encrypted-pem.key \
46-
ssl/client-encrypted-der.key
49+
ssl/client-encrypted-der.key \
50+
ssl/server-rsapss.key
4751

4852
#
4953
# These files are just concatenations of other files. You can add new ones to
@@ -66,7 +70,13 @@ CRLS := ssl/root.crl \
6670
ssl/client.crl \
6771
ssl/server.crl
6872

69-
SSLFILES := $(STANDARD_CERTS) $(STANDARD_KEYS) $(SPECIAL_KEYS) $(COMBINATIONS) $(CRLS)
73+
SSLFILES := \
74+
$(STANDARD_CERTS) \
75+
$(STANDARD_KEYS) \
76+
$(SPECIAL_CERTS) \
77+
$(SPECIAL_KEYS) \
78+
$(COMBINATIONS) \
79+
$(CRLS)
7080
SSLDIRS := ssl/client-crldir \
7181
ssl/server-crldir \
7282
ssl/root+client-crldir \
@@ -86,6 +96,10 @@ sslfiles: $(SSLFILES) $(SSLDIRS)
8696
ssl/root_ca.crt: ssl/root_ca.key conf/root_ca.config
8797
$(OPENSSL) req -new -x509 -config conf/root_ca.config -days 10000 -key $< -out $@
8898

99+
# Certificate using RSA-PSS algorithm. Also self-signed.
100+
ssl/server-rsapss.crt: ssl/server-rsapss.key conf/server-rsapss.config
101+
$(OPENSSL) req -new -x509 -config conf/server-rsapss.config -key $< -out $@
102+
89103
#
90104
# Special-case keys
91105
#
@@ -96,6 +110,10 @@ ssl/root_ca.crt: ssl/root_ca.key conf/root_ca.config
96110
ssl/server-password.key: ssl/server-cn-only.key
97111
$(OPENSSL) rsa -aes256 -in $< -out $@ -passout 'pass:secret1'
98112

113+
# Key that uses the RSA-PSS algorithm
114+
ssl/server-rsapss.key:
115+
$(OPENSSL) genpkey -algorithm rsa-pss -out $@
116+
99117
# DER-encoded version of client.key
100118
ssl/client-der.key: ssl/client.key
101119
$(OPENSSL) rsa -in $< -outform DER -out $@

src/test/ssl/t/002_scram.pl

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ sub switch_server_cert
4646
# Determine whether build supports tls-server-end-point.
4747
my $supports_tls_server_end_point =
4848
check_pg_config("#define HAVE_X509_GET_SIGNATURE_NID 1");
49+
# Determine whether build supports detection of hash algorithms for
50+
# RSA-PSS certificates.
51+
my $supports_rsapss_certs =
52+
check_pg_config("#define HAVE_X509_GET_SIGNATURE_INFO 1");
4953

5054
# Allocation of base connection string shared among multiple tests.
5155
my $common_connstr;
@@ -136,4 +140,17 @@ sub switch_server_cert
136140
qr/connection authenticated: identity="ssltestuser" method=scram-sha-256/
137141
]);
138142

143+
# Now test with a server certificate that uses the RSA-PSS algorithm.
144+
# This checks that the certificate can be loaded and that channel binding
145+
# works. (see bug #17760)
146+
if ($supports_rsapss_certs)
147+
{
148+
switch_server_cert($node, certfile => 'server-rsapss');
149+
$node->connect_ok(
150+
"$common_connstr user=ssltestuser channel_binding=require",
151+
"SCRAM with SSL and channel_binding=require, server certificate uses 'rsassaPss'",
152+
log_like => [
153+
qr/connection authenticated: identity="ssltestuser" method=scram-sha-256/
154+
]);
155+
}
139156
done_testing();

src/tools/msvc/Solution.pm

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,6 +371,7 @@ sub GenerateFiles
371371
HAVE_WCSTOMBS_L => 1,
372372
HAVE_VISIBILITY_ATTRIBUTE => undef,
373373
HAVE_X509_GET_SIGNATURE_NID => 1,
374+
HAVE_X509_GET_SIGNATURE_INFO => undef,
374375
HAVE_X86_64_POPCNTQ => undef,
375376
HAVE__BOOL => undef,
376377
HAVE__BUILTIN_BSWAP16 => undef,
@@ -489,7 +490,14 @@ sub GenerateFiles
489490

490491
my ($digit1, $digit2, $digit3) = $self->GetOpenSSLVersion();
491492

492-
# More symbols are needed with OpenSSL 1.1.0 and above.
493+
# Symbols needed with OpenSSL 1.1.1 and above.
494+
if ( ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
495+
|| ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '1'))
496+
{
497+
$define{HAVE_X509_GET_SIGNATURE_INFO} = 1;
498+
}
499+
500+
# Symbols needed with OpenSSL 1.1.0 and above.
493501
if ( ($digit1 >= '3' && $digit2 >= '0' && $digit3 >= '0')
494502
|| ($digit1 >= '1' && $digit2 >= '1' && $digit3 >= '0'))
495503
{

0 commit comments

Comments
 (0)