-
Notifications
You must be signed in to change notification settings - Fork 2
Middleware and Doc Improvements #3
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
base: main
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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