packet_analysis: Do not raise analyzer_confirmation per-packet for tunnels

There's a logic error in the packet analyzer's AnalyzerConfirmation()
method that causes analyzer_confirmation() events to be raised for every
packet rather than stopping after the first confirmation which appears to
have been the intention. This affects, for example, VXLAN and Geneve tunnels.

The optional arg_tag parameter was used for short-circuit'ing, but the return
value of GetAnalyzerTag() used for setting the session state causing the
disconnect.

In scenarios where Zeek receives purely tunneled monitoring traffic, this may
result in a non-negligible performance impact.

Somewhat related, ensure the session state is set to violated before
short-circuiting if no analyzer_violations are installed.

Suggesting this as a 5.0.3 candidate.
This commit is contained in:
Arne Welzel 2022-09-26 17:02:59 +02:00
parent 19ba30d77a
commit 3a320fc6b6
6 changed files with 57 additions and 8 deletions

View file

@ -168,26 +168,30 @@ void Analyzer::Weird(const char* name, Packet* packet, const char* addl) const
void Analyzer::AnalyzerConfirmation(session::Session* session, zeek::Tag arg_tag) void Analyzer::AnalyzerConfirmation(session::Session* session, zeek::Tag arg_tag)
{ {
if ( session->AnalyzerState(arg_tag) == session::AnalyzerConfirmationState::CONFIRMED ) const auto& effective_tag = arg_tag ? arg_tag : GetAnalyzerTag();
if ( session->AnalyzerState(effective_tag) == session::AnalyzerConfirmationState::CONFIRMED )
return; return;
session->SetAnalyzerState(GetAnalyzerTag(), session::AnalyzerConfirmationState::CONFIRMED); session->SetAnalyzerState(effective_tag, session::AnalyzerConfirmationState::CONFIRMED);
if ( ! analyzer_confirmation ) if ( ! analyzer_confirmation )
return; return;
const auto& tval = arg_tag ? arg_tag.AsVal() : tag.AsVal(); event_mgr.Enqueue(analyzer_confirmation, session->GetVal(), effective_tag.AsVal(),
event_mgr.Enqueue(analyzer_confirmation, session->GetVal(), tval, val_mgr->Count(0)); val_mgr->Count(0));
} }
void Analyzer::AnalyzerViolation(const char* reason, session::Session* session, const char* data, void Analyzer::AnalyzerViolation(const char* reason, session::Session* session, const char* data,
int len, zeek::Tag arg_tag) int len, zeek::Tag arg_tag)
{ {
const auto& effective_tag = arg_tag ? arg_tag : GetAnalyzerTag();
session->SetAnalyzerState(effective_tag, session::AnalyzerConfirmationState::VIOLATED);
if ( ! analyzer_violation ) if ( ! analyzer_violation )
return; return;
session->SetAnalyzerState(GetAnalyzerTag(), session::AnalyzerConfirmationState::VIOLATED);
StringValPtr r; StringValPtr r;
if ( data && len ) if ( data && len )
@ -200,8 +204,8 @@ void Analyzer::AnalyzerViolation(const char* reason, session::Session* session,
else else
r = make_intrusive<StringVal>(reason); r = make_intrusive<StringVal>(reason);
const auto& tval = arg_tag ? arg_tag.AsVal() : tag.AsVal(); event_mgr.Enqueue(analyzer_violation, session->GetVal(), effective_tag.AsVal(),
event_mgr.Enqueue(analyzer_violation, session->GetVal(), tval, val_mgr->Count(0), std::move(r)); val_mgr->Count(0), std::move(r));
} }
} // namespace zeek::packet_analysis } // namespace zeek::packet_analysis

View file

@ -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 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 10.1.200.131 50000 10.1.1.172 4789 udp vxlan 0.627090 10203 0 S0 - - 0 D 12 10539 0 0 -
XXXXXXXXXX.XXXXXX ClEkJM2Vm5giqnMf4h 172.16.11.201 40354 54.86.237.188 80 tcp http 0.627052 87 9212 SF - - 0 ShADadFf 7 459 5 9480 CHhAvVGS1DHFjwGM9
#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 ClEkJM2Vm5giqnMf4h 172.16.11.201 40354 54.86.237.188 80 1 GET eu.httpbin.org /image/svg - 1.1 curl/7.76.1 - 0 8984 200 OK - - (empty) - - - - - - FTKnz016WapPYpNaxl - text/plain
#close XXXX-XX-XX-XX-XX-XX

View file

@ -0,0 +1,3 @@
### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
analyzer_confirmation, CHhAvVGS1DHFjwGM9, [orig_h=10.1.200.131, orig_p=50000/udp, resp_h=10.1.1.172, resp_p=4789/udp], 0
analyzer_confirmation, ClEkJM2Vm5giqnMf4h, [orig_h=172.16.11.201, orig_p=40354/tcp, resp_h=54.86.237.188, resp_p=80/tcp], 6

View file

@ -0,0 +1,19 @@
# @TEST-DOC: Check how many analyzer_confirmation events a vxlan-encapsulated HTTP transaction triggers. Should be 2.
# @TEST-EXEC: zeek -b -r $TRACES/tunnels/vxlan-encapsulated-http.pcap %INPUT >out
# @TEST-EXEC: btest-diff out
# @TEST-EXEC: btest-diff conn.log
# @TEST-EXEC: btest-diff http.log
@load base/frameworks/tunnels
@load base/protocols/conn
@load base/protocols/http
event analyzer_confirmation(c: connection, atype: AllAnalyzers::Tag, aid: count)
{
print "analyzer_confirmation", c$uid, c$id, aid;
}
event analyzer_violation(c: connection, atype: AllAnalyzers::Tag, aid: count, reason: string)
{
print "analyzer_violation", c$uid, c$id, aid, reason;
}