public inbox for [email protected]
help / color / mirror / Atom feedFrom: Heikki Linnakangas <[email protected]>
To: Michael Paquier <[email protected]>
Cc: Jacob Champion <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: Direct SSL connection with ALPN and HBA rules
Date: Tue, 23 Apr 2024 01:48:04 +0300
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<[email protected]>
<CAOYmi+kbYPX78nu0k7OODxUhoD6H1OXdugiMK1z9Nn33G-qdcQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
On 22/04/2024 10:47, Heikki Linnakangas wrote:
> On 22/04/2024 10:19, Michael Paquier wrote:
>> On Sat, Apr 20, 2024 at 12:43:24AM +0300, Heikki Linnakangas wrote:
>>> On 19/04/2024 19:48, Jacob Champion wrote:
>>>> On Fri, Apr 19, 2024 at 6:56 AM Heikki Linnakangas <[email protected]> wrote:
>>>>> With direct SSL negotiation, we always require ALPN.
>>>>
>>>> (As an aside: I haven't gotten to test the version of the patch that
>>>> made it into 17 yet, but from a quick glance it looks like we're not
>>>> rejecting mismatched ALPN during the handshake as noted in [1].)
>>>
>>> Ah, good catch, that fell through the cracks. Agreed, the client should
>>> reject a direct SSL connection if the server didn't send ALPN. I'll add that
>>> to the Open Items so we don't forget again.
>>
>> Would somebody like to write a patch for that? I'm planning to look
>> at this code more closely, as well.
>
> I plan to write the patch later today.
Here's the patch for that. The error message is:
"direct SSL connection was established without ALPN protocol negotiation
extension"
That's accurate, but I wonder if we could make it more useful to a user
who's wondering what went wrong. I'd imagine that if the server doesn't
support ALPN, it's because you have some kind of a (not necessarily
malicious) generic SSL man-in-the-middle that doesn't support it. Or
you're trying to connect to an HTTPS server. Suggestions welcome.
--
Heikki Linnakangas
Neon (https://neon.tech)
Attachments:
[text/x-patch] require-alpn-in-direct-mode.patch (2.5K, ../[email protected]/2-require-alpn-in-direct-mode.patch)
download | inline diff:
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index ec20e3f3a9..f9156e29ca 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -3524,6 +3524,13 @@ keep_going: /* We will come back to here until there is
pollres = pqsecure_open_client(conn);
if (pollres == PGRES_POLLING_OK)
{
+ /* ALPN is mandatory with direct SSL negotiation */
+ if (conn->current_enc_method == ENC_DIRECT_SSL && !conn->ssl_alpn_used)
+ {
+ libpq_append_conn_error(conn, "direct SSL connection was established without ALPN protocol negotiation extension");
+ CONNECTION_FAILED();
+ }
+
/*
* At this point we should have no data already buffered.
* If we do, it was received before we performed the SSL
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index e7a4d006e1..8df3c751db 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1585,6 +1585,31 @@ open_client_SSL(PGconn *conn)
}
}
+ /* Get the protocol selected by ALPN */
+ {
+ const unsigned char *selected;
+ unsigned int len;
+
+ SSL_get0_alpn_selected(conn->ssl, &selected, &len);
+
+ /* If ALPN is used, check that we negotiated the expected protocol */
+ if (selected != NULL)
+ {
+ if (len == strlen(PG_ALPN_PROTOCOL) &&
+ memcmp(selected, PG_ALPN_PROTOCOL, strlen(PG_ALPN_PROTOCOL)) == 0)
+ {
+ conn->ssl_alpn_used = true;
+ }
+ else
+ {
+ /* shouldn't happen */
+ libpq_append_conn_error(conn, "SSL connection was established with unexpected ALPN protocol");
+ pgtls_close(conn);
+ return PGRES_POLLING_FAILED;
+ }
+ }
+ }
+
/*
* We already checked the server certificate in initialize_SSL() using
* SSL_CTX_set_verify(), if root.crt exists.
@@ -1632,6 +1657,7 @@ pgtls_close(PGconn *conn)
conn->ssl = NULL;
conn->ssl_in_use = false;
conn->ssl_handshake_started = false;
+ conn->ssl_alpn_used = false;
destroy_needed = true;
}
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
index 3691e5ee96..a6792917cf 100644
--- a/src/interfaces/libpq/libpq-int.h
+++ b/src/interfaces/libpq/libpq-int.h
@@ -569,6 +569,7 @@ struct pg_conn
bool ssl_handshake_started;
bool ssl_cert_requested; /* Did the server ask us for a cert? */
bool ssl_cert_sent; /* Did we send one in reply? */
+ bool ssl_alpn_used; /* Did we negotiate a protocol with TLS ALPN? */
#ifdef USE_SSL
#ifdef USE_OPENSSL
view thread (28+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: Direct SSL connection with ALPN and HBA rules
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox