Merge remote-tracking branch 'origin/topic/awelzel/4605-conn-id-context'

* origin/topic/awelzel/4605-conn-id-context:
  NEWS: Adapt for conn_id$ctx introduction
  conn_key/fivetuple: Drop support for non conn_id records
  Conn: Move conn_id init and flip to IPBasedConnKey
  IPBasedConnKey: Add GetTransportProto() helper
  input/Manager: Ignore empty record types
  external: Bump commit hashes for external suites
  ip/vlan_fivetuple: Populate nested conn_id_context, not conn_id
  ConnKey: Extend DoPopulateConnIdVal() with ctx
  btest: Update tests and baselines after adding ctx to conn_id
  init-bare: Add conn_id_ctx to conn_id

src/packet_analysis/protocol/ip/conn_key/IPBasedConnKey.cc

@@ -3,6 +3,7 @@
 #include "zeek/packet_analysis/protocol/ip/conn_key/IPBasedConnKey.h"
 
 #include "zeek/Conn.h"
+#include "zeek/Desc.h"
 
 using namespace zeek;
 using namespace zeek::packet_analysis::IP;
@@ -28,3 +29,26 @@ void IPBasedConnKey::InitTuple(const IPAddr& src_addr, uint32_t src_port, const
 
     tuple.proto = proto;
 }
+
+void IPBasedConnKey::DoPopulateConnIdVal(RecordVal& conn_id, RecordVal& ctx) {
+    if ( conn_id.GetType() != id::conn_id )
+        zeek::reporter->InternalError("unexpected conn_id type %s", obj_desc_short(conn_id.GetType()).c_str());
+
+    conn_id.Assign(0, make_intrusive<AddrVal>(SrcAddr()));
+    conn_id.Assign(1, val_mgr->Port(ntohs(SrcPort()), GetTransportProto()));
+    conn_id.Assign(2, make_intrusive<AddrVal>(DstAddr()));
+    conn_id.Assign(3, val_mgr->Port(ntohs(DstPort()), GetTransportProto()));
+    conn_id.Assign(4, Proto());
+}
+
+void IPBasedConnKey::DoFlipRoles(RecordVal& conn_id, RecordVal& ctx) {
+    if ( conn_id.GetType() != id::conn_id )
+        zeek::reporter->InternalError("unexpected conn_id type %s", obj_desc_short(conn_id.GetType()).c_str());
+
+    const auto& tmp_addr = conn_id.GetField<zeek::AddrVal>(0);
+    const auto& tmp_port = conn_id.GetField<zeek::PortVal>(1);
+    conn_id.Assign(0, conn_id.GetField<zeek::AddrVal>(2));
+    conn_id.Assign(1, conn_id.GetField<zeek::PortVal>(3));
+    conn_id.Assign(2, tmp_addr);
+    conn_id.Assign(3, tmp_port);
+}

src/packet_analysis/protocol/ip/conn_key/IPBasedConnKey.h

@@ -7,6 +7,7 @@
 #include "zeek/Conn.h"
 #include "zeek/ConnKey.h"
 #include "zeek/IPAddr.h"
+#include "zeek/net_util.h"
 
 namespace zeek {
 
@@ -63,6 +64,41 @@ public:
      */
     uint16_t Proto() const { return PackedTuple().proto; }
 
+    /**
+     * @return The TransportProto value for this key's IP proto.
+     */
+    TransportProto GetTransportProto() const {
+        switch ( Proto() ) {
+            case IPPROTO_TCP: return TRANSPORT_TCP;
+            case IPPROTO_UDP: return TRANSPORT_UDP;
+            case IPPROTO_ICMP:
+            case IPPROTO_ICMPV6: return TRANSPORT_ICMP;
+            default: return TRANSPORT_UNKNOWN;
+        }
+    }
+
+    /**
+     * Flips the role of source and destination fields in the packed tuple.
+     */
+    void FlipRoles() { flipped = ! flipped; }
+
+    /**
+     * Flips the role of originator and responder.
+     *
+     * This overload will also flip fields of the conn_id and ctx record
+     * values. The DoFlipRoles hook can be overridden to customize this process,
+     * but that's usually not needed. The default implementation will flip
+     * the orig_h/resp_h and orig_p/resp_p pairs.
+     *
+     * @param conn_id The conn_id record to populate.
+     * @param ctx The conn_id's ctx record to populate.
+     */
+    void FlipRoles(RecordVal& conn_id, RecordVal& ctx) {
+        FlipRoles();
+
+        DoFlipRoles(conn_id, ctx);
+    }
+
     /**
      * Return a modifiable reference to the embedded PackedConnTuple.
      *
@@ -84,6 +120,36 @@ public:
     virtual const detail::PackedConnTuple& PackedTuple() const = 0;
 
 protected:
+    /**
+     * Overridden from ConnKey.
+     *
+     * This implementation sets orig_h, resp_h, orig_p, resp_p and proto
+     * on the \a conn_id record value and leaves \a ctx untouched.
+     *
+     * When implementing subclasses of IPBasedConnKey, redef the script-layer
+     * record type conn_id_ctx with the fields specific to your ConnKey implementation,
+     * e.g. VLAN IDs. Then override this method to populate the fields of \a ctx based
+     * on data stored in your custom ConnKey instance. Ensure to call
+     * IPBasedConnKey::DoPopulateConnIdVal() to populate the common \a conn_id fields, too.
+     *
+     * @param conn_id The conn_id record to populate.
+     * @param ctx The conn_id's ctx record to populate.
+     */
+    void DoPopulateConnIdVal(RecordVal& conn_id, RecordVal& ctx) override;
+
+    /**
+     * Hook for FlipRoles.
+     *
+     * The default implementation will flip the orig_h/resp_h and orig_p/resp_p pairs.
+     *
+     * @param conn_id The conn_id record to flip.
+     * @param ctx The conn_id's ctx record to flip.
+     */
+    virtual void DoFlipRoles(RecordVal& conn_id, RecordVal& ctx);
+
+    /**
+     * Flag for tracking if src and dst addresses provided to InitTuple() were flipped.
+     */
     bool flipped = false;
 };
 

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

@@ -2,6 +2,7 @@
 
 #include "zeek/packet_analysis/protocol/ip/conn_key/fivetuple/Factory.h"
 
+#include "zeek/Desc.h"
 #include "zeek/IP.h"
 #include "zeek/Val.h"
 #include "zeek/packet_analysis/protocol/ip/conn_key/IPBasedConnKey.h"
@@ -12,48 +13,26 @@ namespace zeek::conn_key::fivetuple {
 zeek::ConnKeyPtr Factory::DoNewConnKey() const { return std::make_unique<zeek::IPConnKey>(); }
 
 zeek::expected<zeek::ConnKeyPtr, std::string> Factory::DoConnKeyFromVal(const zeek::Val& v) const {
-    static auto unexpected_conn_id = zeek::unexpected<std::string>{"invalid connection ID record encountered"};
     auto ck = NewConnKey();
     auto* ick = static_cast<zeek::IPBasedConnKey*>(ck.get());
     auto& pt = ick->PackedTuple();
-    const auto& vt = v.GetType();
 
-    if ( ! IsRecord(vt->Tag()) )
-        return unexpected_conn_id;
+    if ( v.GetType() != id::conn_id )
+        return zeek::unexpected<std::string>{
+            util::fmt("expected conn_id, got %s", obj_desc_short(v.GetType()).c_str())};
 
-    auto* vr = vt->AsRecordType();
     auto vl = v.AsRecordVal();
 
     // Indices into conn_id's record field value list:
-    int orig_h = 0;
-    int orig_p = 1;
-    int resp_h = 2;
-    int resp_p = 3;
-    int proto = 4;
-
-    if ( vr != id::conn_id ) {
-        // While it's not a conn_id, it may have equivalent fields.
-        orig_h = vr->FieldOffset("orig_h");
-        resp_h = vr->FieldOffset("resp_h");
-        orig_p = vr->FieldOffset("orig_p");
-        resp_p = vr->FieldOffset("resp_p");
-        proto = vr->FieldOffset("proto");
-
-        // clang-format off
-        if ( orig_h < 0 || vr->GetFieldType(orig_h)->Tag() != TYPE_ADDR ||
-	     resp_h < 0 || vr->GetFieldType(resp_h)->Tag() != TYPE_ADDR ||
-	     orig_p < 0 || vr->GetFieldType(orig_p)->Tag() != TYPE_PORT ||
-	     resp_p < 0 || vr->GetFieldType(resp_p)->Tag() != TYPE_PORT ||
-	     proto < 0  || vr->GetFieldType(proto)->Tag() != TYPE_COUNT ) {
-            return unexpected_conn_id;
-        }
-        // clang-format on
-    }
-
+    constexpr int orig_h = 0;
+    constexpr int orig_p = 1;
+    constexpr int resp_h = 2;
+    constexpr int resp_p = 3;
+    constexpr int proto = 4;
+    constexpr int ctx = 5;
     if ( ! vl->HasField(orig_h) || ! vl->HasField(resp_h) || ! vl->HasField(orig_p) || ! vl->HasField(resp_p) ||
-         ! vl->HasField(proto) ) {
-        return unexpected_conn_id;
-    }
+         ! vl->HasField(proto) || ! vl->HasField(ctx) )
+        return zeek::unexpected<std::string>{"invalid connection ID record encountered"};
 
     const IPAddr& orig_addr = vl->GetFieldAs<AddrVal>(orig_h);
     const IPAddr& resp_addr = vl->GetFieldAs<AddrVal>(resp_h);
@@ -71,8 +50,6 @@ zeek::expected<zeek::ConnKeyPtr, std::string> Factory::DoConnKeyFromVal(const ze
 
     ick->InitTuple(orig_addr, htons(orig_portv->Port()), resp_addr, htons(resp_portv->Port()), proto16_t);
 
-    // Asserting here on the absence of errors can fail btests.
-
     return ck;
 }
 

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"
@@ -37,33 +38,34 @@ protected:
         return {reinterpret_cast<const void*>(&key), sizeof(key), session::detail::Key::CONNECTION_KEY_TYPE};
     }
 
-    void DoPopulateConnIdVal(RecordVal& conn_id) override {
-        if ( conn_id.NumFields() <= 5 )
-            return;
+    void DoPopulateConnIdVal(RecordVal& conn_id, RecordVal& ctx) override {
+        IPBasedConnKey::DoPopulateConnIdVal(conn_id, ctx);
 
         // 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;
         static int inner_vlan_offset = -2;
 
         if ( vlan_offset == -2 && inner_vlan_offset == -2 ) {
-            vlan_offset = id::conn_id->FieldOffset("vlan");
-            if ( vlan_offset < 0 || id::conn_id->GetFieldType(vlan_offset)->Tag() != TYPE_INT )
+            vlan_offset = conn_id_ctx->FieldOffset("vlan");
+            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");
-            if ( inner_vlan_offset < 0 || id::conn_id->GetFieldType(inner_vlan_offset)->Tag() != TYPE_INT )
+            inner_vlan_offset = conn_id_ctx->FieldOffset("inner_vlan");
+            if ( inner_vlan_offset < 0 || conn_id_ctx->GetFieldType(inner_vlan_offset)->Tag() != TYPE_INT )
                 inner_vlan_offset = -1;
         }
 
@@ -79,7 +81,6 @@ protected:
 private:
     friend class Factory;
 
-    // Key bytes:
     struct {
         struct detail::PackedConnTuple tuple;
         // Add 802.1Q vlan tags to connection tuples. The tag representation
@@ -99,31 +100,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;
-    int inner_vlan_offset;
-    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");
-    }
+    auto [vlan_offset, inner_vlan_offset] = k->GetConnCtxFieldOffsets();
 
     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 )
-        return zeek::unexpected<std::string>{"vlan or inner_vlan field not of type int"};
+    if ( ctx->HasField(vlan_offset) )
+        k->key.vlan = ctx->GetFieldAs<zeek::IntVal>(vlan_offset);
 
-    if ( vl->HasField(vlan_offset) )
-        k->key.vlan = vl->GetFieldAs<zeek::IntVal>(vlan_offset);
-
-    if ( vl->HasField(inner_vlan_offset) )
-        k->key.inner_vlan = vl->GetFieldAs<zeek::IntVal>(inner_vlan_offset);
+    if ( ctx->HasField(inner_vlan_offset) )
+        k->key.inner_vlan = ctx->GetFieldAs<zeek::IntVal>(inner_vlan_offset);
 
     return ck;
 }