From e4ab7b2d7073b414e0c3b2c03cd429a0746c1662 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Sat, 4 Feb 2023 12:02:37 +0100 Subject: [PATCH 1/4] files/main: No empty file_ids When an analyzer calls DataIn(), there's a costly callback construct going through the event queue. If an analyzer does not have a get_file_handle() handler installed, the produced file_id would end up empty and ignored. Consequently, the get_file_handle() callback was invoked for every new DataIn() invocations. This is surprising and costly. Log a warning when this happens and instead set a generically generated file handle value instead to prevent the repeated get_file_handle() invocations. --- NEWS | 7 +++++++ scripts/base/frameworks/files/main.zeek | 4 ++++ 2 files changed, 11 insertions(+) diff --git a/NEWS b/NEWS index 240c41f1a9..44e5bf6f9b 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,13 @@ release. For an exhaustive list of changes, see the ``CHANGES`` file Zeek 6.0.0 ========== +Changed Functionality +--------------------- + +- When ``get_file_handle()`` is invoked for an analyzer that did not register + an appropriate callback function, log a warning and return a generic handle + value based on the analyzer and connection information. + Zeek 5.2.0 ========== diff --git a/scripts/base/frameworks/files/main.zeek b/scripts/base/frameworks/files/main.zeek index da5ef00fc9..7dc6667185 100644 --- a/scripts/base/frameworks/files/main.zeek +++ b/scripts/base/frameworks/files/main.zeek @@ -513,7 +513,11 @@ function describe(f: fa_file): string event get_file_handle(tag: Files::Tag, c: connection, is_orig: bool) &priority=5 { if ( tag !in registered_protocols ) + { + Reporter::warning(fmt("get_file_handle() invoked for %s", tag)); + set_file_handle(fmt("%s-fallback-%s-%s-%s", tag, c$uid, is_orig, network_time())); return; + } local handler = registered_protocols[tag]; set_file_handle(handler$get_file_handle(c, is_orig)); From b66cd313db9a4695196fc1c9642689bb9045e4f8 Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Sat, 4 Feb 2023 12:15:10 +0100 Subject: [PATCH 2/4] EventHandler: Support unsetting generate_always --- src/EventHandler.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/EventHandler.h b/src/EventHandler.h index c8947f1082..e7c9458971 100644 --- a/src/EventHandler.h +++ b/src/EventHandler.h @@ -49,7 +49,10 @@ public: // Flags the event as interesting even if there is no body defined. In // particular, this will then still pass the event on to plugins. - void SetGenerateAlways() { generate_always = true; } + void SetGenerateAlways(bool arg_generate_always = true) + { + generate_always = arg_generate_always; + } bool GenerateAlways() const { return generate_always; } uint64_t CallCount() const { return call_count; } From d8b4667f806b7fef33bf24a3114d089b791bc8fc Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Sat, 4 Feb 2023 12:15:33 +0100 Subject: [PATCH 3/4] fuzzer-setup: Do not always generate new_event new_event should never be used on production systems, so don't turn it on for fuzzing either as it showed up as bottlenecks in flamegraphs. --- src/fuzzers/fuzzer-setup.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fuzzers/fuzzer-setup.h b/src/fuzzers/fuzzer-setup.h index 24cdc7b43a..7a3ca8343f 100644 --- a/src/fuzzers/fuzzer-setup.h +++ b/src/fuzzers/fuzzer-setup.h @@ -47,6 +47,7 @@ extern "C" int LLVMFuzzerInitialize(int* argc, char*** argv) // even if they don't, because otherwise we lose a bit of coverage where if // statements return false that would otherwise not. zeek::event_registry->ActivateAllHandlers(); + zeek::event_registry->Lookup("new_event")->SetGenerateAlways(false); return 0; } From b928a7d84d0e98adbfe62bdc05a0b682ca14876b Mon Sep 17 00:00:00 2001 From: Arne Welzel Date: Sun, 5 Feb 2023 21:22:50 +0100 Subject: [PATCH 4/4] krb/smb2_krb_nokeytab: Register get_file_handle() to avoid warnings Now that the common event handler logs a warning, ensure there's one in place, even if it's just returning stub data. --- .../base/protocols/krb/smb2_krb_nokeytab.test | 36 +++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/testing/btest/scripts/base/protocols/krb/smb2_krb_nokeytab.test b/testing/btest/scripts/base/protocols/krb/smb2_krb_nokeytab.test index 2f7cff52ea..794e8958e0 100644 --- a/testing/btest/scripts/base/protocols/krb/smb2_krb_nokeytab.test +++ b/testing/btest/scripts/base/protocols/krb/smb2_krb_nokeytab.test @@ -8,13 +8,37 @@ # @TEST-EXEC: btest-diff .stdout # @TEST-EXEC: btest-diff .stderr +module SMB; + +export { + global get_file_handle: function(c: connection, is_orig: bool): string; + global describe_file: function(f: fa_file): string; +} + global monitor_ports: set[port] = { 445/tcp, 139/tcp } &redef; -event zeek_init() &priority=5{ + +# Stubs for testing so that we don't produce a warning due +# to missing get_file_handle() handlers for SMB. +function get_file_handle(c: connection, is_orig: bool): string + { + return cat(c$uid); + } + +function describe_file(f: fa_file): string + { + return ""; + } + +event zeek_init() &priority=5 + { Analyzer::register_for_ports(Analyzer::ANALYZER_SMB, monitor_ports); -} - -event krb_ap_request(c: connection, ticket: KRB::Ticket, opts: KRB::AP_Options){ - print ticket?$authenticationinfo; -} + Files::register_protocol(Analyzer::ANALYZER_SMB, + [$get_file_handle = SMB::get_file_handle, + $describe = SMB::describe_file]); + } +event krb_ap_request(c: connection, ticket: KRB::Ticket, opts: KRB::AP_Options) + { + print ticket?$authenticationinfo; + }