IPBasedAnalyzer/TCPSessionAdapter: Fix TCP reassembly decision for known port analyzers

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
This commit is contained in:
Arne Welzel 2022-10-25 15:42:46 +02:00
parent 5aa7d80e88
commit f3f593c523
4 changed files with 34 additions and 0 deletions

View file

@ -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 )

View file

@ -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

View file

@ -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

View file

@ -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