Add support for B.A.T.M.A.N. Advanced

[T-X]

This adds support for the layer 2 mesh routing protocol B.A.T.M.A.N. Advanced. "batadv" can be used to filter on batman-adv packets. It also allows later filters to look at frames inside the tunnel when both "version" and "type" are specified.

Documentation for the batman-adv protocol can be found at the following locations:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/networking/batman-adv.rst https://www.open-mesh.org/

Signed-off-by: Linus Lüssing <[email protected]>

[T-X]

One remark: pcap/batadv_packet.h and pcap/batadv_legacy_packet.h are basically copies from the Linux kernel and are therefore GPLv2 licensed. Let me know if that's okay for you or if I should ask the other maintainers and contributors for a BSD-3-Clause (compatible) dual-license for these two header files.

[infrastation]

Thank you for preparing this feature. Neither macOS nor FreeBSD like the new Linux-specific headers. As you mentioned, these are GPL, also the amount of filter-specific code seems to be much smaller than the amount of declarations. So it might be worth to consider if the new code could have just the declarations it needs, then it would be simpler to make these portable and have the same licence as the rest of libpcap. Other opinions are welcome.

[guyharris]

So it might be worth to consider if the new code could have just the declarations it needs, then it would be simpler to make these portable and have the same licence as the rest of libpcap. Other opinions are welcome.

My opinion can be summarized as "+1". :-)

I.e., no GPLed code, please.

[guyharris]

Here's a tarball containing a patch, pcap/batadv_legacy_packet.h, and pcap/batadv_packet.h. The patch is a variant of your change that compiles on macOS Catalina and Xcode 11.7, so it rips out enough Linuxisms to get it to compile there, at least.

It also gets rid of all #pragma pack stuff (not all compilers necessarily support it), instead declaring 2-byte and 4-byte integral values as arrays of uint8_t of the appropriate length.

Do those headers need to be part of the libpcap distribution, rather than being internal to the libpcap source?

tarfile.gz

[T-X]

Thanks for the great feedback!

Changelog v2:

• changes to batadv_{legacy_,}packet.h:
  • integrated @guyharris' suggested changes: removal of #pragma packed(2), bitfield endianess #ifdefs, Linux specific includes, unused definitions, conversion to uint8_t only
  • further removed kernel doc comments and instead added a small note to consult the Linux kernel version for a complete definition
  • changed the license to BSD-3 for these new, tiny versions
  • moved these two header files from the pcap directory to the root directory
  • shortened the include guards, the UAPI directory for instance was Linux specific

[T-X]

Changelog v3:

• removed unused variables "s" and "b1" in gen_batadv() to satisfy the AppVeyor CI / Visual Studio builds

[infrastation]

Passed FreeBSD build this time, that's better. One other thing that would complement this feature is some tcpdump test(s) to flex the new keywords. Do you have enough batman packets with the right protocol fields?

[T-X]

Changelog v4:

• changed offset parameter of gen_batadv_push_offset() from size_t to u_int and added according type cast on callers, to make AppVeyor CI / Visual Studio happy

[T-X]

@infrastation sure, generating some test packets is no issue, can do that next weekend. According tests need to be added to the tcpdump repository in a separate commit over there, right?

[infrastation]

Yes, that's correct, and the tests will not pass until this feature has been merged, but eventually it would be consistent.

[T-X]

Some simple tests for tcpdump for the batman-adv version 15 packet types and their decoding offsets can be found here: https://github.com/the-tcpdump-group/tcpdump/pull/891

[infrastation]

Thank you. For what it's worth, I have just looked through the proposed changes one more time and could not find any immediate issues to fix (which does not automatically mean everything is good, but still). Other reviewers are welcome.

[T-X]

I'm a bit hesitant to ask, as I can see with all the frequent commits that everyone is very eagerly working on libpcap/tcpdump. But any chance to get this merged? Would help me adding it to the collaborative projects I would want to use this in. Which are usually reluctant in adding changes that are not upstream yet.

[mcr]

Hi, @fenner , you've been down the pcap compiler space more than I, do you have any final comments before I merge this?

[fxlb]

Rebased. "All checks have failed" => There are some errors fo fix.

[T-X]

Changelog v5:

• adding "static" attribute to functions "gen_batadv_check_version" and "gen_batadv_check_type" to fix "no previous prototype for function" warnings
• removing unnecessary CHECK_INT_VAL($1) and CHECK_PTR_VAL($2) in grammar.y.in in pbatadv, fixing an unsigned vs. signed check warning with CHECK_INT_VAL($1)

[T-X]

Changelog v6:

• rebasing to current master (tiny conflict since 5f634cf36c in pcap-filter.manmisc.in, resolved it)

[AiyionPrime]

@guyharris is this ready to merge? Or am I overlooking some remark?

[infrastation]

If it helps, I have not learned the parser code sufficiently well yet, so far I can make sense of roughly half of the proposed C code changes.

[AiyionPrime]

Hi, @fenner , you've been down the pcap compiler space more than I, do you have any final comments before I merge this?

@mcr did you get reply on that? I'd really like to see this in libpcap; and I'm confident @T-X would be available on the matter, if there were some urgent changes to this PR before it could get merged.

[mcr]

@mcr did you get reply on that? I'd really like to see this in libpcap; and I'm confident @T-X would be available on the matter, if there were some urgent changes to this PR before it could get merged.

I did not, but if @guyharris is okay with it, then I am.

[AiyionPrime]

@guyharris its been three months :/ Are you by any chance okay with these changes?

[AiyionPrime]

@guyharris its been four months, could you give this another round of a review?

[fxlb]

To have the new header files included in a release archive, they must be added in Makefile.in (PUBHDR or HDR). (See the result of new target make releasecheck here: "fatal error: batadv_packet.h: No such file or directory" https://github.com/the-tcpdump-group/libpcap/pull/980/checks?check_run_id=9728030848) @guyharris @mcr: Is it PUBHDR or HDR in this case?

[T-X]

Changelog v7:

• added batadv_legacy_packet.h and batadv_packet.h to Makefile.in's HDR variable, to fix "make releasecheck" and "make releasetar"

@fxlb I've added it to HDR and not PUBHDR. I don't think it's necessary to expose these header files to a user's system include directory. It's more similar to ieee80211.h, ppp.h or pcap-usb-linux-common.h for instance.

[T-X]

Is there anything else I can do to progress this or to motivate for its adoption?

batman-adv is a quite popular mesh routing protocol in various non-profit wireless community networks. Like Freifunk (via the Gluon firmware) or AlterMundi (via the LibreMesh firmware). But also commercial vendors use it, like Datto or Plasma Cloud.

On embedded devices Wireshark/tshark is not an option, even though libwireshark has a batman-adv dissector, due to the limited amount of flash and RAM in that case. So having this more tunable and better performing capture method would help a lot, as especially in these open, non-profit wireless community networks various layer 2 traffic can happen on top.

Next, I want to use this in Gluon with bpfcountd to create more detailed traffic statistics, to be able to better react to issues and to improve batman-adv through real-world data: https://github.com/freifunk-gluon/gluon/pull/2367. And by that improving the stability and the features users in these communities can use, as well as the sizes they can scale their networks to.

[fxlb]

@guyharris: Do you think your comments have been taken into account? Do you think this PR is ready to be merged?

[fxlb]

(Currently rebased on the-tcpdump-group:master.)

[AiyionPrime]

@fxlb Have you gotten any feedback from @guyharris on this? It's been another four months; is there anything I can provide to get this thing moving forward?

[fxlb]

The current types/sub-types in libpcap filter syntax use "-" and not "_".

Examples:

wlan:   
assoc-req,  assoc-resp, ...

The  following   ICMP   type   field   values   are   available:
icmp-echoreply,  icmp-unreach, ...

The  following  ICMPv6  type   field   values   are   available:
icmp6-destinationunreach, icmp6-packettoobig, ...

The following TCP flags field  values  are  available:  tcp-fin,
tcp-syn, tcp-rst, tcp-push, tcp-ack, tcp-urg, tcp-ece, tcp-cwr.

Thus for batman-adv packet types,

The following packet type aliases are available for compat  ver-
sion  14:  iv_ogm,  unicast-frag (?), tt_query, roam_adv, unicast_4addr.

The following packet type aliases are available for compat  ver-
sion 15: iv_ogm, unicast_frag, unicast_4addr, unicast_tvlv.

it should be better to replace "_" by "-".