GH-887: improve GRE/ERSPAN parsing of non-IPv4/IPv6 inner payload

This changes the decapsulation logic for GRE/ERSPAN payloads to re-use
existing Layer 2 parsing logic that already handles things like 802.1Q
tags correctly before going on to process the inner IPv4/IPv6 payload.

src/Sessions.cc

@@ -351,6 +351,8 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 	id.dst_addr = ip_hdr->DstAddr();
 	ConnectionMap* d = nullptr;
 	BifEnum::Tunnel::Type tunnel_type = BifEnum::Tunnel::IP;
+	int gre_version = -1;
+	int gre_link_type = DLT_RAW;
 
 	switch ( proto ) {
 	case IPPROTO_TCP:
@@ -415,9 +417,8 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 
 		uint16_t flags_ver = ntohs(*((uint16_t*)(data + 0)));
 		uint16_t proto_typ = ntohs(*((uint16_t*)(data + 2)));
-		int gre_version = flags_ver & 0x0007;
+		gre_version = flags_ver & 0x0007;
 
-		// If a carried packet has ethernet, this will help skip it.
 		unsigned int eth_len = 0;
 		unsigned int gre_len = gre_header_len(flags_ver);
 		unsigned int ppp_len = gre_version == 1 ? 4 : 0;
@@ -438,6 +439,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 				if ( len > gre_len + 14 )
 					{
 					eth_len = 14;
+					gre_link_type = DLT_EN10MB;
 					proto_typ = ntohs(*((uint16_t*)(data + gre_len + eth_len - 2)));
 					}
 				else
@@ -454,6 +456,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 					{
 					erspan_len = 8;
 					eth_len = 14;
+					gre_link_type = DLT_EN10MB;
 					proto_typ = ntohs(*((uint16_t*)(data + gre_len + erspan_len + eth_len - 2)));
 					}
 				else
@@ -470,6 +473,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 					{
 					erspan_len = 12;
 					eth_len = 14;
+					gre_link_type = DLT_EN10MB;
 
 					auto flags = data + gre_len + erspan_len - 1;
 					bool have_opt_header = ((*flags & 0x01) == 0x01);
@@ -493,19 +497,6 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 					return;
 					}
 				}
-
-			if ( proto_typ == 0x0800 )
-				proto = IPPROTO_IPV4;
-			else if ( proto_typ == 0x86dd )
-				proto = IPPROTO_IPV6;
-			else
-				{
-				// Not IPv4/IPv6 payload.
-				Weird("unknown_gre_protocol", ip_hdr, encapsulation,
-				      fmt("%d", proto_typ));
-				return;
-				}
-
 			}
 
 		else // gre_version == 1
@@ -554,9 +545,12 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 			proto = (ppp_proto == 0x0021) ? IPPROTO_IPV4 : IPPROTO_IPV6;
 			}
 
-		data += gre_len + ppp_len + eth_len + erspan_len;
+		// If we know there's an Ethernet header here, it's not skipped yet.
-		len -= gre_len + ppp_len + eth_len + erspan_len;
+		// The Packet::init() that happens later will process all layer 2
-		caplen -= gre_len + ppp_len + eth_len + erspan_len;
+		// data, including things like vlan tags.
+		data += gre_len + ppp_len + erspan_len;
+		len -= gre_len + ppp_len + erspan_len;
+		caplen -= gre_len + ppp_len + erspan_len;
 
 		// Treat GRE tunnel like IP tunnels, fallthrough to logic below now
 		// that GRE header is stripped and only payload packet remains.
@@ -580,20 +574,24 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 			return;
 			}
 
-		// Check for a valid inner packet first.
 		IP_Hdr* inner = 0;
-		int result = ParseIPPacket(caplen, data, proto, inner);
-		if ( result == -2 )
-			Weird("invalid_inner_IP_version", ip_hdr, encapsulation);
-		else if ( result < 0 )
-			Weird("truncated_inner_IP", ip_hdr, encapsulation);
-		else if ( result > 0 )
-			Weird("inner_IP_payload_length_mismatch", ip_hdr, encapsulation);
 
-		if ( result != 0 )
+		if ( gre_version != 0 )
 			{
-			delete inner;
+			// Check for a valid inner packet first.
-			return;
+			int result = ParseIPPacket(caplen, data, proto, inner);
+			if ( result == -2 )
+				Weird("invalid_inner_IP_version", ip_hdr, encapsulation);
+			else if ( result < 0 )
+				Weird("truncated_inner_IP", ip_hdr, encapsulation);
+			else if ( result > 0 )
+				Weird("inner_IP_payload_length_mismatch", ip_hdr, encapsulation);
+
+			if ( result != 0 )
+				{
+				delete inner;
+				return;
+				}
 			}
 
 		// Look up to see if we've already seen this IP tunnel, identified
@@ -617,8 +615,12 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt, const IP_Hdr* ip_hdr
 		else
 			it->second.second = network_time;
 
-		DoNextInnerPacket(t, pkt, inner, encapsulation,
+		if ( gre_version == 0 )
-		                  ip_tunnels[tunnel_idx].first);
+			DoNextInnerPacket(t, pkt, caplen, len, data, gre_link_type,
+			                  encapsulation, ip_tunnels[tunnel_idx].first);
+		else
+			DoNextInnerPacket(t, pkt, inner, encapsulation,
+			                  ip_tunnels[tunnel_idx].first);
 
 		return;
 		}
@@ -727,7 +729,6 @@ void NetSessions::DoNextInnerPacket(double t, const Packet* pkt,
 
 	pkt_timeval ts;
 	int link_type;
-	Layer3Proto l3_proto;
 
 	if ( pkt )
 		ts = pkt->ts;
@@ -741,15 +742,9 @@ void NetSessions::DoNextInnerPacket(double t, const Packet* pkt,
 	const u_char* data = 0;
 
 	if ( inner->IP4_Hdr() )
-		{
 		data = (const u_char*) inner->IP4_Hdr();
-		l3_proto = L3_IPV4;
-		}
 	else
-		{
 		data = (const u_char*) inner->IP6_Hdr();
-		l3_proto = L3_IPV6;
-		}
 
 	EncapsulationStack* outer = prev ?
 			new EncapsulationStack(*prev) : new EncapsulationStack();
@@ -765,6 +760,40 @@ void NetSessions::DoNextInnerPacket(double t, const Packet* pkt,
 	delete outer;
 	}
 
+void NetSessions::DoNextInnerPacket(double t, const Packet* pkt,
+                                    uint32_t caplen, uint32_t len,
+                                    const u_char* data, int link_type,
+                                    const EncapsulationStack* prev,
+                                    const EncapsulatingConn& ec)
+	{
+	pkt_timeval ts;
+
+	if ( pkt )
+		ts = pkt->ts;
+	else
+		{
+		ts.tv_sec = (time_t) network_time;
+		ts.tv_usec = (suseconds_t)
+		    ((network_time - (double)ts.tv_sec) * 1000000);
+		}
+
+	EncapsulationStack* outer = prev ?
+			new EncapsulationStack(*prev) : new EncapsulationStack();
+	outer->Add(ec);
+
+	// Construct fake packet for DoNextPacket
+	Packet p;
+	p.Init(link_type, &ts, caplen, len, data, false, "");
+
+	if ( p.Layer2Valid() && (p.l3_proto == L3_IPV4 || p.l3_proto == L3_IPV6) )
+		{
+		auto inner = p.IP();
+		DoNextPacket(t, &p, &inner, outer);
+		}
+
+	delete outer;
+	}
+
 int NetSessions::ParseIPPacket(int caplen, const u_char* const pkt, int proto,
 		IP_Hdr*& inner)
 	{

src/Sessions.h

@@ -100,7 +100,7 @@ public:
 	 * Wrapper that recurses on DoNextPacket for encapsulated IP packets.
 	 *
 	 * @param t Network time.
-	 * @param hdr If the outer pcap header is available, this pointer can be set
+	 * @param pkt If the outer pcap header is available, this pointer can be set
 	 *        so that the fake pcap header passed to DoNextPacket will use
 	 *        the same timeval.  The caplen and len fields of the fake pcap
 	 *        header are always set to the TotalLength() of \a inner.
@@ -114,6 +114,27 @@ public:
 	                      const IP_Hdr* inner, const EncapsulationStack* prev,
 	                      const EncapsulatingConn& ec);
 
+	/**
+	 * Recurses on DoNextPacket for encapsulated Ethernet/IP packets.
+	 *
+	 * @param t  Network time.
+	 * @param pkt  If the outer pcap header is available, this pointer can be
+	 *        set so that the fake pcap header passed to DoNextPacket will use
+	 *        the same timeval.
+	 * @param caplen  number of captured bytes remaining
+	 * @param len  number of bytes remaining as claimed by outer framing
+	 * @param data  the remaining packet data
+	 * @param link_type  layer 2 link type used for initializing inner packet
+	 * @param prev  Any previous encapsulation stack of the caller, not
+	 *        including the most-recently found depth of encapsulation.
+	 * @param ec The most-recently found depth of encapsulation.
+	 */
+	void DoNextInnerPacket(double t, const Packet* pkt,
+                           uint32_t caplen, uint32_t len,
+                           const u_char* data, int link_type,
+                           const EncapsulationStack* prev,
+                           const EncapsulatingConn& ec);
+
 	/**
 	 * Returns a wrapper IP_Hdr object if \a pkt appears to be a valid IPv4
 	 * or IPv6 header based on whether it's long enough to contain such a header,

testing/btest/Baseline/core.tunnels.gre-erspan3-dot1q/out

@@ -0,0 +1,5 @@
+echo request, 43, 4
+echo reply, 43, 4
+[orig_h=172.31.10.31, orig_p=8/icmp, resp_h=172.31.10.2, resp_p=0/icmp]
+[[cid=[orig_h=172.31.1.23, orig_p=0/unknown, resp_h=172.31.1.135, resp_p=0/unknown], tunnel_type=Tunnel::GRE, uid=CHhAvVGS1DHFjwGM9]]
+vlans 10, nil

testing/btest/core/tunnels/gre-erspan3-dot1q.zeek

@@ -0,0 +1,19 @@
+# @TEST-EXEC: zeek -b -r $TRACES/tunnels/gre-erspan3-dot1q.pcap %INPUT > out
+# @TEST-EXEC: btest-diff out
+
+event icmp_echo_request(c: connection, icmp: icmp_conn, id: count, seq: count, payload: string)
+	{
+	print "echo request", id, seq;
+	}
+
+event icmp_echo_reply(c: connection, icmp: icmp_conn, id: count, seq: count, payload: string)
+	{
+	print "echo reply", id, seq;
+	}
+
+event connection_state_remove(c: connection)
+	{
+	print c$id;
+	print c$tunnel;
+	print fmt("vlans %s, %s", c$vlan, c?$inner_vlan ? "shouldn't be set" : "nil");
+	}