-
Notifications
You must be signed in to change notification settings - Fork 105
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 sessions resumption #1054
Comments
Update as discussed in the chat: there should be to 2 handshakes limits - the 1st one for successful handshakes and the 2nd for protocol errors on handshakes. Due to TLS tickets asymmetry, it makes sense for a bot just to send us garbage instead of valid crypto data, so the server struggles on meaningless crypto operations. |
The #1430 fixed the issue partly, there are still a few todos:
|
Probably need to add client IP address into ticket. At least OpenSSL allows to serialise session in one application and deserialise it in an another application. Attacker may use that to establish TLS session once and then propagate the session to all the bots and resume the session there without any need to pass through full handshake. |
Clients are likely changing their IP addresses thanks to DHCP and maybe mobile cells.... |
closed by #1458 |
Scope
Session tickets defined by RFC 5077 must be enabled. TLS cache is considered as a bad practice due to unnecessary server memory usage, so I completely removed
TTLS_CACHE_C
option.A new config option with 2 attributes must be introduced:
random
orsecret
- define how a ticket key must be generated - using random generator or the static data for tickets sharing among cluster nodes (a hash function must be used to not to send the secret as is).In case of using shared secret, the perfect forward secrecy must be kept and the secret must be regenerated in pseudo-random (to make sure that all cluster nodes have the same state) fashion.
It makes sense to use the same secret as for
sticky_secret
, so I propose to rename the option tosecret
and use it for the TLS tickets as well as for the session tickets. (Please update Wiki).Notice (probably good to add to Wiki) it's attractive to reuse TLS Session ID instead of sticky cookie or vice versa, but Session ID is generated (and controlled) by a client, so we can not rely on it and can not force a client to use our cookie.
AES_128_GCM should be used (bedtls/programs/ssl/ssl_server2.c) for encryption.
Note and fix
TODO #1054
comments around the code.Handshakes limits
It's supposed that normal clients use TLS sessions instead of establish new TLS connections, so as part of the issue, a rate limit for the new handshakes (not only RSA as in #1038) must be introduced. This is the right way to limit TLS.
The limit must be per-vhost in sense of #715.
Code reworking
Use
ttls_ticket_write()
andttls_ticket_parse()
directly instead of indirect calls off_ticket_write()
andf_ticket_parse()
fromtls->conf
. This allows to fix record size estimation inttls_write_new_session_ticket()
.ssl_load_session()
must not parse X.509 each time, instead it must cache only the required information from the certificate.I believe
ttls_ticket_context->mutex
inttls_ticket_parse()
should experience a serious contention, so the lock must be acquired only to update the key inssl_ticket_update_keys()
- all the other operations with the context are read only.In general, please review the code and move all computations which can be precomputed to configuration phase, reduce copies, extra
memset()
s, and other inneficiencies.I didn't finish code cleanups in /tls, so please fix coding style in
ssl_ticket.[ch]
.Known problems
It seems we broke TLS handshakes FSM for session resumption: in
ttls_write_server_hello()
we set next stateTTLS_SERVER_CHANGE_CIPHER_SPEC
, but the FSM inttls_handshake_server_hello()
doesn't expect the state. Need to fix this and send all the server messages created on the consequent server states in one shot. See comment and the newgoto
inttls_handshake_server_hello()
in branchak-bn-mem-profiles
.Testing
As part of the feature development and debugging, please develop appropriate functional test in
tls/
:tls_connection_rate
,tls_connection_burst
, andtls_uncomplete_rate
limits. Probably it makes sense to move these 3 tests into a separate test which will be developed further in Functional test for Frang #673.Besides the functional test, please make sure that a regular browser (at least Firefox and Chrome) normally uses a ticket to restore a session.
Performance evaluation
Need to benchmark TLS sessions resumption on HTTP/1.1 and HTTP/2 (#309) cases and compare with OpenSSL's TLS 1.2 and 1.3 in zero rtt handshake coupled with either HAProxy or Nginx (one of them is enough). Analyze perf report and create issue for 0.7 if some anomalies are found and/or Tempesta shows worse results than 30% better than OpenSSL TLS 1.2 with Nginx for both HTTP/1.1 and HTTP/2 (we're basically fine if TLS 1.3 0-rtt bets TLS 1.2 session resumption - it's left for #1031).
tls-perf extension tempesta-tech/tls-perf#3 is required to perform the measurements.
h2load seems a good tools. Check that we're not worse than H2O results.
The text was updated successfully, but these errors were encountered: