From 785bb2ee13f855df27e3019f2b63f59f7d5d2789 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Wed, 23 Jul 2025 20:32:22 +0200 Subject: [PATCH] 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 --- .../protocol/ip/conn_key/fivetuple/Factory.cc | 12 ++++++++++-- .../Baseline/bifs.icmp_connection_exists/.stderr | 1 + testing/btest/bifs/icmp_connection_exists.zeek | 12 ++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 testing/btest/Baseline/bifs.icmp_connection_exists/.stderr create mode 100644 testing/btest/bifs/icmp_connection_exists.zeek diff --git a/src/packet_analysis/protocol/ip/conn_key/fivetuple/Factory.cc b/src/packet_analysis/protocol/ip/conn_key/fivetuple/Factory.cc index bfe40ac1b2..8f27830f3f 100644 --- a/src/packet_analysis/protocol/ip/conn_key/fivetuple/Factory.cc +++ b/src/packet_analysis/protocol/ip/conn_key/fivetuple/Factory.cc @@ -5,6 +5,7 @@ #include "zeek/Desc.h" #include "zeek/IP.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/util-types.h" @@ -43,12 +44,19 @@ zeek::expected Factory::DoConnKeyFromVal(const ze const auto& protov = vl->GetField(proto); auto proto16_t = static_cast(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( "invalid connection ID record encountered: the proto field has the \"unknown\" 65535 value. " "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; } diff --git a/testing/btest/Baseline/bifs.icmp_connection_exists/.stderr b/testing/btest/Baseline/bifs.icmp_connection_exists/.stderr new file mode 100644 index 0000000000..49d861c74c --- /dev/null +++ b/testing/btest/Baseline/bifs.icmp_connection_exists/.stderr @@ -0,0 +1 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. diff --git a/testing/btest/bifs/icmp_connection_exists.zeek b/testing/btest/bifs/icmp_connection_exists.zeek new file mode 100644 index 0000000000..3818181f91 --- /dev/null +++ b/testing/btest/bifs/icmp_connection_exists.zeek @@ -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]); + }