Merge remote-tracking branch 'origin/topic/awelzel/3439-bump-tunnel-max-depth'

* origin/topic/awelzel/3439-bump-tunnel-max-depth:
  NEWS: Update news for tunnel depth changes
  tunnels: Add 'X' to history when reaching Tunnel::max_depth
  Session/TCP/UDP: Reserve HIST_UNKNOWN_PKT mask
  Conn: Deprecated AppendAddl
  Conn/Session: Lift history logic into Session
  init-bare: Default Tunnel::max_depth to 4

CHANGES

@@ -1,3 +1,40 @@
+6.2.0-dev.355 | 2024-01-11 10:59:05 +0100
+
+  * NEWS: Update news for tunnel depth changes (Arne Welzel, Corelight)
+
+  * tunnels: Add 'X' to history when reaching Tunnel::max_depth (Arne Welzel, Corelight)
+
+  * Session/TCP/UDP: Reserve HIST_UNKNOWN_PKT mask (Arne Welzel, Corelight)
+
+    This is meant to be used for a new 'X' code in the history in scenarios when
+    packets are knowingly not processed or an unexpected unknown situation
+    is recognized.
+
+    Usually, these situations are currently reported via weirds or analyzer violations,
+    but being able to include it in the history field allows them to be more visible.
+
+    Will be used for exceeding tunnel depths first.
+
+  * Conn: Deprecated AppendAddl (Arne Welzel, Corelight)
+
+  * Conn/Session: Lift history logic into Session (Arne Welzel, Corelight)
+
+    This should allow to mangle a Session's history also from packet
+    analyzers without necessarily knowing the concrete connection type.
+
+    Given Connection is a subclass of Session, I don't think this
+    changes much.
+
+  * GH-3439: init-bare: Default Tunnel::max_depth to 4 (Arne Welzel, Corelight)
+
+    In AWS GLB environments, the max_depth of 2 is easily reached due to packets
+    being encapsulated with GENEVE and VXLAN [1]. Any additional encapsulation
+    layer causes Zeek raise a weird and ignore the inner traffic. Bump the default
+    maximum depth to 4, while not common it's not unusual either to observe
+    this in the wild.
+
+    [1] https://docs.aws.amazon.com/vpc/latest/mirroring/traffic-mirroring-packet-formats.html
+
 6.2.0-dev.347 | 2024-01-10 12:32:43 -0700
 
   * Use std::move in return values from bif methods to avoid copies (Tim Wojtulewicz, Corelight)

NEWS

@@ -153,6 +153,10 @@ New Functionality
   Note that generally you should prefer record extension in conditionally loaded
   scripts rather than using conditional directives in the original record definition.
 
+- The 'X' code can now appear in a connection's history. It is meant to indicate
+  situations where Zeek stopped analyzing traffic due to exceeding certain limits or
+  when encountering unknown/unsupported protocols. Its first use is to indicate
+  ``Tunnel::max_depth`` being exceeded.
 
 Changed Functionality
 ---------------------
@@ -172,6 +176,10 @@ Changed Functionality
   detail API from ``digest.h`` to compute hashes likely need to accommodate for
   this change.
 
+- The ``Tunnel::max_depth`` default was changed from 2 to 4 allowing for more than
+  two encapsulation layers. Two layers are already easily reached in AWS GLB
+  environments.
+
 Removed Functionality
 ---------------------
 

VERSION

@@ -1 +1 @@
-6.2.0-dev.347
+6.2.0-dev.355

scripts/base/init-bare.zeek

@@ -5220,7 +5220,7 @@ module Tunnel;
 export {
 	## The maximum depth of a tunnel to decapsulate until giving up.
 	## Setting this to zero will disable all types of tunnel decapsulation.
-	const max_depth: count = 2 &redef;
+	const max_depth: count = 4 &redef;
 
 	## With this set, the Teredo analyzer waits until it sees both sides
 	## of a connection using a valid Teredo encapsulation before issuing

scripts/base/protocols/conn/main.zeek

@@ -125,6 +125,7 @@ export {
 		## i       inconsistent packet (e.g. FIN+RST bits set)
 		## q       multi-flag packet (SYN+FIN or SYN+RST bits set)
 		## ^       connection direction was flipped by Zeek's heuristic
+		## x       connection analysis partial (e.g. limits exceeded)
 		## ======  ====================================================
 		##
 		## If the event comes from the originator, the letter is in

src/Conn.cc

@@ -158,39 +158,6 @@ void Connection::NextPacket(double t, bool is_orig, const IP_Hdr* ip, int len, i
 
 bool Connection::IsReuse(double t, const u_char* pkt) { return adapter && adapter->IsReuse(t, pkt); }
 
-bool Connection::ScaledHistoryEntry(char code, uint32_t& counter, uint32_t& scaling_threshold, uint32_t scaling_base) {
-    if ( ++counter == scaling_threshold ) {
-        AddHistory(code);
-
-        auto new_threshold = scaling_threshold * scaling_base;
-
-        if ( new_threshold <= scaling_threshold )
-            // This can happen due to wrap-around.  In that
-            // case, reset the counter but leave the threshold
-            // unchanged.
-            counter = 0;
-
-        else
-            scaling_threshold = new_threshold;
-
-        return true;
-    }
-
-    return false;
-}
-
-void Connection::HistoryThresholdEvent(EventHandlerPtr e, bool is_orig, uint32_t threshold) {
-    if ( ! e )
-        return;
-
-    if ( threshold == 1 )
-        // This will be far and away the most common case,
-        // and at this stage it's not a *multiple* instance.
-        return;
-
-    EnqueueEvent(e, nullptr, GetVal(), val_mgr->Bool(is_orig), val_mgr->Count(threshold));
-}
-
 namespace {
 // Flip everything that needs to be flipped in the connection
 // record that is known on this level. This needs to align

src/Conn.h

@@ -148,7 +148,8 @@ public:
     /**
      * Append additional entries to the history field in the connection record.
      */
-    void AppendAddl(const char* str);
+    [[deprecated("Remove in v7.1 - Appears unused and named rough. Use CheckHistory() or AddHistory() instead.")]] void
+    AppendAddl(const char* str);
 
     void Match(detail::Rule::PatternType type, const u_char* data, int len, bool is_orig, bool bol, bool eol,
                bool clear_state);
@@ -179,30 +180,6 @@ public:
     static uint64_t TotalConnections() { return total_connections; }
     static uint64_t CurrentConnections() { return current_connections; }
 
-    // Returns true if the history was already seen, false otherwise.
-    bool CheckHistory(uint32_t mask, char code) {
-        if ( (hist_seen & mask) == 0 ) {
-            hist_seen |= mask;
-            AddHistory(code);
-            return false;
-        }
-        else
-            return true;
-    }
-
-    // Increments the passed counter and adds it as a history
-    // code if it has crossed the next scaling threshold.  Scaling
-    // is done in terms of powers of the third argument.
-    // Returns true if the threshold was crossed, false otherwise.
-    bool ScaledHistoryEntry(char code, uint32_t& counter, uint32_t& scaling_threshold, uint32_t scaling_base = 10);
-
-    void HistoryThresholdEvent(EventHandlerPtr e, bool is_orig, uint32_t threshold);
-
-    void AddHistory(char code) { history += code; }
-
-    const std::string& GetHistory() const { return history; }
-    void ReplaceHistory(std::string new_h) { history = std::move(new_h); }
-
     // Sets the root of the analyzer tree as well as the primary PIA.
     void SetSessionAdapter(packet_analysis::IP::SessionAdapter* aa, analyzer::pia::PIA* pia);
     packet_analysis::IP::SessionAdapter* GetSessionAdapter() { return adapter; }
@@ -246,9 +223,6 @@ private:
     unsigned int finished : 1;
     unsigned int saw_first_orig_packet : 1, saw_first_resp_packet : 1;
 
-    uint32_t hist_seen;
-    std::string history;
-
     packet_analysis::IP::SessionAdapter* adapter;
     analyzer::pia::PIA* primary_PIA;
 

src/analyzer/protocol/tcp/TCP_Endpoint.h

@@ -203,6 +203,7 @@ public:
 #define HIST_CORRUPT_PKT 0x80
 #define HIST_RXMIT 0x100
 #define HIST_WIN0 0x200
+    // #define HIST_UNKNOWN_PKT 0x400  (do not use - used in Session.h)
     bool CheckHistory(uint32_t mask, char code);
     void AddHistory(char code);
 

src/packet_analysis/protocol/ayiya/AYIYA.cc

@@ -20,6 +20,7 @@ bool AYIYAAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packe
         return false;
 
     if ( packet->encap && packet->encap->Depth() >= BifConst::Tunnel::max_depth ) {
+        packet->session->CheckHistory(zeek::session::detail::HIST_UNKNOWN_PKT, 'X');
         Weird("exceeded_tunnel_max_depth", packet);
         return false;
     }

src/packet_analysis/protocol/geneve/Geneve.cc

@@ -21,6 +21,7 @@ bool GeneveAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack
         return false;
 
     if ( packet->encap && packet->encap->Depth() >= BifConst::Tunnel::max_depth ) {
+        packet->session->CheckHistory(zeek::session::detail::HIST_UNKNOWN_PKT, 'X');
         Weird("exceeded_tunnel_max_depth", packet);
         return false;
     }

src/packet_analysis/protocol/iptunnel/IPTunnel.cc

@@ -23,6 +23,7 @@ bool IPTunnelAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pa
     }
 
     if ( packet->encap && packet->encap->Depth() >= BifConst::Tunnel::max_depth ) {
+        packet->session->CheckHistory(zeek::session::detail::HIST_UNKNOWN_PKT, 'X');
         Weird("exceeded_tunnel_max_depth", packet);
         return false;
     }

src/packet_analysis/protocol/teredo/Teredo.cc

@@ -148,6 +148,7 @@ bool TeredoAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack
         return false;
 
     if ( packet->encap && packet->encap->Depth() >= BifConst::Tunnel::max_depth ) {
+        packet->session->CheckHistory(zeek::session::detail::HIST_UNKNOWN_PKT, 'X');
         Analyzer::Weird("exceeded_tunnel_max_depth", packet);
         return false;
     }

src/packet_analysis/protocol/udp/UDP.cc

@@ -18,6 +18,7 @@ constexpr uint32_t HIST_ORIG_DATA_PKT = 0x1;
 constexpr uint32_t HIST_RESP_DATA_PKT = 0x2;
 constexpr uint32_t HIST_ORIG_CORRUPT_PKT = 0x4;
 constexpr uint32_t HIST_RESP_CORRUPT_PKT = 0x8;
+// constexpr uint32_t HIST_UNKNOWN_PKT = 0x400;  (do not use - used in Session.h)
 
 UDPAnalyzer::UDPAnalyzer() : IPBasedAnalyzer("UDP", TRANSPORT_UDP, UDP_PORT_MASK, false) {}
 

src/packet_analysis/protocol/vxlan/VXLAN.cc

@@ -21,6 +21,7 @@ bool VXLAN_Analyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack
         return false;
 
     if ( packet->encap && packet->encap->Depth() >= BifConst::Tunnel::max_depth ) {
+        packet->session->CheckHistory(zeek::session::detail::HIST_UNKNOWN_PKT, 'X');
         Weird("exceeded_tunnel_max_depth", packet);
         return false;
     }

src/session/Session.cc

@@ -186,4 +186,37 @@ void Session::SetAnalyzerState(const zeek::Tag& tag, AnalyzerConfirmationState v
     analyzer_confirmations.insert_or_assign(tag, value);
 }
 
+bool Session::ScaledHistoryEntry(char code, uint32_t& counter, uint32_t& scaling_threshold, uint32_t scaling_base) {
+    if ( ++counter == scaling_threshold ) {
+        AddHistory(code);
+
+        auto new_threshold = scaling_threshold * scaling_base;
+
+        if ( new_threshold <= scaling_threshold )
+            // This can happen due to wrap-around.  In that
+            // case, reset the counter but leave the threshold
+            // unchanged.
+            counter = 0;
+
+        else
+            scaling_threshold = new_threshold;
+
+        return true;
+    }
+
+    return false;
+}
+
+void Session::HistoryThresholdEvent(EventHandlerPtr e, bool is_orig, uint32_t threshold) {
+    if ( ! e )
+        return;
+
+    if ( threshold == 1 )
+        // This will be far and away the most common case,
+        // and at this stage it's not a *multiple* instance.
+        return;
+
+    EnqueueEvent(e, nullptr, GetVal(), val_mgr->Bool(is_orig), val_mgr->Count(threshold));
+}
+
 } // namespace zeek::session

src/session/Session.h

@@ -24,7 +24,9 @@ class Analyzer;
 namespace session {
 namespace detail {
 class Timer;
-}
+
+constexpr uint32_t HIST_UNKNOWN_PKT = 0x400; // Initially for exceeded_tunnel_max_depth.
+} // namespace detail
 
 class Session;
 using timer_func = void (Session::*)(double t);
@@ -185,6 +187,67 @@ public:
     AnalyzerConfirmationState AnalyzerState(const zeek::Tag& tag) const;
     void SetAnalyzerState(const zeek::Tag& tag, AnalyzerConfirmationState);
 
+    /**
+     * Add \a code to history unless already seen.
+     *
+     * @param mask Bitmask used for the given code character.
+     * @param code The character to add to the history.
+     *
+     * @return True if the given \a code was already seen (mask set),
+     * otherwise false after adding it.
+     */
+    bool CheckHistory(uint32_t mask, char code) {
+        if ( (hist_seen & mask) == 0 ) {
+            hist_seen |= mask;
+            AddHistory(code);
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * Increments the passed counter and adds it as a history
+     * code if it has crossed the next scaling threshold.  Scaling
+     * is done in terms of powers of the third argument.
+     *
+     * @param code The history code.
+     * @param counter Reference to counter for this code.
+     * @param scaling_threshold The next threshold, updated to next threshold if crossed.
+     * @param scaling_base Base to compute the next scaling_threshold.
+     *
+     * @return True if the threshold was crossed, false otherwise.
+     */
+    bool ScaledHistoryEntry(char code, uint32_t& counter, uint32_t& scaling_threshold, uint32_t scaling_base = 10);
+
+    /**
+     *  Helper to enqueue a history threshold event \a e with the Connection object of this session.
+     *
+     * @param e The event to enqueue
+     * @param is_orig True if this is the originator of the session.
+     * @param threshold Crossed threshold to use as event argument.
+     */
+    void HistoryThresholdEvent(EventHandlerPtr e, bool is_orig, uint32_t threshold);
+
+    /**
+     * Add \a code to the history.
+     *
+     * @param code Code to add
+     */
+    void AddHistory(char code) { history += code; }
+
+    /**
+     * @return The current history value.
+     */
+    const std::string& GetHistory() const { return history; }
+
+    /**
+     * Replace the history of this session with a new one.
+     *
+     * @param new_h The new history.
+     */
+    void ReplaceHistory(std::string new_h) { history = std::move(new_h); }
+
 protected:
     friend class detail::Timer;
 
@@ -234,6 +297,9 @@ protected:
     bool in_session_table;
 
     std::map<zeek::Tag, AnalyzerConfirmationState> analyzer_confirmations;
+
+    uint32_t hist_seen;
+    std::string history;
 };
 
 namespace detail {

testing/btest/Baseline/core.tunnels.max-depth-exceeded/conn.log.cut

@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+uid	id.orig_h	id.resp_p	id.resp_h	id.resp_p	proto	history	service	tunnel_parents
+CHhAvVGS1DHFjwGM9	1.1.1.1	4789	1.1.1.9	4789	udp	D	vxlan	-
+ClEkJM2Vm5giqnMf4h	2.2.2.2	4789	2.2.2.9	4789	udp	D	vxlan	CHhAvVGS1DHFjwGM9
+C4J4Th3PJpwUYZZ6gc	3.3.3.3	4789	3.3.3.9	4789	udp	DX	-	ClEkJM2Vm5giqnMf4h

testing/btest/Baseline/core.tunnels.max-depth-exceeded/tunnel.log

@@ -0,0 +1,14 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	tunnel
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	tunnel_type	action
+#types	time	string	addr	port	addr	port	enum	enum
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	1.1.1.1	4789	1.1.1.9	4789	Tunnel::VXLAN	Tunnel::DISCOVER
+XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	2.2.2.2	4789	2.2.2.9	4789	Tunnel::VXLAN	Tunnel::DISCOVER
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	1.1.1.1	4789	1.1.1.9	4789	Tunnel::VXLAN	Tunnel::CLOSE
+XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	2.2.2.2	4789	2.2.2.9	4789	Tunnel::VXLAN	Tunnel::CLOSE
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/core.tunnels.max-depth-exceeded/weird.log

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	weird
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	name	addl	notice	peer	source
+#types	time	string	addr	port	addr	port	string	string	bool	string	string
+XXXXXXXXXX.XXXXXX	-	3.3.3.3	0	3.3.3.9	0	exceeded_tunnel_max_depth_in_tunnel	-	F	zeek	VXLAN
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/core.tunnels.max-depth/conn.log.cut

@@ -0,0 +1,6 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+uid	id.orig_h	id.resp_p	id.resp_h	id.resp_p	proto	history	service	tunnel_parents
+CtPZjS20MLrsMUOJi2	4.4.4.4	53	4.4.4.9	53	udp	D	dns	C4J4Th3PJpwUYZZ6gc
+CHhAvVGS1DHFjwGM9	1.1.1.1	4789	1.1.1.9	4789	udp	D	vxlan	-
+ClEkJM2Vm5giqnMf4h	2.2.2.2	4789	2.2.2.9	4789	udp	D	vxlan	CHhAvVGS1DHFjwGM9
+C4J4Th3PJpwUYZZ6gc	3.3.3.3	4789	3.3.3.9	4789	udp	D	vxlan	ClEkJM2Vm5giqnMf4h

testing/btest/Baseline/core.tunnels.max-depth/dns.log.cut

@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+uid	id.orig_h	id.resp_p	id.resp_h	id.resp_p	query
+CtPZjS20MLrsMUOJi2	4.4.4.4	53	4.4.4.9	53	www.bbc.com

testing/btest/Baseline/core.tunnels.max-depth/tunnel.log

@@ -0,0 +1,16 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	tunnel
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	tunnel_type	action
+#types	time	string	addr	port	addr	port	enum	enum
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	1.1.1.1	4789	1.1.1.9	4789	Tunnel::VXLAN	Tunnel::DISCOVER
+XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	2.2.2.2	4789	2.2.2.9	4789	Tunnel::VXLAN	Tunnel::DISCOVER
+XXXXXXXXXX.XXXXXX	C4J4Th3PJpwUYZZ6gc	3.3.3.3	4789	3.3.3.9	4789	Tunnel::VXLAN	Tunnel::DISCOVER
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	1.1.1.1	4789	1.1.1.9	4789	Tunnel::VXLAN	Tunnel::CLOSE
+XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	2.2.2.2	4789	2.2.2.9	4789	Tunnel::VXLAN	Tunnel::CLOSE
+XXXXXXXXXX.XXXXXX	C4J4Th3PJpwUYZZ6gc	3.3.3.3	4789	3.3.3.9	4789	Tunnel::VXLAN	Tunnel::CLOSE
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/core/tunnels/max-depth-exceeded.zeek

@@ -0,0 +1,15 @@
+# @TEST-DOC: Set a too small Tunnel::max_depth value, observe the effects.
+#
+# @TEST-EXEC: zeek -b -r $TRACES/tunnels/vxlan-triple-v2.pcap %INPUT
+# @TEST-EXEC: zeek-cut -m uid id.orig_h id.resp_p id.resp_h id.resp_p proto history service tunnel_parents < conn.log > conn.log.cut
+# @TEST-EXEC: btest-diff conn.log.cut
+# @TEST-EXEC: btest-diff tunnel.log
+# @TEST-EXEC: btest-diff weird.log
+# @TEST-EXEC: test ! -f dns.log
+#
+@load base/frameworks/notice/weird
+@load base/frameworks/tunnels
+@load base/protocols/conn
+@load base/protocols/dns
+
+redef Tunnel::max_depth = 2;

testing/btest/core/tunnels/max-depth.zeek

@@ -0,0 +1,11 @@
+# @TEST-DOC: A DNS request encapsulated in 3 layers of VXLAN. Funky but not all that unusual.
+# @TEST-EXEC: zeek -b -r $TRACES/tunnels/vxlan-triple-v2.pcap %INPUT
+# @TEST-EXEC: zeek-cut -m uid id.orig_h id.resp_p id.resp_h id.resp_p proto history service tunnel_parents < conn.log > conn.log.cut
+# @TEST-EXEC: zeek-cut -m uid id.orig_h id.resp_p id.resp_h id.resp_p query < dns.log > dns.log.cut
+# @TEST-EXEC: btest-diff conn.log.cut
+# @TEST-EXEC: btest-diff tunnel.log
+# @TEST-EXEC: btest-diff dns.log.cut
+#
+@load base/frameworks/tunnels
+@load base/protocols/conn
+@load base/protocols/dns