From bb0ab89615d25fe0fe7853c7dfc92d9a42b72131 Mon Sep 17 00:00:00 2001 From: Dominik Pataky Date: Sat, 19 Aug 2023 09:24:49 +0200 Subject: [PATCH] Squashed commit of branch feature/ipfix-padding: commit 63abf52ec640a019f8c45c1208f0dfb585641781 Padding: add offset!=length check to reduce safety check calls Adds another check when parsing a set. The check "offset != self.header.length" allows to skip the padding checks if the offset is the same as the length, not calling rest_is_padding_zeroes and wasting CPU time. commit 8d1cf9cac12c45c0af70591b646d898ba5c923fc Finish IPFIX padding handling Tested implementation of IPFIX set padding handling. Uses TK-Khaw's proposed no_padding_last_offset calculation, extended as modulo calculation to match multiple data set records. Tests were conducted by capturing live traffic on a test machine with tcpdump, then this capture file was read in by softflowd 1.1.0, with the collector.py as the export target. The exported IPFIX (v10) packets were then using both no padding and padding, so that tests could be validated. Closes #34 Signed-off-by: Dominik Pataky commit 51ce4eaa268e4bda5be89e1d430477d12fc8a72c Fix and optimize padding calculation for IPFIX sets. Refs #34 commit 9d3c4135385ca9714b7631a0c5af46feb891a9fb Author: Khaw Teng Kang Date: Tue Jul 5 16:29:12 2022 +0800 Reverted changes to template_record, data_length is now computed using field length in template. Signed-off-by: Khaw Teng Kang commit 3c4f8e62892876d4a2d42288843890b97244df55 IPFIX: handle padding (zero bytes) in sets Adds a check to each IPFIX set ID branch, checking if the rest of the bytes in this set is padding/zeroes. Refs #34 Signed-off-by: Dominik Pataky --- netflow/collector.py | 6 ++-- netflow/ipfix.py | 79 ++++++++++++++++++++++++++++++++++++-------- 2 files changed, 69 insertions(+), 16 deletions(-) diff --git a/netflow/collector.py b/netflow/collector.py index 0cc9538..81c27fc 100644 --- a/netflow/collector.py +++ b/netflow/collector.py @@ -19,9 +19,9 @@ import threading import time from collections import namedtuple -from .ipfix import IPFIXTemplateNotRecognized -from .utils import UnknownExportVersion, parse_packet -from .v9 import V9TemplateNotRecognized +from netflow.ipfix import IPFIXTemplateNotRecognized +from netflow.utils import UnknownExportVersion, parse_packet +from netflow.v9 import V9TemplateNotRecognized RawPacket = namedtuple('RawPacket', ['ts', 'client', 'data']) ParsedPacket = namedtuple('ParsedPacket', ['ts', 'client', 'export']) diff --git a/netflow/ipfix.py b/netflow/ipfix.py index 1524f5a..57276ce 100644 --- a/netflow/ipfix.py +++ b/netflow/ipfix.py @@ -635,6 +635,10 @@ class IPFIXTemplateNotRecognized(KeyError): pass +class PaddingCalculationError(Exception): + pass + + class IPFIXHeader: """The header of the IPFIX export packet """ @@ -663,9 +667,6 @@ class IPFIXTemplateRecord: offset += offset_add if len(self.fields) != self.field_count: raise IPFIXMalformedRecord - - # TODO: if padding is needed, implement here - self._length = offset def get_length(self): @@ -697,8 +698,6 @@ class IPFIXOptionsTemplateRecord: raise IPFIXMalformedRecord offset += offset_add - # TODO: if padding is needed, implement here - self._length = offset def get_length(self): @@ -812,17 +811,29 @@ class IPFIXSet: self.records = [] self._templates = {} - offset = IPFIXSetHeader.size + offset = IPFIXSetHeader.size # fixed size + if self.header.set_id == 2: # template set while offset < self.header.length: # length of whole set template_record = IPFIXTemplateRecord(data[offset:]) self.records.append(template_record) if template_record.field_count == 0: + # Should not happen, since RFC says "one or more" self._templates[template_record.template_id] = None else: self._templates[template_record.template_id] = template_record.fields offset += template_record.get_length() + # If the rest of the data is deemed to be too small for another + # template record, check existence of padding + if ( + offset != self.header.length + and self.header.length - offset <= 16 # 16 is chosen as a guess + and rest_is_padding_zeroes(data[:self.header.length], offset) + ): + # Rest should be padding zeroes + break + elif self.header.set_id == 3: # options template while offset < self.header.length: optionstemplate_record = IPFIXOptionsTemplateRecord(data[offset:]) @@ -834,16 +845,47 @@ class IPFIXSet: optionstemplate_record.scope_fields + optionstemplate_record.fields offset += optionstemplate_record.get_length() + # If the rest of the data is deemed to be too small for another + # options template record, check existence of padding + if ( + offset != self.header.length + and self.header.length - offset <= 16 # 16 is chosen as a guess + and rest_is_padding_zeroes(data[:self.header.length], offset) + ): + # Rest should be padding zeroes + break + elif self.header.set_id >= 256: # data set, set_id is template id - while offset < self.header.length: - template = templates.get( - self.header.set_id) # type: List[Union[TemplateField, TemplateFieldEnterprise]] - if not template: - raise IPFIXTemplateNotRecognized - data_record = IPFIXDataRecord(data[offset:], template) + # First, get the template behind the ID. Returns a list of fields or raises an exception + template_fields = templates.get( + self.header.set_id) # type: List[Union[TemplateField, TemplateFieldEnterprise]] + if not template_fields: + raise IPFIXTemplateNotRecognized + + # All template fields have a known length. Add them all together to get the length of the data set. + dataset_length = functools.reduce(lambda a, x: a + x.length, template_fields, 0) + + # This is the last possible offset value possible if there's no padding. + # If there is padding, this value marks the beginning of the padding. + # Two cases possible: + # 1. No padding: then (4 + x * dataset_length) == self.header.length + # 2. Padding: then (4 + x * dataset_length + p) == self.header.length, + # where p is the remaining length of padding zeroes. The modulo calculates p + no_padding_last_offset = self.header.length - ((self.header.length - IPFIXSetHeader.size) % dataset_length) + + while offset < no_padding_last_offset: + data_record = IPFIXDataRecord(data[offset:], template_fields) self.records.append(data_record) offset += data_record.get_length() - self._length = offset + + # Safety check + if ( + offset != self.header.length + and not rest_is_padding_zeroes(data[:self.header.length], offset) + ): + raise PaddingCalculationError + + self._length = self.header.length def get_length(self): return self._length @@ -973,3 +1015,14 @@ def parse_fields(data: bytes, count: int) -> (list, int): ) offset += 4 return fields, offset + + +def rest_is_padding_zeroes(data: bytes, offset: int) -> bool: + if offset <= len(data): + # padding zeros, so rest of bytes must be summed to 0 + if sum(data[offset:]) != 0: + return False + return True + + # If offset > len(data) there is an error + raise ValueError