conn_key/fivetuple: Handle one-way ICMP conns in DoConnKeyFromVal()

When a conn_id represents a ICMP "connection", we need to determine the
is_one_way flag for InitTuple() in order to skip any flipping of address
and ports for one-way ICMP connections.

Fixes #4645
This commit is contained in:
Arne Welzel 2025-07-23 20:32:22 +02:00
parent 4399f171ae
commit 785bb2ee13
3 changed files with 23 additions and 2 deletions

View file

@ -5,6 +5,7 @@
#include "zeek/Desc.h" #include "zeek/Desc.h"
#include "zeek/IP.h" #include "zeek/IP.h"
#include "zeek/Val.h" #include "zeek/Val.h"
#include "zeek/packet_analysis/protocol/icmp/ICMP.h"
#include "zeek/packet_analysis/protocol/ip/conn_key/IPBasedConnKey.h" #include "zeek/packet_analysis/protocol/ip/conn_key/IPBasedConnKey.h"
#include "zeek/util-types.h" #include "zeek/util-types.h"
@ -43,12 +44,19 @@ zeek::expected<zeek::ConnKeyPtr, std::string> Factory::DoConnKeyFromVal(const ze
const auto& protov = vl->GetField<CountVal>(proto); const auto& protov = vl->GetField<CountVal>(proto);
auto proto16_t = static_cast<uint16_t>(protov->AsCount()); auto proto16_t = static_cast<uint16_t>(protov->AsCount());
if ( proto16_t == UNKNOWN_IP_PROTO ) bool is_one_way = false;
// For ICMP connections, ensure we have a proper is_one_way flag.
if ( proto16_t == IPPROTO_ICMP )
packet_analysis::ICMP::ICMP4_counterpart(ntohs(orig_portv->Port()), ntohs(resp_portv->Port()), is_one_way);
else if ( proto16_t == IPPROTO_ICMPV6 )
packet_analysis::ICMP::ICMP6_counterpart(ntohs(orig_portv->Port()), ntohs(resp_portv->Port()), is_one_way);
else if ( proto16_t == UNKNOWN_IP_PROTO )
return zeek::unexpected<std::string>( return zeek::unexpected<std::string>(
"invalid connection ID record encountered: the proto field has the \"unknown\" 65535 value. " "invalid connection ID record encountered: the proto field has the \"unknown\" 65535 value. "
"Did you forget to set it?"); "Did you forget to set it?");
ick->InitTuple(orig_addr, htons(orig_portv->Port()), resp_addr, htons(resp_portv->Port()), proto16_t); ick->InitTuple(orig_addr, htons(orig_portv->Port()), resp_addr, htons(resp_portv->Port()), proto16_t, is_one_way);
return ck; return ck;
} }

View file

@ -0,0 +1 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

View file

@ -0,0 +1,12 @@
# @TEST-DOC: Test connection_exists() within new_connection() for ICMP traces. Regression test for #4645.
#
# @TEST-EXEC: zeek -b -r $TRACES/icmp/icmp-destunreach-ip.pcap %INPUT
# @TEST-EXEC: zeek -b -r $TRACES/icmp/icmp-destunreach-no-context.pcap %INPUT
# @TEST-EXEC: zeek -b -r $TRACES/icmp/icmp-destunreach-udp.pcap %INPUT
# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff .stderr
event new_connection(c: connection)
{
assert connection_exists(c$id), fmt("%s does not exist (pcap %s)", c$id, split_string(packet_source()$path, /\//)[-1]);
}