Fix

src/conntuple/vlan/Builder.cc

@@ -18,13 +18,11 @@ public:
         memset(static_cast<void*>(&key), '\0', sizeof(key));
     }
 
-    void DoInit(const Packet& pkt) override {
+    zeek::session::detail::Key SessionKey() const override {
-        key.vlan = pkt.vlan;
+        return zeek::session::detail::Key(reinterpret_cast<const void*>(&key), sizeof(key),
-        key.inner_vlan = pkt.inner_vlan;
+                                          // XXX: Not sure we need CONNECTION_KEY_TYPE and it adds an extra comparison
-    }
+                                          // that's anyhow true.
-
+                                          session::detail::Key::CONNECTION_KEY_TYPE);
-    zeek::Span<const std::byte> Key() const override {
-        return {reinterpret_cast<const std::byte*>(&key), reinterpret_cast<const std::byte*>(&key) + sizeof(key)};
     }
 
     detail::RawConnTuple& RawTuple() const override { return key.tuple; }
@@ -42,6 +40,13 @@ public:
             conn_id->Assign(6, key.inner_vlan);
     };
 
+protected:
+    void DoInit(const Packet& pkt) override {
+        key.vlan = pkt.vlan;
+        key.inner_vlan = pkt.inner_vlan;
+    }
+
+
 private:
     friend class Builder;
 

src/conntuple/vlan/Builder.h

@@ -9,6 +9,8 @@ class Builder : public conntuple::Builder {
 public:
     virtual zeek::ConnKeyPtr NewConnKey() override;
     virtual zeek::ConnKeyPtr FromVal(const zeek::ValPtr& v) override;
+
+    static zeek::conntuple::BuilderPtr Instantiate() { return std::make_unique<Builder>(); }
 };
 
 } // namespace zeek::plugin::Zeek_Conntuple_VLAN

src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc

@@ -16,6 +16,18 @@
 using namespace zeek;
 using namespace zeek::packet_analysis::IP;
 
+std::optional<std::string> IPBasedConnKey::Error() const {
+    auto& rt = RawTuple();
+    if ( rt.transport == detail::INVALID_CONN_KEY_IP_PROTO )
+        return "invalid connection ID record encountered";
+    if ( rt.transport == UNKNOWN_IP_PROTO )
+        return "invalid connection ID record encountered: the proto field has the \"unknown\" 65535 value. Did you "
+               "forget to set it?";
+
+    return std::nullopt;
+}
+
+
 IPBasedAnalyzer::IPBasedAnalyzer(const char* name, TransportProto proto, uint32_t mask, bool report_unknown_protocols)
     : zeek::packet_analysis::Analyzer(name, report_unknown_protocols), transport(proto), server_port_mask(mask) {}
 

src/packet_analysis/protocol/ip/IPBasedAnalyzer.h

@@ -84,16 +84,7 @@ public:
         ConnKey::Init(pkt);
     }
 
-    std::optional<std::string> Error() const override {
+    std::optional<std::string> Error() const override;
-        auto& rt = RawTuple();
-        if ( rt.transport == detail::INVALID_CONN_KEY_IP_PROTO )
-            return "invalid connection ID record";
-        if ( rt.transport == UNKNOWN_IP_PROTO )
-            return "invalid connection ID record: the proto field has the \"unknown\" 65535 value. Did you forget to "
-                   "set it?";
-
-        return std::nullopt;
-    }
 
     /**
      * Return a modifiable version of the embedded RawConnTuple.
@@ -113,12 +104,15 @@ using IPBasedConnKeyPtr = std::unique_ptr<IPBasedConnKey>;
 class IPConnKey : public IPBasedConnKey {
 public:
     IPConnKey() {
-        // Fill holes as we use the full tuple as a Key!
+        // Fill holes as we use the full tuple for the Key!
         memset(static_cast<void*>(&key), '\0', sizeof(key));
     }
 
-    zeek::Span<const std::byte> Key() const override {
+    zeek::session::detail::Key SessionKey() const override {
-        return {reinterpret_cast<const std::byte*>(&key), reinterpret_cast<const std::byte*>(&key) + sizeof(key)};
+        return zeek::session::detail::Key(reinterpret_cast<const void*>(&key), sizeof(key),
+                                          // XXX: Not sure we need CONNECTION_KEY_TYPE and it adds an extra comparison
+                                          // that's anyhow true.
+                                          session::detail::Key::CONNECTION_KEY_TYPE);
     }
 
     detail::RawConnTuple& RawTuple() const override { return key.tuple; }

src/session/Key.h

@@ -8,7 +8,6 @@
 
 #include "zeek/Hash.h"
 #include "zeek/IntrusivePtr.h"
-#include "zeek/Span.h"
 
 namespace zeek::session::detail {
 
@@ -93,54 +92,56 @@ using RecordValPtr = zeek::IntrusivePtr<RecordVal>;
 
 /**
  * Abstract ConnKey - not IP specific.
- *
- * Should move this elsewhere to avoid circular dependencies. Conn really is IP specific,
- * while ConnKey is not.
  */
 class ConnKey {
 public:
     virtual ~ConnKey() {}
 
-    /**
-     * ConnKeys created from Vals may be invalid, Error() can be used to determine validity.
-     */
-    virtual std::optional<std::string> Error() const = 0;
-
     /**
      * Initialization of this key with the current packet.
      */
     void Init(const Packet& pkt) { DoInit(pkt); }
 
     /**
-     * Hook method for further initialization.
+     * Given the ConnKey, fill a script layer record with
+     * its custom information. E.g. VLAN.
      *
-     * This may also take information from the context.
+     * Empty implementation by default.
-     *
-     * @param p The current packet
-     */
-    virtual void DoInit(const Packet& pkt) {};
-
-    /**
-     * Using this ConnKey and its contents, populate a conn_id or other script layer record.
      */
     virtual void FillConnIdVal(RecordValPtr& conn_id) {};
 
     /**
-     * They Key over which to compute a hash or use for comparison with other keys.
+     * Return a non-owning session::detail::Key instance.
      *
-     * The returned Span is only valid as long as this ConnKey instance is valid.
+     * Callers that need more than a View should copy
+     * the data. Callers are not supposed to hold on
+     * to the Key for longer than the ConnKey instance
+     * exists. Think string_view or span!
+     *
+     * @return A zeek::session::detail::Key
      */
-    virtual zeek::Span<const std::byte> Key() const = 0;
+    virtual zeek::session::detail::Key SessionKey() const = 0;
 
     /**
-     * View over key data as returned by Key() as session::detail::Key instance.
+     * Get the error state of a ConnKey, if any.
      *
-     * Mostly for plumbing the session/Manager.h code.
+     * Instances of a ConnKey created from zeek::Val instances
+     * via Builder::FromVal() may not be valid. Calling Error()
+     * can be used to gather a description of the encountered
+     * error.
      */
-    zeek::session::detail::Key SessionKey() const {
+    virtual std::optional<std::string> Error() const = 0;
-        auto span = Key();
+
-        return zeek::session::detail::Key(span.data(), span.size(), session::detail::Key::CONNECTION_KEY_TYPE);
+protected:
-    }
+    /**
+     * Hook method for custom initialization.
+     *
+     * This may also take information from the global context rather
+     * than just the packet.
+     *
+     * @param p The current packet
+     */
+    virtual void DoInit(const Packet& pkt) {};
 };
 
 using ConnKeyPtr = std::unique_ptr<ConnKey>;