Skip to content

Use handshake timeout for Tls listener callback #62177

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

Merged

Conversation

DeagleGross
Copy link
Member

Problem

HttpsConnectionMiddleware carefully considers a handshake timeout.

New TlsListenerMiddleware does not use it, and in case of a configured timeout request will not be cancelled.

Solution

I renamed TlsListenerMiddleware to TlsListener to be used as part of HttpsConnectionMiddleware. It will use the same cancellation token as the latter. We can move the code directly into HttpsConnectionMiddleware but IMO TlsListener has a bit different purpose and can be living in a separate instance. TlsListener now does not invoke next middleware delegate.

Also changed the parsing code to remember the TLS client hello record length, and if that is known re-parsing of TLS client hello "header" will not be happening on each iteration.

Closes #62172

@DeagleGross DeagleGross self-assigned this May 30, 2025
@github-actions github-actions bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label May 30, 2025
Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

Thanks!

@DeagleGross
Copy link
Member Author

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@@ -21,7 +21,7 @@

namespace InMemory.FunctionalTests;

public class TlsListenerMiddlewareTests : TestApplicationErrorLoggerLoggedTest
public class TlsListenerTests : TestApplicationErrorLoggerLoggedTest
Copy link
Member

Choose a reason for hiding this comment

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

We can probably add a cancellation test here as well. Set the handshake timeout to something small like 1 millisecond and check that the request was canceled.

Copy link
Member Author

Choose a reason for hiding this comment

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

added

@DeagleGross DeagleGross enabled auto-merge (squash) June 4, 2025 12:21
@DeagleGross DeagleGross merged commit 3aa03f4 into dotnet:main Jun 4, 2025
27 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview6 milestone Jun 4, 2025
@DeagleGross DeagleGross deleted the dmkorolev/tlsclienthello-improvements branch June 4, 2025 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TlsListenerMiddleware does not use handshake timeout
3 participants