From 16f6cafd9a6b63a0ba43a283d1a57d73e045acdc Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 13 Feb 2023 16:42:11 -0700 Subject: [PATCH 1/2] Add validation of session to start of AYIYA/VXLAN/Geneve analysis This mimics how the Teredo analyzer is already doing it, including sending a weird if the session is invalid and bailing out if the protocol was already violated. --- src/packet_analysis/protocol/ayiya/AYIYA.cc | 11 +++++++++++ src/packet_analysis/protocol/geneve/Geneve.cc | 11 +++++++++++ src/packet_analysis/protocol/vxlan/VXLAN.cc | 11 +++++++++++ 3 files changed, 33 insertions(+) diff --git a/src/packet_analysis/protocol/ayiya/AYIYA.cc b/src/packet_analysis/protocol/ayiya/AYIYA.cc index 203f354dc2..70b46c3df2 100644 --- a/src/packet_analysis/protocol/ayiya/AYIYA.cc +++ b/src/packet_analysis/protocol/ayiya/AYIYA.cc @@ -13,6 +13,17 @@ bool AYIYAAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packe if ( ! BifConst::Tunnel::enable_ayiya ) return false; + // AYIYA always comes from a TCP or UDP connection, which means that session + // should always be valid and always be a connection. Return a weird if we + // didn't have a session stored. + if ( ! packet->session ) + { + Analyzer::Weird("ayiya_missing_connection"); + return false; + } + else if ( AnalyzerViolated(packet->session) ) + return false; + if ( packet->encap && packet->encap->Depth() >= BifConst::Tunnel::max_depth ) { Weird("exceeded_tunnel_max_depth", packet); diff --git a/src/packet_analysis/protocol/geneve/Geneve.cc b/src/packet_analysis/protocol/geneve/Geneve.cc index 8dac14533b..c74711c0cb 100644 --- a/src/packet_analysis/protocol/geneve/Geneve.cc +++ b/src/packet_analysis/protocol/geneve/Geneve.cc @@ -11,6 +11,17 @@ GeneveAnalyzer::GeneveAnalyzer() : zeek::packet_analysis::Analyzer("Geneve") { } bool GeneveAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) { + // Geneve always comes from a UDP connection, which means that session should always + // be valid and always be a connection. Return a weird if we didn't have a session + // stored. + if ( ! packet->session ) + { + Analyzer::Weird("geneve_missing_connection"); + return false; + } + else if ( AnalyzerViolated(packet->session) ) + return false; + if ( packet->encap && packet->encap->Depth() >= BifConst::Tunnel::max_depth ) { Weird("exceeded_tunnel_max_depth", packet); diff --git a/src/packet_analysis/protocol/vxlan/VXLAN.cc b/src/packet_analysis/protocol/vxlan/VXLAN.cc index 2c063b329d..227df039c6 100644 --- a/src/packet_analysis/protocol/vxlan/VXLAN.cc +++ b/src/packet_analysis/protocol/vxlan/VXLAN.cc @@ -11,6 +11,17 @@ VXLAN_Analyzer::VXLAN_Analyzer() : zeek::packet_analysis::Analyzer("VXLAN") { } bool VXLAN_Analyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) { + // VXLAN always comes from a UDP connection, which means that session should always + // be valid and always be a connection. Return a weird if we didn't have a session + // stored. + if ( ! packet->session ) + { + Analyzer::Weird("vxlan_missing_connection"); + return false; + } + else if ( AnalyzerViolated(packet->session) ) + return false; + if ( packet->encap && packet->encap->Depth() >= BifConst::Tunnel::max_depth ) { Weird("exceeded_tunnel_max_depth", packet); From 02b3202453a7c3c9b3eb57d0cd80b15df7f258cd Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Mon, 13 Feb 2023 16:43:36 -0700 Subject: [PATCH 2/2] Call AnalyzerConfirmation earlier in VXLAN/Geneve analysis --- src/packet_analysis/protocol/geneve/Geneve.cc | 21 +++++++------------ src/packet_analysis/protocol/vxlan/VXLAN.cc | 19 +++++++---------- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/packet_analysis/protocol/geneve/Geneve.cc b/src/packet_analysis/protocol/geneve/Geneve.cc index c74711c0cb..219c8ba7be 100644 --- a/src/packet_analysis/protocol/geneve/Geneve.cc +++ b/src/packet_analysis/protocol/geneve/Geneve.cc @@ -70,6 +70,9 @@ bool GeneveAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack len -= hdr_size; data += hdr_size; + // We've successfully parsed everything, so we might as well confirm this. + AnalyzerConfirmation(packet->session); + int encap_index = 0; auto inner_packet = packet_analysis::IPTunnel::build_inner_packet( packet, &encap_index, nullptr, len, data, DLT_RAW, BifEnum::Tunnel::GENEVE, @@ -81,21 +84,13 @@ bool GeneveAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack if ( len > hdr_size ) fwd_ret_val = ForwardPacket(len, data, inner_packet.get(), next_header); - if ( fwd_ret_val ) + if ( fwd_ret_val && geneve_packet ) { - AnalyzerConfirmation(packet->session); - - if ( geneve_packet && packet->session ) - { - EncapsulatingConn* ec = inner_packet->encap->At(encap_index); - if ( ec && ec->ip_hdr ) - inner_packet->session->EnqueueEvent(geneve_packet, nullptr, - packet->session->GetVal(), - ec->ip_hdr->ToPktHdrVal(), val_mgr->Count(vni)); - } + EncapsulatingConn* ec = inner_packet->encap->At(encap_index); + if ( ec && ec->ip_hdr ) + inner_packet->session->EnqueueEvent(geneve_packet, nullptr, packet->session->GetVal(), + ec->ip_hdr->ToPktHdrVal(), val_mgr->Count(vni)); } - else - AnalyzerViolation("Geneve invalid inner packet", packet->session); return fwd_ret_val; } diff --git a/src/packet_analysis/protocol/vxlan/VXLAN.cc b/src/packet_analysis/protocol/vxlan/VXLAN.cc index 227df039c6..ed8b93c4df 100644 --- a/src/packet_analysis/protocol/vxlan/VXLAN.cc +++ b/src/packet_analysis/protocol/vxlan/VXLAN.cc @@ -47,6 +47,9 @@ bool VXLAN_Analyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack len -= hdr_size; data += hdr_size; + // We've successfully parsed everything, so we might as well confirm this. + AnalyzerConfirmation(packet->session); + int encap_index = 0; auto inner_packet = packet_analysis::IPTunnel::build_inner_packet( packet, &encap_index, nullptr, len, data, DLT_RAW, BifEnum::Tunnel::VXLAN, @@ -56,18 +59,12 @@ bool VXLAN_Analyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* pack if ( len > hdr_size ) fwd_ret_val = ForwardPacket(len, data, inner_packet.get()); - if ( fwd_ret_val ) + if ( fwd_ret_val && vxlan_packet ) { - AnalyzerConfirmation(packet->session); - - if ( vxlan_packet && packet->session ) - { - EncapsulatingConn* ec = inner_packet->encap->At(encap_index); - if ( ec && ec->ip_hdr ) - inner_packet->session->EnqueueEvent(vxlan_packet, nullptr, - packet->session->GetVal(), - ec->ip_hdr->ToPktHdrVal(), val_mgr->Count(vni)); - } + EncapsulatingConn* ec = inner_packet->encap->At(encap_index); + if ( ec && ec->ip_hdr ) + inner_packet->session->EnqueueEvent(vxlan_packet, nullptr, packet->session->GetVal(), + ec->ip_hdr->ToPktHdrVal(), val_mgr->Count(vni)); } return fwd_ret_val;