Conversation
|
Hi @JustinTimperio! Thanks for the contribution! 🚀 The current diff is unfortunately made more difficult by the file renaming. Can we keep the naming for now? Seems like hardly a necessary change? |
|
@costela oof, my bad, i had originally broken it up into two wrappers but opted for one in the end thus the changed names. |
|
As an aside, you can verify that everything works as intended by running the unit test and pinging each test while its up. I went ahead and copied over a bit of code for generating self signed certs so you can verify the full handshake. For the HTTP server use: ❯ curl https://localhost:9000 --insecure -v
* Host localhost:9000 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
* Trying [::1]:9000...
* Connected to localhost (::1) port 9000
* ALPN: curl offers h2,http/1.1
* (304) (OUT), TLS handshake, Client hello (1):
* (304) (IN), TLS handshake, Server hello (2):
* (304) (IN), TLS handshake, Unknown (8):
* (304) (IN), TLS handshake, Certificate (11):
* (304) (IN), TLS handshake, CERT verify (15):
* (304) (IN), TLS handshake, Finished (20):
* (304) (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / AEAD-CHACHA20-POLY1305-SHA256 / [blank] / UNDEF
* ALPN: server accepted h2
* Server certificate:
* subject: O=ja4plus-test-cert
* start date: Jun 7 21:19:55 2025 GMT
* expire date: Jun 7 21:19:55 2026 GMT
* issuer: O=ja4plus-test-cert
* SSL certificate verify result: unable to get local issuer certificate (20), continuing anyway.
* using HTTP/2
* [HTTP/2] [1] OPENED stream for https://localhost:9000/
* [HTTP/2] [1] [:method: GET]
* [HTTP/2] [1] [:scheme: https]
* [HTTP/2] [1] [:authority: localhost:9000]
* [HTTP/2] [1] [:path: /]
* [HTTP/2] [1] [user-agent: curl/8.7.1]
* [HTTP/2] [1] [accept: */*]
> GET / HTTP/2
> Host: localhost:9000
> User-Agent: curl/8.7.1
> Accept: */*
>
* Request completely sent off
< HTTP/2 200
< content-type: text/plain; charset=utf-8
< content-length: 36
< date: Sat, 07 Jun 2025 21:20:00 GMT
<
* Connection #0 to host localhost left intact
t13d4907h2_0d8feac7bc37_7395dae3b2f3For the Port Listener use: ❯ openssl s_client -connect localhost:9001 -showcerts
Connecting to ::1
CONNECTED(00000005)
Can't use SSL_get_servername
depth=0 O=ja4plus-test-cert
verify error:num=18:self-signed certificate
verify return:1
depth=0 O=ja4plus-test-cert
verify return:1
---
Certificate chain
0 s:O=ja4plus-test-cert
i:O=ja4plus-test-cert
a:PKEY: id-ecPublicKey, 256 (bit); sigalg: ecdsa-with-SHA256
v:NotBefore: Jun 7 21:20:20 2025 GMT; NotAfter: Jun 7 21:20:20 2026 GMT
-----BEGIN CERTIFICATE-----
MIIBazCCARGgAwIBAgIQb90uCWL3XXG0y/abasc/UzAKBggqhkjOPQQDAjAcMRow
GAYDVQQKExFqYTRwbHVzLXRlc3QtY2VydDAeFw0yNTA2MDcyMTIwMjBaFw0yNjA2
MDcyMTIwMjBaMBwxGjAYBgNVBAoTEWphNHBsdXMtdGVzdC1jZXJ0MFkwEwYHKoZI
zj0CAQYIKoZIzj0DAQcDQgAECG/sOLHaGMqkNGVXljgcdtkVosLaPGGRp1fTwZVn
0JOiNY1J3hnwxpw/Y9asSMlFnlHfHhB6iMwDoBUBJkIJlqM1MDMwDgYDVR0PAQH/
BAQDAgWgMBMGA1UdJQQMMAoGCCsGAQUFBwMBMAwGA1UdEwEB/wQCMAAwCgYIKoZI
zj0EAwIDSAAwRQIgWCnrkYTTCmn09bi+fdbYv4IOm/txsqkk2JBNml9kX8UCIQDU
oXYJ20u85/aFsdSE7pgRP6y77eYpsisoCMlmVvJVyw==
-----END CERTIFICATE-----
---
Server certificate
subject=O=ja4plus-test-cert
issuer=O=ja4plus-test-cert
---
No client certificate CA names sent
Peer signing digest: SHA256
Peer signature type: ECDSA
Server Temp Key: X25519, 253 bits
---
SSL handshake has read 722 bytes and written 366 bytes
Verification error: self-signed certificate
---
New, TLSv1.3, Cipher is TLS_AES_128_GCM_SHA256
Protocol: TLSv1.3
Server public key is 256 bit
This TLS version forbids renegotiation.
Compression: NONE
Expansion: NONE
No ALPN negotiated
Early data was not sent
Verify return code: 18 (self-signed certificate)
---
---
Post-Handshake New Session Ticket arrived:
SSL-Session:
Protocol : TLSv1.3
Cipher : TLS_AES_128_GCM_SHA256
Session-ID: AF16B55C69FBAC87AB6E0ACCC3C22E6B9A5876A2009E7D7D5A2BDD835A10CB60
Session-ID-ctx:
Resumption PSK: E2C729A65F76D877D409473054A1C3CA141CCBFE0EE0CEBE58B822E1A08A4088
PSK identity: None
PSK identity hint: None
SRP username: None
TLS session ticket lifetime hint: 604800 (seconds)
TLS session ticket:
0000 - 3a f3 ec ed 35 0d 6d e5-33 a4 a0 a9 c3 cf 45 3c :...5.m.3.....E<
0010 - 61 c4 c3 0e 89 bc ce f8-8a a9 ba c1 66 73 45 b0 a...........fsE.
0020 - df 77 cf a5 a7 ef b4 0d-db 42 93 c1 2e 20 e7 d3 .w.......B... ..
0030 - 4f 60 6e 28 28 30 d1 5f-44 59 87 c3 84 bc 2c cd O`n((0._DY....,.
0040 - 2e 87 e9 74 a9 fc 6e 33-07 6f c6 40 37 95 e1 32 [email protected]
0050 - 5a 95 36 23 7b 14 b4 eb-bd ef 95 54 53 96 f0 83 Z.6#{......TS...
0060 - 82 ed 30 90 18 67 e2 34-21 ..0..g.4!
Start Time: 1749331235
Timeout : 7200 (sec)
Verify return code: 18 (self-signed certificate)
Extended master secret: no
Max Early Data: 0
---
read R BLOCK |
|
Hi @costela, Just a quick update, I did some testing with this in the wild and observed some panics that i just patched. I also made some small changes to the ergonomics of the new api. |
|
Hi @costela, just checking up here on the PR |
costela
left a comment
There was a problem hiding this comment.
Thanks again for the contribution. I have a few thoughts we could go through before merging. 🙏
| * Security monitoring | ||
|
|
||
| Contributions are welcome for the other fingerprints in the family 😉 | ||
| Currently, JA4Plus offers a single fingerprinting function based on [TLS ClientHello](https://pkg.go.dev/crypto/tls#ClientHelloInfo) information. Contributions are welcome for implementing other fingerprints in the JA4 family! |
There was a problem hiding this comment.
| Currently, JA4Plus offers a single fingerprinting function based on [TLS ClientHello](https://pkg.go.dev/crypto/tls#ClientHelloInfo) information. Contributions are welcome for implementing other fingerprints in the JA4 family! | |
| Currently, JA4Plus offers a single fingerprinting function based on [TLS ClientHello](https://pkg.go.dev/crypto/tls#ClientHelloInfo) information. Contributions are welcome for implementing other fingerprints in the JA4+ family! |
| The `JA4Middleware` struct and its associated methods are designed to bridge the gap between the TLS handshake and the HTTP request context. | ||
|
|
||
| 1. **TLS Configuration:** The middleware modifies the TLS configuration to intercept the `ClientHello` information. | ||
| 2. **Fingerprint Storage:** When a `ClientHello` is received, the corresponding JA4 fingerprint is stored in a `sync.Map` associated with the client's remote address. |
There was a problem hiding this comment.
not sure this implementation detail is necessary for the consumer?
| 2. **Fingerprint Storage:** When a `ClientHello` is received, the corresponding JA4 fingerprint is stored in a `sync.Map` associated with the client's remote address. | |
| 2. **Fingerprint Storage:** When a `ClientHello` is received, the corresponding JA4 fingerprint is stored. |
|
|
||
| ### Important Considerations | ||
|
|
||
| * **Certificate Management:** Ensure you have valid TLS certificates configured for your server. |
There was a problem hiding this comment.
why is the certificate validity relevant here? Isn't it valid to extract the client fingerprint regardless?
| // Load TLS certificates (replace with your actual certificates) | ||
| cert, err := tls.LoadX509KeyPair("server.crt", "server.key") // Example. Consider using a proper configuration. | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } | ||
|
|
||
| tlsConfig := &tls.Config{ | ||
| Certificates: []tls.Certificate{cert}, | ||
| } |
There was a problem hiding this comment.
why do we need this in the example? We're already calling ListenAndServeTLS, which eventually sets the certs as well?
| func JA4FromContext(ctx context.Context) string { | ||
| if fingerprint, ok := ctx.Value(ja4FingerprintCtxKey{}).(string); ok { | ||
| return fingerprint | ||
| func JA4FromContext(ctx context.Context) (string, bool) { |
There was a problem hiding this comment.
I don't think there's a big advantage is the extra bool return value? This is not a generic cache, where we might need to differentiate between an existing nil entry and a non-existing one. If the function returns an empty string, it's pretty clear the fingerprint was not injected?
| type ja4FingerprintCtxKey struct{} | ||
| // NewHandlerWrapper takes a middleware, a tls config, and a http.handler and returns a modified handler that injects | ||
| // fingerprints into the context of the a connection so they can be consumed later. | ||
| func (m *JA4Middleware) NewHandlerWrapper(middleware *JA4Middleware, tlsConfig *tls.Config, next http.Handler) http.Handler { |
There was a problem hiding this comment.
this feels a bit weird: for once, we're passing *JA4Middleware twice, but also, we're doing "magic stuff" to the passed tlsConfig, potentially overwriting existing settings.
I think the API might be more ergonomic if we just added a convenience setup method like the following. That way we could even unexport some methods.
func (m *JA4Middleware) SetupServer(srv http.Server) http.Server {
newSrv := srv // shallow copy
newSrv.Handler = m.Wrap(srv.Handler)
if origConnState := newSrv.ConnState; origConnState == nil {
newSrv.ConnState = m.httpCallback // ← unexported
} else {
newSrv.ConnState = func(c net.Conn, cs ConnState) {
m.httpCallback(c, cs)
origConnState(c, cs)
}
}
newTLSConfig := cmp.Or(srv.tlsConfig, &tls.Config{})
if origGetConfigForClient := newTLSConfig.GetConfigForClient; origGetConfigForClient == nil {
newTLSConfig.GetConfigForClient = m.getConfigForClient // ← unexported and matched method signature for simplicity
} else {
newTLSConfig.GetConfigForClient = func(chi *tls.ClientHelloInfo) (*tls.Config, error) {
_, _ = m.getConfigForClient(chi) // we know we never return anything
return origGetConfigForClient(chi)
},
}
}|
|
||
| tlsConfig.GetConfigForClient = func(chi *tls.ClientHelloInfo) (*tls.Config, error) { | ||
| // Protects against panics when generating the JA4 | ||
| if chi != nil { |
There was a problem hiding this comment.
GetConfigForClient should be guaranteed to get a non-nil ClientHelloInfo?
| // Returns the modified TLS config | ||
| func (m *JA4Middleware) ReturnTLSConfig() *tls.Config { | ||
| return m.tlsConfig | ||
| } |
There was a problem hiding this comment.
are we using this for anything? Not sure I see why we're storing the tlsConfig at all? 🤔
| } | ||
|
|
||
| // JA4FromContext extracts the JA4 fingerprint from the provided [http.Request.Context]. | ||
| // It requires a server set up with [JA4Middleware] to work. |
There was a problem hiding this comment.
we should probably keep this in some form? It's not directly clear that this function will not work at all without the setup?
| fingerprint, ok := m.connectionFingerprints.Load(conn.RemoteAddr().String()) | ||
| if !ok { | ||
| return "", ok | ||
| } | ||
| return "" | ||
|
|
||
| f, ok := fingerprint.(string) | ||
| return f, ok |
There was a problem hiding this comment.
this is another case where I'm not sure the ok return is necessary. We'll always have either fingerprint == "" && !ok OR fingerprint != "" && ok?
Hi @costela,
I stumbled on this project earlier this morning when I was looking for something to use in my own project(https://github.com/JustinTimperio/tripwire). I noticed though that there were no easy ways to wrap the net.listener so I went and ahead and made some small additions to the middleware wrapper to allow for that. I also did a bit of clean up on the http middleware and added some docs.
Change log:
Thanks for publishing this, and if you have any change requests for the PR just ping me.
Best,
Justin