IPBasedAnalyzer: Ensure a connection's ConnVal is updated

One idea for the issue that a connection record's endpoint fields may be stale
when events are raised before the ConnSize analyzer saw the packet and no
analyzer calls GetVal() for the connection afterwards.

While this looks a bit ad hoc, I'm leaning towards a follow-up, extending
GetVal(skip_update_connval=false) API and then updating BinPAC/Spicy to
generate code that passes ``false``. That could avoid a number of unnecessary
UpateConnVal() invocations when calling GetVal() multiple times for the
same connection and instead just do it once when the packet has been
processed.

Closes #4214

src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc

@@ -3,6 +3,7 @@
 #include "zeek/packet_analysis/protocol/ip/IPBasedAnalyzer.h"
 
 #include "zeek/Conn.h"
+#include "zeek/Event.h"
 #include "zeek/RunState.h"
 #include "zeek/Val.h"
 #include "zeek/analyzer/Manager.h"
@@ -41,6 +42,7 @@ bool IPBasedAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pkt
 
     key->Init(*pkt);
 
+    const auto queued_events_before = zeek::event_mgr.Size(); // Used for determining if any events were raised.
     const std::shared_ptr<IP_Hdr>& ip_hdr = pkt->ip_hdr;
     auto src_addr = key->SrcAddr();
     auto src_port = key->SrcPort();
@@ -108,6 +110,24 @@ bool IPBasedAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pkt
     run_state::current_timestamp = 0;
     run_state::current_pkt = nullptr;
 
+    // If any events were queued while processing the packet, call conn->GetVal()
+    // once more here to trigger a recursive UpdateConnVal() call on the analyzer tree.
+    // This is primarily for the ConnSize analyzer that implements UpdateConnVal(),
+    // but does usually not raise any events and therefore does not call GetVal().
+    // The ConnSize data is exposed on the script-level connection record through the
+    // orig and resp endpoint fields, however. This call ensures that these fields
+    // aren't stale events were only raised *before* the ConnSize analyzer.
+    //
+    // This is a bit unfortunate, as the GetVal() call will result in an extra
+    // GetVal() call that might not be needed. Maybe we could extend GetVal() to
+    // skip UpdateConnVal() when called from BinPac or Spicy going forward so that
+    // it is just called once after processing a packet. @awelzel hasn't seen
+    // UpdateConnVal() prominently in flame graphs though, so skipping this for now
+    // as it's not clear it'd actually be worth it.
+    const auto queued_events_after = zeek::event_mgr.Size();
+    if ( queued_events_after > queued_events_before )
+        (void)conn->GetVal();
+
     // If the packet is reassembled, disable packet dumping because the
     // pointer math to dump the data wouldn't work.
     if ( pkt->ip_hdr->Reassembled() )

testing/btest/Baseline/core.conn-size-endpoint-update/out

@@ -0,0 +1,37 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+==== zeek_init, syn.pcap
+new_connection, CHhAvVGS1DHFjwGM9
+  orig, [size=0, state=1, num_pkts=1, num_bytes_ip=64, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+  resp, [size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+connection_SYN_packet, CHhAvVGS1DHFjwGM9, orig
+  orig, [size=0, state=1, num_pkts=1, num_bytes_ip=64, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+  resp, [size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+connection_state_remove, CHhAvVGS1DHFjwGM9
+  orig, [size=0, state=1, num_pkts=1, num_bytes_ip=64, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+  resp, [size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+==== zeek_init, synack.pcap
+new_connection, CHhAvVGS1DHFjwGM9
+  orig, [size=0, state=2, num_pkts=1, num_bytes_ip=60, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+  resp, [size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+connection_SYN_packet, CHhAvVGS1DHFjwGM9, orig
+  orig, [size=0, state=2, num_pkts=1, num_bytes_ip=60, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+  resp, [size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+connection_state_remove, CHhAvVGS1DHFjwGM9
+  orig, [size=0, state=2, num_pkts=1, num_bytes_ip=60, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+  resp, [size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+==== zeek_init, get.trace
+new_connection, CHhAvVGS1DHFjwGM9
+  orig, [size=0, state=1, num_pkts=1, num_bytes_ip=64, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+  resp, [size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+connection_SYN_packet, CHhAvVGS1DHFjwGM9, orig
+  orig, [size=0, state=1, num_pkts=1, num_bytes_ip=64, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+  resp, [size=0, state=0, num_pkts=0, num_bytes_ip=0, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+connection_SYN_packet, CHhAvVGS1DHFjwGM9, resp
+  orig, [size=0, state=4, num_pkts=1, num_bytes_ip=64, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+  resp, [size=0, state=4, num_pkts=1, num_bytes_ip=60, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+connection_established, CHhAvVGS1DHFjwGM9
+  orig, [size=0, state=4, num_pkts=1, num_bytes_ip=64, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+  resp, [size=0, state=4, num_pkts=1, num_bytes_ip=60, flow_label=0, l2_addr=00:10:db:88:d2:ef]
+connection_state_remove, CHhAvVGS1DHFjwGM9
+  orig, [size=136, state=5, num_pkts=7, num_bytes_ip=512, flow_label=0, l2_addr=c8:bc:c8:96:d2:a0]
+  resp, [size=5007, state=5, num_pkts=7, num_bytes_ip=5379, flow_label=0, l2_addr=00:10:db:88:d2:ef]

testing/btest/core/conn-size-endpoint-update.zeek

@@ -0,0 +1,41 @@
+# @TEST-DOC: Ensure that a connection's orig and resp records have up-to-date data
+
+
+# @TEST-EXEC: zeek -b -r $TRACES/tcp/syn.pcap %INPUT >> out
+# @TEST-EXEC: zeek -b -r $TRACES/tcp/synack.pcap %INPUT >> out
+# @TEST-EXEC: zeek -b -r $TRACES/http/get.trace %INPUT >> out
+#
+# @TEST-EXEC: btest-diff out
+
+event zeek_init()
+	{
+	print "==== zeek_init", split_string(packet_source()$path, /\//)[-1];
+	}
+
+event new_connection(c: connection)
+	{
+	print "new_connection", c$uid;
+	print "  orig", c$orig;
+	print "  resp", c$resp;
+	}
+
+event connection_SYN_packet(c: connection, pkt: SYN_packet)
+	{
+	print "connection_SYN_packet", c$uid, pkt$is_orig ? "orig" : "resp";
+	print "  orig", c$orig;
+	print "  resp", c$resp;
+	}
+
+event connection_established(c: connection)
+	{
+	print "connection_established", c$uid;
+	print "  orig", c$orig;
+	print "  resp", c$resp;
+	}
+
+event connection_state_remove(c: connection)
+	{
+	print "connection_state_remove", c$uid;
+	print "  orig", c$orig;
+	print "  resp", c$resp;
+	}