Thread: 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
theclient. 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
sessioninformation. 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
informationneeds to be cached locally.
The TLSv1.3 protocol only supports tickets and does not directly support session ids. However, OpenSSL
allowstwo 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
wrappedup 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
servercan 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
notbe 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-sideoption 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
Attachment
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] 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
Attachment
> 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 forme. 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
Attachment
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
On 2024-07-26 13:55:29 +0200, Daniel Gustafsson wrote: > Thanks for review, I've applied this backpatched all the way. Thanks for working on this!