-
Notifications
You must be signed in to change notification settings - Fork 579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TLS 1.3] Support Session Resumption for TLS Server #3140
Conversation
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #3140 +/- ##
==========================================
+ Coverage 88.04% 88.07% +0.02%
==========================================
Files 615 601 -14
Lines 69740 67366 -2374
Branches 6913 6737 -176
==========================================
- Hits 61405 59333 -2072
+ Misses 5443 5200 -243
+ Partials 2892 2833 -59
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. |
97ecc2c
to
9613928
Compare
9e5d476
to
e4fc53f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving - just reviewed the last few commits here.
Your outline for a new session manager interface seems a better match with the realities of how TLS 1.3 does session management. I'm guessing here but it feels like having the implementation notify the session manager that a session was used is a more flexible approach with regards to policy. It also enables LRU style cache pruning. We may also want to rethink what facilities are offered by the session manager base class itself. In particular right now we do offer out of the box encryption for SQL session managers derived from |
Re: Session Management revamp My central idea is to give the I'm currently putting together a working prototype that (roughly) follows the interface I sketched above. Once that is remotely presentable, I'll open a draft PR. Hopefully this will still happen this year. 🤡 |
e4fc53f
to
40d3747
Compare
"Server-Verify-*-TLS13": "TODO: FIXME - missing deny-list for outdated signature schemes", | ||
"Server-VerifyDefault-*-TLS13": "TODO: FIXME - missing deny-list for outdated signature schemes", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fixed in a follow-up PR. It seems to be an issue in the selection of signature schemes with client auth (?). The test was not enabled before, as it required session resumption
Note: Lets move the |
40d3747
to
689c567
Compare
689c567
to
4d9fd4f
Compare
This is now rebased (finally) onto the Nevertheless, this PR chain (#3244, #3150 and #3140) comes together and should be ready for review. Some refactorings or prep-work could probably be moved from here down to #3244 though I'm a bit unsure whether this is worth the head ache. Also I'd like to have another look at the BoGo tests. Maybe we could enable a few more. |
4d9fd4f
to
80a73dc
Compare
65fb231
to
c1a086e
Compare
Rebased after enum changes prevented a clean merge. Ready for review after #3150 is merged. |
1f3d353
to
096083f
Compare
d16cd63
to
32df109
Compare
Rebased to master, after #3150 was merged. |
* PSK in Client Hello shows up in TLS::Callbacks::modify_extensions() * TLS::Policy::session_ticket_lifetime() returns std::chrono::seconds * TLS::Session::lifetime_hint() returns std::chrono::seconds * Handling of Client Hello is more flexible * Additions of previously missing sanity checks * Extract a class for TLS 1.3 Tickets Co-Authored-By: Ingo Roda <[email protected]>
Co-Authored-By: Ingo Roda <[email protected]>
…ckets Implementations and configurations that do not support sending such tickets (i.e. TLS 1.2 and TLS 1.3 clients) always return 0.
c33fff6
to
f8be24e
Compare
Rebased yet again after #3323 caused a conflict. Also fixed the last bogo test for this PR. |
bcd4281
to
6a203b6
Compare
1db89a0
to
44ba343
Compare
Pull Request Dependencies
[TLS 1.3] Basic Server Implementation #3053[TLS 1.3] Prepare for Session Consolidation #3244[TLS 1.3] Consolidate the Session Manager #3150Description
This adds facilities for PSK-based session resumption to the TLS 1.3 server implementation. The resumption mechanism of TLS 1.3 differs substantially from previous protocol versions: Therefore, this required changes in the
Session_Manager
API that can be found in #3150.New Public API
TLS::Server::send_new_session_tickets(const size_t tickets)
When called on TLS 1.3 server channel that finished its handshake, this sends "
tickets
"NewSessionTicket
messages to the peer. Note that applications may call this at any time after the handshake was completed and as often as they need (up to 2^16 times -- then the ticket nonce depletes).On any other channel object (i.e. TLS 1.2 server), this throws.
TLS::Server::new_session_ticket_supported()
Returns
false
if the channel object is not capable of TLS 1.3 session tickets (e.g. because it is a TLS 1.2 server or the TLS 1.3 client did not advertise support for any PSK key exchange mode). If::send_new_session_ticket()
would likely succeed, this will returntrue
.TLS::Policy::new_session_tickets_upon_handshake_success()
Once a TLS 1.3 server successfully established a TLS connection with a compatible client, it will automatically send as many session tickets as defined by this policy. This is a convenience function to enable session resumption support without the using application needing to call
::send_new_session_ticket()
. By default the TLS 1.2 implementation offers session resumption, so this returns1
by default: I.e. clients will receive a single session ticket after a successful handshake.