crypto/tls: add Extensions to ClientHelloInfo

[phuslu]

Similar aws/s2n-tls#607, Having access to raw ClientHello can be useful for fingerprinting clients [1] for further analysis.
Plus, With raw ClientHello message, we could also implements SNI Proxy in tls.Config.GetConfigForClient [2] , e.g. tlsrouter [3] more easily.
In openssl this can be done by setting up callback through SSL_CTX_set_msg_callback.
Would be nice to have similar ability for golang crypto.

[1] https://github.com/salesforce/ja3
[2] https://golang.org/pkg/crypto/tls/#Config
[3] https://github.com/google/tcpproxy/tree/master/cmd/tlsrouter

[bcmills]

CC @FiloSottile

[mysticaltech]

That is really needed!

[deancn]

Possibly add a raw byte for TLS?

Just reference:
https://github.com/bfenetworks/bfe/blob/develop/bfe_tls/handshake_messages.go#L27

[elindsey]

JA3 fingerprinting in particular has proven to be quite useful, and we've been running a patched stdlib for a few years in order to support it.

I personally see less need to eg. more easily support SNI proxying - tlsrouter's support is already straightforward, and I'm not sure that's a common enough use case that it'd make sense to adjust the stdlib to better accommodate. Exposing the raw ClientHello would also bring up a discussion about how we want to handle the client random, if it should be included or sanitized.

For fingerprinting use cases, the only thing missing is exposing extensions. That's a small change to the API surface, in line with existing ClientHelloInfo fields, and addresses a concrete need. I'd be happy to contribute a patch if there was agreement in that direction.

cc @FiloSottile this seems to be in your wheelhouse. Do you have any thoughts on the request or how we can move this towards a decision?

[komuw]

I ran into a need for this because I wanted to implement ja3 fingerprinting on my app.

Note that ja3 is now kind of going mainstream with AWS[1], cloudflare[2], fastly[3], among others, supporting it.

Also, it seems like caddy[4] ran into a need for something like this.

1. https://aws.amazon.com/about-aws/whats-new/2022/11/amazon-cloudfront-supports-ja3-fingerprint-headers/
2. https://developers.cloudflare.com/bots/concepts/ja3-fingerprint/
3. https://docs.rs/fastly/latest/fastly/struct.Request.html#method.get_tls_ja3_md5
4. Detect HTTPS interception caddyserver/caddy#1430

PS: @elindsey , is your patch somewhere public that someone like me can have a look?

[phuslu]

As supplementary material, I also implement a high performance nginx module in https://github.com/phuslu/nginx-ssl-fingerprint

[mysticaltech]

This is extremely useful in controlling bot abuses on a website!

[gopherbot]

Change https://go.dev/cl/471396 mentions this issue: crypto/tls: expose extensions presented by client to GetCertificate

[komuw]

if this is done, I think we should also update http.Request.TLS *tls.ConnectionState. This is because the initial ask in this issue was ClientHello can be useful for fingerprinting clients, as such I believe this is best utilised in the request-response lifecyle.
http.Request.TLS is already documented as

// allows HTTP servers and other software to record
// information about the TLS connection on which the request
// was received. 

[bpowers]

I think the problem with extending *tls.ConnectionState is that (a) it would either entail retaining the entire ClientHello, which can be up to 64k, or (b) require blessing a specific TLS fingerprinting technique. While the JA3 format is the most widely used today, its not the only format or dimension that one could imagine. With the change I posted above, folks who need fingerprinting can wrap http.Server such that their handler gets the fingerprint in each request's Context, while folks who don't need it pay no performance or memory penalty

[komuw]

On Sat, 4 Mar 2023 at 20:22, Bobby Powers ***@***.***> wrote: I think the problem with extending *tls.ConnectionState is that (a) it would either entail retaining the entire ClientHello, which can be up to 64k, or (b) require blessing a specific TLS fingerprinting technique. While the JA3 format is the most widely used today, its not the only format or dimension that one could imagine.

I’m in agreement.

With the change I posted above, folks who need fingerprinting can wrap http.Server such that their handler gets the fingerprint in each request's Context, while folks who don't need it pay no performance or memory penalty

I think my imagination is failing me here. http.Server.Basecontext & http.Server.ConnContext are the ones that i can think of whose context will end up in the request. But those two methods do not have access to clientHello. Do you mind elaborating a little bit of what you have in mind? Thanks.

…

— Reply to this email directly, view it on GitHub <#32936 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABHMWUJ23QA645OMA726HVLW2N24HANCNFSM4H5WPGZQ> . You are receiving this because you commented.Message ID: ***@***.***>

[bpowers]

@komuw I wrote an example, along with a test showing that its working: https://github.com/bpowers/go-fingerprint-example/blob/main/fingerprinting_server.go

The approach requires wrapping the underlying net.Conn, and then storing a shared struct on both the wrapped conn (Which is accessible from GetCertificate, which is called with the *tls.ClientHelloInfo) and the context returned from ConnContext, which is used to derive every request's context

[1366613]

Any progress on this?

I've developed a WAF for my own private use and just because the stdlib doesn't support having access to the raw TLS handshake, I had to go so deep intercepting all TLS handshakes with gopacket and correlating it with the HTTPS requests using some dirty tricks.

[phuslu]

Finally I managed to extract clienthello raw bytes by MirrorHeaderConn trick.

func (ln TCPListener) Accept() (c net.Conn, err error) {
	c, err = ln.Listener.Accept()
	if err != nil {
		return
	}
	if ln.MirrorHeader {
		c = &MirrorHeaderConn{Conn: c, Header: nil}
	}
	if ln.TLSConfig != nil {
		c = tls.Server(c, ln.TLSConfig)
	}
	return
}

type MirrorHeaderConn struct {
	net.Conn
	Header []byte
}

func (c *MirrorHeaderConn) Read(b []byte) (n int, err error) {
	n, err = c.Conn.Read(b)
	if c.Header == nil && n > 0 && err == nil {
		c.Header = make([]byte, n)
		copy(c.Header, b[:n])
	}

	return
}

For more details please see phuslu/liner@594e552 and phuslu/liner@9f6ef2d

[gospider007]

Packaging based on net. Conn, achieving ja3 fingerprint, http2 fingerprint, and ja4 fingerprint recognition
https://github.com/gospider007/fp#quick-start-with-gin

[elindsey]

@gospider007 your code appears to be making the assumption that the full ClientHello will be present in the first call to Read().

[gospider007]

@gospider007 your code appears to be making the assumption that the full ClientHello will be present in the first call to Read().

Thank you for the reminder. It has now been fixed

[murph12F]

is this been implemented yet? i can see here https://go-review.googlesource.com/c/go/+/471396 that it should have been added but dont seem to be in the documentation. I ve seen that somebody been using a patched stdlib, is that public? thx

[elindsey]

The extensions change has not been merged, and it seems unlikely given the (very reasonable) upstream policy of not exposing API surface if it only supports finger printing use cases. The two options are to patch stdlib or to separately store and re-parse the client handshake bytes (dealing with the extra overhead that entails).

[murph12F]

and can i patch the stdlib? would i have to just add the same changes of the change i ve sent before? sorry never patched stdlibs

[phuslu]

@murph12F here're an example of "patched stdlib" golang https://github.com/phuslu/go

[joeshaw]

You may be interested in the https://github.com/AGWA/tlshacks package, which adds net.Conn and net.Listener implementations that capture the ClientHello and parse it. It also contains functions for generating a JA3 fingerprint from it.

[bobby-stripe]

@rsc I think the original title of this issue and the proposed solution in CL 471396 (that I was the author of) have diverged slightly:

• originally the PR asked for the raw bytes of the ClientHello to be preserved and available on tls.ClientHelloInfo
• The CL "just" adds the list of extensions to tls.ClientHelloInfo in the order they were presented.

To me this felt lighter weight, doesn't force users to re-parse the ClientHello using some third party library, and is suitable for the JA3 TLS fingerprinting technique. There is a JA4 format - the major relevant change is that it requires access to the (first) ALPN value from the ClientHello. We have already parsed that by the time we're building ClientHelloInfo, it seems useful to add that to this proposal.

So the way I'd frame/word this proposal is:

• Add the slice of Extension IDs in the order presented by the ClientHello to ClientHelloInfo
• Add AlpnProtocols []string to ClientHelloInfo

And the concrete advantage this brings to Go users is the ability to fingerprint incoming TLS connections to servers, which is important for DDoS mitigation. If this isn't accepted, its possible for applications to implement fingerprinting themselves (example), but requires wrapping the TCP listener, buffering on the TCP connections, dual-parsing the ClientHello, and duplicating/getting the timeouts right when reading handshake packets. It would significantly reduce risk (especially in getting timeouts wrong/drift from Go defaults) + complexity for the *tls.ClientHelloInfo passed to the tls.Config.GetCertificate function to add the 2 additional fields needed for full fidelity fingerprinting.

[rsc]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

[rolandshoemaker]

@bobby-stripe is the proposed AlpnProtocols different from the (unfortunately confusingly named) SupportedProtos?

Adding a slice of extension IDs seems reasonable, it'll require some piping of that information around to get it into the ClientHelloInfo, but that doesn't seem too cumbersome.

[bobby-stripe]

@rolandshoemaker ah! I had grepped for ALPN and missed SupportedProtos -- thats exactly what we need I think! I have a draft PR that pipes the extension IDs through, its indeed not too cumbersome: https://go-review.googlesource.com/c/go/+/471396 (needs rebasing, I'll do that tonight). The small added test case currently looks to see if extensions is just non-empty; I suspect we want to be more explicit checking for a set of expected extensions, but you might have better or stronger opinions here

[rolandshoemaker]

👍, will take a look. Updating the issue title to reflect the current state of this proposal: to add Extensions []uint16 to ClientHelloInfo.

[rsc]

Have all remaining concerns about this proposal been addressed?

The proposal is to add one field “Extensions []uint16” to ClientHelloInfo and have the server populate it during the handshake.

[rsc]

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

The proposal is to add one field “Extensions []uint16” to ClientHelloInfo and have the server populate it during the handshake.

[rsc]

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

The proposal is to add one field “Extensions []uint16” to ClientHelloInfo and have the server populate it during the handshake.

[bobby-stripe]

@rsc excited for this to be accepted! I've rebased CL 471396, but not sure how to properly tag someone like you or @rolandshoemaker to review it or kick off test execution

[bobby-stripe]

@rolandshoemaker I've addressed your feedback on https://go-review.googlesource.com/c/go/+/471396 - I think its ready to submit (I need your help hitting the button, I don't have bits), but I'm also happy to make further revisions if you have additional comments or concerns! Thanks!

[rolandshoemaker]

I've submitted it for you, thanks for the patch!