Merge remote-tracking branch 'origin/topic/awelzel/2998-ntp-flip-roles'

* origin/topic/awelzel/2998-ntp-flip-roles:
  NTP: Detect out-of-order packets

CHANGES

@@ -1,3 +1,13 @@
+6.0.0-dev.505 | 2023-05-04 20:08:33 +0200
+
+  * GH-2998: NTP: Detect out-of-order packets (Arne Welzel, Corelight)
+
+    The NTP mode provides us with the identity of the endpoints. For the
+    simple CLIENT / SERVER modes, flip the connection if we detect
+    orig/resp disagreeing with what the message says. This mainly
+    results in the history getting a ^ and the ntp.log / conn.log
+    showing the corrected endpoints.
+
 6.0.0-dev.503 | 2023-05-04 10:56:33 -0700
 
   * Include compiler in --show-config output (Dominik Charousset, Corelight)

NEWS

@@ -212,6 +212,11 @@ New Functionality
 - The ``ip4_hdr`` record was extended by ``DF``, ``MF``, ``offset`` and ``sum``
   to aid packet-level analysis use-cases.
 
+- The NTP analyzer now recognizes when client and server mode messages disagree
+  with the notion of "originator" and "responder" and flips the connection. This
+  can happen in packet loss or packet re-ordering scenarios. Such connections will
+  have a ``^`` added to their history.
+
 Changed Functionality
 ---------------------
 

VERSION

@@ -1 +1 @@
-6.0.0-dev.503
+6.0.0-dev.505

src/analyzer/protocol/ntp/ntp-analyzer.pac

@@ -130,11 +130,25 @@
 
 refine flow NTP_Flow += {
 
+	%member{
+		bool flipped_;
+	%}
+
+	%init{
+		flipped_ = false;
+	%}
 
 	function proc_ntp_message(msg: NTP_PDU): bool
 		%{
 		connection()->zeek_analyzer()->AnalyzerConfirmation();
 
+		// Flip roles for SERVER mode message from orig or a CLIENT mode message from resp.
+		if ( ((${msg.mode} == SERVER && is_orig()) || (${msg.mode} == CLIENT && ! is_orig())) && ! flipped_ )
+			{
+			connection()->zeek_analyzer()->Conn()->FlipRoles();
+			flipped_ = true;
+			}
+
 		if ( ! ntp_message )
 			return false;
 

src/analyzer/protocol/ntp/ntp-protocol.pac

@@ -1,5 +1,14 @@
 # This is the common part in the header format.
 # See RFC 5905 for details
+enum NTP_Mode {
+	SYMMETRIC_ACTIVE = 1,
+	SYMMETRIC_PASSIVE = 2,
+	CLIENT = 3,
+	SERVER = 4,
+	BROADCAST_SERVER = 5,
+	BROADCAST_CLIENT = 6,
+};
+
 type NTP_PDU(is_orig: bool) = record {
 	# The first byte of the NTP header contains the leap indicator,
 	# the version and the mode

testing/btest/Baseline/scripts.base.protocols.ntp.misordered-ntp/.stdout

@@ -0,0 +1,3 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+ntp_message 192.168.1.95 -> 17.253.4.253:123 (D^)
+ntp_message 192.168.1.95 -> 17.253.4.253:123 (D^)

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

@@ -0,0 +1,11 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	conn
+#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	-
+#close XXXX-XX-XX-XX-XX-XX

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

@@ -0,0 +1,12 @@
+### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
+#separator \x09
+#set_separator	,
+#empty_field	(empty)
+#unset_field	-
+#path	ntp
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	version	mode	stratum	poll	precision	root_delay	root_disp	ref_id	ref_time	org_time	rec_time	xmt_time	num_exts
+#types	time	string	addr	port	addr	port	count	count	count	interval	interval	interval	interval	string	time	time	time	time	count
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.95	123	17.253.4.253	123	4	4	1	256.000000	0.000001	0.000000	0.000992	GPSs	XXXXXXXXXX.XXXXXX	XXXXXXXXXX.XXXXXX	XXXXXXXXXX.XXXXXX	XXXXXXXXXX.XXXXXX	0
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	192.168.1.95	123	17.253.4.253	123	4	3	0	256.000000	1.000000	0.000000	0.000000	\x00\x00\x00\x00	0.000000	0.000000	0.000000	XXXXXXXXXX.XXXXXX	0
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/scripts/base/protocols/ntp/misordered-ntp.test

@@ -0,0 +1,12 @@
+# @TEST-EXEC: zeek -b -C -r $TRACES/ntp/misordered-ntp.pcap %INPUT
+# @TEST-EXEC: btest-diff ntp.log
+# @TEST-EXEC: btest-diff conn.log
+# @TEST-EXEC: btest-diff .stdout
+
+@load base/protocols/conn
+@load base/protocols/ntp
+
+event ntp_message(c: connection, is_orig: bool, msg: NTP::Message)
+	{
+	print fmt("ntp_message %s -> %s:%d (%s)", c$id$orig_h, c$id$resp_h, c$id$resp_p, c$history);
+	}