Fixed libcap with "any" device setting #5
Conversation
…nterface to "any".
| fprintf(stderr, "pcap_lookupdev: %s\n", error); | ||
| return 1; | ||
| } | ||
| device = "any"; |
There was a problem hiding this comment.
Why not continue using pcap_lookupdev()? This change looks unrelated to fixing support for the "any" setting, when that is specified in SCANLOGD_DEVICE.
There was a problem hiding this comment.
Oh, I see you wrote in the PR description:
I have also changed the default libpcap interface to "any".
I think if we do this, it should be a separate commit.
BTW, your current commit message is over-long - I suggest that first lines (titles) of commit messages be limited to 72 chars, and subsequent lines (if any) wrapped at 75 (so that it's 79 max after git log indents them by 4).
There was a problem hiding this comment.
pi@pi:~/scanlogd $ make libpcap
gcc -Wall -O2 -fomit-frame-pointer -c scanlogd.c
gcc -Wall -O2 -fomit-frame-pointer -I/usr/include/pcap -c in_pcap.c
in_pcap.c: In function ‘in_init’:
in_pcap.c:21:2: warning: ‘pcap_lookupdev’ is deprecated: use 'pcap_findalldevs' and use the first device [-Wdeprecated-declarations]
21 | if (!(device = pcap_lookupdev(error))) {
| ^~
In file included from in_pcap.c:5:
/usr/include/pcap/pcap.h:394:16: note: declared here
394 | PCAP_API char *pcap_lookupdev(char *)
| ^~~~~~~~~~~~~~
gcc -s scanlogd.o in_pcap.o -lpcap -o scanlogd
There was a problem hiding this comment.
I also think that making the "any" as default should work for most users. It is much more useful than picking a random first interface.
There was a problem hiding this comment.
Well, if we change the default, that also needs to be reflected in the comment in params.h.
| @@ -64,6 +63,9 @@ void in_run(void (*process_packet)(struct header *packet, int size)) | |||
| hw_size = 14; | |||
| } | |||
|
|
|||
| if(device == NULL || strcmp(device, "any") == 0) | |||
There was a problem hiding this comment.
Inconsistent coding style with the rest of the file.
There was a problem hiding this comment.
Could you be more specific about what's wrong with this coding style?
There was a problem hiding this comment.
The missing space between if and its opening brace is wrong.
Also, while there isn't an exact example like this in scanlogd in particular, other checks generally use !function(...) instead of function(...) == 0. I understand that for strcmp in particular the latter is regarded by many as a more descriptive way to write it, so I don't mind.
| @@ -64,6 +63,9 @@ void in_run(void (*process_packet)(struct header *packet, int size)) | |||
| hw_size = 14; | |||
| } | |||
|
|
|||
| if(device == NULL || strcmp(device, "any") == 0) | |||
| hw_size += 2; | |||
There was a problem hiding this comment.
Now the most important aspect: skipping the 2 bytes unconditionally probably breaks proper behavior with certain older versions of libpcap - ideally, we should investigate that and make this skipping conditional upon libpcap version (if our understanding is correct, or do something different if the understanding is not confirmed).
There was a problem hiding this comment.
I've just tested - the skipping of 2 bytes is needed even with libpcap 0.9.4. So we're good making this change unconditionally, and not skipping was probably always a scanlogd bug.
I have fixed the libcap with "any" device setting. I have also changed the default libpcap interface to "any".
Explanation of my changes:
https://github.com/janw-cz/scanlogd/wiki/Remarks-to-my-fork-of-openwall-scanlogd