Merge remote-tracking branch 'origin/topic/awelzel/no-reassembly-for-…

…known-ports'

* origin/topic/awelzel/no-reassembly-for-known-ports:
  IPBasedAnalyzer/TCPSessionAdapter: Fix TCP reassembly decision for known port analyzers

CHANGES

@@ -1,3 +1,24 @@
+5.2.0-dev.167 | 2022-10-31 14:57:21 -0700
+
+  * IPBasedAnalyzer/TCPSessionAdapter: Fix TCP reassembly decision for known port analyzers (Arne Welzel, Corelight)
+
+    This seems to be an age-old bug. Reported by mchen on discourse [1].
+
+    The TCPSessionAdapter decides in AddExtraAnalyzers() whether to enable
+    reassembly or not. When dpd_reassemble_first_packets is F, this boils down to
+    ! GetChildren().empty(). The intention being that if any analyzers have been
+    added to the connection based on known ports, reassembly is to be enabled.
+
+    However, GetChildren() does not take into account new_children and so
+    ! GetChildren().empty() is always false here and reassembly solely
+    based on dpd_reassemble_first_packets=F (or the tcp_content... options).
+    Ouch.
+
+    Call AppendNewChildren() before AddExtraAnalyzers() as a fix. Without this,
+    the new test does not produce an http.log and service "http" isn't in conn.log.
+
+    [1] https://community.zeek.org/t/how-to-activate-an-application-layer-analyzer-when-signature-dpd-reassemble-first-packets-is-off/6763
+
 5.2.0-dev.162 | 2022-10-28 16:54:23 -0700
 
   * GH-2425: cat_sep: Make fully vararg and do explicit runtime type checks (Arne Welzel, Corelight)

VERSION

@@ -1 +1 @@
-5.2.0-dev.162
+5.2.0-dev.167

src/packet_analysis/protocol/ip/IPBasedAnalyzer.cc

@@ -216,6 +216,9 @@ void IPBasedAnalyzer::BuildSessionAnalyzerTree(Connection* conn)
 			}
 		}
 
+	// Make analyzers added above through known ports visible via GetChildren()
+	root->AppendNewChildren();
+
 	root->AddExtraAnalyzers(conn);
 
 	if ( pia )

testing/btest/Baseline/core.tcp.reassembly-known-ports/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	141.142.228.5	59856	192.150.187.43	80	tcp	http	0.211484	136	5007	SF	-	-	0	ShADadFf	7	512	7	5379	-
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/Baseline/core.tcp.reassembly-known-ports/http.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	http
+#open XXXX-XX-XX-XX-XX-XX
+#fields	ts	uid	id.orig_h	id.orig_p	id.resp_h	id.resp_p	trans_depth	method	host	uri	referrer	version	user_agent	origin	request_body_len	response_body_len	status_code	status_msg	info_code	info_msg	tags	username	password	proxied	orig_fuids	orig_filenames	orig_mime_types	resp_fuids	resp_filenames	resp_mime_types
+#types	time	string	addr	port	addr	port	count	string	string	string	string	string	string	string	count	count	count	string	count	string	set[enum]	string	string	set[string]	vector[string]	vector[string]	vector[string]	vector[string]	vector[string]	vector[string]
+XXXXXXXXXX.XXXXXX	CHhAvVGS1DHFjwGM9	141.142.228.5	59856	192.150.187.43	80	1	GET	bro.org	/download/CHANGES.bro-aux.txt	-	1.1	Wget/1.14 (darwin12.2.0)	-	0	4705	200	OK	-	-	(empty)	-	-	-	-	-	-	FMnxxt3xjVcWNS2141	-	text/plain
+#close XXXX-XX-XX-XX-XX-XX

testing/btest/core/tcp/reassembly-known-ports.zeek

@@ -0,0 +1,9 @@
+# @TEST-DOC: Set dpd_reassemble_first_packets=F, but expect reassembly to be enabled and the HTTP analyzer to work due to being registered for port 80.
+# @TEST-EXEC: zeek -b -r $TRACES/http/get.trace %INPUT
+# @TEST-EXEC: btest-diff conn.log
+# @TEST-EXEC: btest-diff http.log
+
+redef dpd_reassemble_first_packets = F;
+
+@load base/protocols/conn
+@load base/protocols/http