-
Notifications
You must be signed in to change notification settings - Fork 5.3k
improve TLS resume on server #63030
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
improve TLS resume on server #63030
Conversation
|
Tagging subscribers to this area: @dotnet/ncl, @vcsjones Issue DetailsThis is follow-up on #57079. When I did the original change, I run to problem that once ALPN callback is set it MUST return some valid value and that breaks use case when server does not wish to use ALPN. To keep the lookup simple I simply avoided resume with no ALPN. That of course bit me now in testing and I realized there is easy fix. Each member of SslProtocols uses 2 bits starting with Ssl2 -> 12 -> 1100 and going up. The lover two bits are never used. Second part I realized is that Server caching is on by default (even prior) #57079 So second part of this change is to turn caching off explicitly in case we use ephemeral context and resume will not be possible. Aside from resource saving it also makes it more obvious in packet capture if TLS resume is possible or not. Note that client resume is coming soon. I wanted to split this part to make that change smaller and easier to review. contributes to #22977
|
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs
Show resolved
Hide resolved
|
@wfurt can this change be responsible for an improvement for YARP proxy? |
|
I'm not sure how to read the graph. It seems like this is HTTP-HTTP -> no ssl involved. There may be some small improvement for SSL as the server will not calculate and send session tickets if resume is not possible but I'm not sure if that is applicable. |
|
Me neither actually 🙂 it's https://aka.ms/aspnet/benchmarks (16th page) it says something happened between 1/10 and 1/11 and this PR looked like the only suspect but it could also be a change in the YARP itself. |
|
@EgorBo that change was a result of using non-validated headers in YARP: dotnet/yarp#1507 |
Nice! 🙂 |

This is follow-up on #57079.
When I did the original change, I run to problem that once ALPN callback is set it MUST return some valid value and that breaks use case when server does not wish to use ALPN. To keep the lookup simple I simply avoided resume with no ALPN. That of course bit me now in testing and I realized there is easy fix.
Each member of SslProtocols uses 2 bits starting with Ssl2 -> 12 -> 1100 and going up. The lover two bits are never used.
This change use the lowest bit to separate contexts with and without ALPN. With that, we can keep
SslProtocolsas only key for the hash lookup.Second part I realized is that Server caching is on by default (even prior) #57079
https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_set_session_cache_mode.html
So even in cases we know resume will not work, we would calculate the ticket, send New Ticket message on wire and possibly cause burden on client to store session that will never be resumable.
So second part of this change is to turn caching off explicitly in case we use ephemeral context and resume will not be possible. Aside from resource saving it also makes it more obvious in packet capture if TLS resume is possible or not.
Note that client resume is coming soon. I wanted to split this part to make that change smaller and easier to review.
contributes to #22977