Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix/ipfix-attribute-error #50

Open
wants to merge 2 commits into
base: release
Choose a base branch
from

Conversation

dolevha
Copy link

@dolevha dolevha commented Nov 11, 2024

line 740 would be reached when field_type is None, So field_type.type raises AttributeError. Replacing with the more indicative error.

@dolevha dolevha changed the title bugfix/ipfix-value-error bugfix/ipfix-attribute-error Nov 11, 2024
@bitkeks
Copy link
Owner

bitkeks commented Nov 22, 2024

Hi @dolevha, thanks for your contribution!
Looking into it - what do you mean with field_type.type raises AttributeError? Where is field_type.type called in your patch?

type(field) does not utilize field_type, if that's some misunderstanding?

@bitkeks bitkeks self-assigned this Nov 22, 2024
@dolevha
Copy link
Author

dolevha commented Nov 24, 2024

Hi @dolevha, thanks for your contribution! Looking into it - what do you mean with field_type.type raises AttributeError? Where is field_type.type called in your patch?

type(field) does not utilize field_type, if that's some misunderstanding?

Hi @bitkeks, thank you for the quick response. The main issue I have is the error thrown by parse_packet.

Looking at the case when field_type == None, type(field) == TemplateFieldEnterprise:
Before my PR, NotImplementedError would not be raised, and the following line would be executed - datatype = field_type.type, that will raise an AttributeError as field_type == None and has no such attribute
After my PR, NotImplementedError would be raised with a more precise error description.

@bitkeks
Copy link
Owner

bitkeks commented Nov 24, 2024

Ah, I see, the original check does not catch your combination, since you are using an enterprise field. Hmm..

According to https://www.rfc-editor.org/rfc/rfc7011#appendix-A.2.2 the enterprise "Information Element ids" might overlap with the IANA registered ones. I'd therefore propose to extend your patch by adding a second check: if field_type is None and type(field) is TemplateFieldEnterprise. And leave the original one for NotImplemented as-is.

Where do you define the ID for your field, if that's an enterprise field? I mean: do you have a list of identifiers for your enterprise-specific fields?

In v9 for example there are Cisco ASA-extension IDs, see

# ASA extensions
# https://www.cisco.com/c/en/us/td/docs/security/asa/special/netflow/guide/asa_netflow.html
148: 'NF_F_CONN_ID', # An identifier of a unique flow for the device
176: 'NF_F_ICMP_TYPE', # ICMP type value
177: 'NF_F_ICMP_CODE', # ICMP code value
178: 'NF_F_ICMP_TYPE_IPV6', # ICMP IPv6 type value
179: 'NF_F_ICMP_CODE_IPV6', # ICMP IPv6 code value
225: 'NF_F_XLATE_SRC_ADDR_IPV4', # Post NAT Source IPv4 Address
226: 'NF_F_XLATE_DST_ADDR_IPV4', # Post NAT Destination IPv4 Address
227: 'NF_F_XLATE_SRC_PORT', # Post NATT Source Transport Port
228: 'NF_F_XLATE_DST_PORT', # Post NATT Destination Transport Port
281: 'NF_F_XLATE_SRC_ADDR_IPV6', # Post NAT Source IPv6 Address
282: 'NF_F_XLATE_DST_ADDR_IPV6', # Post NAT Destination IPv6 Address
233: 'NF_F_FW_EVENT', # High-level event code
33002: 'NF_F_FW_EXT_EVENT', # Extended event code
323: 'NF_F_EVENT_TIME_MSEC', # The time that the event occurred, which comes from IPFIX
152: 'NF_F_FLOW_CREATE_TIME_MSEC',
231: 'NF_F_FWD_FLOW_DELTA_BYTES', # The delta number of bytes from source to destination
232: 'NF_F_REV_FLOW_DELTA_BYTES', # The delta number of bytes from destination to source
33000: 'NF_F_INGRESS_ACL_ID', # The input ACL that permitted or denied the flow
33001: 'NF_F_EGRESS_ACL_ID', # The output ACL that permitted or denied a flow
40000: 'NF_F_USERNAME', # AAA username

But there do not seem to be public enterprise identifier lists for IPFIX.

@dolevha
Copy link
Author

dolevha commented Dec 12, 2024

Ah, I see, the original check does not catch your combination, since you are using an enterprise field. Hmm..

According to https://www.rfc-editor.org/rfc/rfc7011#appendix-A.2.2 the enterprise "Information Element ids" might overlap with the IANA registered ones. I'd therefore propose to extend your patch by adding a second check: if field_type is None and type(field) is TemplateFieldEnterprise. And leave the original one for NotImplemented as-is.

Where do you define the ID for your field, if that's an enterprise field? I mean: do you have a list of identifiers for your enterprise-specific fields?

In v9 for example there are Cisco ASA-extension IDs, see

# ASA extensions
# https://www.cisco.com/c/en/us/td/docs/security/asa/special/netflow/guide/asa_netflow.html
148: 'NF_F_CONN_ID', # An identifier of a unique flow for the device
176: 'NF_F_ICMP_TYPE', # ICMP type value
177: 'NF_F_ICMP_CODE', # ICMP code value
178: 'NF_F_ICMP_TYPE_IPV6', # ICMP IPv6 type value
179: 'NF_F_ICMP_CODE_IPV6', # ICMP IPv6 code value
225: 'NF_F_XLATE_SRC_ADDR_IPV4', # Post NAT Source IPv4 Address
226: 'NF_F_XLATE_DST_ADDR_IPV4', # Post NAT Destination IPv4 Address
227: 'NF_F_XLATE_SRC_PORT', # Post NATT Source Transport Port
228: 'NF_F_XLATE_DST_PORT', # Post NATT Destination Transport Port
281: 'NF_F_XLATE_SRC_ADDR_IPV6', # Post NAT Source IPv6 Address
282: 'NF_F_XLATE_DST_ADDR_IPV6', # Post NAT Destination IPv6 Address
233: 'NF_F_FW_EVENT', # High-level event code
33002: 'NF_F_FW_EXT_EVENT', # Extended event code
323: 'NF_F_EVENT_TIME_MSEC', # The time that the event occurred, which comes from IPFIX
152: 'NF_F_FLOW_CREATE_TIME_MSEC',
231: 'NF_F_FWD_FLOW_DELTA_BYTES', # The delta number of bytes from source to destination
232: 'NF_F_REV_FLOW_DELTA_BYTES', # The delta number of bytes from destination to source
33000: 'NF_F_INGRESS_ACL_ID', # The input ACL that permitted or denied the flow
33001: 'NF_F_EGRESS_ACL_ID', # The output ACL that permitted or denied a flow
40000: 'NF_F_USERNAME', # AAA username

But there do not seem to be public enterprise identifier lists for IPFIX.

Sounds good, I'll change my patch.
I didn't define these templates, it's Cisco's.
Some examples:
cisco.app.category.name,12232,9,32,string
cisco.app.http.host,12235,9,var,string
cisco.client.bytes.network,8338,9,8,uint64
cisco.client.ipv4,12236,9,4,uint32
cisco.server.bytes.network,8337,9,8,uint64
cisco.server.ipv4,12237,9,4,uint32

@dolevha
Copy link
Author

dolevha commented Dec 23, 2024

@bitkeks I Found Cisco's field types:
https://www.cisco.com/c/dam/en/us/td/docs/security/stealthwatch/Information_Elements/7_4_Information_Elements_DV_2_0.pdf
Is it possible to add it to the library? Or a way to configure it?

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

Successfully merging this pull request may close these issues.

3 participants