GH-545: add "addl" parameter to flow_weird and net_weird events

NEWS

@@ -298,9 +298,14 @@ Changed Functionality
   that didn't otherwise handle them (like syslog, modbus, dnp3) are now
   a ProtocolViolation instead
 
+- An "addl" parameter was added to the ``flow_weird`` and ``net_weird`` events
+  for describing additional information about the weird.  The ``conn_weird``
+  event already had such a parameter.
+
 - Weird names that contained variable content and may result in an unbounded
   number of weird names have been renamed to remove the variable content
-  (which has been made available in the "addl" field of conn_weirds):
+  (which has been made available in the "addl" field of ``conn_weird``,
+   ``flow_weird``, or ``net_weird`` events):
 
     - "unknown_dce_rpc_auth_type_%d" -> unknown_dce_rpc_auth_type
     - "gtp_invalid_info_element_%d" -> gtp_invalid_info_element

scripts/base/frameworks/notice/weird.zeek

@@ -406,7 +406,7 @@ event conn_weird(name: string, c: connection, addl: string)
 	weird(i);
 	}
 
-event flow_weird(name: string, src: addr, dst: addr)
+event flow_weird(name: string, src: addr, dst: addr, addl: string)
 	{
 	# We add the source and destination as port 0/unknown because that is
 	# what fits best here.
@@ -414,12 +414,20 @@ event flow_weird(name: string, src: addr, dst: addr)
 	                   $resp_h=dst, $resp_p=count_to_port(0, unknown_transport));
 
 	local i = Info($ts=network_time(), $name=name, $id=id, $identifier=flow_id_string(src,dst));
+
+	if ( addl != "" )
+		i$addl = addl;
+
 	weird(i);
 	}
 
-event net_weird(name: string)
+event net_weird(name: string, addl: string)
 	{
 	local i = Info($ts=network_time(), $name=name);
+
+	if ( addl != "" )
+		i$addl = addl;
+
 	weird(i);
 	}
 

scripts/base/misc/find-checksum-offloading.zeek

@@ -67,7 +67,7 @@ event zeek_init()
 	schedule check_interval { ChecksumOffloading::check() };
 	}
 
-event net_weird(name: string)
+event net_weird(name: string, addl: string)
 	{
 	if ( name == "bad_IP_checksum" )
 		++bad_ip_checksums;

src/IP.cc

@@ -288,7 +288,7 @@ RecordVal* IPv6_Hdr::BuildRecordVal(VectorVal* chain) const
 			}
 
 		default:
-			reporter->Weird("unknown_mobility_type");
+			reporter->Weird("unknown_mobility_type", fmt("%d", mob->ip6mob_type));
 			break;
 		}
 
@@ -553,7 +553,8 @@ void IPv6_Hdr_Chain::ProcessRoutingHeader(const struct ip6_rthdr* r, uint16_t le
 #endif
 
 	default:
-		reporter->Weird(SrcAddr(), DstAddr(), "unknown_routing_type");
+		reporter->Weird(SrcAddr(), DstAddr(), "unknown_routing_type",
+		                fmt("%d", r->ip6r_type));
 		break;
 	}
 	}

src/Reporter.cc

@@ -219,34 +219,14 @@ void Reporter::Syslog(const char* fmt, ...)
 	va_end(ap);
 	}
 
-void Reporter::WeirdHelper(EventHandlerPtr event, Val* conn_val, file_analysis::File* f, const char* addl, const char* fmt_name, ...)
+void Reporter::WeirdHelper(EventHandlerPtr event, val_list vl, const char* fmt_name, ...)
 	{
-	val_list vl(2);
-
-	if ( conn_val )
-		vl.push_back(conn_val);
-	else if ( f )
-		vl.push_back(f->GetVal()->Ref());
-
-	if ( addl )
-		vl.push_back(new StringVal(addl));
-
 	va_list ap;
 	va_start(ap, fmt_name);
 	DoLog("weird", event, 0, 0, &vl, false, false, 0, fmt_name, ap);
 	va_end(ap);
 	}
 
-void Reporter::WeirdFlowHelper(const IPAddr& orig, const IPAddr& resp, const char* fmt_name, ...)
-	{
-	val_list vl{new AddrVal(orig), new AddrVal(resp)};
-
-	va_list ap;
-	va_start(ap, fmt_name);
-	DoLog("weird", flow_weird, 0, 0, &vl, false, false, 0, fmt_name, ap);
-	va_end(ap);
-	}
-
 void Reporter::UpdateWeirdStats(const char* name)
 	{
 	++weird_count;
@@ -331,7 +311,7 @@ bool Reporter::PermitFlowWeird(const char* name,
 		return false;
 	}
 
-void Reporter::Weird(const char* name)
+void Reporter::Weird(const char* name, const char* addl)
 	{
 	UpdateWeirdStats(name);
 
@@ -341,7 +321,7 @@ void Reporter::Weird(const char* name)
 			return;
 		}
 
-	WeirdHelper(net_weird, 0, 0, 0, "%s", name);
+	WeirdHelper(net_weird, {new StringVal(addl)}, "%s", name);
 	}
 
 void Reporter::Weird(file_analysis::File* f, const char* name, const char* addl)
@@ -355,7 +335,8 @@ void Reporter::Weird(file_analysis::File* f, const char* name, const char* addl)
 			return;
 		}
 
-	WeirdHelper(file_weird, 0, f, addl, "%s", name);
+	WeirdHelper(file_weird, {f->GetVal()->Ref(), new StringVal(addl)},
+	            "%s", name);
 	}
 
 void Reporter::Weird(Connection* conn, const char* name, const char* addl)
@@ -369,10 +350,11 @@ void Reporter::Weird(Connection* conn, const char* name, const char* addl)
 			return;
 		}
 
-	WeirdHelper(conn_weird, conn->BuildConnVal(), 0, addl, "%s", name);
+	WeirdHelper(conn_weird, {conn->BuildConnVal(), new StringVal(addl)},
+	            "%s", name);
 	}
 
-void Reporter::Weird(const IPAddr& orig, const IPAddr& resp, const char* name)
+void Reporter::Weird(const IPAddr& orig, const IPAddr& resp, const char* name, const char* addl)
 	{
 	UpdateWeirdStats(name);
 
@@ -382,7 +364,9 @@ void Reporter::Weird(const IPAddr& orig, const IPAddr& resp, const char* name)
 			 return;
 		}
 
-	WeirdFlowHelper(orig, resp, "%s", name);
+	WeirdHelper(flow_weird,
+	            {new AddrVal(orig), new AddrVal(resp), new StringVal(addl)},
+	            "%s", name);
 	}
 
 void Reporter::DoLog(const char* prefix, EventHandlerPtr event, FILE* out,

src/Reporter.h

@@ -84,10 +84,10 @@ public:
 
 	// Report a traffic weirdness, i.e., an unexpected protocol situation
 	// that may lead to incorrectly processing a connnection.
-	void Weird(const char* name);	// Raises net_weird().
+	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 IPAddr& orig, const IPAddr& resp, const char* name);	// Raises flow_weird().
+	void Weird(const IPAddr& orig, const IPAddr& resp, const char* name, const char* addl = "");	// Raises flow_weird().
 
 	// Syslog a message. This methods does nothing if we're running
 	// offline from a trace.
@@ -245,10 +245,9 @@ private:
 		   Connection* conn, val_list* addl, bool location, bool time,
 		   const char* postfix, const char* fmt, va_list ap) __attribute__((format(printf, 10, 0)));
 
-	// The order if addl, name needs to be like that since fmt_name can
+	// WeirdHelper doesn't really have to be variadic, but it calls DoLog
-	// contain format specifiers
+	// and that takes va_list anyway.
-	void WeirdHelper(EventHandlerPtr event, Val* conn_val, file_analysis::File* f, const char* addl, const char* fmt_name, ...) __attribute__((format(printf, 6, 7)));;
+	void WeirdHelper(EventHandlerPtr event, val_list vl, const char* fmt_name, ...) __attribute__((format(printf, 4, 5)));;
-	void WeirdFlowHelper(const IPAddr& orig, const IPAddr& resp, const char* fmt_name, ...) __attribute__((format(printf, 4, 5)));;
 	void UpdateWeirdStats(const char* name);
 	inline bool WeirdOnSamplingWhiteList(const char* name)
 		{ return weird_sampling_whitelist.find(name) != weird_sampling_whitelist.end(); }

src/Sessions.cc

@@ -503,7 +503,8 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 
 		if ( gre_version != 0 && gre_version != 1 )
 			{
-			Weird("unknown_gre_version", ip_hdr, encapsulation);
+			Weird("unknown_gre_version", ip_hdr, encapsulation,
+			      fmt("%d", gre_version));
 			return;
 			}
 
@@ -578,7 +579,8 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 			else
 				{
 				// Not IPv4/IPv6 payload.
-				Weird("unknown_gre_protocol", ip_hdr, encapsulation);
+				Weird("unknown_gre_protocol", ip_hdr, encapsulation,
+				      fmt("%d", proto_typ));
 				return;
 				}
 
@@ -589,7 +591,8 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 			if ( proto_typ != 0x880b )
 				{
 				// Enhanced GRE payload must be PPP.
-				Weird("egre_protocol_type", ip_hdr, encapsulation);
+				Weird("egre_protocol_type", ip_hdr, encapsulation,
+				      fmt("%d", proto_typ));
 				return;
 				}
 			}
@@ -711,7 +714,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 		}
 
 	default:
-		Weird("unknown_protocol", pkt, encapsulation);
+		Weird("unknown_protocol", pkt, encapsulation, fmt("%d", proto));
 		return;
 	}
 
@@ -1412,25 +1415,25 @@ void NetSessions::DumpPacket(const Packet *pkt, int len)
 	}
 
 void NetSessions::Weird(const char* name, const Packet* pkt,
-                        const EncapsulationStack* encap)
+                        const EncapsulationStack* encap, const char* addl)
 	{
 	if ( pkt )
 		dump_this_packet = 1;
 
 	if ( encap && encap->LastType() != BifEnum::Tunnel::NONE )
-		reporter->Weird(fmt("%s_in_tunnel", name));
+		reporter->Weird(fmt("%s_in_tunnel", name), addl);
 	else
-		reporter->Weird(name);
+		reporter->Weird(name, addl);
 	}
 
 void NetSessions::Weird(const char* name, const IP_Hdr* ip,
-                        const EncapsulationStack* encap)
+                        const EncapsulationStack* encap, const char* addl)
 	{
 	if ( encap && encap->LastType() != BifEnum::Tunnel::NONE )
 		reporter->Weird(ip->SrcAddr(), ip->DstAddr(),
-		                fmt("%s_in_tunnel", name));
+		                fmt("%s_in_tunnel", name), addl);
 	else
-		reporter->Weird(ip->SrcAddr(), ip->DstAddr(), name);
+		reporter->Weird(ip->SrcAddr(), ip->DstAddr(), name, addl);
 	}
 
 unsigned int NetSessions::ConnectionMemoryUsage()

src/Sessions.h

@@ -90,9 +90,9 @@ public:
 	void GetStats(SessionStats& s) const;
 
 	void Weird(const char* name, const Packet* pkt,
-	    const EncapsulationStack* encap = 0);
+	    const EncapsulationStack* encap = 0, const char* addl = "");
 	void Weird(const char* name, const IP_Hdr* ip,
-	    const EncapsulationStack* encap = 0);
+	    const EncapsulationStack* encap = 0, const char* addl = "");
 
 	PacketFilter* GetPacketFilter()
 		{

src/event.bif

@@ -450,13 +450,15 @@ event conn_weird%(name: string, c: connection, addl: string%);
 ##
 ## dst: The destination address corresponding to the activity.
 ##
+## addl: Optional additional context further describing the situation.
+##
 ## .. zeek:see:: conn_weird net_weird file_weird
 ##
 ## .. note:: "Weird" activity is much more common in real-world network traffic
 ##    than one would intuitively expect. While in principle, any protocol
 ##    violation could be an attack attempt, it's much more likely that an
 ##    endpoint's implementation interprets an RFC quite liberally.
-event flow_weird%(name: string, src: addr, dst: addr%);
+event flow_weird%(name: string, src: addr, dst: addr, addl: string%);
 
 ## Generated for unexpected activity that is not tied to a specific connection
 ## or pair of hosts. When Zeek's packet analysis encounters activity that
@@ -468,13 +470,15 @@ event flow_weird%(name: string, src: addr, dst: addr%);
 ##       scripts use this name in filtering policies that specify which
 ##       "weirds" are worth reporting.
 ##
+## addl: Optional additional context further describing the situation.
+##
 ## .. zeek:see:: flow_weird file_weird
 ##
 ## .. note:: "Weird" activity is much more common in real-world network traffic
 ##    than one would intuitively expect. While in principle, any protocol
 ##    violation could be an attack attempt, it's much more likely that an
 ##    endpoint's implementation interprets an RFC quite liberally.
-event net_weird%(name: string%);
+event net_weird%(name: string, addl: string%);
 
 ## Generated for unexpected activity that is tied to a file.
 ## When Zeek's packet analysis encounters activity that

testing/btest/Baseline/core.disable-mobile-ipv6/weird.log

@@ -3,8 +3,8 @@
 #empty_field	(empty)
 #unset_field	-
 #path	weird
-#open	2019-06-07-01-59-20
+#open	2019-08-21-02-16-33
 #fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	name	addl	notice	peer
 #types	time	string	addr	port	addr	port	string	string	bool	string
-1333663011.602839	-	-	-	-	-	unknown_protocol	-	F	zeek
+1333663011.602839	-	-	-	-	-	unknown_protocol	135	F	zeek
-#close	2019-06-07-01-59-20
+#close	2019-08-21-02-16-33

testing/btest/core/ipv6_ext_headers.test

@@ -9,7 +9,7 @@ event ipv6_ext_headers(c: connection, p: pkt_hdr)
 	}
 
 # Also check the weird for routing type 0 extensions headers
-event flow_weird(name: string, src: addr, dst: addr)
+event flow_weird(name: string, src: addr, dst: addr, addl: string)
 	{
 	print fmt("weird %s from %s to %s", name, src, dst);
 	}

testing/btest/core/leaks/ipv6_ext_headers.test

@@ -15,7 +15,7 @@ event ipv6_ext_headers(c: connection, p: pkt_hdr)
 	}
 
 # Also check the weird for routing type 0 extensions headers
-event flow_weird(name: string, src: addr, dst: addr)
+event flow_weird(name: string, src: addr, dst: addr, addl: string)
 	{
 	print fmt("weird %s from %s to %s", name, src, dst);
 	}

testing/btest/core/reassembly.zeek

@@ -10,12 +10,12 @@ event zeek_init()
 	print "----------------------";
 	}
 
-event flow_weird(name: string, src: addr, dst: addr)
+event flow_weird(name: string, src: addr, dst: addr, addl: string)
 	{
 	print "flow weird", name, src, dst;
 	}
 
-event net_weird(name: string)
+event net_weird(name: string, addl: string)
 	{
 	print "net_weird", name;
 	}

testing/btest/core/reporter-weird-sampling-disable.zeek

@@ -4,7 +4,7 @@
 redef Weird::sampling_threshold = 1;
 redef Weird::sampling_rate = 0;
 
-event net_weird(name: string)
+event net_weird(name: string, addl: string)
 	{
 	print "net_weird", name;
 	}

testing/btest/core/reporter-weird-sampling.zeek

@@ -13,12 +13,12 @@ event conn_weird(name: string, c: connection, addl: string)
 	print "conn_weird", name;
 	}
 
-event flow_weird(name: string, src: addr, dst: addr)
+event flow_weird(name: string, src: addr, dst: addr, addl: string)
 	{
 	print "flow_weird", name;
 	}
 
-event net_weird(name: string)
+event net_weird(name: string, addl: string)
 	{
 	print "net_weird", name;
 	}

testing/btest/scripts/base/frameworks/config/weird.zeek

@@ -24,12 +24,12 @@ event conn_weird(name: string, c: connection, addl: string)
 	print "conn_weird", name;
 	}
 
-event flow_weird(name: string, src: addr, dst: addr)
+event flow_weird(name: string, src: addr, dst: addr, addl: string)
 	{
 	print "flow_weird", name;
 	}
 
-event net_weird(name: string)
+event net_weird(name: string, addl: string)
 	{
 	print "net_weird", name;
 	}