Conn: In-place val flip and connection_flipped()

Avoids loosing state on a connection value when a connection is flipped.

Fixes up the NTP baseline as well where this was visible: analyzer_confirmation_info()
was raised for a connection value which was immediately forgotten due to
the subsequent connection flipping.

Closed #3028

NEWS

@@ -38,6 +38,13 @@ New Functionality
 Changed Functionality
 ---------------------
 
+- A connection's value is now updated in-place when its directionality is
+  flipped due to Zeek's heuristics (for example, SYN/SYN-ACK reversal or
+  protocol specific approaches).
+  Previously, a connection's value was discarded when flipped, including any
+  values set in a ``new_connection()`` handler. A new ``connection_flipped()``
+  event is added to allow updating custom state in script-land.
+
 Removed Functionality
 ---------------------
 

src/Conn.cc

@@ -217,6 +217,29 @@ void Connection::HistoryThresholdEvent(EventHandlerPtr e, bool is_orig, uint32_t
 	EnqueueEvent(e, nullptr, GetVal(), val_mgr->Bool(is_orig), val_mgr->Count(threshold));
 	}
 
+namespace
+	{
+// Flip everything that needs to be flipped in the connection
+// record that is known on this level. This needs to align
+// with GetVal() and connection's layout in init-bare.
+void flip_conn_val(const RecordValPtr& conn_val)
+	{
+	// Flip the the conn_id (c$id).
+	const auto& id_val = conn_val->GetField<zeek::RecordVal>(0);
+	const auto& tmp_addr = id_val->GetField<zeek::AddrVal>(0);
+	const auto& tmp_port = id_val->GetField<zeek::PortVal>(1);
+	id_val->Assign(0, id_val->GetField<zeek::AddrVal>(2));
+	id_val->Assign(1, id_val->GetField<zeek::PortVal>(3));
+	id_val->Assign(2, tmp_addr);
+	id_val->Assign(3, tmp_port);
+
+	// Flip the endpoints within connection.
+	const auto& tmp_endp = conn_val->GetField<zeek::RecordVal>(1);
+	conn_val->Assign(1, conn_val->GetField(2));
+	conn_val->Assign(2, tmp_endp);
+	}
+	}
+
 const RecordValPtr& Connection::GetVal()
 	{
 	if ( ! conn_val )
@@ -358,7 +381,8 @@ void Connection::FlipRoles()
 	resp_flow_label = orig_flow_label;
 	orig_flow_label = tmp_flow;
 
-	conn_val = nullptr;
+	if ( conn_val )
+		flip_conn_val(conn_val);
 
 	if ( adapter )
 		adapter->FlipRoles();
@@ -366,6 +390,9 @@ void Connection::FlipRoles()
 	analyzer_mgr->ApplyScheduledAnalyzers(this);
 
 	AddHistory('^');
+
+	if ( connection_flipped )
+		EnqueueEvent(connection_flipped, nullptr, GetVal());
 	}
 
 void Connection::Describe(ODesc* d) const

src/event.bif

@@ -194,6 +194,17 @@ event connection_reused%(c: connection%);
 ##    new_connection new_connection_contents partial_connection
 event connection_status_update%(c: connection%);
 
+## Generated for a connection when the direction was flipped by Zeek's
+## heuristics and originator and responder roles were reversed. If state is
+## kept on a connection record for originator and responder, this event
+## can be used to update or reset such state. The ``orig`` and ``resp`` fields
+## as well as the contents of the ``id`` field reflect the post-flip state.
+##
+## c: The connection.
+##
+## .. zeek:see:: connection_established new_connection
+event connection_flipped%(c: connection%);
+
 ## Generated for a connection over IPv6 when one direction has changed
 ## the flow label that it's using.
 ##

testing/btest/Baseline/core.connection_flipped/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.
+1362692526.939084, new_connection, [orig_h=192.150.187.43, orig_p=80/tcp, resp_h=141.142.228.5, resp_p=59856/tcp, extra_id=42], H, 1362692526.939084
+1362692526.939344, connection_flipped, [orig_h=141.142.228.5, orig_p=59856/tcp, resp_h=192.150.187.43, resp_p=80/tcp, extra_id=42], Hs^, 1362692526.939084
+1362692527.080972, connection_state_remove, [orig_h=141.142.228.5, orig_p=59856/tcp, resp_h=192.150.187.43, resp_p=80/tcp, extra_id=42], Hs^ADadFf, 1362692526.939084

testing/btest/Baseline/scripts.base.protocols.ntp.misordered-ntp/conn.log

@@ -7,5 +7,5 @@
 #open XXXX-XX-XX-XX-XX-XX
 #fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	proto	service	duration	orig_bytes	resp_bytes	conn_state	local_orig	local_resp	missed_bytes	history	orig_pkts	orig_ip_bytes	resp_pkts	resp_ip_bytes	tunnel_parents
 #types	time	string	addr	port	addr	port	enum	string	interval	count	count	string	bool	bool	count	string	count	count	count	count	set[string]
-XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.95	123	17.253.4.253	123	udp	-	0.959285	96	0	S0	T	F	0	D^	2	152	0	0	-
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.95	123	17.253.4.253	123	udp	ntp	0.959285	96	0	S0	T	F	0	D^	2	152	0	0	-
 #close XXXX-XX-XX-XX-XX-XX

testing/btest/core/connection_flipped.zeek

@@ -0,0 +1,29 @@
+# @TEST-DOC: A connection flip does not reset the ConnVal. Regression test for #3028.
+
+# @TEST-EXEC: zeek -b -r $TRACES/tcp/handshake-reorder.trace %INPUT >out
+# @TEST-EXEC: TEST_DIFF_CANONIFIER= btest-diff out
+
+redef record conn_id += {
+	extra_id: count &optional;
+};
+
+redef record connection += {
+	my_timestamp: time &optional;
+};
+
+event new_connection(c: connection)
+	{
+	c$id$extra_id = 42;
+	c$my_timestamp = network_time();
+	print network_time(), "new_connection", c$id, c$history, c$my_timestamp;
+	}
+
+event connection_flipped(c: connection)
+	{
+	print network_time(), "connection_flipped", c$id, c$history, c$my_timestamp;
+	}
+
+event connection_state_remove(c: connection)
+	{
+	print network_time(), "connection_state_remove", c$id, c$history, c$my_timestamp;
+	}