diff --git a/CHANGES b/CHANGES index e81fcbf5e4..d765816ae0 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,24 @@ +7.0.0-11 | 2024-08-30 12:38:59 -0700 + + * Spicy: Register well-known ports through an event handler. (Robin Sommer, Corelight) + + This avoids the earlier problem of not tracking ports correctly in + scriptland, while still supporting `port` in EVT files and `%port` in + Spicy files. + + As it turns out we are already following the same approach for file + analyzers' MIME types, so I'm applying the same pattern: it's one + event per port, without further customization points. That leaves the + patch pretty small after all while fixing the original issue. + + (cherry picked from commit a2079bcda6e40180b888240a281c12cc0ca735be) + + * Revert "Remove deprecated port/ports fields for spicy analyzers" (Robin Sommer, Corelight) + + This reverts commit 15d404dd191a723960e4efd956eec22739d3f1c2. + + (cherry picked from commit a2079bcda6e40180b888240a281c12cc0ca735be) + 7.0.0-9 | 2024-08-30 11:47:39 -0700 * ldap: Promote uint8 to uint64 before shifting (Arne Welzel, Corelight) diff --git a/VERSION b/VERSION index b1cd350f84..2283f0636c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.0.0-9 +7.0.0-11 diff --git a/doc b/doc index 4e2413e2fb..b845bee39f 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 4e2413e2fb2f98298430b3e2cb384fc5f18cbde4 +Subproject commit b845bee39fa241892458d274184e2e9f22f4d096 diff --git a/scripts/base/frameworks/spicy/init-framework.zeek b/scripts/base/frameworks/spicy/init-framework.zeek index ae3a3b8e65..de6b528ee4 100644 --- a/scripts/base/frameworks/spicy/init-framework.zeek +++ b/scripts/base/frameworks/spicy/init-framework.zeek @@ -47,12 +47,18 @@ export { # Marked with &is_used to suppress complaints when there aren't any # Spicy file analyzers loaded, and hence this event can't be generated. -# The attribute is only supported for Zeek 5.0 and higher. event spicy_analyzer_for_mime_type(a: Files::Tag, mt: string) &is_used { Files::register_for_mime_type(a, mt); } +# Marked with &is_used to suppress complaints when there aren't any +# Spicy protocol analyzers loaded, and hence this event can't be generated. +event spicy_analyzer_for_port(a: Analyzer::Tag, p: port) &is_used + { + Analyzer::register_for_port(a, p); + } + function enable_protocol_analyzer(tag: Analyzer::Tag) : bool { return Spicy::__toggle_analyzer(tag, T); diff --git a/src/spicy/manager.cc b/src/spicy/manager.cc index 1a9420e22a..423febf1c9 100644 --- a/src/spicy/manager.cc +++ b/src/spicy/manager.cc @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -32,6 +33,7 @@ #include "zeek/spicy/file-analyzer.h" #include "zeek/spicy/packet-analyzer.h" #include "zeek/spicy/protocol-analyzer.h" +#include "zeek/spicy/runtime-support.h" #include "zeek/zeek-config-paths.h" using namespace zeek; @@ -74,9 +76,13 @@ void Manager::registerProtocolAnalyzer(const std::string& name, hilti::rt::Proto info.name_zeek = hilti::rt::replace(name, "::", "_"); info.name_zeekygen = hilti::rt::fmt("", name); info.protocol = proto; - info.ports = ports; info.linker_scope = linker_scope; + // Store ports in a deterministic order. We can't (easily) sort the + // `hilti::rt::Vector` unfortunately. + std::copy(ports.begin(), ports.end(), std::back_inserter(info.ports)); + std::sort(info.ports.begin(), info.ports.end()); + // We may have that analyzer already iff it was previously pre-registered // without a linker scope. We'll then only set the scope now. if ( auto t = _analyzer_name_to_tag_type.find(info.name_zeek); t != _analyzer_name_to_tag_type.end() ) { @@ -701,14 +707,25 @@ void Manager::InitPostScript() { if ( ! tag ) reporter->InternalError("cannot get analyzer tag for '%s'", p.name_analyzer.c_str()); + auto register_analyzer_for_port = [&](auto tag, const hilti::rt::Port& port_) { + SPICY_DEBUG(hilti::rt::fmt(" Scheduling analyzer for port %s", port_)); + + // Well-known ports are registered in scriptland, so we'll raise an + // event that will do it for us through a predefined handler. + zeek::Args vals = Args(); + vals.emplace_back(tag.AsVal()); + vals.emplace_back(zeek::spicy::rt::to_val(port_, base_type(TYPE_PORT))); + EventHandlerPtr handler = event_registry->Register("spicy_analyzer_for_port"); + event_mgr.Enqueue(handler, vals); + }; + for ( const auto& ports : p.ports ) { const auto proto = ports.begin.protocol(); // Port ranges are closed intervals. for ( auto port = ports.begin.port(); port <= ports.end.port(); ++port ) { const auto port_ = hilti::rt::Port(port, proto); - SPICY_DEBUG(hilti::rt::fmt(" Scheduling analyzer for port %s", port_)); - analyzer_mgr->RegisterAnalyzerForPort(tag, transport_protocol(port_), port); + register_analyzer_for_port(tag, port_); // Don't double register in case of single-port ranges. if ( ports.begin.port() == ports.end.port() ) @@ -727,7 +744,7 @@ void Manager::InitPostScript() { continue; SPICY_DEBUG(hilti::rt::fmt(" Scheduling analyzer for port %s", port.port)); - analyzer_mgr->RegisterAnalyzerForPort(tag, transport_protocol(port.port), port.port.port()); + register_analyzer_for_port(tag, port.port); } } } diff --git a/src/spicy/manager.h b/src/spicy/manager.h index 118e03b6c3..55f47c51fd 100644 --- a/src/spicy/manager.h +++ b/src/spicy/manager.h @@ -85,7 +85,7 @@ public: * * @param name name of the analyzer as defined in its EVT file * @param proto analyzer's transport-layer protocol - * @param prts well-known ports for the analyzer; it'll be activated automatically for these + * @param ports well-known ports for the analyzer; it'll be activated automatically for these * @param parser_orig name of the Spicy parser for the originator side; must match the name that * Spicy registers the unit's parser with * @param parser_resp name of the Spicy parser for the originator side; must match the name that @@ -343,7 +343,7 @@ private: std::string name_parser_resp; std::string name_replaces; hilti::rt::Protocol protocol = hilti::rt::Protocol::Undef; - hilti::rt::Vector<::zeek::spicy::rt::PortRange> ports; + std::vector<::zeek::spicy::rt::PortRange> ports; // we keep this sorted std::string linker_scope; // Computed and available once the analyzer has been registered. diff --git a/src/spicy/port-range.h b/src/spicy/port-range.h index bbe0d58c12..7e71d433f8 100644 --- a/src/spicy/port-range.h +++ b/src/spicy/port-range.h @@ -19,6 +19,11 @@ struct PortRange { hilti::rt::Port begin; /**< first port in the range */ hilti::rt::Port end; /**< last port in the range */ + + bool operator<(const PortRange& other) const { + // Just get us a deterministic order. + return std::tie(begin, end) < std::tie(other.begin, other.end); + } }; inline bool operator==(const PortRange& a, const PortRange& b) { diff --git a/testing/btest/Baseline/core.check-unused-event-handlers/.stderr b/testing/btest/Baseline/core.check-unused-event-handlers/.stderr index 69f805dbf7..9fc3532832 100644 --- a/testing/btest/Baseline/core.check-unused-event-handlers/.stderr +++ b/testing/btest/Baseline/core.check-unused-event-handlers/.stderr @@ -22,5 +22,6 @@ warning in , line 1: event handler never invoked: SupervisorControl::res warning in , line 1: event handler never invoked: SupervisorControl::status_request warning in , line 1: event handler never invoked: SupervisorControl::stop_request warning in , line 1: event handler never invoked: spicy_analyzer_for_mime_type +warning in , line 1: event handler never invoked: spicy_analyzer_for_port warning in , line 1: event handler never invoked: terminate_event warning in , line 1: event handler never invoked: this_is_never_used diff --git a/testing/btest/Baseline/spicy.port-deprecated/out.stderr b/testing/btest/Baseline/spicy.port-deprecated/out.stderr deleted file mode 100644 index a033682601..0000000000 --- a/testing/btest/Baseline/spicy.port-deprecated/out.stderr +++ /dev/null @@ -1,2 +0,0 @@ -### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -[warning] <...>/udp-test.evt:4: Remove in v7.1: Analyzer spicy::TEST is using the deprecated 'port' or 'ports' keyword to register well-known ports. Use Analyzer::register_for_ports() in the accompanying Zeek script instead. diff --git a/testing/btest/Baseline/spicy.port-fail/output b/testing/btest/Baseline/spicy.port-fail/output index f572d2e79a..24eb09807d 100644 --- a/testing/btest/Baseline/spicy.port-fail/output +++ b/testing/btest/Baseline/spicy.port-fail/output @@ -1,3 +1,3 @@ ### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. -[error] <...>/port-fail.evt:9: port outside of valid range +[error] <...>/port-fail.evt:7: port outside of valid range [error] error loading EVT file "<...>/port-fail.evt" diff --git a/testing/btest/Baseline/spicy.port/output b/testing/btest/Baseline/spicy.port/output new file mode 100644 index 0000000000..938a6c7b35 --- /dev/null +++ b/testing/btest/Baseline/spicy.port/output @@ -0,0 +1,19 @@ +### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63. +Analyzer::ANALYZER_SPICY_TEST, 11337/udp +Analyzer::ANALYZER_SPICY_TEST, 11338/udp +Analyzer::ANALYZER_SPICY_TEST, 11339/udp +Analyzer::ANALYZER_SPICY_TEST, 11340/udp +Analyzer::ANALYZER_SPICY_TEST, 31337/udp +Analyzer::ANALYZER_SPICY_TEST, 31338/udp +Analyzer::ANALYZER_SPICY_TEST, 31339/udp +Analyzer::ANALYZER_SPICY_TEST, 31340/udp +{ +31339/udp, +31337/udp, +31338/udp, +11339/udp, +11338/udp, +11340/udp, +31340/udp, +11337/udp +} diff --git a/testing/btest/spicy/port-deprecated.evt b/testing/btest/spicy/port-deprecated.evt deleted file mode 100644 index 220a9d1faf..0000000000 --- a/testing/btest/spicy/port-deprecated.evt +++ /dev/null @@ -1,21 +0,0 @@ -# @TEST-REQUIRES: have-spicy -# -# @TEST-EXEC: spicyz -d -o test.hlto ./udp-test.evt 2>out.stderr -# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out.stderr -# -# @TEST-DOC: Remove with v7.1: Specifying ports is deprecated. - -module Test; - -import zeek; - -public type Message = unit { - data: bytes &eod {} -}; - -# @TEST-START-FILE udp-test.evt -protocol analyzer spicy::TEST over UDP: - parse with Test::Message, - port 11337/udp-11340/udp, - ports {31337/udp-31340/udp}; -# @TEST-END-FILE diff --git a/testing/btest/spicy/port-fail.evt b/testing/btest/spicy/port-fail.evt index e51ca0fb79..f00efc6210 100644 --- a/testing/btest/spicy/port-fail.evt +++ b/testing/btest/spicy/port-fail.evt @@ -2,8 +2,6 @@ # # @TEST-EXEC-FAIL: spicyz %INPUT -d -o x.hlto >output 2>&1 # @TEST-EXEC: TEST_DIFF_CANONIFIER=diff-canonifier-spicy btest-diff output -# -# @TEST-DOC: Remove with v7.1 protocol analyzer spicy::SSH over TCP: port 123456/udp; diff --git a/testing/btest/spicy/port-range-one-port.zeek b/testing/btest/spicy/port-range-one-port.zeek index 95c32f2b27..bdc5219791 100644 --- a/testing/btest/spicy/port-range-one-port.zeek +++ b/testing/btest/spicy/port-range-one-port.zeek @@ -5,7 +5,7 @@ # @TEST-EXEC: grep -e 'Scheduling analyzer' -e 'error during parsing' < out > out.filtered # @TEST-EXEC: btest-diff out.filtered -# @TEST-DOC: Remove with v7.1. Expect a single 'Scheduling analyzer ...' message in the debug output and no parsing errors. There was a bug that 'port 31336/udp' would be wrongly interpreted as a 31336/udp-31337/udp port range. Regression test for #3278. +# @TEST-DOC: Expect a single 'Scheduling analyzer ...' message in the debug output and no parsing errors. There was a bug that 'port 31336/udp' would be wrongly interpreted as a 31336/udp-31337/udp port range. Regression test for #3278. # @TEST-START-FILE udp-test.spicy module UDPTest; diff --git a/testing/btest/spicy/port.zeek b/testing/btest/spicy/port.zeek new file mode 100644 index 0000000000..81d3586c68 --- /dev/null +++ b/testing/btest/spicy/port.zeek @@ -0,0 +1,32 @@ +# @TEST-REQUIRES: have-spicy +# +# @TEST-EXEC: spicyz -d -o test.hlto test.spicy test.evt +# @TEST-EXEC: zeek test.hlto %INPUT >output +# @TEST-EXEC: btest-diff output +# +# @TEST-DOC: Check that we raise port events for Spicy analyzers, and that the ports get correctly registered. + +event spicy_analyzer_for_port(a: Analyzer::Tag, p: port){ + print a, p; +} + +event zeek_done() { + print Analyzer::ports[Analyzer::ANALYZER_SPICY_TEST]; +} + +# @TEST-START-FILE test.spicy +module Test; + +import zeek; + +public type Message = unit { + data: bytes &eod {} +}; +# @TEST-END-FILE + +# @TEST-START-FILE test.evt +protocol analyzer spicy::Test over UDP: + parse with Test::Message, + port 11337/udp-11340/udp, + ports {31337/udp-31340/udp}; +# @TEST-END-FILE