Move building session analyzer tree out of analyzer::Manager

src/Dict.h

@@ -177,6 +177,7 @@ public:
 	bool operator!=( const DictIterator& that ) const { return !(*this == that); }
 
 private:
+
 	friend class Dictionary;
 
 	DictIterator(const Dictionary* d, detail::DictEntry* begin, detail::DictEntry* end);

src/analyzer/Analyzer.h

@@ -25,6 +25,7 @@ using FilePtr = zeek::IntrusivePtr<File>;
 using RecordValPtr = zeek::IntrusivePtr<RecordVal>;
 
 namespace detail { class Rule; }
+namespace packet_analysis::IP { class IPBasedAnalyzer; }
 
 } // namespace zeek
 
@@ -601,6 +602,7 @@ protected:
 	friend class Manager;
 	friend class zeek::Connection;
 	friend class zeek::analyzer::tcp::TCP_ApplicationAnalyzer;
+	friend class zeek::packet_analysis::IP::IPBasedAnalyzer;
 
 	/**
 	 * Return a string represantation of an analyzer, containing its name

src/analyzer/Manager.cc

@@ -353,11 +353,6 @@ Manager::tag_set* Manager::LookupPort(TransportProto proto, uint32_t port, bool
 	return l;
 	}
 
-Manager::tag_set* Manager::LookupPort(PortVal* val, bool add_if_not_found)
-	{
-	return LookupPort(val->PortType(), val->Port(), add_if_not_found);
-	}
-
 bool Manager::BuildInitialAnalyzerTree(Connection* conn)
 	{
 	analyzer::tcp::TCP_Analyzer* tcp = nullptr;
@@ -484,57 +479,6 @@ bool Manager::BuildInitialAnalyzerTree(Connection* conn)
 	return true;
 	}
 
-bool Manager::BuildSessionAnalyzerTree(Connection* conn, packet_analysis::IP::IPBasedAnalyzer* analyzer)
-	{
-	packet_analysis::IP::IPBasedTransportAnalyzer* root = nullptr;
-	analyzer::pia::PIA* pia = nullptr;
-	bool check_port = false;
-
-	analyzer->CreateTransportAnalyzer(conn, root, pia, check_port);
-
-	bool scheduled = ApplyScheduledAnalyzers(conn, false, root);
-
-	// Hmm... Do we want *just* the expected analyzer, or all
-	// other potential analyzers as well?  For now we only take
-	// the scheduled ones.
-	if ( ! scheduled )
-		{ // Let's see if it's a port we know.
-		if ( check_port && ! zeek::detail::dpd_ignore_ports )
-			{
-			int resp_port = ntohs(conn->RespPort());
-			tag_set* ports = LookupPort(conn->ConnTransport(), resp_port, false);
-
-			if ( ports )
-				{
-				for ( tag_set::const_iterator j = ports->begin(); j != ports->end(); ++j )
-					{
-					Analyzer* analyzer = analyzer_mgr->InstantiateAnalyzer(*j, conn);
-
-					if ( ! analyzer )
-						continue;
-
-					root->AddChildAnalyzer(analyzer, false);
-					DBG_ANALYZER_ARGS(conn, "activated %s analyzer due to port %d",
-					                  analyzer_mgr->GetComponentName(*j).c_str(), resp_port);
-					}
-				}
-			}
-		}
-
-	root->AddExtraAnalyzers(conn);
-
-	if ( pia )
-		root->AddChildAnalyzer(pia->AsAnalyzer());
-
-	conn->SetRootAnalyzer(root, pia);
-	root->Init();
-	root->InitChildren();
-
-	PLUGIN_HOOK_VOID(HOOK_SETUP_ANALYZER_TREE, HookSetupAnalyzerTree(conn));
-
-	return true;
-	}
-
 void Manager::ExpireScheduledAnalyzers()
 	{
 	if ( ! run_state::network_time )

src/analyzer/Manager.h

@@ -252,18 +252,6 @@ public:
 	 */
 	bool BuildInitialAnalyzerTree(Connection* conn);
 
-	/**
-	 * Builds the analyzer tree used by transport-layer analyzers in the
-	 * packet analysis framework.
-	 *
-	 * @param conn The connection to add the initial set of analyzers to.
-	 * @param analyzer The packet analyzer requesting the tree.
-	 * @return False if the tree cannot be built; that's usually an
-	 * internal error.
-	 */
-	bool BuildSessionAnalyzerTree(Connection* conn,
-	                              packet_analysis::IP::IPBasedAnalyzer* analyzer);
-
 	/**
 	 * Schedules a particular analyzer for an upcoming connection. Once
 	 * the connection is seen, BuildInitAnalyzerTree() will add the
@@ -360,10 +348,11 @@ public:
 
 private:
 
+	friend class packet_analysis::IP::IPBasedAnalyzer;
+
 	using tag_set = std::set<Tag>;
 	using analyzer_map_by_port = std::map<uint32_t, tag_set*>;
 
-	tag_set* LookupPort(PortVal* val, bool add_if_not_found);
 	tag_set* LookupPort(TransportProto proto, uint32_t port, bool add_if_not_found);
 
 	tag_set GetScheduled(const Connection* conn);

src/packet_analysis/protocol/icmp/ICMP.cc

@@ -35,6 +35,15 @@ ICMPAnalyzer::~ICMPAnalyzer()
 	{
 	}
 
+IPBasedTransportAnalyzer* ICMPAnalyzer::MakeTransportAnalyzer(Connection* conn)
+	{
+	auto* root = new ICMPTransportAnalyzer(conn);
+	root->SetParent(this);
+	conn->SetInactivityTimeout(zeek::detail::icmp_inactivity_timeout);
+
+	return root;
+	}
+
 bool ICMPAnalyzer::BuildConnTuple(size_t len, const uint8_t* data, Packet* packet,
                                   ConnTuple& tuple)
 	{
@@ -858,17 +867,6 @@ int ICMPAnalyzer::ICMP6_counterpart(int icmp_type, int icmp_code, bool& is_one_w
 	}
 	}
 
-void ICMPAnalyzer::CreateTransportAnalyzer(Connection* conn, IPBasedTransportAnalyzer*& root,
-                                           analyzer::pia::PIA*& pia, bool& check_port)
-	{
-	root = new ICMPTransportAnalyzer(conn);
-	root->SetParent(this);
-	conn->SetInactivityTimeout(zeek::detail::icmp_inactivity_timeout);
-
-	pia = nullptr;
-	check_port = false;
-	}
-
 void ICMPTransportAnalyzer::AddExtraAnalyzers(Connection* conn)
 	{
 	static analyzer::Tag analyzer_connsize = analyzer_mgr->GetComponentTag("CONNSIZE");

src/packet_analysis/protocol/icmp/ICMP.h

@@ -29,8 +29,7 @@ public:
 		return std::make_shared<ICMPAnalyzer>();
 		}
 
-	void CreateTransportAnalyzer(Connection* conn, IP::IPBasedTransportAnalyzer*& root,
+	packet_analysis::IP::IPBasedTransportAnalyzer* MakeTransportAnalyzer(Connection* conn) override;
-	                             analyzer::pia::PIA*& pia, bool& check_port) override;
 
 protected:
 

src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc

@@ -7,6 +7,8 @@
 #include "zeek/Val.h"
 #include "zeek/session/Manager.h"
 #include "zeek/analyzer/Manager.h"
+#include "zeek/analyzer/protocol/pia/PIA.h"
+#include "zeek/plugin/Manager.h"
 
 using namespace zeek;
 using namespace zeek::packet_analysis::IP;
@@ -201,7 +203,7 @@ zeek::Connection* IPBasedAnalyzer::NewConn(const ConnTuple* id, const detail::Co
 			return nullptr;
 			}
 		}
-	else if ( ! analyzer_mgr->BuildSessionAnalyzerTree(conn, this) )
+	else if ( ! BuildSessionAnalyzerTree(conn) )
 		{
 		conn->Done();
 		Unref(conn);
@@ -213,3 +215,59 @@ zeek::Connection* IPBasedAnalyzer::NewConn(const ConnTuple* id, const detail::Co
 
 	return conn;
 	}
+
+bool IPBasedAnalyzer::BuildSessionAnalyzerTree(Connection* conn)
+	{
+	packet_analysis::IP::IPBasedTransportAnalyzer* root = MakeTransportAnalyzer(conn);
+	analyzer::pia::PIA* pia = MakePIA(conn);
+
+	// TODO: temporary, can be replaced when the port lookup stuff is moved from analyzer_mgr
+	bool check_port = conn->ConnTransport() != TRANSPORT_ICMP;
+
+	bool scheduled = analyzer_mgr->ApplyScheduledAnalyzers(conn, false, root);
+
+	// Hmm... Do we want *just* the expected analyzer, or all
+	// other potential analyzers as well?  For now we only take
+	// the scheduled ones.
+	if ( ! scheduled )
+		{ // Let's see if it's a port we know.
+		if ( check_port && ! zeek::detail::dpd_ignore_ports )
+			{
+			// TODO: ideally this lookup would be local to the packet analyzer instead of
+			// calling out to the analyzer manager. This code can move once the TCP work
+			// is in progress so that it doesn't have to be done piecemeal.
+			//
+			int resp_port = ntohs(conn->RespPort());
+			std::set<analyzer::Tag>* ports = analyzer_mgr->LookupPort(conn->ConnTransport(), resp_port, false);
+
+			if ( ports )
+				{
+				for ( const auto& port : *ports )
+					{
+					analyzer::Analyzer* analyzer = analyzer_mgr->InstantiateAnalyzer(port, conn);
+
+					if ( ! analyzer )
+						continue;
+
+					root->AddChildAnalyzer(analyzer, false);
+					DBG_ANALYZER_ARGS(conn, "activated %s analyzer due to port %d",
+					                  analyzer_mgr->GetComponentName(port).c_str(), resp_port);
+					}
+				}
+			}
+		}
+
+	root->AddExtraAnalyzers(conn);
+
+	if ( pia )
+		root->AddChildAnalyzer(pia->AsAnalyzer());
+
+	conn->SetRootAnalyzer(root, pia);
+	root->Init();
+	root->InitChildren();
+
+	PLUGIN_HOOK_VOID(HOOK_SETUP_ANALYZER_TREE, HookSetupAnalyzerTree(conn));
+
+	// TODO: temporary
+	return true;
+	}

src/packet_analysis/protocol/ip/IPBasedAnalyzer.h

@@ -33,12 +33,6 @@ public:
 	 */
 	virtual bool IsReuse(double t, const u_char* pkt) { return false; }
 
-	/**
-	 * TODO: comment
-	 */
-	virtual void CreateTransportAnalyzer(Connection* conn, IPBasedTransportAnalyzer*& root,
-	                                     analyzer::pia::PIA*& pia, bool& check_port) = 0;
-
 protected:
 
 	/**
@@ -96,6 +90,17 @@ protected:
 		return true;
 		}
 
+	/**
+	 * Returns a transport analyzer appropriate for this IP-based analyzer. This
+	 * can also be used to do any extra initialization of connection timers, etc.
+	 */
+	virtual IPBasedTransportAnalyzer* MakeTransportAnalyzer(Connection* conn) { return nullptr; }
+
+	/**
+	 * Returns a PIA appropriate for this IP-based analyzer.
+	 */
+	virtual analyzer::pia::PIA* MakePIA(Connection* conn) { return nullptr; }
+
 	/**
 	 * Verifies that there is enough data in the packet to process the header
 	 * length requested.
@@ -134,6 +139,8 @@ private:
 	zeek::Connection* NewConn(const ConnTuple* id, const detail::ConnKey& key,
 	                          const Packet* pkt);
 
+	bool BuildSessionAnalyzerTree(Connection* conn);
+
 	TransportProto transport;
 	uint32_t server_port_mask;
 };

src/packet_analysis/protocol/tcp/TCP.cc

@@ -74,8 +74,3 @@ bool TCPAnalyzer::WantConnection(uint16_t src_port, uint16_t dst_port,
 
 	return true;
 	}
-
-void TCPAnalyzer::CreateTransportAnalyzer(Connection* conn, IPBasedTransportAnalyzer*& root,
-                                          analyzer::pia::PIA*& pia, bool& check_port)
-	{
-	}

src/packet_analysis/protocol/tcp/TCP.h

@@ -18,9 +18,6 @@ public:
 		return std::make_shared<TCPAnalyzer>();
 		}
 
-	void CreateTransportAnalyzer(Connection* conn, IP::IPBasedTransportAnalyzer*& root,
-	                             analyzer::pia::PIA*& pia, bool& check_port) override;
-
 protected:
 
 	/**

src/packet_analysis/protocol/udp/UDP.cc

@@ -33,17 +33,20 @@ UDPAnalyzer::~UDPAnalyzer()
 	{
 	}
 
-void UDPAnalyzer::CreateTransportAnalyzer(Connection* conn, IPBasedTransportAnalyzer*& root,
+IPBasedTransportAnalyzer* UDPAnalyzer::MakeTransportAnalyzer(Connection* conn)
-                                          analyzer::pia::PIA*& pia, bool& check_port)
 	{
-	root = new UDPTransportAnalyzer(conn);
+	auto* root = new UDPTransportAnalyzer(conn);
 	root->SetParent(this);
 
 	conn->EnableStatusUpdateTimer();
 	conn->SetInactivityTimeout(zeek::detail::udp_inactivity_timeout);
 
-	pia = new analyzer::pia::PIA_UDP(conn);
+	return root;
-	check_port = true;
+	}
+
+zeek::analyzer::pia::PIA* UDPAnalyzer::MakePIA(Connection* conn)
+	{
+	return new analyzer::pia::PIA_UDP(conn);
 	}
 
 void UDPAnalyzer::Initialize()

src/packet_analysis/protocol/udp/UDP.h

@@ -18,9 +18,6 @@ public:
 		return std::make_shared<UDPAnalyzer>();
 		}
 
-	void CreateTransportAnalyzer(Connection* conn, IP::IPBasedTransportAnalyzer*& root,
-	                             analyzer::pia::PIA*& pia, bool& check_port) override;
-
 	/**
 	 * Initialize the analyzer. This method is called after the configuration
 	 * was read. Derived classes can override this method to implement custom
@@ -54,6 +51,9 @@ protected:
 	bool WantConnection(uint16_t src_port, uint16_t dst_port,
 	                    const u_char* data, bool& flip_roles) const override;
 
+	packet_analysis::IP::IPBasedTransportAnalyzer* MakeTransportAnalyzer(Connection* conn) override;
+	analyzer::pia::PIA* MakePIA(Connection* conn) override;
+
 private:
 
 	// Returns true if the checksum is valid, false if not