-
Notifications
You must be signed in to change notification settings - Fork 327
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
mirror: allow mirroring without explict target #283
Conversation
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.
The first if condition opts->mirrortarget && !opts->mirrorif
on line 513 was supposed to remain, the second on line 517 should go (not both). Because we still require mirrorif if mirrortarget is defined. I guess I wasn't clear, sorry.
I'd appreciate the documentation too if you have time. Otherwise, I can do the rest. As you wish. Thanks Victor. |
21cbb0e
to
687267b
Compare
Whoops, good catch. Wanted to go too fast. |
I've pushed some updates, fixing the option check and updating the man page and default config. Should there be other updates? |
main.c
Outdated
fprintf(stderr, "%s: -I depends on -T.\n", argv0); | ||
exit(EXIT_FAILURE); | ||
} | ||
if (opts->mirrortarget && !opts->mirrorif) { |
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.
ah, this formatting is off, will fix
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.
Done
Allow omitting the -T option, 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.
687267b
to
96d5585
Compare
I think it's looking good, thanks @victorjulien. I can merge this to the develop branch if you are ok too. @droe can review it there, and it's up to him to merge to the master branch. My guess is that a new release will take a while, so people interested in this change will need to fetch the develop branch until then. |
Sounds good @sonertari. I've converted it to a non-draft PR. |
Thanks @sonertari. Would this be something you'd consider for SSLproxy as well? If so I'd be happy to port it and do a PR there as well. |
Thanks @victorjulien, but it's simple enough, I can do that myself, so you can use your time for more difficult stuff like SSLproxy support for Suricata ;-). |
Allow omitting the -T option, 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.
How I've tested this:
Replaces #282. If code looks good I'll do the doc side next.