diff --git a/src/IP.h b/src/IP.h index 7ec5b5ac79..cb012a434a 100644 --- a/src/IP.h +++ b/src/IP.h @@ -318,7 +318,7 @@ public: * already checked that the static IPv6 header is not truncated. If * the packet contains extension headers and they are truncated, that can * be checked afterwards by comparing \a len with \a TotalLen. E.g. - * NetSessions::DoNextPacket does this to skip truncated packets. + * The IP packet analyzer does this to skip truncated packets. * @param arg_ip6 pointer to memory containing an IPv6 packet. * @param arg_del whether to take ownership of \a arg_ip6 pointer's memory. * @param len the packet's length in bytes. diff --git a/src/Sessions.cc b/src/Sessions.cc index f8b0055b21..3f4e58a0ec 100644 --- a/src/Sessions.cc +++ b/src/Sessions.cc @@ -81,11 +81,10 @@ void NetSessions::NextPacket(double t, Packet* pkt) packet_mgr->ProcessPacket(pkt); } -void NetSessions::DoNextPacket(double t, const Packet* pkt) +void NetSessions::ProcessTransportLayer(double t, const Packet* pkt, size_t remaining) { const std::unique_ptr& ip_hdr = pkt->ip_hdr; - uint32_t caplen = pkt->cap_len - pkt->hdr_size; uint32_t len = ip_hdr->TotalLen(); uint16_t ip_hdr_len = ip_hdr->HdrLen(); @@ -95,18 +94,11 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt) return; } - if ( caplen < ip_hdr_len ) - { - sessions->Weird("truncated_IP", pkt); - return; - } - len -= ip_hdr_len; // remove IP header - caplen -= ip_hdr_len; // remove IP header int proto = ip_hdr->NextProto(); - if ( CheckHeaderTrunc(proto, len, caplen, pkt) ) + if ( CheckHeaderTrunc(proto, len, remaining, pkt) ) return; const u_char* data = ip_hdr->Payload(); @@ -232,7 +224,7 @@ void NetSessions::DoNextPacket(double t, const Packet* pkt) conn->EnqueueEvent(new_packet, nullptr, conn->ConnVal(), pkt_hdr_val ? std::move(pkt_hdr_val) : ip_hdr->ToPktHdrVal()); - conn->NextPacket(t, is_orig, ip_hdr.get(), len, caplen, data, + conn->NextPacket(t, is_orig, ip_hdr.get(), len, remaining, data, record_packet, record_content, pkt); // We skip this block for reassembled packets because the pointer diff --git a/src/Sessions.h b/src/Sessions.h index 0d202e6ad2..ad64d85a43 100644 --- a/src/Sessions.h +++ b/src/Sessions.h @@ -93,8 +93,13 @@ public: * Main entry point for processing packets destined for session analyzers. This * method is called by the packet analysis manager when after it has processed * an IP-based packet, and shouldn't be called directly from other places. + * + * @param t The timestamp for this packet. + * @param pkt The packet being processed. + * @param len The number of bytes that haven't been processed yet by packet + * analysis. */ - void DoNextPacket(double t, const Packet *pkt); + void ProcessTransportLayer(double t, const Packet *pkt, size_t len); /** * Returns a wrapper IP_Hdr object if \a pkt appears to be a valid IPv4 diff --git a/src/analyzer/protocol/vxlan/VXLAN.cc b/src/analyzer/protocol/vxlan/VXLAN.cc index 36d7fd7b22..c43ead89a0 100644 --- a/src/analyzer/protocol/vxlan/VXLAN.cc +++ b/src/analyzer/protocol/vxlan/VXLAN.cc @@ -75,25 +75,16 @@ void VXLAN_Analyzer::DeliverPacket(int len, const u_char* data, bool orig, Packet pkt(DLT_EN10MB, &ts, caplen, len, data); pkt.encap = outer; - packet_mgr->ProcessPacket(&pkt); - - if ( ! pkt.l2_valid ) + if ( ! packet_mgr->ProcessInnerPacket(&pkt) ) { - ProtocolViolation("VXLAN invalid inner ethernet frame", - (const char*) data, len); + ProtocolViolation("VXLAN invalid inner packet"); return; } - data += pkt.hdr_size; - len -= pkt.hdr_size; - caplen -= pkt.hdr_size; - + // This isn't really an error. It's just that the inner packet wasn't an IP packet (like ARP). + // Just return without reporting a violation. if ( ! pkt.ip_hdr ) - { - ProtocolViolation("Truncated VXLAN or invalid inner IP", - (const char*) data, len); return; - } ProtocolConfirmation(); diff --git a/src/iosource/Packet.cc b/src/iosource/Packet.cc index 183ba1f80b..b4f1909aa3 100644 --- a/src/iosource/Packet.cc +++ b/src/iosource/Packet.cc @@ -49,7 +49,6 @@ void Packet::Init(int arg_link_type, pkt_timeval *arg_ts, uint32_t arg_caplen, dump_packet = false; time = ts.tv_sec + double(ts.tv_usec) / 1e6; - hdr_size = 0; eth_type = 0; vlan = 0; inner_vlan = 0; @@ -84,11 +83,6 @@ Packet::~Packet() delete [] data; } -const IP_Hdr Packet::IP() const - { - return IP_Hdr((struct ip *) (data + hdr_size), false); - } - void Packet::Weird(const char* name) { sessions->Weird(name, this); diff --git a/src/iosource/Packet.h b/src/iosource/Packet.h index da2332b3bc..17939699ca 100644 --- a/src/iosource/Packet.h +++ b/src/iosource/Packet.h @@ -114,12 +114,6 @@ public: uint32_t len, const u_char *data, bool copy = false, std::string tag = ""); - /** - * Interprets the Layer 3 of the packet as IP and returns a - * corresponding object. - */ - const IP_Hdr IP() const; - /** * Returns a \c raw_pkt_hdr RecordVal, which includes layer 2 and * also everything in IP_Hdr (i.e., IP4/6 + TCP/UDP/ICMP). @@ -156,11 +150,6 @@ public: // These are computed from Layer 2 data. These fields are only valid if // l2_valid returns true. - /** - * Layer 2 header size. Valid iff l2_valid is true. - */ - uint32_t hdr_size; - /** * Layer 3 protocol identified (if any). Valid iff l2_valid is true. */ diff --git a/src/packet_analysis/protocol/ip/IP.cc b/src/packet_analysis/protocol/ip/IP.cc index 2978db9420..ef1deaeac6 100644 --- a/src/packet_analysis/protocol/ip/IP.cc +++ b/src/packet_analysis/protocol/ip/IP.cc @@ -39,10 +39,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) return false; } - // TODO: i feel like this could be generated as we move along the header hierarchy. - // TODO: the sessions code expects that the header size does not include the ip header. Should - // this change? - packet->hdr_size = static_cast(data - packet->data); + int32_t hdr_size = static_cast(data - packet->data); // Cast the current data pointer to an IP header pointer so we can use it to get some // data about the header. @@ -82,10 +79,10 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) sessions->Weird("ip_hdr_len_zero", packet); // Cope with the zero'd out ip_len field by using the caplen. - total_len = packet->cap_len - packet->hdr_size; + total_len = packet->cap_len - hdr_size; } - if ( packet->len < total_len + packet->hdr_size ) + if ( packet->len < total_len + hdr_size ) { sessions->Weird("truncated_IPv6", packet); return false; @@ -158,7 +155,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) else { f = detail::fragment_mgr->NextFragment(run_state::processing_start_time, packet->ip_hdr, - packet->data + packet->hdr_size); + packet->data + hdr_size); std::unique_ptr ih = f->ReassembledPkt(); if ( ! ih ) @@ -173,7 +170,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) len = total_len = packet->ip_hdr->TotalLen(); ip_hdr_len = packet->ip_hdr->HdrLen(); - packet->cap_len = total_len + packet->hdr_size; + packet->cap_len = total_len + hdr_size; if ( ip_hdr_len > total_len ) { @@ -243,7 +240,7 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) case IPPROTO_ICMPV6: DBG_LOG(DBG_PACKET_ANALYSIS, "Analysis in %s succeeded, next layer identifier is %#x.", GetAnalyzerName(), proto); - sessions->DoNextPacket(run_state::processing_start_time, packet); + sessions->ProcessTransportLayer(run_state::processing_start_time, packet, len); break; case IPPROTO_NONE: // If the packet is encapsulated in Teredo, then it was a bubble and diff --git a/src/packet_analysis/protocol/iptunnel/IPTunnel.cc b/src/packet_analysis/protocol/iptunnel/IPTunnel.cc index 51bf9f5997..45b18723ea 100644 --- a/src/packet_analysis/protocol/iptunnel/IPTunnel.cc +++ b/src/packet_analysis/protocol/iptunnel/IPTunnel.cc @@ -128,7 +128,8 @@ bool IPTunnelAnalyzer::ProcessEncapsulatedPacket(double t, const Packet* pkt, auto outer = prev ? prev : std::make_shared(); outer->Add(ec); - // Construct fake packet for DoNextPacket + // Construct fake packet containing the inner packet so it can be processed + // like a normal one. Packet p; p.Init(DLT_RAW, &ts, caplen, len, data, false, ""); p.encap = outer; @@ -164,7 +165,8 @@ bool IPTunnelAnalyzer::ProcessEncapsulatedPacket(double t, const Packet* pkt, auto outer = prev ? prev : std::make_shared(); outer->Add(ec); - // Construct fake packet for DoNextPacket + // Construct fake packet containing the inner packet so it can be processed + // like a normal one. Packet p; p.Init(link_type, &ts, caplen, len, data, false, ""); p.encap = outer; diff --git a/src/packet_analysis/protocol/iptunnel/IPTunnel.h b/src/packet_analysis/protocol/iptunnel/IPTunnel.h index 2928f679d0..ddc8e04466 100644 --- a/src/packet_analysis/protocol/iptunnel/IPTunnel.h +++ b/src/packet_analysis/protocol/iptunnel/IPTunnel.h @@ -30,9 +30,9 @@ public: * * @param t Network time. * @param pkt If the outer pcap header is available, this pointer can be set - * so that the fake pcap header passed to DoNextPacket will use - * the same timeval. The caplen and len fields of the fake pcap - * header are always set to the TotalLength() of \a inner. + * so the fake pcap header passed to the next analyzer will use the + * same timeval. The caplen and len fields of the fake pcap header + * are always set to the TotalLength() of \a inner. * @param inner Pointer to IP header wrapper of the inner packet, ownership * of the pointer's memory is assumed by this function. * @param prev Any previous encapsulation stack of the caller, not including @@ -48,15 +48,15 @@ public: * Wrapper that handles encapsulated Ethernet/IP packets and passes them back into * packet analysis. * - * @param t Network time. - * @param pkt If the outer pcap header is available, this pointer can be - * set so that the fake pcap header passed to DoNextPacket will use - * the same timeval. - * @param caplen number of captured bytes remaining - * @param len number of bytes remaining as claimed by outer framing - * @param data the remaining packet data - * @param link_type layer 2 link type used for initializing inner packet - * @param prev Any previous encapsulation stack of the caller, not + * @param t Network time. + * @param pkt If the outer pcap header is available, this pointer can be set + * so the fake pcap header passed to the next analyzer will use the + * same timeval. + * @param caplen Number of captured bytes remaining + * @param len Number of bytes remaining as claimed by outer framing + * @param data The remaining packet data + * @param link_type Layer 2 link type used for initializing inner packet + * @param prev Any previous encapsulation stack of the caller, not * including the most-recently found depth of encapsulation. * @param ec The most-recently found depth of encapsulation. */ diff --git a/src/packet_analysis/protocol/wrapper/Wrapper.cc b/src/packet_analysis/protocol/wrapper/Wrapper.cc index 5d431af2c6..a08581aece 100644 --- a/src/packet_analysis/protocol/wrapper/Wrapper.cc +++ b/src/packet_analysis/protocol/wrapper/Wrapper.cc @@ -155,8 +155,5 @@ bool WrapperAnalyzer::Analyze(Packet* packet, const uint8_t*& data) } } - // Calculate how much header we've used up. - packet->hdr_size = (data - packet->data); - return AnalyzeInnerPacket(packet, data, protocol); }