Added basic PCAP capture and validation functionality. #24
Conversation
| clientKeyLog, err := os.OpenFile("/logs/client_keylog", os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) | ||
| if err != nil { | ||
| log.Fatal(err) | ||
| } |
There was a problem hiding this comment.
Perhaps this is a dumb question, but here goes. Since the Go standard library is able to produce this output, it's conceivable that there's also code somewhere that consumes it. Asking another way: might there be a Go-native way to read and interpret this log? This would be better than calling tshark on the command line.
There was a problem hiding this comment.
Nope, Go crypto/tls only supports writing this secret. The callback was specifically added to support decryption in Wireshark. There is no way to feed these derived keys to Go crypto/tls and decrypt records.
Dissection (as is done by Wireshark) is a different use case than a client/server implementation. The former needs to play both the client and server at the same time, and try its best to interpret the data.
There was a problem hiding this comment.
Thanks for looking at this @Lekensteyn. I'm thinking in the long term we might want to write a custom Wireshark dissector based on the stock TLS one, in order to handle experimental features (such as KEMTLS) that diverge from ordinary TLS to varying degrees. I believe the current dissector does not handle delegated credentials properly.
There was a problem hiding this comment.
DC is partially supported in Wireshark git master. I have a local patch to dissect the TLS Certificate handshake message properly, but it does not perform actual (DC) certificate chain validation.
There was a problem hiding this comment.
After talking to @chris-wood, we've decided to just do basic best-effort transcript validation for now, but I will get back to you on that.
cmd/validatepcap/parse.go
Outdated
| func parsePcap(tsharkPath string, pcapPath string, keylogPath string) Transcript { | ||
| rawJSON, err := exec.Command(tsharkPath, | ||
| "-r", pcapPath, | ||
| "-d", "tcp.port==4433,ssl", |
There was a problem hiding this comment.
This should not be needed, TLS is explicitly recognized through a heuristic if there is no other port override. It does not hurt though. I would however suggest changing the deprecated ssl to tls. The arguments can also be joined, whether you want to do it is a style question:
| "-d", "tcp.port==4433,ssl", | |
| "-dtcp.port==4433,tls", |
|
This PR is ready for review. I think there's scope for more cleanup/API changes depending on how the CI runner is built, but I'll leave those for when I get to the runner. |
No description provided.