From 6a930c1cf8a23f431282fc046181108f73c8d27d Mon Sep 17 00:00:00 2001
From: Arne Welzel <arne.welzel@corelight.com>
Date: Thu, 12 Sep 2024 21:46:06 +0200
Subject: [PATCH] teredo: Move conn member from analyzer to encapsulation

There's only a single instance of the Teredo analyzer. Mutating the conn
member for every new packet and leaving it set after processing the
packet is confusing. Move conn into TeredoEncapsulation instead, or pass
it explicitly.
---
 src/packet_analysis/protocol/teredo/Teredo.cc |  8 ++++----
 src/packet_analysis/protocol/teredo/Teredo.h  | 11 +++++------
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/packet_analysis/protocol/teredo/Teredo.cc b/src/packet_analysis/protocol/teredo/Teredo.cc
index ca256016e0..0dc34f47ef 100644
--- a/src/packet_analysis/protocol/teredo/Teredo.cc
+++ b/src/packet_analysis/protocol/teredo/Teredo.cc
@@ -153,14 +153,14 @@ bool TeredoAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack
         return false;
     }
 
-    conn = static_cast<Connection*>(packet->session);
+    auto* conn = static_cast<Connection*>(packet->session);
     zeek::detail::ConnKey conn_key = conn->Key();
 
     OrigRespMap::iterator or_it = orig_resp_map.find(conn_key);
     if ( or_it == orig_resp_map.end() )
         or_it = orig_resp_map.insert(or_it, {conn_key, {}});
 
-    detail::TeredoEncapsulation te(this);
+    detail::TeredoEncapsulation te(this, conn);
     if ( ! te.Parse(data, len) ) {
         AnalyzerViolation("Bad Teredo encapsulation", conn, (const char*)data, len);
         return false;
@@ -175,7 +175,7 @@ bool TeredoAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack
         if ( inner->NextProto() == IPPROTO_NONE && inner->PayloadLen() == 0 )
             // Teredo bubbles having data after IPv6 header isn't strictly a
             // violation, but a little weird.
-            Weird("Teredo_bubble_with_payload", true);
+            Weird(conn, "Teredo_bubble_with_payload", true);
         else {
             AnalyzerViolation("Teredo payload length", conn, (const char*)data, len);
             return false;
@@ -193,7 +193,7 @@ bool TeredoAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack
     else
         or_it->second.valid_resp = true;
 
-    Confirm(or_it->second.valid_orig, or_it->second.valid_resp);
+    Confirm(conn, or_it->second.valid_orig, or_it->second.valid_resp);
 
     ValPtr teredo_hdr;
 
diff --git a/src/packet_analysis/protocol/teredo/Teredo.h b/src/packet_analysis/protocol/teredo/Teredo.h
index fcc3c635eb..d482157b92 100644
--- a/src/packet_analysis/protocol/teredo/Teredo.h
+++ b/src/packet_analysis/protocol/teredo/Teredo.h
@@ -26,7 +26,7 @@ public:
      * than helpful.  The *force* param is meant for cases where just one side
      * has a valid encapsulation and so the weird would be informative.
      */
-    void Weird(const char* name, bool force = false) const {
+    void Weird(Connection* conn, const char* name, bool force = false) const {
         if ( AnalyzerConfirmed(conn) || force )
             reporter->Weird(conn, name, "", GetAnalyzerName());
     }
@@ -35,7 +35,7 @@ public:
      * If the delayed confirmation option is set, then a valid encapsulation
      * seen from both end points is required before confirming.
      */
-    void Confirm(bool valid_orig, bool valid_resp) {
+    void Confirm(Connection* conn, bool valid_orig, bool valid_resp) {
         if ( ! BifConst::Tunnel::delay_teredo_confirmation || (valid_orig && valid_resp) ) {
             AnalyzerConfirmation(conn);
         }
@@ -46,8 +46,6 @@ public:
     void RemoveConnection(const zeek::detail::ConnKey& conn_key) { orig_resp_map.erase(conn_key); }
 
 protected:
-    Connection* conn = nullptr;
-
     struct OrigResp {
         bool valid_orig = false;
         bool valid_resp = false;
@@ -63,7 +61,7 @@ namespace detail {
 
 class TeredoEncapsulation {
 public:
-    explicit TeredoEncapsulation(const TeredoAnalyzer* ta) : analyzer(ta) {}
+    TeredoEncapsulation(const TeredoAnalyzer* ta, Connection* conn) : analyzer(ta), conn(conn) {}
 
     /**
      * Returns whether input data parsed as a valid Teredo encapsulation type.
@@ -82,12 +80,13 @@ public:
 private:
     bool DoParse(const u_char* data, size_t& len, bool found_orig, bool found_au);
 
-    void Weird(const char* name) const { analyzer->Weird(name); }
+    void Weird(const char* name) const { analyzer->Weird(conn, name); }
 
     const u_char* inner_ip = nullptr;
     const u_char* origin_indication = nullptr;
     const u_char* auth = nullptr;
     const TeredoAnalyzer* analyzer = nullptr;
+    Connection* conn = nullptr;
 };
 
 } // namespace detail