Merge remote-tracking branch 'origin/topic/timw/1184-additional-weird…

…-info' * origin/topic/timw/1184-additional-weird-info: GH-1184: Add 'source' field to weird log denoting where the weird was reported
zeek · Dec 1, 2020 · 49293c0 · 49293c0
2 parents eccbbb4 + e27008e
commit 49293c0
Show file tree

Hide file tree

Showing 74 changed files with 962 additions and 885 deletions.
diff --git a/CHANGES b/CHANGES
@@ -1,3 +1,6 @@
+3.3.0-dev.587 | 2020-12-01 10:17:42 -0700
+
+  * GH-1184: Add 'source' field to weird log denoting where the weird was reported (Tim Wojtulewicz, Corelight)
 
 3.3.0-dev.585 | 2020-12-01 14:42:54 +0000
 

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-3.3.0-dev.585
+3.3.0-dev.587
diff --git a/doc b/doc
diff --git a/scripts/base/frameworks/notice/weird.zeek b/scripts/base/frameworks/notice/weird.zeek
@@ -54,6 +54,10 @@ export {
 		## trouble to help identify which node is having trouble.
 		peer:   string  &log &optional &default=peer_description;
 
+		## The source of the weird. When reported by an analyzer, this
+		## should be the name of the analyzer.
+		source: string  &log &optional;
+
 		## This field is to be provided when a weird is generated for
 		## the purpose of deduplicating weirds. The identifier string
 		## should be unique for a single instance of the weird. This field
@@ -257,7 +261,7 @@ export {
 
 	## This table is used to track identifier and name pairs that should be
 	## temporarily ignored because the problem has already been reported.
-	## This helps reduce the volume of high volume weirds by only allowing 
+	## This helps reduce the volume of high volume weirds by only allowing
 	## a unique weird every ``create_expire`` interval.
 	global weird_ignore: set[string, string] &create_expire=10min &redef;
 
@@ -400,27 +404,33 @@ function weird(w: Weird::Info)
 	}
 
 # The following events come from core generated weirds typically.
-event conn_weird(name: string, c: connection, addl: string)
+event conn_weird(name: string, c: connection, addl: string, source: string)
 	{
 	local i = Info($ts=network_time(), $name=name, $conn=c, $identifier=id_string(c$id));
 	if ( addl != "" )
 		i$addl = addl;
 
+	if ( source != "" )
+		i$source = source;
+
 	weird(i);
 	}
 
-event expired_conn_weird(name: string, id: conn_id, uid: string, addl: string)
+event expired_conn_weird(name: string, id: conn_id, uid: string, addl: string, source: string)
 	{
 	local i = Info($ts=network_time(), $name=name, $uid=uid, $id=id,
 	               $identifier=id_string(id));
 
 	if ( addl != "" )
 		i$addl = addl;
 
+	if ( source != "" )
+		i$source = source;
+
 	weird(i);
 	}
 
-event flow_weird(name: string, src: addr, dst: addr, addl: string)
+event flow_weird(name: string, src: addr, dst: addr, addl: string, source: string)
 	{
 	# We add the source and destination as port 0/unknown because that is
 	# what fits best here.
@@ -432,25 +442,34 @@ event flow_weird(name: string, src: addr, dst: addr, addl: string)
 	if ( addl != "" )
 		i$addl = addl;
 
+	if ( source != "" )
+		i$source = source;
+
 	weird(i);
 	}
 
-event net_weird(name: string, addl: string)
+event net_weird(name: string, addl: string, source: string)
 	{
 	local i = Info($ts=network_time(), $name=name);
 
 	if ( addl != "" )
 		i$addl = addl;
 
+	if ( source != "" )
+		i$source = source;
+
 	weird(i);
 	}
 
-event file_weird(name: string, f: fa_file, addl: string)
+event file_weird(name: string, f: fa_file, addl: string, source: string)
 	{
 	local i = Info($ts=network_time(), $name=name, $addl=f$id);
 
 	if ( addl != "" )
 		i$addl += fmt(": %s", addl);
 
+	if ( source != "" )
+		i$source = source;
+
 	weird(i);
 	}
diff --git a/src/Conn.cc b/src/Conn.cc
@@ -530,10 +530,10 @@ void Connection::EnqueueEvent(EventHandlerPtr f, analyzer::Analyzer* a,
 	event_mgr.Enqueue(f, std::move(args), util::detail::SOURCE_LOCAL, a ? a->GetID() : 0, this);
 	}
 
-void Connection::Weird(const char* name, const char* addl)
+void Connection::Weird(const char* name, const char* addl, const char* source)
 	{
 	weird = 1;
-	reporter->Weird(this, name, addl ? addl : "");
+	reporter->Weird(this, name, addl ? addl : "", source ? source : "");
 	}
 
 void Connection::AddTimer(timer_func timer, double t, bool do_expire,

diff --git a/src/Conn.h b/src/Conn.h
@@ -238,7 +238,7 @@ class Connection final : public Obj {
 	EnqueueEvent(EventHandlerPtr h, analyzer::Analyzer* analyzer, Args&&... args)
 		{ return EnqueueEvent(h, analyzer, zeek::Args{std::forward<Args>(args)...}); }
 
-	void Weird(const char* name, const char* addl = "");
+	void Weird(const char* name, const char* addl = "", const char* source = "");
 	bool DidWeird() const	{ return weird != 0; }
 
 	// Cancel all associated timers.

diff --git a/src/Reporter.cc b/src/Reporter.cc
@@ -396,7 +396,7 @@ bool Reporter::PermitExpiredConnWeird(const char* name, const RecordVal& conn_id
 		return false;
 	}
 
-void Reporter::Weird(const char* name, const char* addl)
+void Reporter::Weird(const char* name, const char* addl, const char* source)
 	{
 	UpdateWeirdStats(name);
 
@@ -406,10 +406,10 @@ void Reporter::Weird(const char* name, const char* addl)
 			return;
 		}
 
-	WeirdHelper(net_weird, {new StringVal(addl)}, "%s", name);
+	WeirdHelper(net_weird, {new StringVal(addl), new StringVal(source)}, "%s", name);
 	}
 
-void Reporter::Weird(file_analysis::File* f, const char* name, const char* addl)
+void Reporter::Weird(file_analysis::File* f, const char* name, const char* addl, const char* source)
 	{
 	UpdateWeirdStats(name);
 
@@ -424,11 +424,11 @@ void Reporter::Weird(file_analysis::File* f, const char* name, const char* addl)
 			return;
 	}
 
-	WeirdHelper(file_weird, {f->ToVal()->Ref(), new StringVal(addl)},
+	WeirdHelper(file_weird, {f->ToVal()->Ref(), new StringVal(addl), new StringVal(source)},
 	            "%s", name);
 	}
 
-void Reporter::Weird(Connection* conn, const char* name, const char* addl)
+void Reporter::Weird(Connection* conn, const char* name, const char* addl, const char* source)
 	{
 	UpdateWeirdStats(name);
 
@@ -443,12 +443,12 @@ void Reporter::Weird(Connection* conn, const char* name, const char* addl)
 			return;
 	}
 
-	WeirdHelper(conn_weird, {conn->ConnVal()->Ref(), new StringVal(addl)},
+	WeirdHelper(conn_weird, {conn->ConnVal()->Ref(), new StringVal(addl), new StringVal(source)},
 	            "%s", name);
 	}
 
-void Reporter::Weird(RecordValPtr conn_id, StringValPtr uid,
-                     const char* name, const char* addl)
+void Reporter::Weird(RecordValPtr conn_id, StringValPtr uid, const char* name,
+                     const char* addl, const char* source)
 	{
 	UpdateWeirdStats(name);
 
@@ -463,11 +463,11 @@ void Reporter::Weird(RecordValPtr conn_id, StringValPtr uid,
 	}
 
 	WeirdHelper(expired_conn_weird,
-	            {conn_id.release(), uid.release(), new StringVal(addl)},
+	            {conn_id.release(), uid.release(), new StringVal(addl), new StringVal(source)},
 	            "%s", name);
 	}
 
-void Reporter::Weird(const IPAddr& orig, const IPAddr& resp, const char* name, const char* addl)
+void Reporter::Weird(const IPAddr& orig, const IPAddr& resp, const char* name, const char* addl, const char* source)
 	{
 	UpdateWeirdStats(name);
 
@@ -482,7 +482,7 @@ void Reporter::Weird(const IPAddr& orig, const IPAddr& resp, const char* name, c
 	}
 
 	WeirdHelper(flow_weird,
-	            {new AddrVal(orig), new AddrVal(resp), new StringVal(addl)},
+	            {new AddrVal(orig), new AddrVal(resp), new StringVal(addl), new StringVal(source)},
 	            "%s", name);
 	}
 

diff --git a/src/Reporter.h b/src/Reporter.h
@@ -95,12 +95,15 @@ class Reporter {
 
 	// Report a traffic weirdness, i.e., an unexpected protocol situation
 	// that may lead to incorrectly processing a connnection.
-	void Weird(const char* name, const char* addl = "");	// Raises net_weird().
-	void Weird(file_analysis::File* f, const char* name, const char* addl = "");	// Raises file_weird().
-	void Weird(Connection* conn, const char* name, const char* addl = "");	// Raises conn_weird().
+	void Weird(const char* name, const char* addl = "", const char* source = "");	// Raises net_weird().
+	void Weird(file_analysis::File* f, const char* name,
+	           const char* addl = "", const char* source = "");	// Raises file_weird().
+	void Weird(Connection* conn, const char* name,
+	           const char* addl = "", const char* source = "");	// Raises conn_weird().
 	void Weird(RecordValPtr conn_id, StringValPtr uid,
-	           const char* name, const char* addl = "");	// Raises expired_conn_weird().
-	void Weird(const IPAddr& orig, const IPAddr& resp, const char* name, const char* addl = "");	// Raises flow_weird().
+	           const char* name, const char* addl = "", const char* source = "");	// Raises expired_conn_weird().
+	void Weird(const IPAddr& orig, const IPAddr& resp, const char* name,
+	           const char* addl = "", const char* source = "");	// Raises flow_weird().
 
 	// Syslog a message. This methods does nothing if we're running
 	// offline from a trace.

diff --git a/src/Sessions.cc b/src/Sessions.cc
@@ -681,7 +681,7 @@ bool NetSessions::WantConnection(uint16_t src_port, uint16_t dst_port,
 	return true;
 	}
 
-void NetSessions::Weird(const char* name, const Packet* pkt, const char* addl)
+void NetSessions::Weird(const char* name, const Packet* pkt, const char* addl, const char* source)
 	{
 	const char* weird_name = name;
 
@@ -694,12 +694,12 @@ void NetSessions::Weird(const char* name, const Packet* pkt, const char* addl)
 
 		if ( pkt->ip_hdr )
 			{
-			reporter->Weird(pkt->ip_hdr->SrcAddr(), pkt->ip_hdr->DstAddr(), weird_name, addl);
+			reporter->Weird(pkt->ip_hdr->SrcAddr(), pkt->ip_hdr->DstAddr(), weird_name, addl, source);
 			return;
 			}
 		}
 
-	reporter->Weird(weird_name, addl);
+	reporter->Weird(weird_name, addl, source);
 	}
 
 void NetSessions::Weird(const char* name, const IP_Hdr* ip, const char* addl)

diff --git a/src/Sessions.h b/src/Sessions.h
@@ -70,7 +70,7 @@ class NetSessions {
 	void GetStats(SessionStats& s) const;
 
 	void Weird(const char* name, const Packet* pkt,
-	           const char* addl = "");
+	           const char* addl = "", const char* source = "");
 	void Weird(const char* name, const IP_Hdr* ip,
 	           const char* addl = "");
 

diff --git a/src/analyzer/Analyzer.cc b/src/analyzer/Analyzer.cc
@@ -838,7 +838,7 @@ void Analyzer::EnqueueConnEvent(EventHandlerPtr f, Args args)
 
 void Analyzer::Weird(const char* name, const char* addl)
 	{
-	conn->Weird(name, addl);
+	conn->Weird(name, addl, GetAnalyzerName());
 	}
 
 SupportAnalyzer* SupportAnalyzer::Sibling(bool only_active) const

diff --git a/src/analyzer/protocol/ayiya/ayiya-analyzer.pac b/src/analyzer/protocol/ayiya/ayiya-analyzer.pac
@@ -21,7 +21,7 @@ flow AYIYA_Flow
 
 		if ( e && e->Depth() >= zeek::BifConst::Tunnel::max_depth )
 			{
-			zeek::reporter->Weird(c, "tunnel_depth");
+			connection()->zeek_analyzer()->Weird("tunnel_depth");
 			return false;
 			}
 
@@ -34,7 +34,7 @@ flow AYIYA_Flow
 		if ( ${pdu.next_header} != IPPROTO_IPV6 &&
 		     ${pdu.next_header} != IPPROTO_IPV4 )
 			{
-			zeek::reporter->Weird(c, "ayiya_tunnel_non_ip");
+			connection()->zeek_analyzer()->Weird("ayiya_tunnel_non_ip");
 			return false;
 			}
 

diff --git a/src/analyzer/protocol/bittorrent/BitTorrent.cc b/src/analyzer/protocol/bittorrent/BitTorrent.cc
@@ -119,6 +119,8 @@ void BitTorrent_Analyzer::EndpointEOF(bool is_orig)
 void BitTorrent_Analyzer::DeliverWeird(const char* msg, bool orig)
 	{
 	if ( bittorrent_peer_weird )
+
+		// TODO: why does bittorrent have a different set of weirds?
 		EnqueueConnEvent(bittorrent_peer_weird,
 		                 ConnVal(),
 		                 val_mgr->Bool(orig),

diff --git a/src/analyzer/protocol/dce-rpc/dce_rpc-protocol.pac b/src/analyzer/protocol/dce-rpc/dce_rpc-protocol.pac
@@ -190,8 +190,7 @@ flow DCE_RPC_Flow(is_orig: bool) {
 			if ( it != fb.end() )
 				{
 				// We already had a first frag earlier.
-				zeek::reporter->Weird(connection()->zeek_analyzer()->Conn(),
-						"multiple_first_fragments_in_dce_rpc_reassembly");
+				connection()->zeek_analyzer()->Weird("multiple_first_fragments_in_dce_rpc_reassembly");
 				connection()->zeek_analyzer()->SetSkip(true);
 				return false;
 				}
@@ -212,15 +211,13 @@ flow DCE_RPC_Flow(is_orig: bool) {
 
 				if ( fb.size() > zeek::BifConst::DCE_RPC::max_cmd_reassembly )
 					{
-					zeek::reporter->Weird(connection()->zeek_analyzer()->Conn(),
-					                "too_many_dce_rpc_msgs_in_reassembly");
+					connection()->zeek_analyzer()->Weird("too_many_dce_rpc_msgs_in_reassembly");
 					connection()->zeek_analyzer()->SetSkip(true);
 					}
 
 				if ( flowbuf->data_length() > (int)zeek::BifConst::DCE_RPC::max_frag_data )
 					{
-					zeek::reporter->Weird(connection()->zeek_analyzer()->Conn(),
-					                "too_much_dce_rpc_fragment_data");
+					connection()->zeek_analyzer()->Weird("too_much_dce_rpc_fragment_data");
 					connection()->zeek_analyzer()->SetSkip(true);
 					}
 
@@ -235,8 +232,7 @@ flow DCE_RPC_Flow(is_orig: bool) {
 
 			if ( flowbuf->data_length() > (int)zeek::BifConst::DCE_RPC::max_frag_data )
 				{
-				zeek::reporter->Weird(connection()->zeek_analyzer()->Conn(),
-				                "too_much_dce_rpc_fragment_data");
+				connection()->zeek_analyzer()->Weird("too_much_dce_rpc_fragment_data");
 				connection()->zeek_analyzer()->SetSkip(true);
 				}
 

diff --git a/src/analyzer/protocol/gtpv1/gtpv1-analyzer.pac b/src/analyzer/protocol/gtpv1/gtpv1-analyzer.pac
@@ -655,7 +655,7 @@ flow GTPv1_Flow(is_orig: bool)
 
 		if ( e && e->Depth() >= zeek::BifConst::Tunnel::max_depth )
 			{
-			zeek::reporter->Weird(c, "tunnel_depth");
+			a->Weird("tunnel_depth");
 			return false;
 			}
 

diff --git a/src/analyzer/protocol/http/HTTP.cc b/src/analyzer/protocol/http/HTTP.cc
@@ -1262,11 +1262,11 @@ int HTTP_Analyzer::HTTP_RequestLine(const char* line, const char* end_of_line)
 	return 1;
 
 bad_http_request_with_version:
-	reporter->Weird(Conn(), "bad_HTTP_request_with_version");
+	Weird("bad_HTTP_request_with_version");
 	return 0;
 
 error:
-	reporter->Weird(Conn(), "bad_HTTP_request");
+	Weird("bad_HTTP_request");
 	return 0;
 	}
 

diff --git a/src/analyzer/protocol/imap/imap-analyzer.pac b/src/analyzer/protocol/imap/imap-analyzer.pac
@@ -33,7 +33,7 @@ refine connection IMAP_Conn += {
 		if ( is_orig && commands == "starttls" )
 			{
 			if ( !client_starttls_id.empty() )
-				zeek::reporter->Weird(zeek_analyzer()->Conn(), "IMAP: client sent duplicate StartTLS");
+				zeek_analyzer()->Weird("IMAP: client sent duplicate StartTLS");
 
 			client_starttls_id = tags;
 			}
@@ -48,7 +48,7 @@ refine connection IMAP_Conn += {
 					zeek::BifEvent::enqueue_imap_starttls(zeek_analyzer(), zeek_analyzer()->Conn());
 				}
 			else
-				zeek::reporter->Weird(zeek_analyzer()->Conn(), "IMAP: server refused StartTLS");
+				zeek_analyzer()->Weird("IMAP: server refused StartTLS");
 			}
 
 		return true;

diff --git a/src/analyzer/protocol/login/NVT.cc b/src/analyzer/protocol/login/NVT.cc
@@ -539,7 +539,7 @@ void NVT_Analyzer::DeliverChunk(int& len, const u_char*& data)
 			else
 				{
 				if ( Conn()->FlagEvent(SINGULAR_LF) )
-					Conn()->Weird("line_terminated_with_single_LF");
+					Weird("line_terminated_with_single_LF");
 				buf[offset++] = c;
 				}
 			break;

diff --git a/src/analyzer/protocol/login/RSH.cc b/src/analyzer/protocol/login/RSH.cc
@@ -96,7 +96,7 @@ void Contents_Rsh_Analyzer::DoDeliver(int len, const u_char* data)
 		case RSH_PRESUMED_REJECTED:
 			if ( state == RSH_PRESUMED_REJECTED )
 				{
-				Conn()->Weird("rsh_text_after_rejected");
+				Weird("rsh_text_after_rejected");
 				state = RSH_UNKNOWN;
 				}
 
@@ -140,7 +140,7 @@ void Contents_Rsh_Analyzer::DoDeliver(int len, const u_char* data)
 
 void Contents_Rsh_Analyzer::BadProlog()
 	{
-	Conn()->Weird("bad_rsh_prolog");
+	Weird("bad_rsh_prolog");
 	state = RSH_UNKNOWN;
 	}