Skip to content

fix: skip an undecodable IPFIX Set instead of dropping the Sets behind it #332

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.

Sign up for GitHub

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

Open
mikemiles-dev wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: skip an undecodable IPFIX Set instead of dropping the Sets behind it #332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions RELEASES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,32 @@
elsewhere, `pcaps/IPFIX.pcap` included, so retiring both captures loses no
coverage.

* **A Set this parser cannot decode no longer discards the Sets behind it.**
Set iteration stopped at the first Set whose body failed to decode and
dropped the rest of the message without reporting anything — the remaining
half of #310, which the Set-framing check could not reach, because such a Set
is perfectly well framed and only its *body* is undecodable. A Template Set
declaring more fields than its octets carry, for instance, silently destroyed
every valid Data Set behind it while the message reported success.

Because Set framing is now validated across the whole chain before any Set is
parsed, the next Set's offset is known even when the current one fails, so the
walk steps over it and keeps decoding. The skipped Set is reported as
`FlowSetBody::NoTemplate` carrying the new
`NoTemplateInfo::reason` of `NoTemplateReason::UndecodableBody`, so the loss
stays visible without consuming the single `ParseResult::error` slot.

`reason` also distinguishes the pre-existing case: a flowset whose template is
merely absent reports `NoTemplateReason::MissingTemplate` and remains eligible
for pending-flow replay. An undecodable Set never is — no template arriving
later changes its octets, so caching it would retry it forever.
`NoTemplateInfo::is_replayable()` exposes the distinction.

Three bundled test snapshots recorded `flowsets: []` before this change and
now show the Sets that were being dropped, so they were not testing what their
names claimed. `NoTemplateInfo` is `#[non_exhaustive]`, so adding `reason` is
not a breaking change.

* **A malformed IPFIX message no longer discards the rest of the datagram.**
RFC 7011 section 9.1 discards the *Message*, but parsing stopped at the first
message-level error and abandoned every message chained behind it — so one
Expand Down
1 change: 1 addition & 0 deletions src/snapshots/netflow_parser__tests__base_tests__options_no_data.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,3 +147,4 @@ expression: parser.parse_bytes(&packet).packets
- 0
- 0
truncated: false
reason: MissingTemplate
44 changes: 43 additions & 1 deletion ...pshots/netflow_parser__tests__restored_legacy_tests__it_parses_0_length_fields_ipfix.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,46 @@ expression: "NetflowParser::default().parse_bytes(&packet).packets"
export_time: 16909060
sequence_number: 0
observation_domain_id: 16909060
flowsets: []
flowsets:
- header:
header_id: 2
length: 20
body:
NoTemplate:
template_id: 2
raw_data:
- 1
- 0
- 0
- 3
- 0
- 8
- 0
- 4
- 0
- 12
- 0
- 4
- 0
- 65
- 0
- 0
truncated: false
reason: UndecodableBody
- header:
header_id: 256
length: 12
body:
NoTemplate:
template_id: 256
raw_data:
- 1
- 2
- 3
- 4
- 1
- 2
- 3
- 4
truncated: false
reason: MissingTemplate
33 changes: 32 additions & 1 deletion ...tests__restored_legacy_tests__it_parses_ipfix_enterprise_bit_in_non_options_template.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,35 @@ expression: "NetflowParser::default().parse_bytes(&packet).packets"
export_time: 1670052913
sequence_number: 0
observation_domain_id: 0
flowsets: []
flowsets:
- header:
header_id: 2
length: 26
body:
NoTemplate:
template_id: 2
raw_data:
- 1
- 4
- 0
- 2
- 128
- 103
- 255
- 255
- 24
- 77
- 128
- 103
- 255
- 255
- 0
- 0
- 24
- 77
- 129
- 64
- 0
- 4
truncated: false
reason: UndecodableBody
108 changes: 107 additions & 1 deletion ...arser__tests__restored_legacy_tests__it_parses_ipfix_with_v9_options_template_packet.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,110 @@ expression: "NetflowParser::default().parse_bytes(&packet).packets"
export_time: 1744871721
sequence_number: 100000
observation_domain_id: 0
flowsets: []
flowsets:
- header:
header_id: 1
length: 14
body:
NoTemplate:
template_id: 1
raw_data:
- 4
- 232
- 0
- 1
- 0
- 6
- 0
- 34
- 0
- 4
truncated: false
reason: UndecodableBody
- header:
header_id: 1256
length: 8
body:
NoTemplate:
template_id: 1256
raw_data:
- 0
- 0
- 0
- 1
truncated: false
reason: MissingTemplate
- header:
header_id: 2
length: 44
body:
Template:
template_id: 258
field_count: 9
fields:
- field_type_number: 8
field_length: 4
field_type:
IANA: SourceIpv4address
- field_type_number: 12
field_length: 4
field_type:
IANA: DestinationIpv4address
- field_type_number: 7
field_length: 2
field_type:
IANA: SourceTransportPort
- field_type_number: 11
field_length: 2
field_type:
IANA: DestinationTransportPort
- field_type_number: 4
field_length: 1
field_type:
IANA: ProtocolIdentifier
- field_type_number: 2
field_length: 4
field_type:
IANA: PacketDeltaCount
- field_type_number: 1
field_length: 4
field_type:
IANA: OctetDeltaCount
- field_type_number: 22
field_length: 4
field_type:
IANA: FlowStartSysUpTime
- field_type_number: 21
field_length: 4
field_type:
IANA: FlowEndSysUpTime
- header:
header_id: 258
length: 33
body:
Data:
fields:
- - - IANA: SourceIpv4address
- Ip4Addr: 70.70.1.1
- - IANA: DestinationIpv4address
- Ip4Addr: 120.120.1.1
- - IANA: SourceTransportPort
- DataNumber:
U16: 12345
- - IANA: DestinationTransportPort
- DataNumber:
U16: 32777
- - IANA: ProtocolIdentifier
- ProtocolType: Tcp
- - IANA: PacketDeltaCount
- DataNumber:
U32: 100
- - IANA: OctetDeltaCount
- DataNumber:
U32: 125000000
- - IANA: FlowStartSysUpTime
- DataNumber:
U32: 1744871721
- - IANA: FlowEndSysUpTime
- DataNumber:
U32: 1744871721
8 changes: 8 additions & 0 deletions src/variable_versions/ipfix/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::template_store::{
encode_ipfix_options_template, encode_ipfix_template, encode_v9_options_template,
encode_v9_template,
};
use crate::variable_versions::NoTemplateReason;
use crate::variable_versions::config::DEFAULT_MAX_RECORDS_PER_FLOWSET;
use crate::variable_versions::enterprise_registry::EnterpriseFieldRegistry;
use crate::variable_versions::field_value::FieldValue;
Expand Down Expand Up @@ -645,6 +646,12 @@ impl IPFixParser {
for (i, flowset) in ipfix.flowsets.iter_mut().enumerate() {
match &mut flowset.body {
FlowSetBody::NoTemplate(info) => {
// A Set whose body could not be decoded is not waiting on a
// template: no template that arrives later changes its
// octets, so caching it for replay would retry it forever.
if !info.is_replayable() {
continue;
}
// Reject flowsets with impossibly small headers (RFC minimum is 4).
// Also reject truncated raw_data (oversized entry at parse time).
// The flowset is kept in output as diagnostic data.
Expand Down Expand Up @@ -1747,6 +1754,7 @@ impl FlowSetBody {
template_id: id,
raw_data,
truncated,
reason: NoTemplateReason::MissingTemplate,
};
Ok((&[] as &[u8], FlowSetBody::NoTemplate(info)))
} else {
Expand Down
Loading
Loading