Skip to content
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

feat: control SSL protocol version on tls jobs #2049

Merged
merged 13 commits into from
Jan 22, 2025

Conversation

DeagleGross
Copy link
Contributor

@DeagleGross DeagleGross commented Jan 20, 2025

Adding an ability to specify the tls version for the job.

In case of Kestrel, added an option to do it on server side. In case of HTTP.SYS the only available option is to modify the underlying registry (which is unsafe and can fail / leave machine in a transient state). So the easiest is to explicitly set the SslProtocols property on the client causing server to establish a connection using specified tls protocol version.

the option load.httpClient.variables.sslProtocols is being contributed here: dotnet/crank#786 (current PR should be merged after the crank one)

Copy link
Member

@BrennanConroy BrennanConroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to add TLS1.3?
We should probably change the scenario names so tls1.2 and tls1.3 are distinguishable.

@DeagleGross
Copy link
Contributor Author

DeagleGross commented Jan 21, 2025

Are we going to add TLS1.3?

I think we should not add more jobs, but simply run TLS1.3 when we would want to compare the perf. It feels like we are adding too many jobs at a moment if we would want to multiply by TLS / OpenSSL version (x3 jobs).
what do you think?

@DeagleGross DeagleGross merged commit e8d6427 into aspnet:main Jan 22, 2025
2 checks passed
@DeagleGross DeagleGross deleted the dmkorolev/idna/tls-protocols branch January 22, 2025 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants