pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
18+ messages / 5 participants
[nested] [flat]
* BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-01-25 11:32 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 2 replies; 18+ messages in thread
From: PG Bug reporting form @ 2023-01-25 11:32 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: gunnar.bluth@pro-open.de
The following bug has been logged on the website:
Bug reference: 17760
Logged by: Gunnar "Nick" Bluth
Email address: gunnar.bluth@pro-open.de
PostgreSQL version: 13.8
Operating system: Ubuntu 20.04
Description:
My client recently started rolling out new server certificates, which, when
dumped with "openssl x509 [...]", show slightly different information
regarding signature, hash etc.
Old:
Signature Algorithm: sha256WithRSAEncryption
New:
Signature Algorithm: rsassaPss
Hash Algorithm: sha512
Mask Algorithm: mgf1 with sha512
Salt Length: 0x40
Trailer Field: 0xBC (default)
When trying to authenticate on a server using such a certificate using
scram-sha-256, we receive an error message: "could not find digest for NID
UNDEF"
I tried to boil this down a bit. The error comes from
src/interfaces/libpq/fe-secure-openssl.c, pgtls_get_peer_certificate_hash(),
line 440+.
The comment there says "If something else is used, the same hash as the
signature algorithm is used."
And obviously, "EVP_get_digestbynid(NID_rsassaPss)" doesn't return a
result.
Now, openssl's "crypto/objects/obj_xref.txt" contains this:
# OID cross reference table.
# Links signatures OIDs to their corresponding public key algorithms
# and digests.
<snip>
sha256WithRSAEncryption sha256 rsaEncryption
<snip>
# For PSS the digest algorithm can vary and depends on the included
# AlgorithmIdentifier. The digest "undef" indicates the public key
# method should handle this explicitly.
rsassaPss undef rsassaPss
That explains the "UNDEF" in the error message (i.e., I *think* that's where
it's coming from).
I have to say that I'm not all too deep into crypto stuff. I couldn't even
tell how to create one such certificate, let alone what that PSS stuff is
all about ;-/
Maybe this is even fixed with recent OpenSSL versions (client has 1.1.1f,
Ubuntu 20.04)? Though that line was introduced in 2010...
I do think however that this is an oversight on our side and has to be
addressed. If not in code, the docs should point out that certain server
certificate types (PSS) may not work with SCRAM auth (or libpq needs to be
compiled against a minimum version of OpenSSL, if that's the root cause).
PS: the "obj_xref.txt" also lists "undef" for ED25519 and ED448
PPS: excerpt from "openssl s_client -starttls postgres -showcerts -connect
[...]":
Requested Signature Algorithms:
ECDSA+SHA256:ECDSA+SHA384:ECDSA+SHA512:Ed25519:Ed448:RSA-PSS+SHA256:RSA-PSS+SHA384:RSA-PSS+SHA512:RSA-PSS+SHA256:RSA-PSS+SHA384:RSA-PSS+SHA512:RSA+SHA256:RSA+SHA384:RSA+SHA512:ECDSA+SHA224:RSA+SHA224
Shared Requested Signature Algorithms:
ECDSA+SHA256:ECDSA+SHA384:ECDSA+SHA512:Ed25519:Ed448:RSA-PSS+SHA256:RSA-PSS+SHA384:RSA-PSS+SHA512:RSA-PSS+SHA256:RSA-PSS+SHA384:RSA-PSS+SHA512:RSA+SHA256:RSA+SHA384:RSA+SHA512
Peer signing digest: SHA256
Peer signature type: RSA-PSS
Server Temp Key: ECDH, P-256, 256 bits
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-01-31 12:27 Gunnar "Nick" Bluth <gunnar.bluth@pro-open.de>
parent: PG Bug reporting form <noreply@postgresql.org>
1 sibling, 0 replies; 18+ messages in thread
From: Gunnar \"Nick\" Bluth @ 2023-01-31 12:27 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
Am 25.01.23 um 12:32 schrieb PG Bug reporting form:
> I have to say that I'm not all too deep into crypto stuff. I couldn't even
> tell how to create one such certificate, let alone what that PSS stuff is
> all about ;-/
>
> Maybe this is even fixed with recent OpenSSL versions (client has 1.1.1f,
> Ubuntu 20.04)? Though that line was introduced in 2010...
>
> I do think however that this is an oversight on our side and has to be
> addressed. If not in code, the docs should point out that certain server
> certificate types (PSS) may not work with SCRAM auth (or libpq needs to be
> compiled against a minimum version of OpenSSL, if that's the root cause).
Nobody willing to enlighten me on this one? :(
--
Gunnar "Nick" Bluth
Eimermacherweg 106
D-48159 Münster
Mobil +49 172 8853339
Email: gunnar.bluth@pro-open.de
__________________________________________________________________________
"Ceterum censeo SystemD esse delendam" - Cato
Attachments:
[application/pgp-signature] OpenPGP_signature (494B, ../../e0504455-25b3-3b1d-08c0-f3e28f78a545@pro-open.de/2-OpenPGP_signature)
download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-05 01:28 Heikki Linnakangas <hlinnaka@iki.fi>
parent: PG Bug reporting form <noreply@postgresql.org>
1 sibling, 1 reply; 18+ messages in thread
From: Heikki Linnakangas @ 2023-02-05 01:28 UTC (permalink / raw)
To: gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org
On 25/01/2023 13:32, PG Bug reporting form wrote:
> The following bug has been logged on the website:
>
> Bug reference: 17760
> Logged by: Gunnar "Nick" Bluth
> Email address: gunnar.bluth@pro-open.de
> PostgreSQL version: 13.8
> Operating system: Ubuntu 20.04
> Description:
>
> My client recently started rolling out new server certificates, which, when
> dumped with "openssl x509 [...]", show slightly different information
> regarding signature, hash etc.
>
> Old:
> Signature Algorithm: sha256WithRSAEncryption
> New:
> Signature Algorithm: rsassaPss
> Hash Algorithm: sha512
> Mask Algorithm: mgf1 with sha512
> Salt Length: 0x40
> Trailer Field: 0xBC (default)
>
> When trying to authenticate on a server using such a certificate using
> scram-sha-256, we receive an error message: "could not find digest for NID
> UNDEF"
I was able to create a certificate like that with these commands:
openssl genpkey -algorithm rsa-pss -pkeyopt rsa_keygen_bits:2048
-pkeyopt rsa_keygen_pubexp:65537 -out CA.key
openssl req -new -x509 -days 365 -nodes -text -out server.crt -key
CA.key -keyout server.key -subj "/CN=rsapss.localtest.me"
And was able to reproduce the problem with that.
As a workaround, you can disable channel binding with the
"channel_binding=disable" libpq option.
> I tried to boil this down a bit. The error comes from
> src/interfaces/libpq/fe-secure-openssl.c, pgtls_get_peer_certificate_hash(),
> line 440+.
> The comment there says "If something else is used, the same hash as the
> signature algorithm is used."
> And obviously, "EVP_get_digestbynid(NID_rsassaPss)" doesn't return a
> result.
>
> Now, openssl's "crypto/objects/obj_xref.txt" contains this:
> # OID cross reference table.
> # Links signatures OIDs to their corresponding public key algorithms
> # and digests.
> <snip>
> sha256WithRSAEncryption sha256 rsaEncryption
> <snip>
> # For PSS the digest algorithm can vary and depends on the included
> # AlgorithmIdentifier. The digest "undef" indicates the public key
> # method should handle this explicitly.
> rsassaPss undef rsassaPss
>
>
> That explains the "UNDEF" in the error message (i.e., I *think* that's where
> it's coming from).
>
> I have to say that I'm not all too deep into crypto stuff. I couldn't even
> tell how to create one such certificate, let alone what that PSS stuff is
> all about ;-/
>
> Maybe this is even fixed with recent OpenSSL versions (client has 1.1.1f,
> Ubuntu 20.04)? Though that line was introduced in 2010...
Thanks for the investigation!
> I do think however that this is an oversight on our side and has to be
> addressed. If not in code, the docs should point out that certain server
> certificate types (PSS) may not work with SCRAM auth (or libpq needs to be
> compiled against a minimum version of OpenSSL, if that's the root cause).
There are a few things we change in PostgreSQL for this:
1. If the server cannot compute a hash of the certificate, because it
cannot unambiguously determine the hash algorithm to use, it should not
offer the SCRAM-SHA-256-PLUS authentication method to the client. In
other words, if the server cannot do channel binding with it's
certificate, it should say so, instead of erroring out later.
2. Similarly in the client: if libpq cannot determinate the hash
algorithm to use with the server's certificate, it should not attempt
channel binding.
I believe this is OK from a security point of view. If the server is
using a certificate that cannot be used with channel binding, and the
client doesn't require channel binding, it's OK to not do it. A man in
the middle can present a certificate to the client that cannot be used
with channel binding, but if they can do that they could also just not
offer the SCRAM-SHA-256-PLUS option to the client. So I don't see risk
of downgrade attacks here.
3. Add support for channel binding with RSA-PSS. The problem is that
be_tls_get_certificate_hash() doesn't know which digest algorithm to
use. As you noted, OBJ_find_sigid_algs() returns "undef" (NID_undef) for
rsassaPss certificates. I did some googling: when certificate uses
RSASSA-PSS as the signature algorithm, there is a separate field,
RSASSA-PSS-params that specifies the hash algorithm
(https://www.rfc-editor.org/rfc/rfc4055#section-3.1). If you ask me,
OBJ_find_sigid_algs() should look into RSASSA-PSS-params and figure it
out, but I'm no expert in the OpenSSL codebase so maybe that would be
the wrong place to do it.
OpenSSL 3.0 actually has a function called X509_digest_sig(), which
parses the RSASSA-PSS-params field. It also has a fallback mechanism to
use SHA512 for ED25519, SHAKE256 for ED448, and SHA256 for anything
else. We could use that function, perhaps without accepting the
fallbacks, though. Or call lower level OpenSSL functions to get the hash
algorithm from the RSASSA-PSS-params field ourselves.
I'll write a patch for 1 and 2, at least, and maybe 3 if it's not too
complicated. Please poke me if you don't hear from me in a few days.
- Heikki
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-08 16:11 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 18+ messages in thread
From: Heikki Linnakangas @ 2023-02-08 16:11 UTC (permalink / raw)
To: gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org; +Cc: Jacob Champion <jchampion@timescale.com>; Michael Paquier <michael@paquier.xyz>
cc'ing Jacob and Michael who have poked around channel binding before.
If you have a chance to review this patch, I would appreciate that.
On 05/02/2023 03:28, Heikki Linnakangas wrote:
> On 25/01/2023 13:32, PG Bug reporting form wrote:
>> I do think however that this is an oversight on our side and has to be
>> addressed. If not in code, the docs should point out that certain server
>> certificate types (PSS) may not work with SCRAM auth (or libpq needs to be
>> compiled against a minimum version of OpenSSL, if that's the root cause).
>
> There are a few things we change in PostgreSQL for this:
>
> 1. If the server cannot compute a hash of the certificate, because it
> cannot unambiguously determine the hash algorithm to use, it should not
> offer the SCRAM-SHA-256-PLUS authentication method to the client. In
> other words, if the server cannot do channel binding with it's
> certificate, it should say so, instead of erroring out later.
Here's a patch for that.
> 2. Similarly in the client: if libpq cannot determinate the hash
> algorithm to use with the server's certificate, it should not attempt
> channel binding.
>
> I believe this is OK from a security point of view. If the server is
> using a certificate that cannot be used with channel binding, and the
> client doesn't require channel binding, it's OK to not do it. A man in
> the middle can present a certificate to the client that cannot be used
> with channel binding, but if they can do that they could also just not
> offer the SCRAM-SHA-256-PLUS option to the client. So I don't see risk
> of downgrade attacks here.
On second thoughts, the above is wrong. We cannot safely downgrade to
not using channel binding just because we don't support the
certificate's signature algorithm. Otherwise a MITM could indeed do what
I said above and send a certificate that uses an unsupported signature
algorithm, to force downgrade. It cannot just modify the SASL mechanism
negotiation by leaving out the SCRAM-SHA-256-PLUS, because the SCRAM
handshake catches that case. The client sends a different gs2-cbind-flag
when it chooses to not do channel binding even though the server
supports it, and when it doesn't use channel binding because the server
didn't support it.
I improved the error message in the client, but I'm afraid that's the
best we can do in the client side. Fortunately, it is enough to upgrade
the server to include this fix, to allow clients to connect. (Albeit
without channel binding).
> 3. Add support for channel binding with RSA-PSS. The problem is that
> be_tls_get_certificate_hash() doesn't know which digest algorithm to
> use. As you noted, OBJ_find_sigid_algs() returns "undef" (NID_undef) for
> rsassaPss certificates. I did some googling: when certificate uses
> RSASSA-PSS as the signature algorithm, there is a separate field,
> RSASSA-PSS-params that specifies the hash algorithm
> (https://www.rfc-editor.org/rfc/rfc4055#section-3.1). If you ask me,
> OBJ_find_sigid_algs() should look into RSASSA-PSS-params and figure it
> out, but I'm no expert in the OpenSSL codebase so maybe that would be
> the wrong place to do it.
I didn't dare to make any such changes for now. Maybe we could do that
in 'master', but I would be wary of backpatching.
- Heikki
Attachments:
[text/x-patch] 0001-Don-t-advertise-channel-binding-to-client-when-we-ca.patch (24.4K, ../../584842ed-e476-6bac-bb36-2aec204e9eef@iki.fi/2-0001-Don-t-advertise-channel-binding-to-client-when-we-ca.patch)
download | inline diff:
From 9a8e6d11d6fdbfed907de2c999129ba8105d6aff Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Wed, 8 Feb 2023 17:39:25 +0200
Subject: [PATCH 1/1] Don't advertise channel binding to client when we cannot
support it.
With certain kinds of TLS server certificates, we cannot do SCRAM
channel binding (tls-server-end-point), because the hash algorithm to
use is not well defined, or is not supported by OpenSSL. If you try to
use channel binding with such a certificate, you get an obscure error
in the client during the authentication:
could not find digest for NID UNDEF
To fix, don't advertise SCRAM-SHA-256-PLUS if we cannot calculate the
hash. Print a LOG message once on server startup, and at config
reload, if the server is using such a certificate. To do that,
calculate the certificate hash when the certificate is loaded, instead
of when a connection is established.
The availability of channel binding now depends on the installed
certificate, so this removes HAVE_BE_TLS_GET_CERTIFICATE_HASH and
replaces it with a new function, scram_channel_binding_possible(). If
the server is compiled with a version of OpenSSL that doesn't have the
X509_get_signature_nid() function, and hence cannot support our
channel binding at all, it will just always return false.
The libqq client code has a similar issue. It will choose
SCRAM-SHA-256-PLUS if the server advertises it, even if it doesn't
support the server certificate's signature algorithm, only to fail
later in the authentication. Unfortunately, there isn't much we can do
in the client: it cannot safely fall back to not using channel binding
if it doesn't support the server's certificate's hashing algorithm,
because that could lead to a downgrade attack. So in the client, just
make the error message more clear.
We could try harder to determine a hash algorithm if the certificate
doesn't specify one unambiguously. For example, we could fall back to
SHA-256. However, I'm not comfortable inventing new rules especially
in back-branches. OpenSSL also has a function X509_digest_sig() that
contains some fallback rules like that, but AFAICS it wasn't written
with channel binding in mind, so I'm not confident switching to that
either. In the future, we probably should switch to the tls-exporter
channel binding type as specified in RFC 9266.
Fixes bug #17760. We still don't support channel binding with RSA-PSS
certificates, but at least we fall back more gracefully now.
Reported-by: Gunnar "Nick" Bluth
Discussion: https://www.postgresql.org/message-id/17760-b6c61e752ec07060@postgresql.org
Backpatch: 11- (all supported versions)
---
src/backend/libpq/auth-scram.c | 54 +++++++-----
src/backend/libpq/be-secure-openssl.c | 102 ++++++++++++++++++++---
src/backend/libpq/be-secure.c | 1 +
src/include/libpq/libpq-be.h | 9 +-
src/interfaces/libpq/fe-auth-scram.c | 17 +++-
src/interfaces/libpq/fe-secure-openssl.c | 5 +-
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 | 22 ++++-
src/test/ssl/t/002_scram.pl | 14 ++++
11 files changed, 242 insertions(+), 45 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/src/backend/libpq/auth-scram.c b/src/backend/libpq/auth-scram.c
index 4441e0d7745..fb39051bfa6 100644
--- a/src/backend/libpq/auth-scram.c
+++ b/src/backend/libpq/auth-scram.c
@@ -191,6 +191,36 @@ static char *scram_mock_salt(const char *username,
pg_cryptohash_type hash_type,
int key_length);
+/*
+ * Is channel binding possible?
+ *
+ * SCRAM-SHA-256-PLUS in PostgreSQL implies the tls-server-end-point channel
+ * binding type, which is based on the hash of the server's certificate. Try
+ * to get the hash. It might not be available, if the certificate uses an
+ * unsupported or ambiguous digest algorithm.
+ *
+ * XXX: By supporting only tls-server-end-point, we are violating RFC 5802,
+ * which states that the server MUST support tls-unique channel binding type
+ * if channel binding is supported at all. We choose to violate the RFC
+ * because tls-unique was not supported by all versions of OpenSSL when this
+ * was implemented, and also requires Extended Master Key Extension [RFC7627]
+ * or TLS v1.3 to be secure. In the future, we should implement the
+ * tls-exporter channel binding type as defined in RFC9266.
+ */
+static bool
+scram_channel_binding_possible(Port *port)
+{
+ size_t dummy_len;
+
+ if (!port->ssl_in_use)
+ return false;
+
+ if (be_tls_get_certificate_hash(port, &dummy_len) != NULL)
+ return true;
+ else
+ return false;
+}
+
/*
* Get a list of SASL mechanisms that this module supports.
*
@@ -203,17 +233,13 @@ scram_get_mechanisms(Port *port, StringInfo buf)
{
/*
* Advertise the mechanisms in decreasing order of importance. So the
- * channel-binding variants go first, if they are supported. Channel
- * binding is only supported with SSL, and only if the SSL implementation
- * has a function to get the certificate's hash.
+ * channel-binding variants go first, if they are supported.
*/
-#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
- if (port->ssl_in_use)
+ if (scram_channel_binding_possible(port))
{
appendStringInfoString(buf, SCRAM_SHA_256_PLUS_NAME);
appendStringInfoChar(buf, '\0');
}
-#endif
appendStringInfoString(buf, SCRAM_SHA_256_NAME);
appendStringInfoChar(buf, '\0');
}
@@ -252,12 +278,9 @@ scram_init(Port *port, const char *selected_mech, const char *shadow_pass)
* client nevertheless tries to select it, it's a protocol violation like
* selecting any other SASL mechanism we don't support.
*/
-#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
- if (strcmp(selected_mech, SCRAM_SHA_256_PLUS_NAME) == 0 && port->ssl_in_use)
+ if (strcmp(selected_mech, SCRAM_SHA_256_PLUS_NAME) == 0 && scram_channel_binding_possible(port))
state->channel_binding_in_use = true;
- else
-#endif
- if (strcmp(selected_mech, SCRAM_SHA_256_NAME) == 0)
+ else if (strcmp(selected_mech, SCRAM_SHA_256_NAME) == 0)
state->channel_binding_in_use = false;
else
ereport(ERROR,
@@ -1005,14 +1028,12 @@ read_client_first_message(scram_state *state, const char *input)
errmsg("malformed SCRAM message"),
errdetail("The client selected SCRAM-SHA-256-PLUS, but the SCRAM message does not include channel binding data.")));
-#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
- if (state->port->ssl_in_use)
+ if (scram_channel_binding_possible(state->port))
ereport(ERROR,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
errmsg("SCRAM channel binding negotiation error"),
errdetail("The client supports SCRAM channel binding but thinks the server does not. "
"However, this server does support channel binding.")));
-#endif
p++;
if (*p != ',')
ereport(ERROR,
@@ -1301,7 +1322,6 @@ read_client_final_message(scram_state *state, const char *input)
channel_binding = read_attr_value(&p, 'c');
if (state->channel_binding_in_use)
{
-#ifdef HAVE_BE_TLS_GET_CERTIFICATE_HASH
const char *cbind_data = NULL;
size_t cbind_data_len = 0;
size_t cbind_header_len;
@@ -1343,10 +1363,6 @@ read_client_final_message(scram_state *state, const char *input)
ereport(ERROR,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
errmsg("SCRAM channel binding check failed")));
-#else
- /* shouldn't happen, because we checked this earlier already */
- elog(ERROR, "channel binding not supported by this build");
-#endif
}
else
{
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index e3c7c12aa0e..751189600f8 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -61,6 +61,7 @@ static int my_sock_write(BIO *h, const char *buf, int size);
static BIO_METHOD *my_BIO_s_socket(void);
static int my_SSL_set_fd(Port *port, int fd);
+static char *calculate_certificate_hash(X509 *cert, size_t *len, bool isServerStart);
static DH *load_dh_file(char *filename, bool isServerStart);
static DH *load_dh_buffer(const char *buffer, size_t len);
static int ssl_external_passwd_cb(char *buf, int size, int rwflag, void *userdata);
@@ -73,7 +74,11 @@ static const char *SSLerrmessage(unsigned long ecode);
static char *X509_NAME_to_cstring(X509_NAME *name);
+/* current SSL context, and hash of the server certificate in use in the context */
static SSL_CTX *SSL_context = NULL;
+static char *ssl_cert_hash = NULL;
+static int ssl_cert_hash_len = 0;
+
static bool SSL_initialized = false;
static bool dummy_ssl_passwd_cb_called = false;
static bool ssl_is_server_start;
@@ -94,6 +99,9 @@ be_tls_init(bool isServerStart)
SSL_CTX *context;
int ssl_ver_min = -1;
int ssl_ver_max = -1;
+ X509 *new_cert;
+ char *new_hash = NULL;
+ size_t new_hash_len = 0;
/* This stuff need be done only once. */
if (!SSL_initialized)
@@ -156,6 +164,30 @@ be_tls_init(bool isServerStart)
if (!check_ssl_key_file_permissions(ssl_key_file, isServerStart))
goto error;
+ /*
+ * Pre-calculate the hash of the certificate, for use in channel binding.
+ */
+ new_cert = SSL_CTX_get0_certificate(context);
+ if (new_cert == NULL)
+ {
+ /*
+ * The SSL_CTX_use_certificate_chain_file() call above succeeded, but
+ * there is no certificate in the context. Not clear if this can
+ * happen, but we certainly can't do tls-server-end-point channel
+ * binding without a certificate.
+ */
+ new_hash = NULL;
+ new_hash_len = 0;
+ }
+ else
+ {
+ MemoryContext oldcxt;
+
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+ new_hash = calculate_certificate_hash(new_cert, &new_hash_len, isServerStart);
+ MemoryContextSwitchTo(oldcxt);
+ }
+
/*
* OK, try to load the private key file.
*/
@@ -380,8 +412,12 @@ be_tls_init(bool isServerStart)
*/
if (SSL_context)
SSL_CTX_free(SSL_context);
+ if (ssl_cert_hash)
+ pfree(ssl_cert_hash);
SSL_context = context;
+ ssl_cert_hash = new_hash;
+ ssl_cert_hash_len = new_hash_len;
/*
* Set flag to remember whether CA store has been loaded into SSL_context.
@@ -397,6 +433,8 @@ be_tls_init(bool isServerStart)
error:
if (context)
SSL_CTX_free(context);
+ if (new_hash)
+ pfree(new_hash);
return -1;
}
@@ -407,6 +445,9 @@ be_tls_destroy(void)
SSL_CTX_free(SSL_context);
SSL_context = NULL;
ssl_loaded_verify_locations = false;
+ if (ssl_cert_hash)
+ pfree(ssl_cert_hash);
+ ssl_cert_hash = NULL;
}
int
@@ -1429,11 +1470,29 @@ be_tls_get_peer_serial(Port *port, char *ptr, size_t len)
ptr[0] = '\0';
}
-#ifdef HAVE_X509_GET_SIGNATURE_NID
char *
be_tls_get_certificate_hash(Port *port, size_t *len)
{
- X509 *server_cert;
+ *len = ssl_cert_hash_len;
+ return ssl_cert_hash;
+}
+
+/*
+ * Calculate hash of the given certificate, for use in tls-server-end-point
+ * channel binding.
+ *
+ * The returned hash is palloc'd, and its length is returned in *len.
+ *
+ * If the hash could not be calculated because the hash algorithm specified in
+ * the certificate's signature is not supported, or it doesn't unambiguously
+ * specify an algorithm to use, returns NULL and logs the error. On other
+ * less expected errors, ereports the error with FATAL or LOG depending on
+ * whether 'isServerStart' is true.
+ */
+static char *
+calculate_certificate_hash(X509 *cert, size_t *len, bool isServerStart)
+{
+#ifdef HAVE_X509_GET_SIGNATURE_NID
char *cert_hash;
const EVP_MD *algo_type = NULL;
unsigned char hash[EVP_MAX_MD_SIZE]; /* size for SHA-512 */
@@ -1441,17 +1500,18 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
int algo_nid;
*len = 0;
- server_cert = SSL_get_certificate(port->ssl);
- if (server_cert == NULL)
- return NULL;
/*
* 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))
- elog(ERROR, "could not determine server certificate signature algorithm");
+ if (OBJ_find_sigid_algs(X509_get_signature_nid(cert),
+ &algo_nid, NULL) == 0)
+ {
+ ereport(LOG,
+ (errmsg("channel binding is disabled; server certificate has unknown signature algorithm")));
+ return NULL;
+ }
/*
* The TLS server's certificate bytes need to be hashed with SHA-256 if
@@ -1461,6 +1521,10 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
*/
switch (algo_nid)
{
+ case NID_undef:
+ ereport(LOG,
+ (errmsg("channel binding is disabled; server certificate does not unambiguously specify a signature algorithm")));
+ return NULL;
case NID_md5:
case NID_sha1:
algo_type = EVP_sha256();
@@ -1468,22 +1532,34 @@ be_tls_get_certificate_hash(Port *port, size_t *len)
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));
+ {
+ ereport(isServerStart ? FATAL : LOG,
+ errmsg("could not find digest for NID %s",
+ OBJ_nid2sn(algo_nid)));
+ return NULL;
+ }
break;
}
/* generate and save the certificate hash */
- if (!X509_digest(server_cert, algo_type, hash, &hash_size))
- elog(ERROR, "could not generate server certificate hash");
+ if (!X509_digest(cert, algo_type, hash, &hash_size))
+ {
+ ereport(isServerStart ? FATAL : LOG,
+ (errmsg("channel binding is disabled; could not generate server certificate hash")));
+ return NULL;
+ }
+ Assert(hash_size <= sizeof(hash));
cert_hash = palloc(hash_size);
memcpy(cert_hash, hash, hash_size);
*len = hash_size;
return cert_hash;
-}
+#else
+ *len = 0;
+ return NULL;
#endif
+}
/*
* Convert an X509 subject name to a cstring.
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index a0f70840183..8c991099784 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -47,6 +47,7 @@ bool ssl_passphrase_command_supports_reload;
#ifdef USE_SSL
bool ssl_loaded_verify_locations = false;
+char *ssl_cert_hash = NULL;
#endif
/* GUC variable controlling SSL cipher list */
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 8c70b2fd5be..dda299a241f 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -303,15 +303,10 @@ extern void be_tls_get_peer_serial(Port *port, char *ptr, size_t len);
* tls-server-end-point.
*
* The result is a palloc'd hash of the server certificate with its
- * size, and NULL if there is no certificate available.
- *
- * This is not supported with old versions of OpenSSL that don't have
- * the X509_get_signature_nid() function.
+ * size, or NULL if there is no certificate available or we could not
+ * unambiguously calculate a hash for it.
*/
-#if defined(USE_OPENSSL) && defined(HAVE_X509_GET_SIGNATURE_NID)
-#define HAVE_BE_TLS_GET_CERTIFICATE_HASH
extern char *be_tls_get_certificate_hash(Port *port, size_t *len);
-#endif
/* init hook for SSL, the default sets the password callback if appropriate */
#ifdef USE_OPENSSL
diff --git a/src/interfaces/libpq/fe-auth-scram.c b/src/interfaces/libpq/fe-auth-scram.c
index 9c42ea4f819..fc0a5cd264a 100644
--- a/src/interfaces/libpq/fe-auth-scram.c
+++ b/src/interfaces/libpq/fe-auth-scram.c
@@ -487,7 +487,22 @@ build_client_final_message(fe_scram_state *state)
&cbind_data_len);
if (cbind_data == NULL)
{
- /* error message is already set on error */
+ /*
+ * Could not calculate hash of the server's certificate. Error
+ * message is already set by pgtls_get_peer_certificate_hash().
+ *
+ * This is known to happen with certificates using 'rsassaPss',
+ * 'ED25519' and 'ED448' signatures, because they don't specify an
+ * unambiguous digest algorithm to use. The server should not
+ * offer PLUS-variants with such certificates, but older versions
+ * incorrectly did. If channel binding is required, we naturally
+ * must fail, but even if channel binding is merely preferred, we
+ * cannot safely fall back to disabling channel binding, because
+ * that could lead to a downgrade attack: A man-in-the-middle
+ * could intentionally offer us a certificate with an unrecognized
+ * signature algorithm, to force the client to disable channel
+ * binding.
+ */
termPQExpBuffer(&buf);
return NULL;
}
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 983536de251..96168a8bd7e 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -389,7 +389,7 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
if (!OBJ_find_sigid_algs(X509_get_signature_nid(peer_cert),
&algo_nid, NULL))
{
- libpq_append_conn_error(conn, "could not determine server certificate signature algorithm");
+ libpq_append_conn_error(conn, "could not determine signature algorithm to use with server's certificate; channel binding not possible");
return NULL;
}
@@ -401,6 +401,9 @@ pgtls_get_peer_certificate_hash(PGconn *conn, size_t *len)
*/
switch (algo_nid)
{
+ case NID_undef:
+ libpq_append_conn_error(conn, "could not determine signature algorithm to use with server's certificate; channel binding not possible");
+ return NULL;
case NID_md5:
case NID_sha1:
algo_type = EVP_sha256();
diff --git a/src/test/ssl/conf/server-rsapss.config b/src/test/ssl/conf/server-rsapss.config
new file mode 100644
index 00000000000..d4701bdb36b
--- /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
diff --git a/src/test/ssl/ssl/server-rsapss.crt b/src/test/ssl/ssl/server-rsapss.crt
new file mode 100644
index 00000000000..bef29adb1c3
--- /dev/null
+++ b/src/test/ssl/ssl/server-rsapss.crt
@@ -0,0 +1,21 @@
+-----BEGIN CERTIFICATE-----
+MIIDczCCAioCFB6p3YqR83DVMn6wTP1AI9E3akswMD4GCSqGSIb3DQEBCjAxoA0w
+CwYJYIZIAWUDBAIBoRowGAYJKoZIhvcNAQEIMAsGCWCGSAFlAwQCAaIEAgIA3jBG
+MSQwIgYDVQQDDBtjb21tb24tbmFtZS5wZy1zc2x0ZXN0LnRlc3QxHjAcBgNVBAsM
+FVBvc3RncmVTUUwgdGVzdCBzdWl0ZTAeFw0yMzAyMDgxNDIzMzRaFw0yMzAzMTAx
+NDIzMzRaMEYxJDAiBgNVBAMMG2NvbW1vbi1uYW1lLnBnLXNzbHRlc3QudGVzdDEe
+MBwGA1UECwwVUG9zdGdyZVNRTCB0ZXN0IHN1aXRlMIIBIDALBgkqhkiG9w0BAQoD
+ggEPADCCAQoCggEBALaoRpce0lzkRKERq4cUANEoiVtOPfKnG4n4cifMJYto+DlE
+TIB5PfeNeuLoLdKnohYlICQDLuV7W3r93MoimZ8tbYt78OQ06G/6qtyc6FjlGTKP
+6+ULV8AVBbhcyQPtKhpzq+WbX/6Lz4hVtms/aeElB60ZbAs6wzSt8XBTmfjnOrty
+5w/k2/+/X0DzRE1aYTnVlFTlVyuF1Th7Psm1EOfvAdjbVLCso4Ttpsixq0ILarAm
+xQfc5cO6/dXRDsjCnly+5CVNWYCE1+AB4G9NvklORLApkdc4UBjxFpDOoh5pNHZa
+ofExUthbGwyUfYOwGDPTPNYeYVshUZlAvgTJIrMCAwEAATA+BgkqhkiG9w0BAQow
+MaANMAsGCWCGSAFlAwQCAaEaMBgGCSqGSIb3DQEBCDALBglghkgBZQMEAgGiBAIC
+AN4DggEBAIyTGMj5Z+Rl/lhvPkLM5TAtl/pGP23wHa6hsf+SpgwPGFQe5qC5Tbrd
++w/5KK5mh40+QMWGNo+kaDGJY3N+WV69Vk4aMvKvxVIGgH5itwvBE/emYFThbMmw
+sGlW9f33+Edb2pEAEGEeb/AcesCYCF0tptyD1FphGVJaVFxo9uwwI0R3t/Ogok5H
+576WHz6L56/Bqx5qpF+6h3cSDom3uGS0BYJlKBuDG3F89gNSBsixhfPHaIZxy+Jc
+x5Tedp1sAqvxOQ1lez9LMPxdYAo3Qubw7c8H7bzyauiZnTt0xuTwOPJGFxwp5LMN
+7+KYwyAzsHaglC/1ySPHAfsg0Xkq4Q0=
+-----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 00000000000..71a1dcebd33
--- /dev/null
+++ b/src/test/ssl/ssl/server-rsapss.key
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvgIBADALBgkqhkiG9w0BAQoEggSqMIIEpgIBAAKCAQEAtqhGlx7SXOREoRGr
+hxQA0SiJW0498qcbifhyJ8wli2j4OURMgHk994164ugt0qeiFiUgJAMu5Xtbev3c
+yiKZny1ti3vw5DTob/qq3JzoWOUZMo/r5QtXwBUFuFzJA+0qGnOr5Ztf/ovPiFW2
+az9p4SUHrRlsCzrDNK3xcFOZ+Oc6u3LnD+Tb/79fQPNETVphOdWUVOVXK4XVOHs+
+ybUQ5+8B2NtUsKyjhO2myLGrQgtqsCbFB9zlw7r91dEOyMKeXL7kJU1ZgITX4AHg
+b02+SU5EsCmR1zhQGPEWkM6iHmk0dlqh8TFS2FsbDJR9g7AYM9M81h5hWyFRmUC+
+BMkiswIDAQABAoIBAQCA4h3lFf9zQjJWwKQajPfCnerFvWCirl0VLnRGNpsrUmcL
+6XBdmq8p1O+n2hIzOMt6+rlCD+jwTemP0D8RdfaVxbUtHcgtDlQQZ3xWbHTZ/NWZ
+wsm9dRJ2256MpxPDrH3v6jyZp9fFQZuvZGNyK22nSryu3GhLSCxjKHVbP/I+GKE3
+XUqN8coC9w0IznlURzxOCtN1MQff7ZAriXRtAMX0aWSzdkw/h2MkWsWGyB6ZpevA
+iWKqGSzBuQirYcwvk3Ne6IqOU83QRbLO4O7y2IBiwEz+BDB1l0/oIV3mwx2+4qLJ
+8HlFeAhPoCYFD/U3w+ZeQHevU8Y6+lFt6YqcGXfZAoGBANwP6+4gQkO+fRg13ncd
+7plWuebeEi/GZNpIk36/+U14Z88sZ0apB8szmkYrebDw1aDAsMdg313LlRQdbjzS
+9q3Xwx6Zso0CGE+qZ53BByImIcpcOqyuy6XSjd6buAK1mIGtUOvMEQAhCKaKzGZH
+mASN6aQazEPP+bLL43GhTU/9AoGBANR8lFHxMFf0uUX0F9Q0yxcmUGkW2bmHQXQl
+jnSCGdvMZzi21VLN4MjoLnKUsfo63THFZ0IshNeZbcMfA8wts26cPwJPE/EUppwJ
+Okdatp4zb0Pm5rRudhnpMvNELNjIxKSQ1Gpf3eXvpegE97nsApAvO7NzEA0yfNJ9
+hO7dqYRvAoGBAM2adpUqQJ8op5nqIqrqJVXQyKniC93lH68uJdhDprpx55OR6gAD
+x0tcMCSlU/I0YXPq9H+ji64HmoaLpMZhWKY9s4iwkzjZnzs3e83RXkfSlrmJiCx0
+t8J7QZ+dn8OwRQfLlZJpO+0B8CIiBiz9cRO4P3Xar8Qf2Szq/9rXmPNFAoGBALlu
+HmGEAZRAVI7ffQwLJlCFdxa+gjKN/mjFfZPfFYHi1xF/PJqOX+Pz+tSzPr4IMJAG
+nR36i9M6abclkcLU/wlAARyateRlCSCSTPGIEGXurOKs1hgPbbEe+P+iNyDX1ANp
+AHX0Q0kt4bKg4y6072e0UH1BpHhf3t4x+5gYiJ6xAoGBAJdD0GxLpe3AyG577Oo6
+x30W0/l4A4fPPkdRA+9zukt9Ybgs/KnoHzydBAqbuNBlj9DzqRuXlZA+zktJdyY1
+HC2oQ8qem7HbzbD10wLMN56VZWEohhEgw5A4EWdIcwYRKmIAliGTwyuJ+TQ4wl0x
++AVn5Ma2QzxTvU1Rrw0ok0w5
+-----END PRIVATE KEY-----
diff --git a/src/test/ssl/sslfiles.mk b/src/test/ssl/sslfiles.mk
index 5d9dc09a4b0..a565e90e4f1 100644
--- a/src/test/ssl/sslfiles.mk
+++ b/src/test/ssl/sslfiles.mk
@@ -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,9 @@ 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 +92,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 +106,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 0f3d180cfa9..a14fa3cb09b 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -136,4 +136,18 @@ $node->connect_ok(
qr/connection authenticated: identity="ssltestuser" method=scram-sha-256/
]);
+# Now test with a server certificate that uses the RSA-PSS algorithm. It
+# cannot be used for channel binding currently, but check that connecting
+# without channel binding is still possible. (see bug #17760)
+switch_server_cert($node, certfile => 'server-rsapss');
+$node->connect_fails(
+ "$common_connstr user=ssltestuser channel_binding=require",
+ "SCRAM with SSL and channel_binding=require, server certificate uses 'rsassaPss'",
+ expected_stderr =>
+ qr/channel binding is required, but server did not offer an authentication method that supports channel binding/
+ );
+$node->connect_ok(
+ "$common_connstr user=ssltestuser channel_binding=prefer",
+ "SCRAM with SSL and channel_binding=prefer, server certificate uses 'rsassaPss'");
+
done_testing();
--
2.30.2
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-09 08:24 Michael Paquier <michael@paquier.xyz>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 2 replies; 18+ messages in thread
From: Michael Paquier @ 2023-02-09 08:24 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: gunnar.bluth@pro-open.de, pgsql-bugs@lists.postgresql.org, Jacob Champion <jchampion@timescale.com>
On Wed, Feb 08, 2023 at 06:11:24PM +0200, Heikki Linnakangas wrote:
> On 05/02/2023 03:28, Heikki Linnakangas wrote:
>> There are a few things we change in PostgreSQL for this:
Nice digging in finding the set of openssl commands that would be
needed to produce certs able to trigger the error.
>> 1. If the server cannot compute a hash of the certificate, because it
>> cannot unambiguously determine the hash algorithm to use, it should not
>> offer the SCRAM-SHA-256-PLUS authentication method to the client. In
>> other words, if the server cannot do channel binding with it's
>> certificate, it should say so, instead of erroring out later.
>
> Here's a patch for that.
If the server cannot do it, not offering -PLUS makes sense and sending
the mechanisms available back to the client is the first step of the
SASL exchange as far as I recall.
>> 2. Similarly in the client: if libpq cannot determinate the hash
>> algorithm to use with the server's certificate, it should not attempt
>> channel binding.
>>
>> I believe this is OK from a security point of view. If the server is
>> using a certificate that cannot be used with channel binding, and the
>> client doesn't require channel binding, it's OK to not do it. A man in
>> the middle can present a certificate to the client that cannot be used
>> with channel binding, but if they can do that they could also just not
>> offer the SCRAM-SHA-256-PLUS option to the client. So I don't see risk
>> of downgrade attacks here.
>
> On second thoughts, the above is wrong. We cannot safely downgrade to not
> using channel binding just because we don't support the certificate's
> signature algorithm. Otherwise a MITM could indeed do what I said above and
> send a certificate that uses an unsupported signature algorithm, to force
> downgrade. It cannot just modify the SASL mechanism negotiation by leaving
> out the SCRAM-SHA-256-PLUS, because the SCRAM handshake catches that case.
> The client sends a different gs2-cbind-flag when it chooses to not do
> channel binding even though the server supports it, and when it doesn't use
> channel binding because the server didn't support it.
>
> I improved the error message in the client, but I'm afraid that's the best
> we can do in the client side. Fortunately, it is enough to upgrade the
> server to include this fix, to allow clients to connect. (Albeit without
> channel binding).
But the client has the choice to decide if it wants to use channel
binding, does it? In this case, it would send the non-PLUS mechanism
followed by 'n' as gs2-cbind-flag, no? If the client requires channel
binding with channel_binding=require and the server does not support
it, the client should fail if the server has a certificate that does
not support channel binding.
>> 3. Add support for channel binding with RSA-PSS. The problem is that
>> be_tls_get_certificate_hash() doesn't know which digest algorithm to
>> use. As you noted, OBJ_find_sigid_algs() returns "undef" (NID_undef) for
>> rsassaPss certificates. I did some googling: when certificate uses
>> RSASSA-PSS as the signature algorithm, there is a separate field,
>> RSASSA-PSS-params that specifies the hash algorithm
>> (https://www.rfc-editor.org/rfc/rfc4055#section-3.1). If you ask me,
>> OBJ_find_sigid_algs() should look into RSASSA-PSS-params and figure it
>> out, but I'm no expert in the OpenSSL codebase so maybe that would be
>> the wrong place to do it.
>
> I didn't dare to make any such changes for now. Maybe we could do that in
> 'master', but I would be wary of backpatching.
Perhaps that's something worth reporting to upstream? Not having the
interface to be able to find out the hash algo to use for a rsapss
certificate sounds like an issue on its own. Or it's just that rsapss
has nothing of the kind?
be_tls_get_certificate_hash() as presented just returns the hash
calculated in the TLS init phase, cached in TopMemoryContext. So you
are doing so because you need to know at a stage earlier than the SASL
exchange if channel binding would be a possibility or not. I am
wondering how this would complicate a potential implementation of
tls-exporter, as deciding if -PLUS should be sent depends on more
parameters. Anyway, this part is fine here.
+ * or TLS v1.3 to be secure. In the future, we should implement the
+ * tls-exporter channel binding type as defined in RFC9266.
One thing here is the complexity that we get in terms of channel
binding negotiation, and what to do with endpoint once tls-exporter is
around. See recent patch https://commitfest.postgresql.org/40/3850/
and its associated thread.
+ /*
+ * Pre-calculate the hash of the certificate, for use in channel binding.
+ */
+ new_cert = SSL_CTX_get0_certificate(context);
The minimal version supported on HEAD is OpenSSL 1.0.1, so this will
not compile as this routine exists only since 1.0.2. I am afraid that
this is going to require an extra configure flag. Using a wrapper
that englobes calculate_certificate_hash() would do the work, as
well.
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+ new_hash = calculate_certificate_hash(new_cert, &new_hash_len, isServerStart);
+ MemoryContextSwitchTo(oldcxt);
Shouldn't this bit in be_tls_init() have a "goto error" if the result
of calculate_certificate_hash() is NULL in some cases? On server
startup, that would be a FATAL, but not on reloads where we would have
a LOG. There are indeed two code paths where you still want to
proceed with the next TLS init steps, so this had better return a
boolean to let its caller know if an error should be triggered or not?
+ ereport(LOG,
+ (errmsg("channel binding is disabled; server certificate does not unambiguously specify a signature algorithm")));
[...]
+ ereport(LOG,
+ (errmsg("channel binding is disabled; server certificate has unknown signature algorithm")));
No FATAL if isServerStart, because the certificate in use does not
provide a hash algorithm that can be used.. I think that we'd better
document that, with a warning for example. On the client-side, it is
fortunate that we have channel_binding in libpq so as one would know
when the server does not support it, still it worries me that if
things are not set up correctly, then instances could finish by being
unprotected without knowing about it, especially since the default is
channel_binding=prefer.
It seems so wrong to me that we would just silently disable this
feature because RSA-PSS does not give you an algo type to do the
computation work. I'll try to look at bit at the OpenSSL code base
and see if we could not extract the algo information in this case.
Unfortunately, RFC 5929 is very clear:
"if the certificate's signatureAlgorithm uses no hash functions or
uses multiple hash functions, then this channel binding type's
channel bindings are undefined at this time (updates to is channel
binding type may occur to address this issue if it ever arises)."
I understand from this sentence that if a certificate has no hash
functions, then there's nothing you can do about it.
So as much as I'd like to be aggressive and potentially enforce the
use of SHA256 to compute the certificate hash, what you are doing is
RFC-compliant. I am wondering whether it would make sense to force a
failure with a GUC if the server cannot compute the hash of the
certificate at startup, though, rather than hope that a user knows
about that via a client forcing channel_binding=require at connection
time.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../Y+St0GKY9FsTTJ1U@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-09 21:54 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 18+ messages in thread
From: Heikki Linnakangas @ 2023-02-09 21:54 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: gunnar.bluth@pro-open.de, pgsql-bugs@lists.postgresql.org, Jacob Champion <jchampion@timescale.com>
On 09/02/2023 10:24, Michael Paquier wrote:
> It seems so wrong to me that we would just silently disable this
> feature because RSA-PSS does not give you an algo type to do the
> computation work. I'll try to look at bit at the OpenSSL code base
> and see if we could not extract the algo information in this case.
> Unfortunately, RFC 5929 is very clear:
> "if the certificate's signatureAlgorithm uses no hash functions or
> uses multiple hash functions, then this channel binding type's
> channel bindings are undefined at this time (updates to is channel
> binding type may occur to address this issue if it ever arises)."
>
> I understand from this sentence that if a certificate has no hash
> functions, then there's nothing you can do about it.
>
> So as much as I'd like to be aggressive and potentially enforce the
> use of SHA256 to compute the certificate hash, what you are doing is
> RFC-compliant.
Yeah, it seems silly to use anything else than SHA-256, given that
SCRAM-SHA-256 is totally dependent on the security of SHA-256 anyway.
Reading RFC5929 section "10.1. Cryptographic Algorithm Agility" [1], I
think the idea behind deriving the hash algorithm from the certificate's
signature algorithm is to allow the algorithm to be upgraded
automatically as new certificates with stronger algorithms are
introduced. But again, that seems silly with SCRAM-SHA-256.
I'm a little uncomfortable with this patch as I wrote it. It's not
trivial. The LOG messages might not be seen by admins. If we add a GUC
as you suggested, then an admin might need to change the GUC as part of
a minor version upgrade, or the server will refuse to start up. And we
cannot do anything about the client. And with all of this, we still
won't actually support channel binding with those certificates.
Perhaps we should just ignore the RFC and use SHA256 for these known
"undef" cases, after all. Having slept over this, I feel that would be
the best option. What do you think?
We could use a new "tls-server-end-point-sha-256" channel binding name
for that, instead of "tls-server-end-point", to make it clear when we're
violating the RFC.
[1] https://www.rfc-editor.org/rfc/rfc5929#section-10.1
- Heikki
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-09 22:22 Jacob Champion <jchampion@timescale.com>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 18+ messages in thread
From: Jacob Champion @ 2023-02-09 22:22 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Michael Paquier <michael@paquier.xyz>; gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org
On Thu, Feb 9, 2023 at 1:54 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
> On 09/02/2023 10:24, Michael Paquier wrote:
> > It seems so wrong to me that we would just silently disable this
> > feature because RSA-PSS does not give you an algo type to do the
> > computation work. I'll try to look at bit at the OpenSSL code base
> > and see if we could not extract the algo information in this case.
+1. I'm still coming up to speed on RSA-PSS and the conversation at
https://github.com/openssl/openssl/issues/15477
but it makes me really uncomfortable to fail open rather than shut in
this case, as part of a backport.
> > Unfortunately, RFC 5929 is very clear:
> > "if the certificate's signatureAlgorithm uses no hash functions or
> > uses multiple hash functions, then this channel binding type's
> > channel bindings are undefined at this time (updates to is channel
> > binding type may occur to address this issue if it ever arises)."
> >
> > I understand from this sentence that if a certificate has no hash
> > functions, then there's nothing you can do about it.
But we also don't have to proceed as if everything is okay.
> > So as much as I'd like to be aggressive and potentially enforce the
> > use of SHA256 to compute the certificate hash, what you are doing is
> > RFC-compliant.
>
> Yeah, it seems silly to use anything else than SHA-256, given that
> SCRAM-SHA-256 is totally dependent on the security of SHA-256 anyway.
How will this evolve when SCRAM-SHA-512 is implemented? Do you plan to
upgrade the undef-hash in that case to SHA-512?
Knee-jerk reaction: this departure feels unnecessary since Gunnar's
certificate does in fact contain a hash algorithm...
--Jacob
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-09 22:45 Michael Paquier <michael@paquier.xyz>
parent: Jacob Champion <jchampion@timescale.com>
0 siblings, 2 replies; 18+ messages in thread
From: Michael Paquier @ 2023-02-09 22:45 UTC (permalink / raw)
To: Jacob Champion <jchampion@timescale.com>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org
On Thu, Feb 09, 2023 at 02:22:28PM -0800, Jacob Champion wrote:
> +1. I'm still coming up to speed on RSA-PSS and the conversation at
>
> https://github.com/openssl/openssl/issues/15477
>
> but it makes me really uncomfortable to fail open rather than shut in
> this case, as part of a backport.
I could get that some users would want to be able to use such certs,
though. At least we have one such user as of this thread.
>>> Unfortunately, RFC 5929 is very clear:
>>> "if the certificate's signatureAlgorithm uses no hash functions or
>>> uses multiple hash functions, then this channel binding type's
>>> channel bindings are undefined at this time (updates to is channel
>>> binding type may occur to address this issue if it ever arises)."
>>>
>>> I understand from this sentence that if a certificate has no hash
>>> functions, then there's nothing you can do about it.
>
> But we also don't have to proceed as if everything is okay.
The RFC is kind of silly here? It just looks like a wording to give
a lot of freedom about future changes, dropping the responsibility
down to folks using it.. Reading the thread you mentioned, it looks
like even the OpenSSL folks are confused by the choice to have, and
that libcrypto should have little knowledge about which hash type to
pass down depending on the cert type, still they are enforcing the
hash type to use for some cert types.
>>> So as much as I'd like to be aggressive and potentially enforce the
>>> use of SHA256 to compute the certificate hash, what you are doing is
>>> RFC-compliant.
>>
>> Yeah, it seems silly to use anything else than SHA-256, given that
>> SCRAM-SHA-256 is totally dependent on the security of SHA-256 anyway.
>
> How will this evolve when SCRAM-SHA-512 is implemented? Do you plan to
> upgrade the undef-hash in that case to SHA-512?
Yes, that's the second part of the patch that makes me uncomfortable,
actually. If there is a SASL mechanism that supports channel binding
with a hash higher than SHA-256, the default would likely be such a
hash for functions that are weaker. For example, SCRAM-SHA-512-PLUS
would enforce SHA-512 as hash function when a cert is linked to MD5,
SHA-1 or SHA-256. So, there is a possibility that we need to compile
the hash of a certificate depending on the mechanism name, which is
known at authentication time. As proposed, the patch moves the
compilation at TLS init time. It should not make reloads much slower,
but removes the possibility to switch on-the-fly depending on the
mechanism name received from the client. It is not a problem with
only SCRAM-SHA-256 supported, but could become one in the future.
> Knee-jerk reaction: this departure feels unnecessary since Gunnar's
> certificate does in fact contain a hash algorithm...
What do you mean by Gunnar here?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../Y+V3p2k6yBUWm9N5@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-10 00:24 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 18+ messages in thread
From: Heikki Linnakangas @ 2023-02-10 00:24 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; Jacob Champion <jchampion@timescale.com>; +Cc: gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org
On 10/02/2023 00:45, Michael Paquier wrote:
> On Thu, Feb 09, 2023 at 02:22:28PM -0800, Jacob Champion wrote:
>> +1. I'm still coming up to speed on RSA-PSS and the conversation at
>>
>> https://github.com/openssl/openssl/issues/15477
>>
>> but it makes me really uncomfortable to fail open rather than shut in
>> this case, as part of a backport.
>
> I could get that some users would want to be able to use such certs,
> though. At least we have one such user as of this thread.
Note that the discussion in that thread is in a slightly different
context. They are not talking about channel binding, but something else
(https://github.com/openssl/openssl/issues/15477#issuecomment-852928702).
The problem is similar: what hash algorithm to use if it's not well
defined. But there's no guarantee that the rules they come up with will
be the same as the rules for tls-server-end-point. So I'm not very
comfortable relying on X509_digest_sig() to do the right thing for us in
the future, even if it does today.
>>>> So as much as I'd like to be aggressive and potentially enforce the
>>>> use of SHA256 to compute the certificate hash, what you are doing is
>>>> RFC-compliant.
>>>
>>> Yeah, it seems silly to use anything else than SHA-256, given that
>>> SCRAM-SHA-256 is totally dependent on the security of SHA-256 anyway.
>>
>> How will this evolve when SCRAM-SHA-512 is implemented? Do you plan to
>> upgrade the undef-hash in that case to SHA-512?
Yes, that's what I had in mind. Or for SCRAM-SHA-512, we could go
further and define it as "always use SHA-512", because we wouldn't need
to worry about backwards compatibility anymore.
>> Knee-jerk reaction: this departure feels unnecessary since Gunnar's
>> certificate does in fact contain a hash algorithm...
>
> What do you mean by Gunnar here?
Gunnar reported the bug, he's cc'd in this thread.
Ok, let me refine my earlier idea to make this more concrete:
> We could use a new "tls-server-end-point-sha-256" channel binding name
> for that, instead of "tls-server-end-point", to make it clear when we're
> violating the RFC.
Let's define a new channel binding type: tls-server-end-point-sha-256.
with "tls-server-end-point-sha-256", the SHA-256 hash of the certificate
is used, regardless of what algorithm is in the certificate's signature.
1. When the client sees a "normal" server certificate that uses SHA-256,
SHA-512, or something else that it recognizes and handles fine today, it
does the same thing as it does today. It sends "tls-server-end-point" as
the the cb-name, and calculates the hash using the certificate's hash
algorithm.
2. When the client sees a certificate with RSA-PSS, ED25519, ED448, or
any other signature method that it doesn't recognize, it selects the
tls-server-end-point-sha-256 channel binding type. It sends
"tls-server-end-point-sha-256" as the cb-name, and calculates the
SHA-256 hash of the certificate.
3. The server follows the current rules, or uses SHA-256, depending on
whether the client sent "tls-server-end-point" or
"tls-server-end-point-sha-256" in the cb-name.
These rules ensure that with certificates that work today, nothing
changes. Rule 1 ensures that you can connect with a new client to an old
server, and it still works. And rule 3. ensures that you can still
connect to a new server with an old client.
At the same time, we are using a new channel binding type to make it
clear when we are deviating from the standard rules of
"tls-server-end-point".
If you have an RSA-PSS certificate, you need to upgrade the client and
the server, or you get an error when you try to connect. (Unless you set
channel_binding=disable). But after upgrading, it will work.
- Heikki
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-10 06:32 Michael Paquier <michael@paquier.xyz>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 18+ messages in thread
From: Michael Paquier @ 2023-02-10 06:32 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Jacob Champion <jchampion@timescale.com>; gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org
On Fri, Feb 10, 2023 at 02:24:12AM +0200, Heikki Linnakangas wrote:
> Note that the discussion in that thread is in a slightly different context.
> They are not talking about channel binding, but something else
> (https://github.com/openssl/openssl/issues/15477#issuecomment-852928702).
> The problem is similar: what hash algorithm to use if it's not well defined.
> But there's no guarantee that the rules they come up with will be the same
> as the rules for tls-server-end-point. So I'm not very comfortable relying
> on X509_digest_sig() to do the right thing for us in the future, even if it
> does today.
Okay. Yes, it does not seem like a good idea in the long run to avoid
that.
>>> How will this evolve when SCRAM-SHA-512 is implemented? Do you plan to
>>> upgrade the undef-hash in that case to SHA-512?
>
> Yes, that's what I had in mind. Or for SCRAM-SHA-512, we could go further
> and define it as "always use SHA-512", because we wouldn't need to worry
> about backwards compatibility anymore.
Hmm. There is a risk of breakage here with older versions of libpq
because we'd still need to support both 256 and 512, no? Or do you
mean to drop SCRAM-SHA-256 and do a drop-in replacement with
SCRAM-SHA-512? Doing that in the server may be fine, not in libpq.
As long as you control that with the channel binding name, there
should be enough control I guess.
> Let's define a new channel binding type: tls-server-end-point-sha-256. with
> "tls-server-end-point-sha-256", the SHA-256 hash of the certificate is used,
> regardless of what algorithm is in the certificate's signature.
>
> 1. When the client sees a "normal" server certificate that uses SHA-256,
> SHA-512, or something else that it recognizes and handles fine today, it
> does the same thing as it does today. It sends "tls-server-end-point" as the
> the cb-name, and calculates the hash using the certificate's hash algorithm.
>
> 2. When the client sees a certificate with RSA-PSS, ED25519, ED448, or any
> other signature method that it doesn't recognize, it selects the
> tls-server-end-point-sha-256 channel binding type. It sends
> "tls-server-end-point-sha-256" as the cb-name, and calculates the SHA-256
> hash of the certificate.
>
> 3. The server follows the current rules, or uses SHA-256, depending on
> whether the client sent "tls-server-end-point" or
> "tls-server-end-point-sha-256" in the cb-name.
>
> These rules ensure that with certificates that work today, nothing changes.
> Rule 1 ensures that you can connect with a new client to an old server, and
> it still works. And rule 3. ensures that you can still connect to a new
> server with an old client.
>
> At the same time, we are using a new channel binding type to make it clear
> when we are deviating from the standard rules of "tls-server-end-point".
So, using a new channel binding name with tls-server-end-point-sha-256
is actually the way to violate the RFC, while tls-server-end-point
still tries to stick with it? I am not sure if there's much benefit
in a second channel binding, enforcing SHA-256 for undefined
signatures could just be better, and simpler. :)
There be a risk of a downgrade attack here, where the client could
feign the server to do a SHA-256 computation where SHA-512 or higher
should be used. Or, if the server certificate has a hash function but
the client feigns in not knowing that, do you mean to go through a
mock authentication in this case and fail?
> If you have an RSA-PSS certificate, you need to upgrade the client and the
> server, or you get an error when you try to connect. (Unless you set
> channel_binding=disable). But after upgrading, it will work.
Indeed.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../Y+Xk8N%2FatQixeE2%2F@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-10 09:33 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 18+ messages in thread
From: Heikki Linnakangas @ 2023-02-10 09:33 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Jacob Champion <jchampion@timescale.com>; gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org
On 10/02/2023 08:32, Michael Paquier wrote:
> On Fri, Feb 10, 2023 at 02:24:12AM +0200, Heikki Linnakangas wrote:
>>>> How will this evolve when SCRAM-SHA-512 is implemented? Do you
>>>> plan to upgrade the undef-hash in that case to SHA-512?
>>
>> Yes, that's what I had in mind. Or for SCRAM-SHA-512, we could go
>> further and define it as "always use SHA-512", because we wouldn't
>> need to worry about backwards compatibility anymore.
>
> Hmm. There is a risk of breakage here with older versions of libpq
> because we'd still need to support both 256 and 512, no? Or do you
> mean to drop SCRAM-SHA-256 and do a drop-in replacement with
> SCRAM-SHA-512? Doing that in the server may be fine, not in libpq.
> As long as you control that with the channel binding name, there
> should be enough control I guess.
We would still support for SCRAM-SHA-256-PLUS in client and server. But
*if* the server supports and advertises SCRAM-SHA-512-PLUS as a
supported mechanism, and the client supports and selects it, then with
SCRAM-SHA-512-PLUS, we could always use SHA-512 as the hash algorithm
for the channel binding.
Yeah, a MITM could force a downgrade from SCRAM-SHA-512 to
SCRAM-SHA-256, by modifying the list of supported SASL mechanisms that
the server sends. Then again, usually the server would not advertise
both SCRAM-SHA-256 and SCRAM-SHA-512, because pg_role contains only one
password verifier. Unless we change that too. I don't have a solution
for these issues right now, but they are not related to channel binding.
The point is that if we add a new SASL mechanism, like
SCRAM-SHA-512-PLUS, we are free to define how to calculate the
certificate hash with that mechanism however we want, without breaking
backwards compatibility.
> So, using a new channel binding name with
> tls-server-end-point-sha-256 is actually the way to violate the RFC,
> while tls-server-end-point still tries to stick with it?
Correct.
> I am not sure if there's much benefit in a second channel binding,
> enforcing SHA-256 for undefined signatures could just be better, and
> simpler. :)
The benefit is that it's more explicit, which helps to reduce the
confusion. As new signature algorithms are invented in the future, who
knows what OBJ_find_sigid_algs() in different versions of OpenSSL, or
other TLS libraries, will do with them. With a new
"tls-server-end-point-sha-256" channel binding type, it's clear.
> There be a risk of a downgrade attack here, where the client could
> feign the server to do a SHA-256 computation where SHA-512 or higher
> should be used. Or, if the server certificate has a hash function
> but the client feigns in not knowing that, do you mean to go through
> a mock authentication in this case and fail?
Let's always allow tls-server-end-point-sha-256 in the server. The point
of channel binding is to protect the client from a MITM. The client
chooses the channel binding type to use, so a MITM cannot force it to
downgrade.
To be honest, I am not at all worried about losing security by using
SHA-256 instead of SHA-512 or whatever else is in the certificate. If
you don't trust SHA-256, you wouldn't be comfortable using SCRAM-SHA-256
at all.
"tls-server-end-point-sha-256" is simpler than the current rules, so new
client implementations that are not worried about
backwards-compatibility might not bother with the current rules at all,
and only implement "tls-server-end-point-sha-256".
- Heikki
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-10 14:02 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 0 replies; 18+ messages in thread
From: Heikki Linnakangas @ 2023-02-10 14:02 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: gunnar.bluth@pro-open.de, pgsql-bugs@lists.postgresql.org, Jacob Champion <jchampion@timescale.com>
The new "tls-server-end-point-sha-256" channel binding type now seems
like the best approach to me, but I still wanted to reply to this:
On 09/02/2023 10:24, Michael Paquier wrote:
> But the client has the choice to decide if it wants to use channel
> binding, does it? In this case, it would send the non-PLUS mechanism
> followed by 'n' as gs2-cbind-flag, no?
No, that makes a downgrade attack possible with channel_binding=prefer.
Imagine that you have a server with a typical RSA certificate that
supports channel binding. And a client with channel_binding=prefer.
There is a Man-In-The-Middle between the client and the server. The MITM
works as a proxy, and opens two separate TLS connections: server <->
MITM and MITM <-> client. To the client, it presents a TLS server
certificate that uses 'rsassaPss'. The client connects to the MITM, sees
the rsassaPss certificate, and decides that it cannot do channel binding
because the server doesn't support it. It sends 'n' gs2-cbind-flag, and
the server happily accepts that.
Currently, such a downgrade attack is not possible. If client and server
both support channel binding, it will be used. If a MITM tries to modify
the SASL mechanism list, leaving out SCRAM-SHA-256-PLUS, the client will
use gs2-cbind-flag='y' which notifies the server that the mechanism list
was modified, and the server will fail the authentication. And if the
MITM doesn't change the negotiation, then channel binding will be used,
and the authentication will fail because the client and server will
compute a different server certificate hash.
There is no way for the client to tell the server "I support channel
binding in general, but not with this server certificate".
- Heikki
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-11 00:13 Jacob Champion <jchampion@timescale.com>
parent: Michael Paquier <michael@paquier.xyz>
1 sibling, 1 reply; 18+ messages in thread
From: Jacob Champion @ 2023-02-11 00:13 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>; gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org
On Thu, Feb 9, 2023 at 2:46 PM Michael Paquier <michael@paquier.xyz> wrote:
> I could get that some users would want to be able to use such certs,
> though. At least we have one such user as of this thread.
My take on this bug is that Gunnar doesn't need to solve the general
"undef" case. They've got a certificate that's supposed to be using
SHA512.
It looks like OpenSSL 1.1.1 gives us a better API for grabbing that;
see attached draft (not mesonified, needs independent verification),
which fixes Heikki's certificate case at least. It's unfortunate that
it doesn't reach back to 1.0.2, or to LibreSSL, but that doesn't
appear to be a problem for Gunnar's situation, and you'd mentioned
wanting to drop 1.0.2 support in HEAD soon anyway.
Maybe someone really wants to use EdDSA certs, which aren't handled by
that API. But this stopgap would buy us some time for the
cryptographers to settle on things -- or at least to ask them? And if
people want new crypto they're going to need to upgrade eventually.
(Maybe by that point we'll know that X509_digest_sig() is in fact
correct for bindings.)
<strong opinions>
- Our current departures from the spec (e.g. no tls-unique) mean that
we already can't interoperate with standard SASL libraries. I've been
trying to realign them, slowly.
- Coming up with our own binding type takes time and resources away
from other things this thread highlighted (the ability to force the
use of a binding at the server side. better behavior when we can't
actually compute a binding value. tls-exporter support...).
- Worst case, using SHA256 for a future certificate type might be
*catastrophically* wrong, but no one's going to warn us, and it's not
going to be obvious to the DBA or their users that our nonstandard
binding is in effect.
- Best case, we choose exactly right, but if/when tls-server-end-point
gets updated for EdDSA, the world will move on and we'll still have to
support that vestigial binding type for the next X years.
</strong opinions>
--Jacob
Attachments:
[application/x-patch] WIP-handle-RSA-PSS-certs.patch (5.9K, ../../CAAWbhmgjYym7AsH1fqOx+bNqctPpSW1DzyLv_0VhBa_ng+NVyQ@mail.gmail.com/2-WIP-handle-RSA-PSS-certs.patch)
download | inline diff:
From f7fe7fce4cf5172743a12eb0958565eba216176b Mon Sep 17 00:00:00 2001
From: Jacob Champion <jchampion@timescale.com>
Date: Fri, 10 Feb 2023 13:28:04 -0800
Subject: [PATCH] WIP: handle RSA-PSS certs
---
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 +-
7 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/configure b/configure
index 2270417f8e..573baee45f 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
+
+ # Functions 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 7ffe3b9a01..fb5335c230 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])
+ # Functions 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..1c67969b3e 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..4a2d32c1b2 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
--
2.25.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-11 10:58 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Jacob Champion <jchampion@timescale.com>
0 siblings, 1 reply; 18+ messages in thread
From: Heikki Linnakangas @ 2023-02-11 10:58 UTC (permalink / raw)
To: Jacob Champion <jchampion@timescale.com>; Michael Paquier <michael@paquier.xyz>; +Cc: gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org
On 11/02/2023 02:13, Jacob Champion wrote:
> On Thu, Feb 9, 2023 at 2:46 PM Michael Paquier <michael@paquier.xyz> wrote:
>> I could get that some users would want to be able to use such certs,
>> though. At least we have one such user as of this thread.
>
> My take on this bug is that Gunnar doesn't need to solve the general
> "undef" case. They've got a certificate that's supposed to be using
> SHA512.
>
> It looks like OpenSSL 1.1.1 gives us a better API for grabbing that;
> see attached draft (not mesonified, needs independent verification),
> which fixes Heikki's certificate case at least. It's unfortunate that
> it doesn't reach back to 1.0.2, or to LibreSSL, but that doesn't
> appear to be a problem for Gunnar's situation, and you'd mentioned
> wanting to drop 1.0.2 support in HEAD soon anyway.
A-ha! I browsed around OpenSSL APIs and the source code, but missed
X509_get_signature_info(). Good find!
X509_get_signature_info() calls X509_check_purpose(), which calls
internal function ossl_x509v3_cache_extensions(), which extracts and
caches quite a lot of information from the certificate. It calculates
and caches its SHA1 hash, for example. That seems acceptable, the
overhead is negligible and I don't see any scenario where
X509_get_signature_nid() would succeed but X509_get_signature_info()
would fail.
+1 on your patch. I think the only thing it's missing is changes in
meson.build and Solution.pm to match the configure.ac changes.
> Maybe someone really wants to use EdDSA certs, which aren't handled by
> that API. But this stopgap would buy us some time for the
> cryptographers to settle on things -- or at least to ask them? And if
> people want new crypto they're going to need to upgrade eventually.
> (Maybe by that point we'll know that X509_digest_sig() is in fact
> correct for bindings.)
Agreed, if we have an easy solution for RSA-PSS, that's good enough for now.
> <strong opinions>
> - Our current departures from the spec (e.g. no tls-unique) mean that
> we already can't interoperate with standard SASL libraries. I've been
> trying to realign them, slowly.
> - Coming up with our own binding type takes time and resources away
> from other things this thread highlighted (the ability to force the
> use of a binding at the server side. better behavior when we can't
> actually compute a binding value. tls-exporter support...).
> - Worst case, using SHA256 for a future certificate type might be
> *catastrophically* wrong, but no one's going to warn us, and it's not
> going to be obvious to the DBA or their users that our nonstandard
> binding is in effect.
> - Best case, we choose exactly right, but if/when tls-server-end-point
> gets updated for EdDSA, the world will move on and we'll still have to
> support that vestigial binding type for the next X years.
> </strong opinions>
Fair points.
- Heikki
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-13 01:58 Michael Paquier <michael@paquier.xyz>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 18+ messages in thread
From: Michael Paquier @ 2023-02-13 01:58 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Jacob Champion <jchampion@timescale.com>; gunnar.bluth@pro-open.de; pgsql-bugs@lists.postgresql.org
On Sat, Feb 11, 2023 at 12:58:02PM +0200, Heikki Linnakangas wrote:
> X509_get_signature_info() calls X509_check_purpose(), which calls internal
> function ossl_x509v3_cache_extensions(), which extracts and caches quite a
> lot of information from the certificate. It calculates and caches its SHA1
> hash, for example. That seems acceptable, the overhead is negligible and I
> don't see any scenario where X509_get_signature_nid() would succeed but
> X509_get_signature_info() would fail.
Excellent find! On 1.1.1, that's x509v3_cache_extensions, it seems.
The extra overhead at authentication does not worry me, FWIW. I am
wondering why the OpenSSL folks have done nothing for
X509_get_signature_nid() in 1.1.1~ in this case, but whatever.
From my notes, I was also wondering whether we should improve the
situation with the two following things:
- Addition of a new GUC called, say, ssl_check_cert_hash to check at
TLS init time in the backend if a certificate's hash can be found or
not.
- Improvement of error messages in this area when a hash function
cannot be found.
However, this proves to be unnecessary once we use
X509_get_signature_info() as loading an RSA-PSS cert with OpenSSL
1.0.2 or 1.1.0 leads to this error, simply:
FATAL: could not load server certificate file "server.crt": unsupported algorithm
Would more facility make sense for other cert types where OpenSSL
cannot map with these yet, though? I am not really convinced that we
need to burn more resources until we get a complaint and adapt again,
as it may be possible that OpenSSL also improves in-between for such
cases.
The SSL tests need to be patched so as they adapt on-the-fly depending
on if RSA-PSS is supported, of course, and we cannot do a
switch_server_cert() for versions older than 1.1.1.
> +1 on your patch. I think the only thing it's missing is changes in
> meson.build and Solution.pm to match the configure.ac changes.
Done.
> Agreed, if we have an easy solution for RSA-PSS, that's good enough
> for now.
Indeed.
With all that in mind, I am finishing with the attached with the
tests, the meson tweaks and the MSVC tweaks. I have tested it on
HEAD, with OpenSSL down to 1.0.1 which is the minimum version of this
branch. Making sure that this mostly works with 1.0.0 and 0.9.8 on
older branches would not be an issue here.
--
Michael
Attachments:
[text/x-diff] 0001-Fix-handling-of-RSS-PSA-certificates-with-channel-bi.patch (16.2K, ../../Y+mZWf6NCCHiTjzM@paquier.xyz/2-0001-Fix-handling-of-RSS-PSA-certificates-with-channel-bi.patch)
download | inline diff:
From bf83fdbb48b3d5b5ea6013a3a1475818c353e018 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Mon, 13 Feb 2023 10:51:52 +0900
Subject: [PATCH] Fix handling of RSS-PSA certificates with channel binding
TODO, write me, please..
This issue exists since channel binding exists, so backpatch all the way
down.
Reported-by: Gunnar "Nick" Bluth
Author: Jacob Champion, Heikki Linnakangas
Discussion: https://postgr.es/m/17760-b6c61e752ec07060@postgresql.org
Backpatch-through: 11
---
src/include/libpq/libpq-be.h | 2 +-
src/include/pg_config.h.in | 3 +++
src/backend/libpq/be-secure-openssl.c | 9 ++++++--
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 ++++++++++++++
configure | 12 ++++++++++
configure.ac | 2 ++
meson.build | 3 +++
src/tools/msvc/Solution.pm | 10 ++++++++-
15 files changed, 148 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/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 8c70b2fd5b..ac6407e9f6 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -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
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index c5a80b829e..2490bf8ace 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -523,6 +523,9 @@
/* 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
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index e3c7c12aa0..0bde8dd3a9 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -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");
/*
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 983536de25..40829177c0 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -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;
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index d94b648ea5..d7ec5ed429 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -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
diff --git a/src/test/ssl/README b/src/test/ssl/README
index b328203c7c..2101a466d2 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.
Note
====
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 5d9dc09a4b..e63342469d 100644
--- a/src/test/ssl/sslfiles.mk
+++ b/src/test/ssl/sslfiles.mk
@@ -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 $@
diff --git a/src/test/ssl/t/002_scram.pl b/src/test/ssl/t/002_scram.pl
index 0f3d180cfa..1d3905d3a1 100644
--- a/src/test/ssl/t/002_scram.pl
+++ b/src/test/ssl/t/002_scram.pl
@@ -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();
diff --git a/configure b/configure
index 5d07fd0bb9..7bb829ddf4 100755
--- 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
diff --git a/configure.ac b/configure.ac
index e9b74ced6c..137a40a942 100644
--- a/configure.ac
+++ b/configure.ac
@@ -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])
diff --git a/meson.build b/meson.build
index e379a252a5..b5daed9f38 100644
--- a/meson.build
+++ b/meson.build
@@ -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
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 355ea0f9e5..5399050492 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -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'))
{
--
2.39.1
[application/pgp-signature] signature.asc (832B, ../../Y+mZWf6NCCHiTjzM@paquier.xyz/3-signature.asc)
download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-13 12:54 Gunnar "Nick" Bluth <gunnar.bluth@pro-open.de>
parent: Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 18+ messages in thread
From: Gunnar \"Nick\" Bluth @ 2023-02-13 12:54 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; Heikki Linnakangas <hlinnaka@iki.fi>; +Cc: Jacob Champion <jchampion@timescale.com>; pgsql-bugs@lists.postgresql.org
Am 13.02.23 um 02:58 schrieb Michael Paquier:
[...]
>> Agreed, if we have an easy solution for RSA-PSS, that's good enough
>> for now.
>
> Indeed.
>
> With all that in mind, I am finishing with the attached with the
> tests, the meson tweaks and the MSVC tweaks. I have tested it on
> HEAD, with OpenSSL down to 1.0.1 which is the minimum version of this
> branch. Making sure that this mostly works with 1.0.0 and 0.9.8 on
> older branches would not be an issue here.
FWIW:
I only understand a fraction of the underlying crypto, but the patch LGTM!
Much appreciated, thanks for the effort (and sorry for causing it ;-)!
--
Gunnar "Nick" Bluth
Eimermacherweg 106
D-48159 Münster
Mobil +49 172 8853339
Email: gunnar.bluth@pro-open.de
__________________________________________________________________________
"Ceterum censeo SystemD esse delendam" - Cato
Attachments:
[application/pgp-signature] OpenPGP_signature (494B, ../../29c8e8cb-d476-727e-c257-3f9cfa6ed527@pro-open.de/2-OpenPGP_signature)
download
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-13 17:44 Jacob Champion <jchampion@timescale.com>
parent: Gunnar "Nick" Bluth <gunnar.bluth@pro-open.de>
0 siblings, 1 reply; 18+ messages in thread
From: Jacob Champion @ 2023-02-13 17:44 UTC (permalink / raw)
To: Gunnar Nick Bluth <gunnar.bluth@pro-open.de>; +Cc: Michael Paquier <michael@paquier.xyz>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
On Mon, Feb 13, 2023 at 4:54 AM Gunnar "Nick" Bluth
<gunnar.bluth@pro-open.de> wrote:
> FWIW:
> I only understand a fraction of the underlying crypto, but the patch LGTM!
LGTM too, thanks Michael! I tested against LibreSSL 3.5.3 to
double-check the fallback.
--Jacob
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate
@ 2023-02-15 01:22 Michael Paquier <michael@paquier.xyz>
parent: Jacob Champion <jchampion@timescale.com>
0 siblings, 0 replies; 18+ messages in thread
From: Michael Paquier @ 2023-02-15 01:22 UTC (permalink / raw)
To: Jacob Champion <jchampion@timescale.com>; +Cc: Gunnar Nick Bluth <gunnar.bluth@pro-open.de>; Heikki Linnakangas <hlinnaka@iki.fi>; pgsql-bugs@lists.postgresql.org
On Mon, Feb 13, 2023 at 09:44:03AM -0800, Jacob Champion wrote:
> LGTM too, thanks Michael! I tested against LibreSSL 3.5.3 to
> double-check the fallback.
Thanks for checking with this one, I don't have LibreSSL in my
environment, at least not now. Perhaps I should.. So, I have spent a
couple of hours on that, and backpatched the fix down to 11. There
were different conflicts for each branch.
The new tests have been added in 15~, where the generation of the cert
and key files is more straight-forward than ~14. Actually, make
sslfiles fails on these branches when using OpenSSL 1.1.1~. Perhaps
that may be worth addressing, but the existing tests pass anyway when
relying on X509_get_signature_info(), as much as they pass with older
versions of OpenSSL. I have done some manual checks with RSA-PSS
certs and keys to make sure that channel binding works correctly for
these versions (one can just reuse the ones generated on HEAD or
REL_15_STABLE in src/test/ssl/ for that).
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../../Y+wz7kNLIYnvEBgB@paquier.xyz/2-signature.asc)
download
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2023-02-15 01:22 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-01-25 11:32 BUG #17760: SCRAM authentication fails with "modern" (rsassaPss signature) server certificate PG Bug reporting form <noreply@postgresql.org>
2023-01-31 12:27 ` Gunnar "Nick" Bluth <gunnar.bluth@pro-open.de>
2023-02-05 01:28 ` Heikki Linnakangas <hlinnaka@iki.fi>
2023-02-08 16:11 ` Heikki Linnakangas <hlinnaka@iki.fi>
2023-02-09 08:24 ` Michael Paquier <michael@paquier.xyz>
2023-02-09 21:54 ` Heikki Linnakangas <hlinnaka@iki.fi>
2023-02-09 22:22 ` Jacob Champion <jchampion@timescale.com>
2023-02-09 22:45 ` Michael Paquier <michael@paquier.xyz>
2023-02-10 00:24 ` Heikki Linnakangas <hlinnaka@iki.fi>
2023-02-10 06:32 ` Michael Paquier <michael@paquier.xyz>
2023-02-10 09:33 ` Heikki Linnakangas <hlinnaka@iki.fi>
2023-02-11 00:13 ` Jacob Champion <jchampion@timescale.com>
2023-02-11 10:58 ` Heikki Linnakangas <hlinnaka@iki.fi>
2023-02-13 01:58 ` Michael Paquier <michael@paquier.xyz>
2023-02-13 12:54 ` Gunnar "Nick" Bluth <gunnar.bluth@pro-open.de>
2023-02-13 17:44 ` Jacob Champion <jchampion@timescale.com>
2023-02-15 01:22 ` Michael Paquier <michael@paquier.xyz>
2023-02-10 14:02 ` Heikki Linnakangas <hlinnaka@iki.fi>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox