droe / sslsplit

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

mirror: support special 'none' target value for -T #282

Closed victorjulien closed 3 years ago

victorjulien commented 3 years ago

Support "none" as a target value, indicating the target is irrelevant.

The use case is an IDS sensor listening on a dummy interface for the packets sslsplit produces. The IDS will listen in promisc mode, so the target is irrelevant.

victorjulien commented 3 years ago

This is an proposed solution for #272. The use case for me is having Suricata running on the same host as sslsplit. I want to use something like a dummy interface or veth pair where sslsplit just outputs packets, and Suricata can read them. Since Suricata will open the interface in promisc mode, the "destination" of the interface is not relevant.

Sslsplit currently tries to do an arp lookup for the target, but neither dummy nor veth can answer these.

If this method looks acceptable I will make a better PR with updates to docs, etc.

sonertari commented 3 years ago

Looking good to me. I guess that's all we need to do, the rest is up to the IDS listening on that interface. I will update the documentation of the -T option after merging. Please let me know if it is alright to merge now.

victorjulien commented 3 years ago

Thanks @sonertari. I've added a small patch to update the usage info. Please let me know if I should squash the commits or do new PR instead of pushing to an existing one.

sonertari commented 3 years ago

On second thought, passing "-T none" seems redundant because it is actually equivalent to not passing -T at all. So the condition in your if statement would simply check if dst_ip_s is NULL: !dst_ip_s. As far as I remember, leaving mirrortarget NULL shouldn't cause any issues in the rest of the code (but we should try, it's been 2 years since I worked on that code).

We should of course make -T not required if -I is used, in main.c around line 522, to allow NULL mirrortarget.

Btw, we should update the documentation in the man pages and the sample conf file too.

If we do these changes, I guess we'd better squash.

victorjulien commented 3 years ago

Makes sense, please see #283

victorjulien commented 3 years ago

Replaced by #283.