ip/vlan_fivetuple: Populate nested conn_id_context, not conn_id

This also enforces conn_id and conn_id_ctx types instead of being
able to handle any conn_id-like record.

scripts/policy/frameworks/conn_key/vlan_fivetuple.zeek

@@ -3,12 +3,11 @@
 ##! lookups; this change makes it factor them in and also makes those VLAN tags
 ##! part of the :zeek:see:`conn_id` record.
 
-redef record conn_id += {
+redef record conn_id_ctx += {
 	## The outer VLAN for this connection, if applicable.
-	vlan: int      &log &optional;
+	vlan: int &log &optional;
-
 	## The inner VLAN for this connection, if applicable.
-	inner_vlan: int      &log &optional;
+	inner_vlan: int &log &optional;
 };
 
 redef ConnKey::factory = ConnKey::CONNKEY_VLAN_FIVETUPLE;

src/packet_analysis/protocol/ip/conn_key/vlan_fivetuple/Factory.cc

@@ -4,6 +4,7 @@
 
 #include <memory>
 
+#include "zeek/Desc.h"
 #include "zeek/ID.h"
 #include "zeek/Val.h"
 #include "zeek/iosource/Packet.h"
@@ -38,31 +39,30 @@ protected:
     }
 
     void DoPopulateConnIdVal(RecordVal& conn_id, RecordVal& ctx) override {
-        if ( conn_id.NumFields() <= 5 )
-            return;
-
         // Nothing to do if we have no VLAN tags at all.
         if ( key.vlan == 0 && key.inner_vlan == 0 )
             return;
 
-        auto [vlan_offset, inner_vlan_offset] = GetConnIdFieldOffsets();
+        auto [vlan_offset, inner_vlan_offset] = GetConnCtxFieldOffsets();
 
         if ( key.vlan && vlan_offset >= 0 )
-            conn_id.Assign(vlan_offset, static_cast<int>(key.vlan));
+            ctx.Assign(vlan_offset, static_cast<int>(key.vlan));
         if ( key.inner_vlan && inner_vlan_offset >= 0 )
-            conn_id.Assign(inner_vlan_offset, static_cast<int>(key.inner_vlan));
+            ctx.Assign(inner_vlan_offset, static_cast<int>(key.inner_vlan));
     };
 
-    std::pair<int, int> GetConnIdFieldOffsets() {
+    std::pair<int, int> GetConnCtxFieldOffsets() {
+        static const auto& conn_id_ctx = zeek::id::find_type<zeek::RecordType>("conn_id_ctx");
+
         static int vlan_offset = -2, inner_vlan_offset = -2;
 
         if ( vlan_offset == -2 && inner_vlan_offset == -2 ) {
-            vlan_offset = id::conn_id->FieldOffset("vlan");
+            vlan_offset = conn_id_ctx->FieldOffset("vlan");
-            if ( vlan_offset < 0 || id::conn_id->GetFieldType(vlan_offset)->Tag() != TYPE_INT )
+            if ( vlan_offset < 0 || conn_id_ctx->GetFieldType(vlan_offset)->Tag() != TYPE_INT )
                 vlan_offset = -1;
 
-            inner_vlan_offset = id::conn_id->FieldOffset("inner_vlan");
+            inner_vlan_offset = conn_id_ctx->FieldOffset("inner_vlan");
-            if ( inner_vlan_offset < 0 || id::conn_id->GetFieldType(inner_vlan_offset)->Tag() != TYPE_INT )
+            if ( inner_vlan_offset < 0 || conn_id_ctx->GetFieldType(inner_vlan_offset)->Tag() != TYPE_INT )
                 inner_vlan_offset = -1;
         }
 
@@ -78,7 +78,6 @@ protected:
 private:
     friend class Factory;
 
-    // Key bytes:
     struct {
         struct detail::PackedConnTuple tuple;
         // Add 802.1Q vlan tags to connection tuples. The tag representation
@@ -98,30 +97,19 @@ zeek::expected<zeek::ConnKeyPtr, std::string> Factory::DoConnKeyFromVal(const ze
         return ck;
 
     auto* k = static_cast<IPVlanConnKey*>(ck.value().get());
-    auto rt = v.GetType()->AsRecordType();
     auto vl = v.AsRecordVal();
+    auto ctx = vl->GetFieldAs<zeek::RecordVal>(5);
 
-    int vlan_offset, inner_vlan_offset;
+    auto [vlan_offset, inner_vlan_offset] = k->GetConnCtxFieldOffsets();
-    if ( rt == id::conn_id ) {
-        std::tie(vlan_offset, inner_vlan_offset) = k->GetConnIdFieldOffsets();
-    }
-    else {
-        // We don't know what we've been passed.
-        vlan_offset = rt->FieldOffset("vlan");
-        inner_vlan_offset = rt->FieldOffset("inner_vlan");
-    }
 
     if ( vlan_offset < 0 || inner_vlan_offset < 0 )
-        return zeek::unexpected<std::string>{"missing vlan or inner_vlan field"};
+        return zeek::unexpected<std::string>{"missing vlan or inner_vlan field in context"};
 
-    if ( rt->GetFieldType(vlan_offset)->Tag() != TYPE_INT || rt->GetFieldType(inner_vlan_offset)->Tag() != TYPE_INT )
+    if ( ctx->HasField(vlan_offset) )
-        return zeek::unexpected<std::string>{"vlan or inner_vlan field not of type int"};
+        k->key.vlan = ctx->GetFieldAs<zeek::IntVal>(vlan_offset);
 
-    if ( vl->HasField(vlan_offset) )
+    if ( ctx->HasField(inner_vlan_offset) )
-        k->key.vlan = vl->GetFieldAs<zeek::IntVal>(vlan_offset);
+        k->key.inner_vlan = ctx->GetFieldAs<zeek::IntVal>(inner_vlan_offset);
-
-    if ( vl->HasField(inner_vlan_offset) )
-        k->key.inner_vlan = vl->GetFieldAs<zeek::IntVal>(inner_vlan_offset);
 
     return ck;
 }

testing/btest/Baseline/scripts.policy.frameworks.conn_key.vlan_fivetuple-2/conn.log.cut

@@ -1,5 +1,5 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	id.vlan	id.inner_vlan	orig_pkts	resp_pkts	service
+ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	id.ctx.vlan	id.ctx.inner_vlan	orig_pkts	resp_pkts	service
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	141.142.228.5	59856	192.150.187.43	80	-	-	7	7	http
 XXXXXXXXXX.XXXXXX	C4J4Th3PJpwUYZZ6gc	141.142.228.5	59856	192.150.187.43	80	10	20	7	7	http
 XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	141.142.228.5	59856	192.150.187.43	80	42	-	7	7	http

testing/btest/Baseline/scripts.policy.frameworks.conn_key.vlan_fivetuple-4/conn.log.cut

@@ -1,5 +1,5 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	id.vlan	id.inner_vlan	orig_pkts	resp_pkts	service
+ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	id.ctx.vlan	id.ctx.inner_vlan	orig_pkts	resp_pkts	service
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	141.142.228.5	59856	192.150.187.43	80	-	-	7	7	http
 XXXXXXXXXX.XXXXXX	C4J4Th3PJpwUYZZ6gc	141.142.228.5	59856	192.150.187.43	80	10	20	7	7	http
 XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	141.142.228.5	59856	192.150.187.43	80	42	-	7	7	http

testing/btest/Baseline/scripts.policy.frameworks.conn_key.vlan_fivetuple-5/conn.log.cut

@@ -1,5 +1,5 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	id.vlan	id.inner_vlan	orig_pkts	resp_pkts	service
+ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	id.ctx.vlan	id.ctx.inner_vlan	orig_pkts	resp_pkts	service
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	141.142.228.5	59856	192.150.187.43	80	-	-	7	7	http
 XXXXXXXXXX.XXXXXX	C4J4Th3PJpwUYZZ6gc	141.142.228.5	59856	192.150.187.43	80	10	20	7	7	http
 XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	141.142.228.5	59856	192.150.187.43	80	42	-	7	7	http

testing/btest/Baseline/scripts.policy.frameworks.conn_key.vlan_fivetuple-6/conn.log.cut

@@ -1,5 +1,5 @@
 ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	id.vlan	id.inner_vlan	orig_pkts	resp_pkts	service
+ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	id.ctx.vlan	id.ctx.inner_vlan	orig_pkts	resp_pkts	service
 XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	141.142.228.5	59856	192.150.187.43	80	-	-	7	7	http
 XXXXXXXXXX.XXXXXX	C4J4Th3PJpwUYZZ6gc	141.142.228.5	59856	192.150.187.43	80	-	-	7	7	http
 XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	141.142.228.5	59856	192.150.187.43	80	-	-	7	7	http

testing/btest/Baseline/scripts.policy.frameworks.conn_key.vlan_fivetuple_conn_id_ctx_usage-2/out

@@ -0,0 +1,4 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+141.142.228.5, [vlan=42, inner_vlan=<uninitialized>], GET, 1
+141.142.228.5, [vlan=<uninitialized>, inner_vlan=<uninitialized>], GET, 1
+141.142.228.5, [vlan=10, inner_vlan=20], GET, 1

testing/btest/Baseline/scripts.policy.frameworks.conn_key.vlan_fivetuple_conn_id_ctx_usage/out

@@ -0,0 +1,2 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+141.142.228.5, [], GET, 1

testing/btest/Baseline/scripts.policy.frameworks.conn_key.vlan_fivetuple_lookup_connection/.stderr

@@ -0,0 +1 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.

testing/btest/Baseline/scripts.policy.frameworks.conn_key.vlan_fivetuple_lookup_connection/conn.log.cut

@@ -0,0 +1,5 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	id.ctx.vlan	id.ctx.inner_vlan	orig_pkts	resp_pkts	service
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	141.142.228.5	59856	192.150.187.43	80	-	-	7	7	http
+XXXXXXXXXX.XXXXXX	C4J4Th3PJpwUYZZ6gc	141.142.228.5	59856	192.150.187.43	80	10	20	7	7	http
+XXXXXXXXXX.XXXXXX	ClEkJM2Vm5giqnMf4h	141.142.228.5	59856	192.150.187.43	80	42	-	7	7	http

testing/btest/scripts/policy/frameworks/conn_key/vlan_fivetuple.zeek

@@ -4,7 +4,7 @@
 # To create: tcprewrite --enet-vlan=add --enet-vlan-tag 20 --enet-vlan-cfi=1 --enet-vlan-pri=2 -i in.pcap -o out.pcap
 #
 # @TEST-EXEC: zeek -r $TRACES/vlan-collisions.pcap %INPUT
-# @TEST-EXEC: zeek-cut -m ts uid id.orig_h id.orig_p id.resp_h id.resp_p id.vlan id.inner_vlan orig_pkts resp_pkts service <conn.log >conn.log.cut
+# @TEST-EXEC: zeek-cut -m ts uid id.orig_h id.orig_p id.resp_h id.resp_p id.ctx.vlan id.ctx.inner_vlan orig_pkts resp_pkts service <conn.log >conn.log.cut
 # @TEST-EXEC: btest-diff conn.log.cut
 
 # Default operation: Zeek isn't VLAN-aware, a single conn.log entry results.
@@ -27,7 +27,7 @@ redef ConnKey::factory = ConnKey::CONNKEY_VLAN_FIVETUPLE;
 
 # Add an extra field before the VLAN ones, to throw off any fixed-offset code.
 
-redef record conn_id += {
+redef record conn_id_ctx += {
 	foo: int &default=1;
 };
 
@@ -35,11 +35,11 @@ redef record conn_id += {
 
 # @TEST-START-NEXT
 
-# Add the right fields, but in the wrong order. (zeek-cut obscures the difference.)
+# Add the right fields, but in a different order. (zeek-cut obscures the difference.)
 
-redef record conn_id += {
+redef record conn_id_ctx += {
-	inner_vlan: int      &log &optional;
+	inner_vlan: int &log &optional;
-	vlan: int      &log &optional;
+	vlan: int &log &optional;
 };
 
 redef ConnKey::factory = ConnKey::CONNKEY_VLAN_FIVETUPLE;
@@ -48,9 +48,9 @@ redef ConnKey::factory = ConnKey::CONNKEY_VLAN_FIVETUPLE;
 
 # Add the right fields, but with the wrong types.
 
-redef record conn_id += {
+redef record conn_id_ctx += {
-	vlan: string      &log &optional;
+	vlan: string &log &optional;
-	inner_vlan: string      &log &optional;
+	inner_vlan: string &log &optional;
 };
 
 redef ConnKey::factory = ConnKey::CONNKEY_VLAN_FIVETUPLE;

testing/btest/scripts/policy/frameworks/conn_key/vlan_fivetuple_conn_id_ctx_usage.zeek

@@ -0,0 +1,31 @@
+# @TEST-DOC: Demo for using conn_id$ctx in a table to track HTTP request methods per originator IP and their context.
+#
+# The test pcap has 3 overlapping healthy TCP connections, each with different VLAN tagging: none, one VLAN tag, two VLAN tags.
+# To create: tcprewrite --enet-vlan=add --enet-vlan-tag 20 --enet-vlan-cfi=1 --enet-vlan-pri=2 -i in.pcap -o out.pcap
+#
+# @TEST-EXEC: zeek -b -r $TRACES/vlan-collisions.pcap base/protocols/http ./count-http-request-methods.zeek %INPUT >out
+# @TEST-EXEC: btest-diff out
+
+# Default operation: Zeek isn't VLAN-aware, a single conn.log entry results.
+
+# @TEST-START-NEXT
+
+# Switch to VLAN-aware flow tuples: multiple conn.log entries with full
+# information.
+
+@load frameworks/conn_key/vlan_fivetuple
+
+# @TEST-START-FILE count-http-request-methods.zeek
+global http_requests: table[addr, conn_id_ctx, string] of count &default=0;
+
+event http_request(c: connection, method: string, original_URI: string, unescaped_URI: string, version: string)
+	{
+	++http_requests[c$id$orig_h, c$id$ctx, method];
+	}
+
+event zeek_done()
+	{
+	for ( [h, ctx, method], c in http_requests )
+		print h, ctx, method, c;
+	}
+# @TEST-END-FILE

testing/btest/scripts/policy/frameworks/conn_key/vlan_fivetuple_lookup_connection.zeek

@@ -0,0 +1,37 @@
+# @TEST-DOC: Verify connections can be be looked up using lookup_connection() when using vlan aware conn_id's
+#
+# The test pcap has 3 overlapping healthy TCP connections, each with different VLAN tagging: none, one VLAN tag, two VLAN tags.
+# To create: tcprewrite --enet-vlan=add --enet-vlan-tag 20 --enet-vlan-cfi=1 --enet-vlan-pri=2 -i in.pcap -o out.pcap
+#
+# @TEST-EXEC: zeek -r $TRACES/vlan-collisions.pcap %INPUT
+# @TEST-EXEC: zeek-cut -m ts uid id.orig_h id.orig_p id.resp_h id.resp_p id.ctx.vlan id.ctx.inner_vlan orig_pkts resp_pkts service <conn.log >conn.log.cut
+#
+# @TEST-EXEC: btest-diff conn.log.cut
+# @TEST-EXEC: btest-diff .stderr
+
+@load frameworks/conn_key/vlan_fivetuple
+
+event new_connection(c: connection)
+	{
+	local c1 = lookup_connection(c$id);
+	local c2 = lookup_connection(copy(c$id));
+
+	local c3_id = conn_id($orig_h=c$id$orig_h, $orig_p=c$id$orig_p,
+	                      $resp_h=c$id$resp_h, $resp_p=c$id$resp_p,
+	                      $proto=c$id$proto, $ctx=copy(c$id$ctx));
+	local c3 = lookup_connection(c3_id);
+
+	# Ensure all the uids are the same!
+	assert c$uid == c1$uid && c1$uid == c2$uid && c2$uid == c3$uid;
+	}
+
+event new_connection(c: connection)
+	{
+	assert connection_exists(c$id);
+
+	local nx_id = copy(c$id);
+	nx_id$ctx = copy(c$id$ctx);
+	nx_id$ctx$vlan = 1000;
+	nx_id$ctx$inner_vlan = 2000;
+	assert ! connection_exists(nx_id);
+	}