Skip to content

Conversation

msune
Copy link

@msune msune commented Aug 6, 2025

This commit fixes the Python types of a number of fields defined in Netflowv9/10 for the following pre-defined fields (mostly the ones in RFC3954 https://datatracker.ietf.org/doc/html/rfc3954).

It appears that in absence of an assigned field type, it defaults to BytesField instead of IntField. Fields:

  • IN_BYTES, IN_PKTS, OUT_BYTES, OUT_PKTS: IntField (4 bytes). Please note code doesn't seem to support the larget 8byte optional length.
  • MUL_DST_PKTS, MUL_DST_BYTES: same as above
  • INPUT_SNMP, OUTPUT_SNMP: ShortField (2 bytes), iface index. Note: code doesn't seem to support optional larger values (undefined max. length, but 8 bytes might be sufficient).

Note there are many other field types defined that are not part of the spec / unknown without field type and length.

Fixes #4810

msune added 2 commits August 6, 2025 23:54
This commit fixes the Python types of a number of fields defined
in Netflowv9/10 for the following pre-defined fields (mostly
the ones in RFC3954 https://datatracker.ietf.org/doc/html/rfc3954).

It appears that in absence of an assigned field type, it defaults
to BytesField instead of IntField. Fields:

* IN_BYTES, IN_PKTS, OUT_BYTES, OUT_PKTS: IntField (4 bytes). Please note
  code doesn't seem to support the larget 8byte optional length.
* MUL_DST_PKTS, MUL_DST_BYTES: same as above
* INPUT_SNMP, OUTPUT_SNMP: ShortField (2 bytes), iface index. Note: code
  doesn't seem to support optional larger values (undefined max. length,
  but 8 bytes might be sufficient).

Note there are _many_ other field types defined that are not part
of the spec / unknown without field type and length.

Fixes secdev#4810

Signed-off-by: Marc Sune <[email protected]>
Remove unnecessary `print()` in _GenNetflowRecordV9().

Fixes secdev#4810

Signed-off-by: Marc Sune <[email protected]>
@msune
Copy link
Author

msune commented Aug 6, 2025

This comes from #4810. I've revised a number of fields.

A couple of thoughts:

  • I didn't test the changes, I am just opening this Draft PR for discussion.
  • I assumed this was the case, but I'd like someone to confirm:

It appears that in absence of an assigned field type, it defaults to BytesField instead of IntField. Fields:

  • I am not sure which coverage exists for this layer, and where should that coverage be added (not familiar with Scapy's testing approach). Some directions might help here.

cc @gpotter2

Copy link
Member

@gpotter2 gpotter2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for looking into this, we likely should try to adapt the code so that it supports NBytesField to support integer values of arbitrary lengths. Right now this code likely won't work.

78: _N910F("MPLS_LABEL_9", length=3),
79: _N910F("MPLS_LABEL_10", length=3),
70: _N910F("MPLS_LABEL_1", length=3,
field=IntField),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An IntField is 4 octets long, this probably won't work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's a 24 bit value.

30: _N910F("IPV6_DST_MASK", length=1,
field=ByteField),
field=IP6Field),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IP6Field expects a full IPv6 address, probably won't work with a 1bytes long data.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will, it's 1byte value because it's the length of the mask in contiguos bits:


                                           Length of the IPv6
   IPV6_DST_MASK                30   1     destination mask in
                                           contiguous bits

@msune
Copy link
Author

msune commented Aug 7, 2025

we likely should try to adapt the code so that it supports NBytesField to support integer values of arbitrary lengths. Right now this code likely won't work.

Ack. It would be good to support the 8bytes counters, as this is a commonly used thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Netflowv9 flowrecord serializes IN_PKTS=0,IN_BYTES=0
2 participants