Squashed commit of branch feature/ipfix-padding:

commit 63abf52

    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 8d1cf9c

    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 bitkeks#34

    Signed-off-by: Dominik Pataky <[email protected]>

commit 51ce4ea

    Fix and optimize padding calculation for IPFIX sets.
    Refs bitkeks#34

commit 9d3c413
Author: Khaw Teng Kang <[email protected]>
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 <[email protected]>

commit 3c4f8e6

    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 bitkeks#34

Signed-off-by: Dominik Pataky <[email protected]>

netflow/collector.py

@@ -19,9 +19,9 @@
 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'])

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 @@ def __init__(self, data):
         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 @@ def __init__(self, data):
             raise IPFIXMalformedRecord
         offset += offset_add
 
-        # TODO: if padding is needed, implement here
-
         self._length = offset
 
     def get_length(self):
@@ -812,17 +811,29 @@ def __init__(self, data: bytes, templates):
         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 @@ def __init__(self, data: bytes, templates):
                         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