From 90eb97876ff4fe48c8baffa64cbe1d6ebae03fb3 Mon Sep 17 00:00:00 2001 From: Jan Grashoefer Date: Mon, 31 Aug 2020 17:13:22 +0200 Subject: [PATCH] Improve packet analyzer API. --- src/iosource/Packet.cc | 5 ---- src/iosource/Packet.h | 9 +------ src/packet_analysis/Analyzer.cc | 10 ++++---- src/packet_analysis/Analyzer.h | 24 +++++++++---------- src/packet_analysis/Manager.cc | 7 +----- src/packet_analysis/protocol/CMakeLists.txt | 1 - src/packet_analysis/protocol/arp/ARP.cc | 16 +++++++++---- src/packet_analysis/protocol/arp/ARP.h | 2 +- .../protocol/ethernet/Ethernet.cc | 19 +++++++-------- .../protocol/ethernet/Ethernet.h | 2 +- src/packet_analysis/protocol/fddi/FDDI.cc | 10 ++++---- src/packet_analysis/protocol/fddi/FDDI.h | 2 +- .../protocol/ieee802_11/IEEE802_11.cc | 14 +++++------ .../protocol/ieee802_11/IEEE802_11.h | 2 +- .../ieee802_11_radio/IEEE802_11_Radio.cc | 15 +++++------- .../ieee802_11_radio/IEEE802_11_Radio.h | 2 +- src/packet_analysis/protocol/ip/IP.cc | 8 +++---- src/packet_analysis/protocol/ip/IP.h | 2 +- src/packet_analysis/protocol/ipv4/IPv4.cc | 4 +++- src/packet_analysis/protocol/ipv4/IPv4.h | 2 +- src/packet_analysis/protocol/ipv6/IPv6.cc | 4 +++- src/packet_analysis/protocol/ipv6/IPv6.h | 2 +- .../protocol/linux_sll/LinuxSLL.cc | 9 +++---- .../protocol/linux_sll/LinuxSLL.h | 2 +- src/packet_analysis/protocol/mpls/MPLS.cc | 11 +++++---- src/packet_analysis/protocol/mpls/MPLS.h | 2 +- src/packet_analysis/protocol/nflog/NFLog.cc | 17 +++++++++---- src/packet_analysis/protocol/nflog/NFLog.h | 2 +- src/packet_analysis/protocol/null/Null.cc | 10 ++++---- src/packet_analysis/protocol/null/Null.h | 2 +- .../protocol/ppp_serial/PPPSerial.cc | 14 +++++++---- .../protocol/ppp_serial/PPPSerial.h | 2 +- src/packet_analysis/protocol/pppoe/PPPoE.cc | 10 ++++---- src/packet_analysis/protocol/pppoe/PPPoE.h | 2 +- src/packet_analysis/protocol/skip/Skip.cc | 6 ++--- src/packet_analysis/protocol/skip/Skip.h | 2 +- src/packet_analysis/protocol/vlan/VLAN.cc | 10 ++++---- src/packet_analysis/protocol/vlan/VLAN.h | 2 +- .../plugins/packet-protocol-plugin/src/Bar.cc | 7 +++--- .../plugins/packet-protocol-plugin/src/Bar.h | 2 +- 40 files changed, 138 insertions(+), 136 deletions(-) diff --git a/src/iosource/Packet.cc b/src/iosource/Packet.cc index 15617ee7f2..7b40c75e43 100644 --- a/src/iosource/Packet.cc +++ b/src/iosource/Packet.cc @@ -79,11 +79,6 @@ void Packet::Weird(const char* name) l2_valid = false; } -const u_char* const Packet::GetEndOfData() const - { - return data + cap_len; - } - IntrusivePtr Packet::ToRawPktHdrVal() const { static auto raw_pkt_hdr_type = id::find_type("raw_pkt_hdr"); diff --git a/src/iosource/Packet.h b/src/iosource/Packet.h index d99979010d..dfe5d86562 100644 --- a/src/iosource/Packet.h +++ b/src/iosource/Packet.h @@ -140,13 +140,6 @@ public: [[deprecated("Remove in v4.1. Use ToRawPktHdrval() instead.")]] RecordVal* BuildPktHdrVal() const; - /** - * Returns the end of the captured data for bound checking. - * - * @return End of the packet data. - */ - const u_char* const GetEndOfData() const; - /** * Describes the packet, with standard signature. */ @@ -228,7 +221,7 @@ public: */ bool l3_checksummed; - // Wrapper to generate a packet-level weird. Has to be public for llanalyzers to use it. + // Wrapper to generate a packet-level weird. Has to be public for packet analyzers to use it. void Weird(const char* name); private: diff --git a/src/packet_analysis/Analyzer.cc b/src/packet_analysis/Analyzer.cc index 55b7e5fd9c..3262e22fe9 100644 --- a/src/packet_analysis/Analyzer.cc +++ b/src/packet_analysis/Analyzer.cc @@ -57,8 +57,8 @@ AnalyzerPtr Analyzer::Lookup(uint32_t identifier) const return dispatcher.Lookup(identifier); } -AnalyzerResult Analyzer::AnalyzeInnerPacket(Packet* packet, - const uint8_t*& data, uint32_t identifier) const +AnalyzerResult Analyzer::ForwardPacket(size_t len, const uint8_t* data, Packet* packet, + uint32_t identifier) const { auto inner_analyzer = Lookup(identifier); if ( ! inner_analyzer ) @@ -74,13 +74,13 @@ AnalyzerResult Analyzer::AnalyzeInnerPacket(Packet* packet, DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s succeeded, next layer identifier is %#x.", GetAnalyzerName(), identifier); - return inner_analyzer->Analyze(packet, data); + return inner_analyzer->AnalyzePacket(len, data, packet); } -AnalyzerResult Analyzer::AnalyzeInnerPacket(Packet* packet, const uint8_t*& data) const +AnalyzerResult Analyzer::ForwardPacket(size_t len, const uint8_t* data, Packet* packet) const { if ( default_analyzer ) - return default_analyzer->Analyze(packet, data); + return default_analyzer->AnalyzePacket(len, data, packet); DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s stopped, no default analyzer available.", GetAnalyzerName()); diff --git a/src/packet_analysis/Analyzer.h b/src/packet_analysis/Analyzer.h index 2bc92afa7b..82ddf3a686 100644 --- a/src/packet_analysis/Analyzer.h +++ b/src/packet_analysis/Analyzer.h @@ -10,7 +10,6 @@ namespace zeek::packet_analysis { /** * Result of packet analysis. */ - //TODO: Replace with bool? enum class AnalyzerResult { Failed, // Analysis failed Terminate // Analysis succeeded and there is no further analysis to do @@ -85,18 +84,19 @@ public: void RegisterDefaultAnalyzer(AnalyzerPtr default_analyzer); /** - * Analyzes the given packet. The data reference points to the part of the - * raw packet to be analyzed. If the analyzed protocol encapsulates another - * protocol, the data reference should be updated to point to that payload. + * Analyzes the given packet. A common case is that the analyzed protocol + * encapsulates another protocol, which can be determined by an identifier + * in the header. In this case, derived classes may use ForwardPacket() to + * forward the payload to the corresponding analyzer. * - * @param packet The packet to analyze. - * @param data Reference to the payload pointer into the raw packet. + * @param len The number of bytes passed in. + * @param data Pointer to the input to process. + * @param packet Object that maintains the packet's meta data. * - * @return A tuple of analysis result and identifier. The result indicates - * how to proceed. If analysis can continue, the identifier determines the - * encapsulated protocol. + * @return The outcome of the analysis. */ - virtual AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) = 0; + virtual AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, + Packet* packet) = 0; protected: friend class Manager; @@ -121,7 +121,7 @@ protected: * * @return The outcome of the analysis. */ - AnalyzerResult AnalyzeInnerPacket(Packet* packet, const uint8_t*& data, + AnalyzerResult ForwardPacket(size_t len, const uint8_t* data, Packet* packet, uint32_t identifier) const; /** @@ -133,7 +133,7 @@ protected: * * @return The outcome of the analysis. */ - AnalyzerResult AnalyzeInnerPacket(Packet* packet, const uint8_t*& data) const; + AnalyzerResult ForwardPacket(size_t len, const uint8_t* data, Packet* packet) const; private: Tag tag; diff --git a/src/packet_analysis/Manager.cc b/src/packet_analysis/Manager.cc index 0ad05ed4d6..7e44c10883 100644 --- a/src/packet_analysis/Manager.cc +++ b/src/packet_analysis/Manager.cc @@ -120,8 +120,6 @@ void Manager::ProcessPacket(Packet* packet) DBG_LOG(DBG_PACKET_ANALYSIS, "Analyzing packet %ld, ts=%.3f...", ++counter, packet->time); #endif // Start packet analysis - const uint8_t* data = packet->data; - auto root_analyzer = root_dispatcher.Lookup(packet->link_type); auto analyzer = root_analyzer ? root_analyzer : default_analyzer; if ( !analyzer ) @@ -130,10 +128,7 @@ void Manager::ProcessPacket(Packet* packet) return; } - auto result = analyzer->Analyze(packet, data); - - // Calculate header size after processing packet layers. - packet->hdr_size = static_cast(data - packet->data); + auto result = analyzer->AnalyzePacket(packet->cap_len, packet->data, packet); } AnalyzerPtr Manager::InstantiateAnalyzer(const Tag& tag) diff --git a/src/packet_analysis/protocol/CMakeLists.txt b/src/packet_analysis/protocol/CMakeLists.txt index 4aee498aa2..c7228c2123 100644 --- a/src/packet_analysis/protocol/CMakeLists.txt +++ b/src/packet_analysis/protocol/CMakeLists.txt @@ -1,6 +1,5 @@ add_subdirectory(skip) -add_subdirectory(wrapper) add_subdirectory(null) add_subdirectory(ethernet) add_subdirectory(vlan) diff --git a/src/packet_analysis/protocol/arp/ARP.cc b/src/packet_analysis/protocol/arp/ARP.cc index ae9844be58..c38cea1d34 100644 --- a/src/packet_analysis/protocol/arp/ARP.cc +++ b/src/packet_analysis/protocol/arp/ARP.cc @@ -81,18 +81,24 @@ ARPAnalyzer::ARPAnalyzer() #define ARPOP_INVREPLY ARPOP_InREPLY #endif -zeek::packet_analysis::AnalyzerResult ARPAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult ARPAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { packet->l3_proto = L3_ARP; + // Check whether the header is complete. + if ( sizeof(struct arp_pkthdr) > len ) + { + packet->Weird("truncated_ARP"); + return AnalyzerResult::Failed; + } + // Check whether the packet is OK ("inspired" in tcpdump's print-arp.c). auto ah = (const struct arp_pkthdr*) data; // Check the size. - auto min_length = (ar_tpa(ah) - (char*) data) + ah->ar_pln; - auto pkt_hdr_len = data - packet->data; - auto real_length = packet->cap_len - pkt_hdr_len; - if ( min_length > real_length ) + size_t min_length = (ar_tpa(ah) - (char*) data) + ah->ar_pln; + if ( min_length > len ) { packet->Weird("truncated_ARP"); return AnalyzerResult::Failed; diff --git a/src/packet_analysis/protocol/arp/ARP.h b/src/packet_analysis/protocol/arp/ARP.h index b6f590dc9a..f38ed5ff27 100644 --- a/src/packet_analysis/protocol/arp/ARP.h +++ b/src/packet_analysis/protocol/arp/ARP.h @@ -18,7 +18,7 @@ public: ARPAnalyzer(); ~ARPAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/ethernet/Ethernet.cc b/src/packet_analysis/protocol/ethernet/Ethernet.cc index cd5c5889b4..0bced04b4a 100644 --- a/src/packet_analysis/protocol/ethernet/Ethernet.cc +++ b/src/packet_analysis/protocol/ethernet/Ethernet.cc @@ -31,13 +31,12 @@ zeek::packet_analysis::AnalyzerPtr EthernetAnalyzer::LoadAnalyzer(const std::str return packet_mgr->GetAnalyzer(analyzer_val->AsEnumVal()); } -zeek::packet_analysis::AnalyzerResult EthernetAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult EthernetAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - auto end_of_data = packet->GetEndOfData(); - // Make sure that we actually got an entire ethernet header before trying // to pull bytes out of it. - if ( data + 16 >= end_of_data ) + if ( 16 >= len ) { packet->Weird("truncated_ethernet_frame"); return AnalyzerResult::Failed; @@ -48,13 +47,14 @@ zeek::packet_analysis::AnalyzerResult EthernetAnalyzer::Analyze(Packet* packet, { auto constexpr cfplen = 16; - if ( data + cfplen + 14 >= end_of_data ) + if ( cfplen + 14 >= len ) { packet->Weird("truncated_link_header_cfp"); return AnalyzerResult::Failed; } data += cfplen; + len -= cfplen; } // Get protocol being carried from the ethernet frame. @@ -66,15 +66,12 @@ zeek::packet_analysis::AnalyzerResult EthernetAnalyzer::Analyze(Packet* packet, // Ethernet II frames if ( protocol >= 1536 ) - { - data += 14; - return AnalyzeInnerPacket(packet, data, protocol); - } + return ForwardPacket(len - 14, data + 14, packet, protocol); // Other ethernet frame types if ( protocol <= 1500 ) { - if ( data + 16 >= end_of_data ) + if ( 16 >= len ) { packet->Weird("truncated_ethernet_frame"); return AnalyzerResult::Failed; @@ -96,7 +93,7 @@ zeek::packet_analysis::AnalyzerResult EthernetAnalyzer::Analyze(Packet* packet, eth_analyzer = LLCAnalyzer; if ( eth_analyzer ) - return eth_analyzer->Analyze(packet, data); + return eth_analyzer->AnalyzePacket(len, data, packet); return AnalyzerResult::Terminate; } diff --git a/src/packet_analysis/protocol/ethernet/Ethernet.h b/src/packet_analysis/protocol/ethernet/Ethernet.h index c1f89a6f02..b69b4887ef 100644 --- a/src/packet_analysis/protocol/ethernet/Ethernet.h +++ b/src/packet_analysis/protocol/ethernet/Ethernet.h @@ -13,7 +13,7 @@ public: ~EthernetAnalyzer() override = default; void Initialize() override; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/fddi/FDDI.cc b/src/packet_analysis/protocol/fddi/FDDI.cc index 25235cca3e..3612670f10 100644 --- a/src/packet_analysis/protocol/fddi/FDDI.cc +++ b/src/packet_analysis/protocol/fddi/FDDI.cc @@ -10,17 +10,17 @@ FDDIAnalyzer::FDDIAnalyzer() { } -zeek::packet_analysis::AnalyzerResult FDDIAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult FDDIAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - auto hdr_size = 13 + 8; // FDDI header + LLC + size_t hdr_size = 13 + 8; // FDDI header + LLC - if ( data + hdr_size >= packet->GetEndOfData() ) + if ( hdr_size >= len ) { packet->Weird("FDDI_analyzer_failed"); return AnalyzerResult::Failed; } // We just skip the header and hope for default analysis - data += hdr_size; - return AnalyzeInnerPacket(packet, data); + return ForwardPacket(len - hdr_size, data + hdr_size, packet); } diff --git a/src/packet_analysis/protocol/fddi/FDDI.h b/src/packet_analysis/protocol/fddi/FDDI.h index 4219529c6a..d0e204e7d9 100644 --- a/src/packet_analysis/protocol/fddi/FDDI.h +++ b/src/packet_analysis/protocol/fddi/FDDI.h @@ -12,7 +12,7 @@ public: FDDIAnalyzer(); ~FDDIAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/ieee802_11/IEEE802_11.cc b/src/packet_analysis/protocol/ieee802_11/IEEE802_11.cc index f0a9720605..117677b535 100644 --- a/src/packet_analysis/protocol/ieee802_11/IEEE802_11.cc +++ b/src/packet_analysis/protocol/ieee802_11/IEEE802_11.cc @@ -10,13 +10,12 @@ IEEE802_11Analyzer::IEEE802_11Analyzer() { } -zeek::packet_analysis::AnalyzerResult IEEE802_11Analyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult IEEE802_11Analyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - auto end_of_data = packet->GetEndOfData(); - u_char len_80211 = 24; // minimal length of data frames - if ( data + len_80211 >= end_of_data ) + if ( len_80211 >= len ) { packet->Weird("truncated_802_11_header"); return AnalyzerResult::Failed; @@ -48,7 +47,7 @@ zeek::packet_analysis::AnalyzerResult IEEE802_11Analyzer::Analyze(Packet* packet len_80211 += 2; } - if ( data + len_80211 >= end_of_data ) + if ( len_80211 >= len ) { packet->Weird("truncated_802_11_header"); return AnalyzerResult::Failed; @@ -82,7 +81,8 @@ zeek::packet_analysis::AnalyzerResult IEEE802_11Analyzer::Analyze(Packet* packet // skip 802.11 data header data += len_80211; - if ( data + 8 >= end_of_data ) + len_80211 += 8; + if ( len_80211 >= len ) { packet->Weird("truncated_802_11_header"); return AnalyzerResult::Failed; @@ -108,5 +108,5 @@ zeek::packet_analysis::AnalyzerResult IEEE802_11Analyzer::Analyze(Packet* packet uint32_t protocol = (data[0] << 8) + data[1]; data += 2; - return AnalyzeInnerPacket(packet, data, protocol); + return ForwardPacket(len - len_80211, data, packet, protocol); } diff --git a/src/packet_analysis/protocol/ieee802_11/IEEE802_11.h b/src/packet_analysis/protocol/ieee802_11/IEEE802_11.h index 842f182bcd..a9f4916654 100644 --- a/src/packet_analysis/protocol/ieee802_11/IEEE802_11.h +++ b/src/packet_analysis/protocol/ieee802_11/IEEE802_11.h @@ -12,7 +12,7 @@ public: IEEE802_11Analyzer(); ~IEEE802_11Analyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/ieee802_11_radio/IEEE802_11_Radio.cc b/src/packet_analysis/protocol/ieee802_11_radio/IEEE802_11_Radio.cc index 703906ac82..68eea492da 100644 --- a/src/packet_analysis/protocol/ieee802_11_radio/IEEE802_11_Radio.cc +++ b/src/packet_analysis/protocol/ieee802_11_radio/IEEE802_11_Radio.cc @@ -12,26 +12,23 @@ IEEE802_11_RadioAnalyzer::IEEE802_11_RadioAnalyzer() { } -zeek::packet_analysis::AnalyzerResult IEEE802_11_RadioAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult IEEE802_11_RadioAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - auto end_of_data = packet->GetEndOfData(); - - if ( data + 3 >= end_of_data ) + if ( 3 >= len ) { packet->Weird("truncated_radiotap_header"); return AnalyzerResult::Failed; } // Skip over the RadioTap header - int rtheader_len = (data[3] << 8) + data[2]; + size_t rtheader_len = (data[3] << 8) + data[2]; - if ( data + rtheader_len >= end_of_data ) + if ( rtheader_len >= len ) { packet->Weird("truncated_radiotap_header"); return AnalyzerResult::Failed; } - data += rtheader_len; - - return AnalyzeInnerPacket(packet, data, DLT_IEEE802_11); + return ForwardPacket(len - rtheader_len, data + rtheader_len, packet, DLT_IEEE802_11); } diff --git a/src/packet_analysis/protocol/ieee802_11_radio/IEEE802_11_Radio.h b/src/packet_analysis/protocol/ieee802_11_radio/IEEE802_11_Radio.h index e9f306ef26..9f75eece30 100644 --- a/src/packet_analysis/protocol/ieee802_11_radio/IEEE802_11_Radio.h +++ b/src/packet_analysis/protocol/ieee802_11_radio/IEEE802_11_Radio.h @@ -12,7 +12,7 @@ public: IEEE802_11_RadioAnalyzer(); ~IEEE802_11_RadioAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/ip/IP.cc b/src/packet_analysis/protocol/ip/IP.cc index d6c2b91e9a..1e0de7d171 100644 --- a/src/packet_analysis/protocol/ip/IP.cc +++ b/src/packet_analysis/protocol/ip/IP.cc @@ -10,10 +10,11 @@ IPAnalyzer::IPAnalyzer() { } -zeek::packet_analysis::AnalyzerResult IPAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult IPAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { // Assume we're pointing at IP. Just figure out which version. - if ( data + sizeof(struct ip) >= packet->GetEndOfData() ) + if ( sizeof(struct ip) >= len ) { packet->Weird("packet_analyzer_truncated_header"); return AnalyzerResult::Failed; @@ -23,7 +24,6 @@ zeek::packet_analysis::AnalyzerResult IPAnalyzer::Analyze(Packet* packet, const uint32_t protocol = ip->ip_v; auto inner_analyzer = Lookup(protocol); - if ( inner_analyzer == nullptr ) { DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s failed, could not find analyzer for identifier %#x.", @@ -34,5 +34,5 @@ zeek::packet_analysis::AnalyzerResult IPAnalyzer::Analyze(Packet* packet, const DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s succeeded, next layer identifier is %#x.", GetAnalyzerName(), protocol); - return inner_analyzer->Analyze(packet, data); + return inner_analyzer->AnalyzePacket(len, data, packet); } \ No newline at end of file diff --git a/src/packet_analysis/protocol/ip/IP.h b/src/packet_analysis/protocol/ip/IP.h index f57012247c..7fd5d7a799 100644 --- a/src/packet_analysis/protocol/ip/IP.h +++ b/src/packet_analysis/protocol/ip/IP.h @@ -12,7 +12,7 @@ public: IPAnalyzer(); ~IPAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/ipv4/IPv4.cc b/src/packet_analysis/protocol/ipv4/IPv4.cc index 57aef8eb25..958089c52b 100644 --- a/src/packet_analysis/protocol/ipv4/IPv4.cc +++ b/src/packet_analysis/protocol/ipv4/IPv4.cc @@ -9,9 +9,11 @@ IPv4Analyzer::IPv4Analyzer() { } -zeek::packet_analysis::AnalyzerResult IPv4Analyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult IPv4Analyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { packet->l3_proto = L3_IPV4; + packet->hdr_size = static_cast(data - packet->data); // Leave packet analyzer land return AnalyzerResult::Terminate; diff --git a/src/packet_analysis/protocol/ipv4/IPv4.h b/src/packet_analysis/protocol/ipv4/IPv4.h index 984ad4e532..b2f01e4d34 100644 --- a/src/packet_analysis/protocol/ipv4/IPv4.h +++ b/src/packet_analysis/protocol/ipv4/IPv4.h @@ -12,7 +12,7 @@ public: IPv4Analyzer(); ~IPv4Analyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/ipv6/IPv6.cc b/src/packet_analysis/protocol/ipv6/IPv6.cc index 903dfd1607..e36444d296 100644 --- a/src/packet_analysis/protocol/ipv6/IPv6.cc +++ b/src/packet_analysis/protocol/ipv6/IPv6.cc @@ -9,9 +9,11 @@ IPv6Analyzer::IPv6Analyzer() { } -zeek::packet_analysis::AnalyzerResult IPv6Analyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult IPv6Analyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { packet->l3_proto = L3_IPV6; + packet->hdr_size = static_cast(data - packet->data); // Leave packet analyzer land return AnalyzerResult::Terminate; diff --git a/src/packet_analysis/protocol/ipv6/IPv6.h b/src/packet_analysis/protocol/ipv6/IPv6.h index ffff59a668..1a03540cf9 100644 --- a/src/packet_analysis/protocol/ipv6/IPv6.h +++ b/src/packet_analysis/protocol/ipv6/IPv6.h @@ -12,7 +12,7 @@ public: IPv6Analyzer(); ~IPv6Analyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/linux_sll/LinuxSLL.cc b/src/packet_analysis/protocol/linux_sll/LinuxSLL.cc index 740b63a518..2c998dc8d9 100644 --- a/src/packet_analysis/protocol/linux_sll/LinuxSLL.cc +++ b/src/packet_analysis/protocol/linux_sll/LinuxSLL.cc @@ -9,9 +9,11 @@ LinuxSLLAnalyzer::LinuxSLLAnalyzer() { } -zeek::packet_analysis::AnalyzerResult LinuxSLLAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult LinuxSLLAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - if ( data + sizeof(SLLHeader) >= packet->GetEndOfData() ) + auto len_sll_hdr = sizeof(SLLHeader); + if ( len_sll_hdr >= len ) { packet->Weird("truncated_Linux_SLL_header"); return AnalyzerResult::Failed; @@ -27,6 +29,5 @@ zeek::packet_analysis::AnalyzerResult LinuxSLLAnalyzer::Analyze(Packet* packet, // here will cause crashes elsewhere. packet->l2_dst = Packet::L2_EMPTY_ADDR; - data += sizeof(SLLHeader); - return AnalyzeInnerPacket(packet, data, protocol); + return ForwardPacket(len - len_sll_hdr, data + len_sll_hdr, packet, protocol); } diff --git a/src/packet_analysis/protocol/linux_sll/LinuxSLL.h b/src/packet_analysis/protocol/linux_sll/LinuxSLL.h index b62b3a3f59..65225a1fe6 100644 --- a/src/packet_analysis/protocol/linux_sll/LinuxSLL.h +++ b/src/packet_analysis/protocol/linux_sll/LinuxSLL.h @@ -12,7 +12,7 @@ public: LinuxSLLAnalyzer(); ~LinuxSLLAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/mpls/MPLS.cc b/src/packet_analysis/protocol/mpls/MPLS.cc index 962e206239..f0432a0e17 100644 --- a/src/packet_analysis/protocol/mpls/MPLS.cc +++ b/src/packet_analysis/protocol/mpls/MPLS.cc @@ -9,16 +9,15 @@ MPLSAnalyzer::MPLSAnalyzer() { } -zeek::packet_analysis::AnalyzerResult MPLSAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult MPLSAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - auto end_of_data = packet->GetEndOfData(); - // Skip the MPLS label stack. bool end_of_stack = false; while ( ! end_of_stack ) { - if ( data + 4 >= end_of_data ) + if ( 4 >= len ) { packet->Weird("truncated_link_header"); return AnalyzerResult::Failed; @@ -26,11 +25,13 @@ zeek::packet_analysis::AnalyzerResult MPLSAnalyzer::Analyze(Packet* packet, cons end_of_stack = *(data + 2u) & 0x01; data += 4; + len -= 4; } // According to RFC3032 the encapsulated protocol is not encoded. // We assume that what remains is IP. - if ( data + sizeof(struct ip) >= end_of_data ) + //TODO: Make that configurable + if ( sizeof(struct ip) >= len ) { packet->Weird("no_ip_in_mpls_payload"); return AnalyzerResult::Failed; diff --git a/src/packet_analysis/protocol/mpls/MPLS.h b/src/packet_analysis/protocol/mpls/MPLS.h index caade44f94..58c68b1aa4 100644 --- a/src/packet_analysis/protocol/mpls/MPLS.h +++ b/src/packet_analysis/protocol/mpls/MPLS.h @@ -12,7 +12,7 @@ public: MPLSAnalyzer(); ~MPLSAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/nflog/NFLog.cc b/src/packet_analysis/protocol/nflog/NFLog.cc index e2b7c218d2..49fd6a1656 100644 --- a/src/packet_analysis/protocol/nflog/NFLog.cc +++ b/src/packet_analysis/protocol/nflog/NFLog.cc @@ -10,8 +10,14 @@ NFLogAnalyzer::NFLogAnalyzer() { } -zeek::packet_analysis::AnalyzerResult NFLogAnalyzer::Analyze(Packet* packet, const uint8_t*& data) { - auto end_of_data = packet->GetEndOfData(); +zeek::packet_analysis::AnalyzerResult NFLogAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) + { + if ( 4 >= len ) + { + packet->Weird("truncated_nflog_header"); + return AnalyzerResult::Failed; + } // See https://www.tcpdump.org/linktypes/LINKTYPE_NFLOG.html uint32_t protocol = data[0]; @@ -25,13 +31,14 @@ zeek::packet_analysis::AnalyzerResult NFLogAnalyzer::Analyze(Packet* packet, con // Skip to TLVs. data += 4; + len -= 4; uint16_t tlv_len; uint16_t tlv_type; while ( true ) { - if ( data + 4 >= end_of_data ) + if ( 4 >= len ) { packet->Weird("nflog_no_pcap_payload"); return AnalyzerResult::Failed; @@ -49,6 +56,7 @@ zeek::packet_analysis::AnalyzerResult NFLogAnalyzer::Analyze(Packet* packet, con { // The raw packet payload follows this TLV. data += 4; + len -= 4; break; } else @@ -72,8 +80,9 @@ zeek::packet_analysis::AnalyzerResult NFLogAnalyzer::Analyze(Packet* packet, con } data += tlv_len; + len -= tlv_len; } } - return AnalyzeInnerPacket(packet, data, protocol); + return ForwardPacket(len, data, packet, protocol); } diff --git a/src/packet_analysis/protocol/nflog/NFLog.h b/src/packet_analysis/protocol/nflog/NFLog.h index 6cb1335373..9b725565f9 100644 --- a/src/packet_analysis/protocol/nflog/NFLog.h +++ b/src/packet_analysis/protocol/nflog/NFLog.h @@ -12,7 +12,7 @@ public: NFLogAnalyzer(); ~NFLogAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/null/Null.cc b/src/packet_analysis/protocol/null/Null.cc index bac13dcf07..1e54ceaab1 100644 --- a/src/packet_analysis/protocol/null/Null.cc +++ b/src/packet_analysis/protocol/null/Null.cc @@ -10,16 +10,16 @@ NullAnalyzer::NullAnalyzer() { } -zeek::packet_analysis::AnalyzerResult NullAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult NullAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - if ( data + 4 >= packet->GetEndOfData() ) + if ( 4 >= len ) { packet->Weird("null_analyzer_failed"); return AnalyzerResult::Failed; } uint32_t protocol = (data[3] << 24) + (data[2] << 16) + (data[1] << 8) + data[0]; - data += 4; // skip link header - - return AnalyzeInnerPacket(packet, data, protocol); + // skip link header + return ForwardPacket(len - 4, data + 4, packet, protocol); } diff --git a/src/packet_analysis/protocol/null/Null.h b/src/packet_analysis/protocol/null/Null.h index d25cf8a2d9..e82340a690 100644 --- a/src/packet_analysis/protocol/null/Null.h +++ b/src/packet_analysis/protocol/null/Null.h @@ -12,7 +12,7 @@ public: NullAnalyzer(); ~NullAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/ppp_serial/PPPSerial.cc b/src/packet_analysis/protocol/ppp_serial/PPPSerial.cc index 9ec9596c11..4b0531ba8e 100644 --- a/src/packet_analysis/protocol/ppp_serial/PPPSerial.cc +++ b/src/packet_analysis/protocol/ppp_serial/PPPSerial.cc @@ -10,11 +10,17 @@ PPPSerialAnalyzer::PPPSerialAnalyzer() { } -zeek::packet_analysis::AnalyzerResult PPPSerialAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult PPPSerialAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { + if ( 4 >= len ) + { + packet->Weird("truncated_ppp_serial_header"); + return AnalyzerResult::Failed; + } + // Extract protocol identifier uint32_t protocol = (data[2] << 8) + data[3]; - data += 4; // skip link header - - return AnalyzeInnerPacket(packet, data, protocol); + // skip link header + return ForwardPacket(len - 4, data + 4, packet, protocol); } diff --git a/src/packet_analysis/protocol/ppp_serial/PPPSerial.h b/src/packet_analysis/protocol/ppp_serial/PPPSerial.h index c10c34d92e..c9c067ccac 100644 --- a/src/packet_analysis/protocol/ppp_serial/PPPSerial.h +++ b/src/packet_analysis/protocol/ppp_serial/PPPSerial.h @@ -12,7 +12,7 @@ public: PPPSerialAnalyzer(); ~PPPSerialAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/pppoe/PPPoE.cc b/src/packet_analysis/protocol/pppoe/PPPoE.cc index 899f62d512..adbbb3fbe8 100644 --- a/src/packet_analysis/protocol/pppoe/PPPoE.cc +++ b/src/packet_analysis/protocol/pppoe/PPPoE.cc @@ -10,9 +10,10 @@ PPPoEAnalyzer::PPPoEAnalyzer() { } -zeek::packet_analysis::AnalyzerResult PPPoEAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult PPPoEAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - if ( data + 8 >= packet->GetEndOfData() ) + if ( 8 >= len ) { packet->Weird("truncated_pppoe_header"); return AnalyzerResult::Failed; @@ -20,7 +21,6 @@ zeek::packet_analysis::AnalyzerResult PPPoEAnalyzer::Analyze(Packet* packet, con // Extract protocol identifier uint32_t protocol = (data[6] << 8u) + data[7]; - data += 8; // Skip the PPPoE session and PPP header - - return AnalyzeInnerPacket(packet, data, protocol); + // Skip the PPPoE session and PPP header + return ForwardPacket(len - 8, data + 8, packet, protocol); } diff --git a/src/packet_analysis/protocol/pppoe/PPPoE.h b/src/packet_analysis/protocol/pppoe/PPPoE.h index 164a96b8e6..2c5113815b 100644 --- a/src/packet_analysis/protocol/pppoe/PPPoE.h +++ b/src/packet_analysis/protocol/pppoe/PPPoE.h @@ -12,7 +12,7 @@ public: PPPoEAnalyzer(); ~PPPoEAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/skip/Skip.cc b/src/packet_analysis/protocol/skip/Skip.cc index 966561c630..66002d0811 100644 --- a/src/packet_analysis/protocol/skip/Skip.cc +++ b/src/packet_analysis/protocol/skip/Skip.cc @@ -19,8 +19,8 @@ void SkipAnalyzer::Initialize() skip_bytes = skip_val->AsCount(); } -zeek::packet_analysis::AnalyzerResult SkipAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult SkipAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - data += skip_bytes; - return AnalyzeInnerPacket(packet, data); + return ForwardPacket(len - skip_bytes, data + skip_bytes, packet); } diff --git a/src/packet_analysis/protocol/skip/Skip.h b/src/packet_analysis/protocol/skip/Skip.h index a18a7c8bec..5cef785d69 100644 --- a/src/packet_analysis/protocol/skip/Skip.h +++ b/src/packet_analysis/protocol/skip/Skip.h @@ -13,7 +13,7 @@ public: ~SkipAnalyzer() override = default; void Initialize() override; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/src/packet_analysis/protocol/vlan/VLAN.cc b/src/packet_analysis/protocol/vlan/VLAN.cc index 364e1c9096..a2b245e1ff 100644 --- a/src/packet_analysis/protocol/vlan/VLAN.cc +++ b/src/packet_analysis/protocol/vlan/VLAN.cc @@ -10,9 +10,10 @@ VLANAnalyzer::VLANAnalyzer() { } -zeek::packet_analysis::AnalyzerResult VLANAnalyzer::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult VLANAnalyzer::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - if ( data + 4 >= packet->GetEndOfData() ) + if ( 4 >= len ) { packet->Weird("truncated_VLAN_header"); return AnalyzerResult::Failed; @@ -23,7 +24,6 @@ zeek::packet_analysis::AnalyzerResult VLANAnalyzer::Analyze(Packet* packet, cons uint32_t protocol = ((data[2] << 8u) + data[3]); packet->eth_type = protocol; - data += 4; // Skip the VLAN header - - return AnalyzeInnerPacket(packet, data, protocol); + // Skip the VLAN header + return ForwardPacket(len - 4, data + 4, packet, protocol); } diff --git a/src/packet_analysis/protocol/vlan/VLAN.h b/src/packet_analysis/protocol/vlan/VLAN.h index d2169374f1..0e1ffcfb92 100644 --- a/src/packet_analysis/protocol/vlan/VLAN.h +++ b/src/packet_analysis/protocol/vlan/VLAN.h @@ -12,7 +12,7 @@ public: VLANAnalyzer(); ~VLANAnalyzer() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static zeek::packet_analysis::AnalyzerPtr Instantiate() { diff --git a/testing/btest/plugins/packet-protocol-plugin/src/Bar.cc b/testing/btest/plugins/packet-protocol-plugin/src/Bar.cc index 27cf68235c..3781c62272 100644 --- a/testing/btest/plugins/packet-protocol-plugin/src/Bar.cc +++ b/testing/btest/plugins/packet-protocol-plugin/src/Bar.cc @@ -10,12 +10,11 @@ Bar::Bar() { } -zeek::packet_analysis::AnalyzerResult Bar::Analyze(Packet* packet, const uint8_t*& data) +zeek::packet_analysis::AnalyzerResult Bar::AnalyzePacket(size_t len, + const uint8_t* data, Packet* packet) { - auto end_of_data = packet->GetEndOfData(); - // Rudimentary parsing of 802.2 LLC - if ( data + 17 >= end_of_data ) + if ( 17 >= len ) { packet->Weird("truncated_llc_header"); return AnalyzerResult::Failed; diff --git a/testing/btest/plugins/packet-protocol-plugin/src/Bar.h b/testing/btest/plugins/packet-protocol-plugin/src/Bar.h index ad1ee8185e..e8d64e0783 100644 --- a/testing/btest/plugins/packet-protocol-plugin/src/Bar.h +++ b/testing/btest/plugins/packet-protocol-plugin/src/Bar.h @@ -10,7 +10,7 @@ public: Bar(); ~Bar() override = default; - AnalyzerResult Analyze(Packet* packet, const uint8_t*& data) override; + AnalyzerResult AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) override; static AnalyzerPtr Instantiate() {