search

CESNET / ipfixprobe

IPFIX flow exporter with DPDK support capable of bi-directional flows, per-packet-information statistics, and extensibility via processing plugins (e.g., for application layer parsers).
https://cesnet.github.io/ipfixprobe/
BSD 3-Clause "New" or "Revised" License
41 stars 18 forks source link

process/wg: do not check if transport data length is divisible by 16 #200

Closed PaulosV closed 10 months ago

PaulosV commented 11 months ago

This corrects an implementation flub where I assumed that the length of a WireGuard data packet would always be divisible by 16. That is verifiably false, and the receiving code of WireGuard does not assume that.

Zero-padding the message data to the multiples of 16 relates to memory alignment and is mainly a measure to "complicate traffic analysis". It does not always happen; the most common example is traffic approaching the inner MTU, which is set by default to 1420, but can be set to arbitrary values by the user.

1420 is not divisible by 16, and an IP packet of length 1420 is not padded any further by WireGuard. With additional 32 bytes of WireGuard headers and 32 bytes of UDP, IP and frame headers, the total frame length is 1494, which is, again, not divisible by 16.

So this check needs to go. It may increase the number of false positives, I am going to do some tests about this.

codecov-commenter commented 11 months ago

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (84b4f7d) 40.90% compared to head (e0dbd6f) 40.91%.

Files Patch % Lines
process/wg.cpp 0.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #200 +/- ## ========================================== + Coverage 40.90% 40.91% +0.01% ========================================== Files 100 100 Lines 8826 8826 Branches 1312 1312 ========================================== + Hits 3610 3611 +1 + Misses 4446 4443 -3 - Partials 770 772 +2 ``` | [Flag](https://app.codecov.io/gh/CESNET/ipfixprobe/pull/200/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CESNET) | Coverage Δ | | |---|---|---| | [tests](https://app.codecov.io/gh/CESNET/ipfixprobe/pull/200/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CESNET) | `40.91% <0.00%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=CESNET#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.