Subject: Re: tls 1.3: sending multiple tickets

From: Daniel Gustafsson <[email-masked]>
Date: Tue, 18 Jun 2024 15:11:33 +0200

> On 17 Jun 2024, at 19:38, Andres Freund <[email-masked]> 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

---

From: Heikki Linnakangas <[email-masked]>
Date: Wed, 24 Jul 2024 08:44:16 +0300

> the API in 3.5.4 but only for compatibility since it doesn't support TLS

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.

> 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)

---

From: Daniel Gustafsson <[email-masked]>
Date: Fri, 26 Jul 2024 13:55:29 +0200

> On 24 Jul 2024, at 07:44, Heikki Linnakangas <[email-masked]> wrote:
> 
> 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.
> 
> 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

---

From: Marina Polyakova <[email-masked]>
Date: Fri, 26 Jul 2024 15:03:31 +0300

Hello!

> 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.

[1] 
https://www.postgresql.org/message-id/30511.1546097762%40sss.pgh.pa.us

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

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

---

From: Daniel Gustafsson <[email-masked]>
Date: Fri, 26 Jul 2024 14:27:03 +0200

> On 26 Jul 2024, at 14:03, Marina Polyakova <[email-masked]> 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.

--
Daniel Gustafsson

---

From: Marina Polyakova <[email-masked]>
Date: Fri, 26 Jul 2024 15:47:01 +0300

On 2024-07-26 15:27, Daniel Gustafsson wrote:
>> It looks like the recommended way of using autoheader [1] is now 
>> broken. The attached patch fixes the master branch for me.
> 

Thank you!

---

From: Andres Freund <[email-masked]>
Date: Mon, 29 Jul 2024 13:08:53 -0700

> Thanks for review, I've applied this backpatched all the way.

Thanks for working on this!