fix: correct byte order in IPFIX set length fields #15
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes incorrect bit shifting that caused malformed FlowSet Length values in IPFIX packets. Changes shifts from >>16 to >>8 in both getSenderRecord() and getReceiverInformation() to properly encode length fields in big-endian format.
Updates sender information with no locator template descriptor size from 0x2E to 0x34 bytes to accurately reflect the total length including flow start seconds field. Prevents template bytes from spilling into subsequent IPFIX sets and being misinterpreted as Set headers.
Aligns receiver information packet assembly with sender information approach by placing template ID and length at start of payload rather than appending later. Removes separate output buffer and +4 length offset, making the logic easier to follow and reducing the chance of buffer offset bugs.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes two critical bugs in the PSKReporter upload code.
The first bug is incorrect bit shifting that caused malformed FlowSet Length values in IPFIX packets. Changes shifts from >>16 to >>8 in both
getSenderRecord()andgetReceiverInformation()to properly encode length fields in big-endian format.The impact of this bug is data loss for any IPFIX data set that has more than 255 bytes in it. For example, the sender information spot data payload is lost in the below packet, since Set 5 has a malformed length of 0, even though it contains several hundred bytes of spot sender information.
The second bug is that the sender information template descriptor for the case w/o a locator has the wrong length. This means the template bytes are truncated early by 6 bytes, which will make the receiver have an incorrect template. The extra 6 bytes are then erroneously treated as the template ID (2 bytes), size (2 bytes), and subsequent data, for whatever set is next. This corrupts decoding that set - as now the template used to decode it is wrong, the length is wrong, etc.
The above screenshot shows this:
30351is the enterprise ID used for PSKReporter fields, which is mistakenly interpreted as the template ID of the next flow. This has a knock-on effect -- Set 5 has ID61444, which doesn't correspond to any of the valid template IDs. In reality, what should be Set 4 is ending too early due to the bad length, so whatever bytes appear are mistakenly interpreted as a 5th set with a random template ID/length/payload.