From 26b1558cd17666a314bea5b5b11bbfe15c5555ab Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Mon, 23 Jan 2023 12:09:47 +0100 Subject: [PATCH] analyzer: Move disabling_analyzer() hook into Analyzer module When disabling_analyzer() was introduced, it was added to the GLOBAL module. The awkward side-effect is that implementing a hook handler in another module requires to prefix it with GLOBAL. Alternatively, one can re-open the GLOBAL module and implement the handler in that scope. Both are not great, and prefixing with GLOBAL is ugly, so move the identifier to the Analyzer module and ask users to prefix with Analyzer. --- NEWS | 3 ++ scripts/base/init-bare.zeek | 41 +++++++++++-------- src/zeek.bif | 11 +++++ .../bifs.disable_analyzer-hook-module/out | 16 ++++++++ .../bifs/disable_analyzer-hook-module.zeek | 21 ++++++++++ testing/btest/bifs/disable_analyzer-hook.zeek | 4 +- .../ssl/prevent-disable-analyzer.test | 6 +-- 7 files changed, 81 insertions(+), 21 deletions(-) create mode 100644 testing/btest/Baseline/bifs.disable_analyzer-hook-module/out create mode 100644 testing/btest/bifs/disable_analyzer-hook-module.zeek diff --git a/NEWS b/NEWS index 9df03eb11f..23bc938616 100644 --- a/NEWS +++ b/NEWS @@ -213,6 +213,9 @@ Deprecated Functionality deprecated in favor of the more generic ``analyzer_confirmation_info`` and ``analyzer_violation_info`` events. +- The global ``disabling_analyzer()`` hook has been deprecated and replaced + with ``Analyzer::disabling_analyzer()`` that has the same semantics. + - The const values for toggling individual tunnel packet analyzers have been deprecated in favor of using ``Analyzer::disable_analyzer()`` directly. This affects: diff --git a/scripts/base/init-bare.zeek b/scripts/base/init-bare.zeek index cd27134c73..cc60c00a9b 100644 --- a/scripts/base/init-bare.zeek +++ b/scripts/base/init-bare.zeek @@ -597,23 +597,32 @@ type fa_metadata: record { inferred: bool &default=T; }; -## A hook taking a connection, analyzer tag and analyzer id that can be -## used to veto disabling analyzers. Specifically, an analyzer can be prevented -## from being disabled by using a :zeek:see:`break` statement within the hook. -## This hook is invoked synchronously during a :zeek:see:`disable_analyzer` call. -## -## Scripts implementing this hook should have other logic that will eventually -## disable the analyzer for the given connection. That is, if a script vetoes -## disabling an analyzer, it takes responsibility for a later call to -## :zeek:see:`disable_analyzer`, which may be never. -## -## c: The connection -## -## atype: The type / tag of the analyzer being disabled. -## -## aid: The analyzer ID. -type disabling_analyzer: hook(c: connection, atype: AllAnalyzers::Tag, aid: count); +## Same as :zeek:see:`Analyzer::disabling_analyzer`, but deprecated due +## to living in the global namespace. +type disabling_analyzer: hook(c: connection, atype: AllAnalyzers::Tag, aid: count) &redef &deprecated="Remove in v6.1. Use Analyzer::disabling_analyzer() instead."; +module Analyzer; +export { + ## A hook taking a connection, analyzer tag and analyzer id that can be + ## used to veto disabling protocol analyzers. Specifically, an analyzer + ## can be prevented from being disabled by using a :zeek:see:`break` + ## statement within the hook. + ## This hook is invoked synchronously during a :zeek:see:`disable_analyzer` call. + ## + ## Scripts implementing this hook should have other logic that will eventually + ## disable the analyzer for the given connection. That is, if a script vetoes + ## disabling an analyzer, it takes responsibility for a later call to + ## :zeek:see:`disable_analyzer`, which may be never. + ## + ## c: The connection + ## + ## atype: The type / tag of the analyzer being disabled. + ## + ## aid: The analyzer ID. + type disabling_analyzer: hook(c: connection, atype: AllAnalyzers::Tag, aid: count) &redef; +} + +module GLOBAL; ## Fields of a SYN packet. ## ## .. zeek:see:: connection_SYN_packet diff --git a/src/zeek.bif b/src/zeek.bif index b1c2a15cc0..dd462ab4a5 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -4697,7 +4697,10 @@ function disable_analyzer%(cid: conn_id, aid: count, err_if_no_conn: bool &defau return zeek::val_mgr->False(); } + // Remove in v6.1: Global disabling_analyzer is to be removed. static auto disabling_analyzer_hook = id::find_func("disabling_analyzer"); + static auto analyzer_disabling_analyzer_hook = id::find_func("Analyzer::disabling_analyzer"); + if ( disabling_analyzer_hook ) { auto hook_rval = disabling_analyzer_hook->Invoke(c->GetVal(), a->GetAnalyzerTag().AsVal(), @@ -4706,6 +4709,14 @@ function disable_analyzer%(cid: conn_id, aid: count, err_if_no_conn: bool &defau return zeek::val_mgr->False(); } + if ( analyzer_disabling_analyzer_hook ) + { + auto hook_rval = analyzer_disabling_analyzer_hook->Invoke(c->GetVal(), a->GetAnalyzerTag().AsVal(), + zeek::val_mgr->Count(aid)); + if ( hook_rval && ! hook_rval->AsBool() ) + return zeek::val_mgr->False(); + } + if ( prevent ) a->Parent()->PreventChildren(a->GetAnalyzerTag()); diff --git a/testing/btest/Baseline/bifs.disable_analyzer-hook-module/out b/testing/btest/Baseline/bifs.disable_analyzer-hook-module/out new file mode 100644 index 0000000000..f8370ed6a0 --- /dev/null +++ b/testing/btest/Baseline/bifs.disable_analyzer-hook-module/out @@ -0,0 +1,16 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +http_request, GET, /style/enhanced.css +prevent disabling +F +http_request, GET, /script/urchin.js +prevent disabling +F +http_request, GET, /images/template/screen/bullet_utility.png +prevent disabling +F +http_request, GET, /images/template/screen/key-point-top.png +prevent disabling +F +http_request, GET, /projects/calendar/images/header-sunbird.png +prevent disabling +F diff --git a/testing/btest/bifs/disable_analyzer-hook-module.zeek b/testing/btest/bifs/disable_analyzer-hook-module.zeek new file mode 100644 index 0000000000..5092f1c952 --- /dev/null +++ b/testing/btest/bifs/disable_analyzer-hook-module.zeek @@ -0,0 +1,21 @@ +# @TEST-DOC: Hook Analyzer::disabling_analyzer in a module +# @TEST-EXEC: zeek -b -r $TRACES/http/pipelined-requests.trace %INPUT >out +# @TEST-EXEC: btest-diff out + +@load base/protocols/http + +module MyHTTP; + + +# Prevent disabling all analyzers. +hook Analyzer::disabling_analyzer(c: connection, atype: AllAnalyzers::Tag, aid: count) + { + print("prevent disabling"); + break; + } + +event http_request(c: connection, method: string, original_URI: string, unescaped_URI: string, version: string) + { + print "http_request", method, original_URI; + print disable_analyzer(c$id, current_analyzer(), T, T); + } diff --git a/testing/btest/bifs/disable_analyzer-hook.zeek b/testing/btest/bifs/disable_analyzer-hook.zeek index 252b7f253d..23b9c8a21b 100644 --- a/testing/btest/bifs/disable_analyzer-hook.zeek +++ b/testing/btest/bifs/disable_analyzer-hook.zeek @@ -6,7 +6,7 @@ global msg_count: table[conn_id] of count &default=0; -event analyzer_confirmation(c: connection, atype: AllAnalyzers::Tag, aid: count) &priority=10 +event analyzer_confirmation_info(atype: AllAnalyzers::Tag, info: AnalyzerConfirmationInfo) &priority=10 { if ( atype != Analyzer::ANALYZER_HTTP ) return; @@ -15,7 +15,7 @@ event analyzer_confirmation(c: connection, atype: AllAnalyzers::Tag, aid: count) } # Prevent disabling all analyzers. -hook disabling_analyzer(c: connection, atype: AllAnalyzers::Tag, aid: count) +hook Analyzer::disabling_analyzer(c: connection, atype: AllAnalyzers::Tag, aid: count) { if ( msg_count[c$id] < 4 ) { diff --git a/testing/btest/scripts/base/protocols/ssl/prevent-disable-analyzer.test b/testing/btest/scripts/base/protocols/ssl/prevent-disable-analyzer.test index e4ff1257d4..acc48ce2b9 100644 --- a/testing/btest/scripts/base/protocols/ssl/prevent-disable-analyzer.test +++ b/testing/btest/scripts/base/protocols/ssl/prevent-disable-analyzer.test @@ -17,7 +17,7 @@ global encrypted_data_wanted = 4; # Prevent disabling the SSL analyzer for this connection until we've seen encrypted_data_wanted # encrypted data events on it. Our ssl_encrypted_data event handler has the inverse condition. -hook disabling_analyzer(c: connection, atype: AllAnalyzers::Tag, aid: count) +hook Analyzer::disabling_analyzer(c: connection, atype: AllAnalyzers::Tag, aid: count) { print "disabling_analyzer", c$id, atype, aid; if ( atype != Analyzer::ANALYZER_SSL || ! c?$ssl ) @@ -37,9 +37,9 @@ event ssl_established(c: connection) print "established", c$id; } -event analyzer_confirmation(c: connection, atype: AllAnalyzers::Tag, aid: count) +event analyzer_confirmation_info(atype: AllAnalyzers::Tag, info: AnalyzerConfirmationInfo) { - print "analyzer_confirmation", c$id, atype, aid; + print "analyzer_confirmation", info$c$id, atype, info$aid; } event ssl_encrypted_data(c: connection, is_client: bool, record_version: count, content_type: count, length: count)