Merge remote-tracking branch 'origin/topic/awelzel/4645-icmp-conns-inconsistent'

* origin/topic/awelzel/4645-icmp-conns-inconsistent:
  conn_key/fivetuple: Handle one-way ICMP conns in DoConnKeyFromVal()
This commit is contained in:
Arne Welzel 2025-07-24 09:48:54 +02:00
commit e231efac0b
5 changed files with 32 additions and 3 deletions

View file

@ -1,3 +1,11 @@
8.0.0-dev.736 | 2025-07-24 09:48:54 +0200
* GH-4645: conn_key/fivetuple: Handle one-way ICMP conns in DoConnKeyFromVal() (Arne Welzel, Corelight)
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.
8.0.0-dev.734 | 2025-07-24 07:55:31 +0100 8.0.0-dev.734 | 2025-07-24 07:55:31 +0100
* PPPoE: add session id logging (Johanna Amann, Corelight) * PPPoE: add session id logging (Johanna Amann, Corelight)

View file

@ -1 +1 @@
8.0.0-dev.734 8.0.0-dev.736

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]);
}