tls 1.3: sending multiple tickets
Hi,
To investigate an unrelated issue, I set up key logging in the backend (we
probably should build that in) and looked at the decrypted data. And noticed
that just after TLS setup finished the server sends three packets in a row:
C->S: TLSv1.3: finished
C->S: TLSv1.3: application data (startup message)
S->C: TLSv1.3: new session ticket
S->C: TLSv1.3: new session ticket
S->C: TLSv1.3: application data (authentication ok, parameter status+)
We try to turn off session resumption, but not completely enough for 1.3:
SSL_OP_NO_TICKET
SSL/TLS supports two mechanisms for resuming sessions: session ids and stateless session tickets.
When using session ids a copy of the session information is cached on the server and a unique id is sent to the client. When the client wishes to
resume it provides the unique id so that the server can retrieve the session information from its cache.
When using stateless session tickets the server uses a session ticket encryption key to encrypt the session information. This encrypted data is
sent to the client as a "ticket". When the client wishes to resume it sends the encrypted data back to the server. The server uses its key to
decrypt the data and resume the session. In this way the server can operate statelessly - no session information needs to be cached locally.
The TLSv1.3 protocol only supports tickets and does not directly support session ids. However, OpenSSL allows two modes of ticket operation in
TLSv1.3: stateful and stateless. Stateless tickets work the same way as in TLSv1.2 and below. Stateful tickets mimic the session id behaviour
available in TLSv1.2 and below. The session information is cached on the server and the session id is wrapped up in a ticket and sent back to the
client. When the client wishes to resume, it presents a ticket in the same way as for stateless tickets. The server can then extract the session id
from the ticket and retrieve the session information from its cache.
By default OpenSSL will use stateless tickets. The SSL_OP_NO_TICKET option will cause stateless tickets to not be issued. In TLSv1.2 and below this
means no ticket gets sent to the client at all. In TLSv1.3 a stateful ticket will be sent. This is a server-side option only.
In TLSv1.3 it is possible to suppress all tickets (stateful and stateless) from being sent by calling SSL_CTX_set_num_tickets(3) or
SSL_set_num_tickets(3).
Note the second to last paragraph: Because we use SSL_OP_NO_TICKET we trigger
use of stateful tickets. Which afaict are never going to be useful, because we
don't share the necessary state.
I guess openssl really could have inferred this from the fact that we *do*
call SSL_CTX_set_session_cache_mode(SSL_SESS_CACHE_OFF), b
Seems we ought to use SSL_CTX_set_num_tickets() to prevent issuing the useless
tickets?
It seems like a buglet in openssl that it forces each session tickets to be
sent in its own packet (it does an explicit BIO_flush(), so even if we
buffered between openssl and OS, as I think we should, we'd still send it
separately), but I don't really understand most of this stuff.
Greetings,
Andres Freund
On 17 Jun 2024, at 19:38, Andres Freund <andres@anarazel.de> wrote:
Note the second to last paragraph: Because we use SSL_OP_NO_TICKET we trigger
use of stateful tickets. Which afaict are never going to be useful, because we
don't share the necessary state.
Nice catch, I learned something new today. I was under the impression that the
flag turned of all tickets but clearly not.
I guess openssl really could have inferred this from the fact that we *do*
call SSL_CTX_set_session_cache_mode(SSL_SESS_CACHE_OFF), b
Every day with the OpenSSL API is an adventure.
Seems we ought to use SSL_CTX_set_num_tickets() to prevent issuing the useless
tickets?
Agreed, in 1.1.1 and above as the API was only introduced then. LibreSSL added
the API in 3.5.4 but only for compatibility since it doesn't support TLS
tickets at all.
It seems like a buglet in openssl that it forces each session tickets to be
sent in its own packet (it does an explicit BIO_flush(), so even if we
buffered between openssl and OS, as I think we should, we'd still send it
separately), but I don't really understand most of this stuff.
I don't see anything in the RFCs so not sure.
The attached applies this, and I think this is backpatching material since we
arguably fail to do what we say in the code. AFAIK we don't have a hard rule
against backpatching changes to autoconf/meson?
--
Daniel Gustafsson
Attachments:
v1-0001-Disable-all-TLS-session-tickets.patchapplication/octet-stream; name=v1-0001-Disable-all-TLS-session-tickets.patch; x-unix-mode=0644Download
From 5c21ebe072d1c877d7fe8f676879b9a8af76eb1c Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Tue, 18 Jun 2024 14:50:39 +0200
Subject: [PATCH v1] Disable all TLS session tickets
OpenSSL supports two types of session tickets for TLSv1.3, stateless
and stateful. The option we've used only turns off stateless tickets
leaving stateful tickets active. Use the new API introduced in 1.1.1
to disable all types of tickets.
Reported-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/20240617173803.6alnafnxpiqvlh3g@awork3.anarazel.de
---
configure | 9 +++++----
configure.ac | 2 +-
meson.build | 1 +
src/backend/libpq/be-secure-openssl.c | 14 +++++++++++++-
src/include/pg_config.h.in | 3 +++
5 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/configure b/configure
index 7b03db56a6..519a2e5ce6 100755
--- a/configure
+++ b/configure
@@ -12591,12 +12591,13 @@ fi
done
# Function introduced in OpenSSL 1.1.1.
- for ac_func in X509_get_signature_info
+ for ac_func in X509_get_signature_info SSL_CTX_set_num_tickets
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 :
+ as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
+ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
+if eval test \"x\$"$as_ac_var"\" = x"yes"; then :
cat >>confdefs.h <<_ACEOF
-#define HAVE_X509_GET_SIGNATURE_INFO 1
+#define `$as_echo "HAVE_$ac_func" | $as_tr_cpp` 1
_ACEOF
fi
diff --git a/configure.ac b/configure.ac
index 63e7be3847..6e28c2d04c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1358,7 +1358,7 @@ if test "$with_ssl" = openssl ; then
# function was removed.
AC_CHECK_FUNCS([CRYPTO_lock])
# Function introduced in OpenSSL 1.1.1.
- AC_CHECK_FUNCS([X509_get_signature_info])
+ AC_CHECK_FUNCS([X509_get_signature_info SSL_CTX_set_num_tickets])
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 2767abd19e..e21f88b529 100644
--- a/meson.build
+++ b/meson.build
@@ -1293,6 +1293,7 @@ if sslopt in ['auto', 'openssl']
# Function introduced in OpenSSL 1.1.1
['X509_get_signature_info'],
+ ['SSL_CTX_set_num_tickets'],
]
are_openssl_funcs_complete = true
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 39b1a66236..d257b93104 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -258,8 +258,20 @@ be_tls_init(bool isServerStart)
}
}
- /* disallow SSL session tickets */
+ /*
+ * Disallow SSL session tickets. OpenSSL use both stateful and stateless
+ * tickets for TLSv1.3, and stateless ticket for TLSv1.2. SSL_OP_NO_TICKET
+ * is available since 1.0.0 but only turns off stateless tickets. In order
+ * to turn off stateful tickets we also need SSL_CTX_set_num_tickets, which
+ * is available since OpenSSL 1.1.1. LibreSSL 3.5.4 (from OpenBSD 7.1)
+ * introduced this API for compatibility, but doesn't support session
+ * tickets at all so it's a no-op there.
+ */
+#ifdef HAVE_SSL_CTX_SET_NUM_TICKETS
+ SSL_CTX_set_num_tickets(context, 0);
+#else
SSL_CTX_set_options(context, SSL_OP_NO_TICKET);
+#endif
/* disallow SSL session caching, too */
SSL_CTX_set_session_cache_mode(context, SSL_SESS_CACHE_OFF);
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index f8d3e3b6b8..e2834912f8 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -510,6 +510,9 @@
/* 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 `SSL_CTX_set_num_tickets' function. */
+#undef HAVE_SSL_CTX_SET_NUM_TICKETS
+
/* Define to 1 if the assembler supports X86_64's POPCNTQ instruction. */
#undef HAVE_X86_64_POPCNTQ
--
2.39.3 (Apple Git-146)
On 18/06/2024 16:11, Daniel Gustafsson wrote:
On 17 Jun 2024, at 19:38, Andres Freund <andres@anarazel.de> wrote:
Seems we ought to use SSL_CTX_set_num_tickets() to prevent issuing the useless
tickets?Agreed, in 1.1.1 and above as the API was only introduced then. LibreSSL added
the API in 3.5.4 but only for compatibility since it doesn't support TLS
tickets at all.
Wow, that's a bizarre API. The OpenSSL docs are not clear on what the
possible values for SSL_CTX_set_num_tickets() are. It talks about 0, and
mentions that 2 is the default, but what does it mean to set it to 1, or
5, for example?
Anyway, it's pretty clear that SSL_CTX_set_num_tickets(0) can be used to
disable tickets, so that's fine.
It seems like a buglet in openssl that it forces each session tickets to be
sent in its own packet (it does an explicit BIO_flush(), so even if we
buffered between openssl and OS, as I think we should, we'd still send it
separately), but I don't really understand most of this stuff.I don't see anything in the RFCs so not sure.
The attached applies this, and I think this is backpatching material since we
arguably fail to do what we say in the code. AFAIK we don't have a hard rule
against backpatching changes to autoconf/meson?
Looks good to me. Backpatching autoconf/meson changes is fine, we've
done it before.
--
Heikki Linnakangas
Neon (https://neon.tech)
On 24 Jul 2024, at 07:44, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 18/06/2024 16:11, Daniel Gustafsson wrote:
On 17 Jun 2024, at 19:38, Andres Freund <andres@anarazel.de> wrote:
Seems we ought to use SSL_CTX_set_num_tickets() to prevent issuing the useless
tickets?Agreed, in 1.1.1 and above as the API was only introduced then. LibreSSL added
the API in 3.5.4 but only for compatibility since it doesn't support TLS
tickets at all.Wow, that's a bizarre API. The OpenSSL docs are not clear on what the possible values for SSL_CTX_set_num_tickets() are. It talks about 0, and mentions that 2 is the default, but what does it mean to set it to 1, or 5, for example?
It means that 1 or 5 tickets can be sent to the user, OpenSSL accepts an
arbitrary number of tickets (tickets can be issued at other points during the
connection than the handshake AFAICT).
Anyway, it's pretty clear that SSL_CTX_set_num_tickets(0) can be used to disable tickets, so that's fine.
It seems like a buglet in openssl that it forces each session tickets to be
sent in its own packet (it does an explicit BIO_flush(), so even if we
buffered between openssl and OS, as I think we should, we'd still send it
separately), but I don't really understand most of this stuff.I don't see anything in the RFCs so not sure.
The attached applies this, and I think this is backpatching material since we
arguably fail to do what we say in the code. AFAIK we don't have a hard rule
against backpatching changes to autoconf/meson?Looks good to me. Backpatching autoconf/meson changes is fine, we've done it before.
Thanks for review, I've applied this backpatched all the way.
--
Daniel Gustafsson
Hello!
On 2024-07-26 14:55, Daniel Gustafsson wrote:
Thanks for review, I've applied this backpatched all the way.
It looks like the recommended way of using autoheader [1]/messages/by-id/30511.1546097762@sss.pgh.pa.us is now broken.
The attached patch fixes the master branch for me.
[1]: /messages/by-id/30511.1546097762@sss.pgh.pa.us
/messages/by-id/30511.1546097762@sss.pgh.pa.us
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
diff_fix_autoheader.patchtext/x-diff; name=diff_fix_autoheader.patchDownload
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index db3fcbecc3ab33c8593eeb4a6d2927f674e2f684..3dea3856aaf06a3c59a77cc46e76b6d6efd5f88c 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -388,6 +388,9 @@
/* Define to 1 if you have the `SSL_CTX_set_cert_cb' function. */
#undef HAVE_SSL_CTX_SET_CERT_CB
+/* Define to 1 if you have the `SSL_CTX_set_num_tickets' function. */
+#undef HAVE_SSL_CTX_SET_NUM_TICKETS
+
/* Define to 1 if stdbool.h conforms to C99. */
#undef HAVE_STDBOOL_H
@@ -517,9 +520,6 @@
/* 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 `SSL_CTX_set_num_tickets' function. */
-#undef HAVE_SSL_CTX_SET_NUM_TICKETS
-
/* Define to 1 if the assembler supports X86_64's POPCNTQ instruction. */
#undef HAVE_X86_64_POPCNTQ
On 26 Jul 2024, at 14:03, Marina Polyakova <m.polyakova@postgrespro.ru> wrote:
On 2024-07-26 14:55, Daniel Gustafsson wrote:Thanks for review, I've applied this backpatched all the way.
It looks like the recommended way of using autoheader [1] is now broken. The attached patch fixes the master branch for me.
Thanks for the report, I'll fix it. Buildfarm animal hamerkop also reminded me
that I had managed to stash the old MSVC buildsystem changes (ENOTENOUGHCOFFEE)
so fixing that at the same time.
--
Daniel Gustafsson
On 2024-07-26 15:27, Daniel Gustafsson wrote:
On 26 Jul 2024, at 14:03, Marina Polyakova
<m.polyakova@postgrespro.ru> wrote:
It looks like the recommended way of using autoheader [1] is now
broken. The attached patch fixes the master branch for me.Thanks for the report, I'll fix it. Buildfarm animal hamerkop also
reminded me
that I had managed to stash the old MSVC buildsystem changes
(ENOTENOUGHCOFFEE)
so fixing that at the same time.
Thank you!
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
On Fri, Jul 26, 2024 at 8:27 AM Daniel Gustafsson <daniel@yesql.se> wrote:
Thanks for the report, I'll fix it. Buildfarm animal hamerkop also reminded me
that I had managed to stash the old MSVC buildsystem changes (ENOTENOUGHCOFFEE)
so fixing that at the same time.
I was just looking at this commit and noticing that nothing in the
commit message explains why we want to turn off tickets. So then I
looked at the comments in the patch and that didn't explain it either.
So then I read through this thread and that also didn't explain it.
I don't doubt that you're doing the right thing here but it'd be nice
to document why it's the right thing someplace.
--
Robert Haas
EDB: http://www.enterprisedb.com
On 26 Jul 2024, at 16:08, Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jul 26, 2024 at 8:27 AM Daniel Gustafsson <daniel@yesql.se> wrote:
Thanks for the report, I'll fix it. Buildfarm animal hamerkop also reminded me
that I had managed to stash the old MSVC buildsystem changes (ENOTENOUGHCOFFEE)
so fixing that at the same time.I was just looking at this commit and noticing that nothing in the
commit message explains why we want to turn off tickets. So then I
looked at the comments in the patch and that didn't explain it either.
So then I read through this thread and that also didn't explain it.
Sorry for the lack of detail, I probably Stockholm syndromed myself after
having spent some time in this code.
We turn off TLS session tickets for two reasons: a) we don't support TLS
session resumption, and some resumption capable client libraries can experience
connection failures if they try to use tickets received in the setup (Npgsql at
least had this problem in the past); b) it's network overhead in the connection
setup phase which doesn't give any value due to us not supporting their use.
TLS tickets were disallowed in 2017 in 97d3a0b09 but as Andres found out,
TLSv1.3 session tickets had a new API which we didn't call and thus issued
tickets.
I don't doubt that you're doing the right thing here but it'd be nice
to document why it's the right thing someplace.
I can add a summary of the above in the comment for future readers if you think
that would be useful.
--
Daniel Gustafsson
On Fri, Jul 26, 2024 at 10:23 AM Daniel Gustafsson <daniel@yesql.se> wrote:
We turn off TLS session tickets for two reasons: a) we don't support TLS
session resumption, and some resumption capable client libraries can experience
connection failures if they try to use tickets received in the setup (Npgsql at
least had this problem in the past); b) it's network overhead in the connection
setup phase which doesn't give any value due to us not supporting their use.TLS tickets were disallowed in 2017 in 97d3a0b09 but as Andres found out,
TLSv1.3 session tickets had a new API which we didn't call and thus issued
tickets.
Thanks much for this explanation.
I don't doubt that you're doing the right thing here but it'd be nice
to document why it's the right thing someplace.I can add a summary of the above in the comment for future readers if you think
that would be useful.
I think having (a) and (b) from above at the end of the "Disallow SSL
session tickets" comment would be helpful.
While I'm complaining, the bit about SSL renegotiation could use a
better comment, too. One of my chronic complaints about comments is
that they should say why we're doing things, not what we're doing. To
me, having a comment that says "Disallow SSL renegotiation" followed
immediately by SSL_CTX_set_options(context, SSL_OP_NO_RENEGOTIATION)
is a good example of what not to do, because, I mean, I can guess
without the comment what that does. Actually, that comment is quite
well-written in terms of explaining why we do it in different ways
depending on the OpenSSL version; it just fails to explain why it's
important in the first place. I'm pretty sure I know in that case that
there are CVEs about that topic, but that's just because I happen to
remember the mailing list discussion on it, and I don't think every
hacker is contractually required to remember that.
I don't want to sound like I'm giving orders and I think it's really
up to you how much effort you want to put in here, but I feel like any
place where we are doing X because of some property of a non-PG code
base with which a particular reader might not be familiar, we should
have a comment explaining why we're doing it. And especially if it's
security-relevant.
--
Robert Haas
EDB: http://www.enterprisedb.com
On 26 Jul 2024, at 20:29, Robert Haas <robertmhaas@gmail.com> wrote:
One of my chronic complaints about comments is
that they should say why we're doing things, not what we're doing.
Agreed.
I feel like any
place where we are doing X because of some property of a non-PG code
base with which a particular reader might not be familiar, we should
have a comment explaining why we're doing it. And especially if it's
security-relevant.
I'm sure there are more interactions with OpenSSL, and TLS in general, which
warrants better comments but the attached takes a stab at the two examples in
question here to get started (to avoid perfect get in the way of progress).
--
Daniel Gustafsson
Attachments:
openssl_comments.diffapplication/octet-stream; name=openssl_comments.diff; x-unix-mode=0644Download
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 7e056abd5a..34967e87ba 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -260,13 +260,18 @@ be_tls_init(bool isServerStart)
}
/*
- * Disallow SSL session tickets. OpenSSL use both stateful and stateless
- * tickets for TLSv1.3, and stateless ticket for TLSv1.2. SSL_OP_NO_TICKET
- * is available since 0.9.8f but only turns off stateless tickets. In
- * order to turn off stateful tickets we need SSL_CTX_set_num_tickets,
- * which is available since OpenSSL 1.1.1. LibreSSL 3.5.4 (from OpenBSD
- * 7.1) introduced this API for compatibility, but doesn't support session
- * tickets at all so it's a no-op there.
+ * Disallow TLS session tickets. PostgreSQL doesn't support TLS session
+ * resumption, and some resumption capable client libraries can experience
+ * connection failures if they try to use tickets received in the
+ * connection setup. Also, since they aren't used, sending them incurs
+ * network overhead in the connection setup phase which provides no value.
+ * OpenSSL use both stateful and stateless tickets for TLSv1.3, and
+ * stateless ticket for TLSv1.2. SSL_OP_NO_TICKET is available since 0.9.8f
+ * but only turns off stateless tickets. In order to turn off stateful
+ * tickets we need SSL_CTX_set_num_tickets, which is available since
+ * OpenSSL 1.1.1. LibreSSL 3.5.4 (from OpenBSD 7.1) introduced this API
+ * for compatibility, but doesn't support session tickets at all so it's a
+ * no-op there.
*/
#ifdef HAVE_SSL_CTX_SET_NUM_TICKETS
SSL_CTX_set_num_tickets(context, 0);
@@ -281,12 +286,15 @@ be_tls_init(bool isServerStart)
SSL_CTX_set_options(context, SSL_OP_NO_COMPRESSION);
/*
- * Disallow SSL renegotiation. This concerns only TLSv1.2 and older
- * protocol versions, as TLSv1.3 has no support for renegotiation.
- * SSL_OP_NO_RENEGOTIATION is available in OpenSSL since 1.1.0h (via a
- * backport from 1.1.1). SSL_OP_NO_CLIENT_RENEGOTIATION is available in
- * LibreSSL since 2.5.1 disallowing all client-initiated renegotiation
- * (this is usually on by default).
+ * Disallow SSL renegotiation. Renegotiation can be used as an attack
+ * vector during MITM attacks allowing the attacker to pose as the
+ * authenticated client and pass commands and data to the server. This
+ * concerns only TLSv1.2 and older protocol versions, as TLSv1.3 has no
+ * support for renegotiation. SSL_OP_NO_RENEGOTIATION is available in
+ * OpenSSL since 1.1.0h (via a backport from 1.1.1).
+ * SSL_OP_NO_CLIENT_RENEGOTIATION is available in LibreSSL since 2.5.1
+ * disallowing all client-initiated renegotiation (this is usually on by
+ * default).
*/
#ifdef SSL_OP_NO_RENEGOTIATION
SSL_CTX_set_options(context, SSL_OP_NO_RENEGOTIATION);
On Mon, Jul 29, 2024 at 5:57 AM Daniel Gustafsson <daniel@yesql.se> wrote:
I'm sure there are more interactions with OpenSSL, and TLS in general, which
warrants better comments but the attached takes a stab at the two examples in
question here to get started (to avoid perfect get in the way of progress).
+1.
--
Robert Haas
EDB: http://www.enterprisedb.com