droe / sslsplit

Transparent SSL/TLS interception
https://www.roe.ch/SSLsplit
BSD 2-Clause "Simplified" License
1.73k stars 327 forks source link

Use interface MTU for mirroring #236

Closed droe closed 5 years ago

droe commented 5 years ago

For content logging to a mirror interface and destination IP, calculate an appropriate MSS based on the interface MTU instead of using a hardcoded value.

Main reason I am asking for a review is that I am not 100% sure how portable ioctl(SIOCGIFMTU) is in practice, but as usual please also object if you disagree with the idea of maxing out the MSS based on the interface MTU of the mirror target interface or if something else is amiss.

sonertari commented 5 years ago

SSLsplit crashes after it receives the first couple of connections. Earlier over night I thought I fixed it by setting the buf size correctly for ipv4 and ipv6 in logpkt_write_packet(), for the crashes introduced by e75c6618a8dd41058392aa115a4437082551f1fa (Enlarge MSS for PCAP files, shrink for mirroring), but the crashes reappear if I use the latest commit. sys_get_mtu() returns 1500 on OpenBSD. I have used MTU + ether/ip6/tcp header sizes for all buffers, but no luck yet. I'll start looking into it again.

droe commented 5 years ago

MTU 1500 is expected, the ioctl works, that’s great. Note that the buffer we control with MAX_PKTSZ only applies to PCAP writing, not mirroring, so if mirroring crashes as a result of this commit, it would be connected to libnet. Can you send me a PCAP of some packets generated by OpenBSD libnet in mirroring mode, when you override ctx->mss to 1400?

droe commented 5 years ago

The above commit should fix the regression introduced in 1f1cf4adc181c39311e3164ec548b077f3423eb6 (crash when accessing sockaddr from logger thread when connection context was already freed).