diff --git a/CHANGES b/CHANGES index 83dafcf4e6..a360c58802 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,29 @@ +6.0.0-dev.11 | 2023-02-06 11:13:08 -0700 + + * krb/smb2_krb_nokeytab: Register get_file_handle() to avoid warnings (Arne Welzel, Corelight) + + Now that the common event handler logs a warning, ensure there's one + in place, even if it's just returning stub data. + + * fuzzer-setup: Do not always generate new_event (Arne Welzel, Corelight) + + 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. + + * EventHandler: Support unsetting generate_always (Arne Welzel, Corelight) + + * files/main: No empty file_ids (Arne Welzel, Corelight) + + 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. + 6.0.0-dev.6 | 2023-02-06 10:33:24 -0700 * CI: Fix the ThreadSanitizer build (Tim Wojtulewicz, Corelight) 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/VERSION b/VERSION index 5b76c8b338..7952ee9ce4 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -6.0.0-dev.6 +6.0.0-dev.11 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)); 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; } 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; } 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; + }