From e5d628548b2bff844c67278ceb810858a5acdf5b Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 24 Mar 2025 15:19:33 -0700 Subject: [PATCH 1/2] Change packet analyzer identifiers to be 64-bit --- src/packet_analysis/Analyzer.cc | 14 +++++++------- src/packet_analysis/Analyzer.h | 8 ++++---- src/packet_analysis/Dispatcher.cc | 8 ++++---- src/packet_analysis/Dispatcher.h | 8 ++++---- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/packet_analysis/Analyzer.cc b/src/packet_analysis/Analyzer.cc index 17f6987b8d..9995ff6092 100644 --- a/src/packet_analysis/Analyzer.cc +++ b/src/packet_analysis/Analyzer.cc @@ -53,12 +53,12 @@ bool Analyzer::IsAnalyzer(const char* name) { return packet_mgr->GetComponentName(tag) == name; } -const AnalyzerPtr& Analyzer::Lookup(uint32_t identifier) const { return dispatcher.Lookup(identifier); } +const AnalyzerPtr& Analyzer::Lookup(uint64_t identifier) const { return dispatcher.Lookup(identifier); } // Find the next inner analyzer using identifier or via DetectProtocol(), // otherwise return the default analyzer. const AnalyzerPtr& Analyzer::FindInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet, - uint32_t identifier) const { + uint64_t identifier) const { const auto& identifier_based_analyzer = Lookup(identifier); if ( identifier_based_analyzer ) return identifier_based_analyzer; @@ -92,11 +92,11 @@ const AnalyzerPtr& Analyzer::DetectInnerAnalyzer(size_t len, const uint8_t* data return nil; } -bool Analyzer::ForwardPacket(size_t len, const uint8_t* data, Packet* packet, uint32_t identifier) const { +bool Analyzer::ForwardPacket(size_t len, const uint8_t* data, Packet* packet, uint64_t identifier) const { const auto& inner_analyzer = FindInnerAnalyzer(len, data, packet, identifier); if ( ! inner_analyzer ) { - DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s failed, could not find analyzer for identifier %#x.", + DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s failed, could not find analyzer for identifier %#" PRIx64 ".", GetAnalyzerName(), identifier); if ( report_unknown_protocols ) @@ -106,12 +106,12 @@ bool Analyzer::ForwardPacket(size_t len, const uint8_t* data, Packet* packet, ui } if ( ! inner_analyzer->IsEnabled() ) { - DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s found disabled next layer analyzer %s for identifier %#x", + DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s found disabled next layer analyzer %s for identifier %#" PRIx64, GetAnalyzerName(), inner_analyzer->GetAnalyzerName(), identifier); return false; } - DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s succeeded, next layer identifier is %#x.", GetAnalyzerName(), + DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s succeeded, next layer identifier is %#" PRIx64 ".", GetAnalyzerName(), identifier); packet_mgr->TrackAnalyzer(inner_analyzer.get(), len, data); @@ -141,7 +141,7 @@ void Analyzer::DumpDebug() const { #endif } -void Analyzer::RegisterProtocol(uint32_t identifier, AnalyzerPtr child) { +void Analyzer::RegisterProtocol(uint64_t identifier, AnalyzerPtr child) { if ( run_state::detail::zeek_init_done ) reporter->FatalError("Packet protocols cannot be registered after zeek_init has finished."); diff --git a/src/packet_analysis/Analyzer.h b/src/packet_analysis/Analyzer.h index 7afaeae45d..6d4d268a2c 100644 --- a/src/packet_analysis/Analyzer.h +++ b/src/packet_analysis/Analyzer.h @@ -108,7 +108,7 @@ public: * @param child The analyzer that will be called for the new protocol during * forwarding. */ - void RegisterProtocol(uint32_t identifier, AnalyzerPtr child); + void RegisterProtocol(uint64_t identifier, AnalyzerPtr child); /** * Registers an analyzer to use for protocol detection if identifier @@ -202,7 +202,7 @@ protected: * @return The analyzer registered for the given identifier. Returns a * nullptr if no analyzer is registered. */ - const AnalyzerPtr& Lookup(uint32_t identifier) const; + const AnalyzerPtr& Lookup(uint64_t identifier) const; /** * Returns an analyzer based on a script-land definition. @@ -238,7 +238,7 @@ protected: * * @return false if the analysis failed, else true. */ - bool ForwardPacket(size_t len, const uint8_t* data, Packet* packet, uint32_t identifier) const; + bool ForwardPacket(size_t len, const uint8_t* data, Packet* packet, uint64_t identifier) const; /** * Triggers default analysis of the encapsulated packet if the default analyzer @@ -266,7 +266,7 @@ private: const zeek::Tag& arg_tag); // Internal helpers to find an appropriate next inner analyzer. - const AnalyzerPtr& FindInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet, uint32_t identifier) const; + const AnalyzerPtr& FindInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet, uint64_t identifier) const; const AnalyzerPtr& FindInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet) const; const AnalyzerPtr& DetectInnerAnalyzer(size_t len, const uint8_t* data, Packet* packet) const; diff --git a/src/packet_analysis/Dispatcher.cc b/src/packet_analysis/Dispatcher.cc index fd7e38a055..f8c846a2d2 100644 --- a/src/packet_analysis/Dispatcher.cc +++ b/src/packet_analysis/Dispatcher.cc @@ -12,7 +12,7 @@ namespace zeek::packet_analysis::detail { Dispatcher::~Dispatcher() { FreeValues(); } -void Dispatcher::Register(uint32_t identifier, AnalyzerPtr analyzer) { +void Dispatcher::Register(uint64_t identifier, AnalyzerPtr analyzer) { // If the table has size 1 and the entry is nullptr, there was nothing added yet. Just add it. if ( table.size() == 1 && table[0] == nullptr ) { table[0] = std::move(analyzer); @@ -27,7 +27,7 @@ void Dispatcher::Register(uint32_t identifier, AnalyzerPtr analyzer) { } else if ( identifier < lowest_identifier ) { // Lower than the lowest registered identifier. Shift up by lowerBound - identifier - uint32_t distance = lowest_identifier - identifier; + uint64_t distance = lowest_identifier - identifier; table.resize(table.size() + distance, nullptr); // Shift values @@ -48,7 +48,7 @@ void Dispatcher::Register(uint32_t identifier, AnalyzerPtr analyzer) { table[index] = std::move(analyzer); } -const AnalyzerPtr& Dispatcher::Lookup(uint32_t identifier) const { +const AnalyzerPtr& Dispatcher::Lookup(uint64_t identifier) const { int64_t index = identifier - lowest_identifier; if ( index >= 0 && index < static_cast(table.size()) ) return table[index]; @@ -75,7 +75,7 @@ void Dispatcher::DumpDebug() const { DBG_LOG(DBG_PACKET_ANALYSIS, "Dispatcher elements (used/total): %lu/%lu", Count(), table.size()); for ( size_t i = 0; i < table.size(); i++ ) { if ( table[i] != nullptr ) - DBG_LOG(DBG_PACKET_ANALYSIS, "%#8lx => %s", i + lowest_identifier, table[i]->GetAnalyzerName()); + DBG_LOG(DBG_PACKET_ANALYSIS, "%#8" PRIx64 " => %s", i + lowest_identifier, table[i]->GetAnalyzerName()); } #endif } diff --git a/src/packet_analysis/Dispatcher.h b/src/packet_analysis/Dispatcher.h index 7068d9e808..96f9e2219d 100644 --- a/src/packet_analysis/Dispatcher.h +++ b/src/packet_analysis/Dispatcher.h @@ -28,7 +28,7 @@ public: * @param identifier The identifier. * @param analyzer The analyzer to register. */ - void Register(uint32_t identifier, AnalyzerPtr analyzer); + void Register(uint64_t identifier, AnalyzerPtr analyzer); /** * Looks up the analyzer for an identifier. @@ -37,7 +37,7 @@ public: * @return The analyzer registered for the given identifier. Returns a * nullptr if no analyzer is registered. */ - const AnalyzerPtr& Lookup(uint32_t identifier) const; + const AnalyzerPtr& Lookup(uint64_t identifier) const; /** * Returns the number of registered analyzers. @@ -56,12 +56,12 @@ public: void DumpDebug() const; private: - uint32_t lowest_identifier = 0; + uint64_t lowest_identifier = 0; std::vector table; void FreeValues(); - inline uint32_t GetHighestIdentifier() const { return lowest_identifier + table.size() - 1; } + inline uint64_t GetHighestIdentifier() const { return lowest_identifier + table.size() - 1; } }; } // namespace detail From 195b87b873362835c528a406d6fbece9bbfaabed Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 24 Mar 2025 15:20:30 -0700 Subject: [PATCH 2/2] Make SNAP analyzer use both OUI and protocol for forwarding --- src/packet_analysis/protocol/snap/SNAP.cc | 12 ++++++------ .../unknown_protocols.log | 11 +++++++++++ testing/btest/Traces/README | 2 ++ testing/btest/Traces/cdp-v1.pcap | Bin 0 -> 340 bytes .../scripts/base/protocols/snap/snap-cdp.test | 4 ++++ 5 files changed, 23 insertions(+), 6 deletions(-) create mode 100644 testing/btest/Baseline/scripts.base.protocols.snap.snap-cdp/unknown_protocols.log create mode 100644 testing/btest/Traces/cdp-v1.pcap create mode 100644 testing/btest/scripts/base/protocols/snap/snap-cdp.test diff --git a/src/packet_analysis/protocol/snap/SNAP.cc b/src/packet_analysis/protocol/snap/SNAP.cc index 36bd4236a6..f09b9950b6 100644 --- a/src/packet_analysis/protocol/snap/SNAP.cc +++ b/src/packet_analysis/protocol/snap/SNAP.cc @@ -36,11 +36,11 @@ bool SNAPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet data += 5; len -= 5; - if ( oui == 0 ) { - // If the OUI is zero, the protocol is a standard ethertype and can be - // forwarded as such. - return ForwardPacket(len, data, packet, protocol); - } + // Protocol values for SNAP can differ based what OUI publishes them, so use a + // combination of them for the identifier used to forward. + int64_t identifier = oui; + identifier <<= 16; + identifier |= protocol; - return true; + return ForwardPacket(len, data, packet, identifier); } diff --git a/testing/btest/Baseline/scripts.base.protocols.snap.snap-cdp/unknown_protocols.log b/testing/btest/Baseline/scripts.base.protocols.snap.snap-cdp/unknown_protocols.log new file mode 100644 index 0000000000..01a7c037c7 --- /dev/null +++ b/testing/btest/Baseline/scripts.base.protocols.snap.snap-cdp/unknown_protocols.log @@ -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 unknown_protocols +#open XXXX-XX-XX-XX-XX-XX +#fields ts analyzer protocol_id first_bytes analyzer_history +#types time string string string vector[string] +XXXXXXXXXX.XXXXXX SNAP 0xc2000 01b4dff0000100065231 ETHERNET,SNAP +#close XXXX-XX-XX-XX-XX-XX diff --git a/testing/btest/Traces/README b/testing/btest/Traces/README index b26ef4424b..6f9576cb71 100644 --- a/testing/btest/Traces/README +++ b/testing/btest/Traces/README @@ -48,3 +48,5 @@ Trace Index/Sources: https://zeekorg.slack.com/archives/CSZBXF6TH/p1738261449655049 - tunnels/geneve-tagged-udp-packet.pcap Provided by Eldon Koyle Corelight for testing. +- cdp-v1.pcap + From the Wireshark library of captures at https://wiki.wireshark.org/samplecaptures. \ No newline at end of file diff --git a/testing/btest/Traces/cdp-v1.pcap b/testing/btest/Traces/cdp-v1.pcap new file mode 100644 index 0000000000000000000000000000000000000000..cde058b52df32b25de2cd1575b60b78eff0bdebf GIT binary patch literal 340 zcmYL@!AiqG5Qb;lS|nD%gV!?01d@_%AvE4fh)}3jlA?G_(`^$pN!hHFK8`o(QG9@c z*BmW)QXj!LsGAmXU=IKM^Ur+mZ!b?3sKH-tw*W+n@j7rn`$wQlQ8s}_Q55jq+-%NB zbG51hG!V$^*N?R79)${=$`~P}fZF<5Ay9>5S451bOqlY)Vd=sFd^m9)X4p@~&RS&r z9*1+r0}-b)^q09{3HsSoECS9@=?{HWMWP|KO!I%PI;A$Q?u>z^;|=3^oTW%