[TLS 1.3] Session Resumption #2974

reneme · 2022-05-05T14:38:02Z

This adds session resumption to the TLS 1.3 implementation.

Central changes in this pull request:

Extend the TLS::Session class to allow storing TLS 1.3 sessions
Provide a new entry point into the TLS 1.3 key schedule: init_with_psk()
Allow premature serialization of Client_Hello_13 to calculate PSK binders
- Implement a Truncate(Client_Hello) function as outlined in RFC 8446 4.2.11.2
- Extend the TLS client state machine to accomodate resumption
~~Provide TLS::Callbacks::tls_current_timestamp() for deterministic timestamps for testing~~ (now extracted here)

Currently, the TLS 1.3 PSK mechanism is exclusively used for session resumption. It should be fairly straight forward to extend it for out-of-band PSKs. Furthermore, this pull request leaves support for early data and early key export to future work.

Caveat: RFC 8446 suggests to not reuse session tickets to prevent passive client tracking. This is not implemented by the client implementation (neither in TLS 1.2). I suppose this is meant to be implemented by a custom Session_Manager if needed?!

src/lib/tls/tls_session.h

src/lib/tls/tls_session.cpp

src/lib/tls/tls_session.h

codecov-commenter · 2022-05-06T07:41:04Z

Codecov Report

Base: 92.59% // Head: 92.57% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (f126b55) compared to base (49080fb).
Patch coverage: 88.96% of modified lines in pull request are covered.

❗ Current head f126b55 differs from pull request most recent head 410bd69. Consider uploading reports for the commit 410bd69 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2974      +/-   ##
==========================================
- Coverage   92.59%   92.57%   -0.02%     
==========================================
  Files         600      601       +1     
  Lines       70092    70637     +545     
  Branches     6628     6672      +44     
==========================================
+ Hits        64903    65394     +491     
- Misses       5156     5210      +54     
  Partials       33       33

Impacted Files	Coverage Δ
src/bogo_shim/bogo_shim.cpp	`88.85% <ø> (ø)`
src/lib/tls/msg_server_hello.cpp	`92.94% <ø> (ø)`
src/lib/tls/tls13/tls_extensions_key_share.cpp	`87.50% <ø> (ø)`
src/lib/tls/tls13/tls_handshake_layer_13.cpp	`97.14% <ø> (ø)`
src/tests/tests.h	`92.14% <0.00%> (ø)`
src/tests/test_tls_messages.cpp	`86.71% <9.52%> (-6.52%)`	⬇️
src/lib/tls/tls_extensions.cpp	`89.60% <61.53%> (-1.80%)`	⬇️
src/tests/test_tls_rfc8448.cpp	`89.43% <80.23%> (-1.84%)`	⬇️
src/lib/tls/msg_session_ticket.cpp	`81.39% <85.00%> (+4.47%)`	⬆️
src/lib/tls/tls13/tls_extensions_psk.cpp	`89.81% <89.81%> (ø)`
... and 20 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

src/lib/tls/tls13/tls_cipher_state.cpp

reneme · 2022-05-17T15:50:24Z

src/bogo_shim/config.json

+        "TLS12SessionID-TLS13": "We don't offer TLS 1.3 when a TLS 1.2 session was found",
+        "Ticket-Forbidden-TLS13": "We don't offer TLS 1.3 when a TLS 1.2 session was found",
+        "Resume-Client-NoResume-TLS12-TLS13-TLS": "We don't offer TLS 1.3 when a TLS 1.2 session was found",
+        "Resume-Client-Mismatch-TLS12-TLS13-TLS": "We don't offer TLS 1.3 when a TLS 1.2 session was found",


We basically query the Session_Manager before instantiating the TLS 1.2 or TLS 1.3 implementation. If we find a TLS 1.2 session we don't even bother offering TLS 1.3 in the first place. Hence, we cannot fulfil those tests. Though, I think it's worth discussing whether the described behaviour is actually wise.

Would of course be nice to switch to TLS 1.3. even than an older session exist. But unless you think the new some API change for that I don't think it is important to have it in the first version.

src/lib/tls/msg_client_hello.cpp

src/lib/tls/tls13/tls_client_impl_13.cpp

reneme · 2022-05-17T16:06:27Z

src/lib/tls/tls13/tls_transcript_hash_13.cpp

+//              buffer without re-parsing it.
+//
+// Finds the truncation offset in a serialization of Client Hello as defined in
+// RFC 8446 4.2.11.2 used for the calculation of PSK binder MACs.


... yeah. The binder MACs are unfortunately a major cross-cutting concern through our layered implementation architecture. Probably it would be a good idea to see how other implementations are handling this. Maybe I missed a short cut. The current implementation works, but its sad to see this.

reneme · 2022-05-17T16:15:00Z

src/lib/tls/tls13/tls_transcript_hash_13.cpp

+
+      // Check whether we should generate a truncated hash for supporting PSK
+      // binder calculation or verification. See RFC 8446 4.2.11.2.
+      if(serialized_message_length > 0 && *serialized_message == CLIENT_HELLO)


Same as find_client_hello_truncation_mark, this is somewhat "magic" and an ugly breach of abstraction. Before, the Transcript_Hash_State was a dumb class that just "hashed the transcript" as it was passed into it. Now it automagically detects whether it is hashing a Client Hello message, whether that Client Hello message contains a PSK extension (in find_client_hello_truncation_mark) and creates a truncated() hash in that case.

Apart from the level-of-abstraction ugliness, this introduces overhead due to additional memory copies and repeated marshalling and parsing of the data structure.

For now, this is the least intrusive implementation I could come up with. Though, I'm very much open to discussing a better approach to this.... @hrantzsch :)

reneme · 2022-07-05T08:27:03Z

Rebased and retargeted to master.

* New_Session_Ticket * PSK extension * EarlyDataIndication extension

… applicable

src/lib/tls/msg_client_hello.cpp

src/lib/tls/msg_session_ticket.cpp

src/lib/tls/tls13/tls_client_impl_13.cpp

src/lib/tls/tls13/tls_client_impl_13.h

src/lib/tls/tls13/tls_cipher_state.h

src/lib/tls/tls13/tls_extensions_psk.cpp

src/lib/tls/tls_session.cpp

src/lib/tls/tls_session.h

reneme commented May 5, 2022

View reviewed changes

src/lib/tls/tls_session.h Show resolved Hide resolved

reneme commented May 5, 2022

View reviewed changes

src/lib/tls/tls_session.cpp Outdated Show resolved Hide resolved

reneme commented May 5, 2022

View reviewed changes

src/lib/tls/tls_session.h Show resolved Hide resolved

reneme changed the title ~~[Tls13] Session Resumption~~ [TLS 1.3] Session Resumption May 5, 2022

reneme force-pushed the tls13/resumption branch from dd9f030 to 0802b9d Compare May 6, 2022 07:15

reneme force-pushed the tls13/resumption branch 2 times, most recently from 00bd2d2 to 9fc97c1 Compare May 8, 2022 07:18

reneme commented May 8, 2022

View reviewed changes

src/lib/tls/tls13/tls_cipher_state.cpp Show resolved Hide resolved

reneme commented May 8, 2022

View reviewed changes

src/lib/tls/tls13/tls_cipher_state.cpp Show resolved Hide resolved

reneme force-pushed the tls13/resumption branch from 87902db to c5282fd Compare May 16, 2022 07:16

reneme requested a review from hrantzsch May 16, 2022 16:54

reneme marked this pull request as ready for review May 16, 2022 16:54

reneme force-pushed the tls13/resumption branch from bc4dac3 to acf72df Compare May 17, 2022 13:03

reneme mentioned this pull request May 17, 2022

[TLS 1.3] Callback for deterministic timestamps #2975

Closed

reneme force-pushed the tls13/resumption branch from acf72df to 5b4843d Compare May 17, 2022 15:34

reneme changed the base branch from master to tls13/timestamp_callback May 17, 2022 15:35

reneme mentioned this pull request May 17, 2022

[TLS 1.3] Limited TLS 1.3 Client Implementation #2922

Merged

16 tasks

reneme commented May 17, 2022

View reviewed changes

src/lib/tls/msg_client_hello.cpp Show resolved Hide resolved

reneme commented May 17, 2022

View reviewed changes

src/lib/tls/tls13/tls_client_impl_13.cpp Show resolved Hide resolved

reneme commented May 17, 2022

View reviewed changes

reneme force-pushed the tls13/resumption branch from 5b4843d to ca9262e Compare May 18, 2022 12:05

reneme changed the base branch from tls13/timestamp_callback to dev/tls-13 May 19, 2022 07:33

reneme force-pushed the dev/tls-13 branch from d52969f to 2ed9cf0 Compare June 1, 2022 16:23

reneme force-pushed the tls13/resumption branch from ca9262e to 8226aed Compare June 1, 2022 16:43

reneme mentioned this pull request Jun 2, 2022

Support for the TLS 1.3 - development #2714

Closed

reneme changed the base branch from dev/tls-13 to master July 5, 2022 08:12

reneme force-pushed the tls13/resumption branch from cc8a4e6 to 5f7544a Compare July 5, 2022 08:26

reneme force-pushed the tls13/resumption branch from 5f7544a to af88496 Compare July 6, 2022 12:31

reneme added this to the Botan 3.0.0 milestone Sep 20, 2022

reneme added 8 commits October 13, 2022 16:57

Extend TLS::Session to allow storing TLS 1.3 sessions

eae6e7e

Extend TLS::Cipher:State to deal with PSKs in key schedule

03fe97c

Implement resumption specific messages and extensions

e2d8892

* New_Session_Ticket * PSK extension * EarlyDataIndication extension

Transcript_Hash_State automagically calculates truncated hashes where…

3e1a406

… applicable

Client/Server hello can deal with PSKs

57bede2

Integrate session resumption into TLS 1.3 client

17099f2

enable relevant BoGo tests for session resumption

41358e2

FIX: missing include

defa8ac

reneme force-pushed the tls13/resumption branch from af88496 to defa8ac Compare October 13, 2022 15:14

reneme requested a review from lieser October 13, 2022 15:17

lieser reviewed Oct 13, 2022

View reviewed changes

review comments

f126b55

reneme force-pushed the tls13/resumption branch from 4177f4b to a66393b Compare October 19, 2022 11:28

reneme requested a review from lieser October 19, 2022 11:42

reneme added 2 commits October 19, 2022 13:43

Revert: No backward compatibility for TLS sessions

6ca0077

amend 3.0 migration guide

410bd69

reneme force-pushed the tls13/resumption branch from 7571118 to 410bd69 Compare October 19, 2022 11:43

lieser approved these changes Oct 19, 2022

View reviewed changes

reneme merged commit 3188de0 into master Oct 20, 2022

randombit deleted the tls13/resumption branch December 1, 2022 13:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TLS 1.3] Session Resumption #2974

[TLS 1.3] Session Resumption #2974

reneme commented May 5, 2022 •

edited

Loading

codecov-commenter commented May 6, 2022 •

edited

Loading

reneme May 17, 2022

lieser Oct 19, 2022

reneme May 17, 2022 •

edited

Loading

reneme May 17, 2022

reneme commented Jul 5, 2022

[TLS 1.3] Session Resumption #2974

[TLS 1.3] Session Resumption #2974

Conversation

reneme commented May 5, 2022 • edited Loading

codecov-commenter commented May 6, 2022 • edited Loading

Codecov Report

reneme May 17, 2022

Choose a reason for hiding this comment

lieser Oct 19, 2022

Choose a reason for hiding this comment

reneme May 17, 2022 • edited Loading

Choose a reason for hiding this comment

reneme May 17, 2022

Choose a reason for hiding this comment

reneme commented Jul 5, 2022

reneme commented May 5, 2022 •

edited

Loading

codecov-commenter commented May 6, 2022 •

edited

Loading

reneme May 17, 2022 •

edited

Loading