-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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: create API to fetch TLS client hello message #60806
base: main
Are you sure you want to change the base?
Conversation
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.
PR Overview
This PR adds an API to fetch the TLS client hello message by introducing a new interface (ITlsAccessFeature) along with sample code changes and necessary updates in the underlying HttpSys and native interop code. Key changes include:
- Introducing ITlsAccessFeature and its implementation to expose raw TLS client hello message bytes.
- Updating the HttpSys request processing to retrieve, log, and expose the TLS client hello message.
- Adding support in HttpApi interop and configuring HTTP service settings to enable caching of the client hello message.
Reviewed Changes
File | Description |
---|---|
src/Servers/HttpSys/samples/TlsFeaturesObserve/Program.cs | Updated sample to invoke new TLS client hello message caching configuration. |
src/Servers/HttpSys/samples/TlsFeaturesObserve/Startup.cs | Sample updated to use ITlsAccessFeature for fetching TLS client hello bytes. |
src/Servers/Connections.Abstractions/src/Features/ITlsAccessFeature.cs | New API interface for accessing TLS client hello message bytes. |
src/Servers/HttpSys/src/RequestProcessing/RequestContext.cs | Integrated methods to retrieve and parse TLS client hello message bytes. |
src/Servers/HttpSys/src/NativeInterop/HttpApi.cs | Updated interop definitions to include HttpSetServiceConfiguration. |
Others | Minor updates in logging, error codes, and feature collection to support the new API. |
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (4)
src/Servers/HttpSys/samples/TlsFeaturesObserve/Program.cs:11
- [nitpick] The namespace 'TlsFeatureObserve' is inconsistent with the folder name 'TlsFeaturesObserve'. Consider aligning the namespace with the folder structure to improve clarity.
namespace TlsFeatureObserve;
src/Servers/HttpSys/src/RequestProcessing/RequestContext.cs:245
- [nitpick] A magic number (11) is used inline for the TLS client hello property. Consider using a named constant to enhance readability and maintainability.
11 /* HTTP_REQUEST_PROPERTY.HttpRequestPropertyTlsClientHello */
src/Servers/HttpSys/src/LoggerEventIds.cs:62
- [nitpick] The logger event name 'TlsClientHelloParseError' is defined here but the LoggerMessage in RequestContext.Log.cs uses 'TlsClientHelloRetrieveError'. Ensure consistent naming for clarity.
public const int TlsClientHelloParseError = 55;
src/Servers/HttpSys/src/RequestProcessing/RequestContext.cs:227
- [nitpick] The implementation of 'GetTlsClientHelloMessageBytes()' appears similar to other copies in the code. Consider refactoring the duplicate logic into a shared utility method to improve maintainability.
internal unsafe byte[]? GetTlsClientHelloMessageBytes()
Expose API to get the TLS client hello message from http.sys (later will be added for other servers)
Description
HTTP.SYS provides new property
HttpRequestPropertyTlsClientHello
, which can be used for accessing raw TLS client hello message bytes. Some customers are interested in accessing TLS client hello message.API
New API exposed is:
ITlsAccessFeature
- I think it should not be added toITlsHandshakeFeature
, because this is not what most of the users would want to use. We can have a separate feature for more advanced scenarios.ITlsAccessFeature.GetTlsClientHelloMessageBytes
- a method to extract the raw bytes of TLS client hello message.I am not sure if this API should look like that, so please let me know if you have other ideas on how to place it.
Also: I am making a hard-copy of the TLS client hello message bytes, so we can provide a more complicated API implemented based on the pooling (i.e.
ReturnTlsClientHelloBytes(byte[] buffer)
and user will need to call it), but I am not sure if this is OK.Fixes #60805