-
Notifications
You must be signed in to change notification settings - Fork 4.9k
[API Proposal]: expose TLS client hello message in SslStream #113729
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
Comments
Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones |
I don't think we want an API which would require us to store the hello message bytes internally, thus increasing the memory footprint for all users, regardless of whether they would use the feature or not. Moreso if the target audience of the feature is very narrow. The approach I would find more permissible is some sort of callback registered in Ssl(Server/Client)AuthenticationOptions that would give you the message as ReadOnlyMemory (or ReadOnlySpan). The buffer would be valid only during the callback so you can copy the data out if you need. Similarly, we might want to also expose the TLS ServerHello message to be symmetric. Would something like below also work for your scenario? +public delegate void TlsHelloMessageCallback(object sender, ReadOnlyMemory<byte>);
public partial class SslClientAuthenticationOptions
{
+ TlsHelloMessageCallback ClientHelloBytesCallback { get; set; };
+ TlsHelloMessageCallback ServerHelloBytesCallback { get; set; };
}
public partial class SslServerAuthenticationOptions
{
+ TlsHelloMessageCallback ClientHelloBytesCallback { get; set; };
+ TlsHelloMessageCallback ServerHelloBytesCallback { get; set; };
}
The only downside I see in the above might be that we need to figure out how those should behave for QUIC, I don't think MsQuic API gives access to those, so we might have to leave them unimplemented until they do :/ |
What would the behavior of these APIs be in the case of renegotiation? Would the callbacks fire for the initial handshake or also renegotiations? |
Also bear in mind that PQC handshakes are much larger and will, eventually, start getting traction soon. X25519MLKEM-Hybrid Hellos are going to be kilobytes in size. This is another point in making it pay to play with callbacks. |
What are the expected use cases for this (especially in the context of Kestrel)? For example one use case I know of is to parse the hellos in order to reject/ratelimit connections before performing the handshake. In that case you want to know it before you call into |
In case of kestrel we know that users at least want to have a raw byte data of the tls client hello message.
I suspect it should be called for renegotiation as well.
Miha, it makes sense, but does it listen to any data on the wire (not only tls)? |
What I was curious about is the why. What are they expected to do with it. Log it? Throw to stop the handshake?
The idea is that you do something like this options.ListenAnyIP(443, listenOptions =>
{
listenOptions.Use(async (context, next) =>
{
// Read context.Transport.Input
// Call TlsFrameHelper.TryGetFrameInfo
// Do whatever you want with the info, short-circuit, log, etc.
// Advance the context.Transport.Input pipe (not consuming anything)
// Call next middleware (https)
await next();
});
listenOptions.UseHttps();
}); There's no extra overhead after reading that first client hello frame. We could also make this simpler to consume on the YARP side, such that you'd end up with something as simple as options.ListenAnyIP(443, listenOptions =>
{
+ listenOptions.Use(async (context, next) =>
+ {
+ var hello = await context.ParseClientHelloAsync(... more options ...);
+ Console.WriteLine($"SNI: {hello.TargetName}");
+ await next();
+ });
listenOptions.UseHttps();
}); Or similar. |
Servers are starting to implement this for in-depth defense against threats. here is a nice blog post from cloudflare folks |
@MihaZupan the purpose of API is to have a view into TLS client hello and analyze the data anyhow. The use case we (ASP.NET team) are aware of would be solvable without a strongly-typed representation of the data: "raw bytes" would be sufficient enough and will be a good starting point (basically any user could use it to their desires). We dont want something like YARP's I think this is much more straightforward to have an API exposed where SslStream invokes a callback (for instance) when the TLS client hello message is detected, instead of building a separate middleware which will try to parse every single packet / data if SslStream internals is anyway doing the same job (it can simply invoke a callback if setup). Let me know if you think this is a non-convenient way to proceed. |
I much prefer the approach YARP took and don’t think the SslStream should store anything. Providing the parser / and a callback seems like a much more performant approach. |
talked to @rzikm more (thanks!), and I added the proposed callback to the original description of issue. |
We have discussed this within the team and @MihaZupan has made a very good point, that if anyone would want to use the ClientHello message to do any kind of decision regarding whether to let the connection continue (i.e. mentioned fingerprint-based DOS protection), SslStream is too late for that and is unnecessarily inefficient (by the time we get to the calling the callback, we already allocated quite some memory both Managed and native) Furthermore, the callback does not give an opportunity to reject the connection (other than throwing exception), which could be fixed, but the above mentioned drawbacks still apply. So, for example, even such addition would not make it usable for YARP for any sort of TLS filtering. From what @MihaZupan posted, intercepting the ClientHello before passing it to SslStream is rather easy via the ASP.NET's Middleware model.
Out of curiosity, do subsequent ClientHello from renegotiation or ClientHelloRequest also enter the fingerprint calculation? If not, then only the first ever frame (or first few frames) needs to be parsed, not every frame in the connection. If there are concerns about duplicating the TlsFrameHelper code, we can add it to the list of shared sources between runtime and ASP.NET (as we share e.g. HTTP header encoding/decoding code), but for your simple use case (just having the raw message bytes), the header is not too difficult to parse to identify the ClientHello portion. @MihaZupan also had an idea to make a TLS-filtering middleware as a shippable package in the future if it would be of any use. |
thanks for your input @rzikm and @MihaZupan; updated aspnetcore proposal - I will come back here with the results of decision if we would need dotnet/runtime changes at all. Personally, I think you are right and we can support it fully on aspnetcore side, but I still think once we would need to have a strongly-typed TLS client hello message representation, we will need to expose |
Background and motivation
There is an effort to expose TLS client hello message in ASP.NET: see API Proposal: Expose TLS client hello message. We have already figured out the implementation for HTTP.SYS as underlying server (see feat: fetch TLS client hello message from HTTP.SYS.
The ask here is to have a similar API to fetch this data from SslStream in case ASP.NET uses Kestrel server.
API Proposal
Is should be a method / property to fetch the raw bytes of the tls client hello message.
public partial class SslStream { + public ReadOnlySpan<byte> GetTlsClientHelloBytes(); }
or since we probably dont want to keep the data increasing the memory footprint, we can introduce a callback, which will be invoked during SslStream TLS client hello message processing:
API Usage
in case of callback:
Risks
there is no risk here - it is just an accessor to underlying data if needed for the user.
API probably will not be used by majority, and it will not be increasing costs of standard use cases, since users will not be passing a callback to invoke.
The text was updated successfully, but these errors were encountered: