From: Tom Peters (thopeter) Date: Tue, 29 Jun 2021 16:39:28 +0000 (+0000) Subject: Merge pull request #2956 in SNORT/snort3 from ~KATHARVE/snort3:http_ooo_injection... X-Git-Tag: 3.1.7.0~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=825630379bb08b7803a3fa34dd16c5b5c7e534e1;p=thirdparty%2Fsnort3.git Merge pull request #2956 in SNORT/snort3 from ~KATHARVE/snort3:http_ooo_injection to master Squashed commit of the following: commit 89629a45d15511a400494d22d3921540476036ec Author: Katura Harvey Date: Fri Jun 25 12:31:16 2021 -0400 payload_injector: don't inject if there are unflushed S2C TCP packets queued --- diff --git a/src/flow/session.h b/src/flow/session.h index 1e19849a5..22146ceea 100644 --- a/src/flow/session.h +++ b/src/flow/session.h @@ -72,6 +72,7 @@ public: virtual bool is_sequenced(uint8_t /*dir*/) { return true; } virtual bool are_packets_missing(uint8_t /*dir*/) { return true; } + virtual bool are_client_segments_queued() { return false; } virtual void disable_reassembly(snort::Flow*) { } virtual uint8_t get_reassembly_direction() { return SSN_DIR_NONE; } diff --git a/src/payload_injector/payload_injector.cc b/src/payload_injector/payload_injector.cc index 672c93a5d..b86a4c2f4 100644 --- a/src/payload_injector/payload_injector.cc +++ b/src/payload_injector/payload_injector.cc @@ -25,6 +25,7 @@ #include "payload_injector.h" #include "detection/detection_engine.h" +#include "flow/session.h" #include "packet_io/active.h" #include "protocols/packet.h" #include "service_inspectors/http2_inspect/http2_flow_data.h" @@ -48,7 +49,8 @@ static const std::map InjectionErrorToStrin { ERR_TRANSLATED_HDRS_SIZE, "HTTP/2 translated header size is bigger than expected. Update max size." }, { ERR_HTTP2_EVEN_STREAM_ID, "HTTP/2 - injection to server initiated stream" }, - { ERR_PKT_FROM_SERVER, "Packet is from server" } + { ERR_PKT_FROM_SERVER, "Packet is from server" }, + { ERR_CONFLICTING_S2C_TRAFFIC, "Conflicting S2C HTTP traffic in progress" } }; InjectionReturnStatus PayloadInjector::inject_http2_payload(Packet* p, @@ -74,6 +76,11 @@ InjectionReturnStatus PayloadInjector::inject_http2_payload(Packet* p, // FIXIT-E mid-frame injection not supported status = ERR_HTTP2_MID_FRAME; } + else if (p->flow->session and + p->flow->session->are_client_segments_queued()) + { + status = ERR_CONFLICTING_S2C_TRAFFIC; + } else { uint8_t* http2_payload; @@ -120,8 +127,17 @@ InjectionReturnStatus PayloadInjector::inject_http_payload(Packet* p, else if (!p->flow->gadget || strcmp(p->flow->gadget->get_name(),"http_inspect") == 0) { - payload_injector_stats.http_injects++; - p->active->send_data(p, df, control.http_page, control.http_page_len); + if (p->flow->session and + p->flow->session->are_client_segments_queued()) + { + status = ERR_CONFLICTING_S2C_TRAFFIC; + p->active->send_data(p, df, nullptr, 0); // To send reset + } + else + { + payload_injector_stats.http_injects++; + p->active->send_data(p, df, control.http_page, control.http_page_len); + } } else if (strcmp(p->flow->gadget->get_name(),"http2_inspect") == 0) status = inject_http2_payload(p, control, df); diff --git a/src/payload_injector/payload_injector.h b/src/payload_injector/payload_injector.h index 1cb0fe68e..c95e717a4 100644 --- a/src/payload_injector/payload_injector.h +++ b/src/payload_injector/payload_injector.h @@ -40,6 +40,7 @@ enum InjectionReturnStatus : int8_t ERR_TRANSLATED_HDRS_SIZE = -7, ERR_HTTP2_EVEN_STREAM_ID = -8, ERR_PKT_FROM_SERVER = -9, + ERR_CONFLICTING_S2C_TRAFFIC = -10, // Update InjectionErrorToString when adding/removing error codes }; diff --git a/src/payload_injector/test/payload_injector_test.cc b/src/payload_injector/test/payload_injector_test.cc index f0feb020a..deb39954d 100644 --- a/src/payload_injector/test/payload_injector_test.cc +++ b/src/payload_injector/test/payload_injector_test.cc @@ -430,6 +430,18 @@ TEST(payload_injector_translate_err_test, http2_hdrs_size) "HTTP/2 translated header size is bigger than expected. Update max size.") == 0); } +TEST(payload_injector_translate_err_test, conflicting_s2c_traffic) +{ + Packet p(false); + set_configured(); + p.packet_flags = PKT_STREAM_EST; + p.flow = &flow; + translation_status = ERR_CONFLICTING_S2C_TRAFFIC; + status = PayloadInjector::inject_http_payload(&p, control); + const char* err_string = PayloadInjector::get_err_string(status); + CHECK(strcmp(err_string, + "Conflicting S2C HTTP traffic in progress") == 0); +} int main(int argc, char** argv) { return CommandLineTestRunner::RunAllTests(argc, argv); diff --git a/src/stream/tcp/tcp_stream_session.cc b/src/stream/tcp/tcp_stream_session.cc index 0a37dabe4..0d154abf9 100644 --- a/src/stream/tcp/tcp_stream_session.cc +++ b/src/stream/tcp/tcp_stream_session.cc @@ -229,6 +229,11 @@ bool TcpStreamSession::are_packets_missing(uint8_t dir) return false; } +bool TcpStreamSession::are_client_segments_queued() +{ + return client.reassembler.get_seg_count() > 0; +} + bool TcpStreamSession::add_alert(Packet* p, uint32_t gid, uint32_t sid) { TcpReassemblerPolicy& trp = p->ptrs.ip_api.get_src()->equals(flow->client_ip) ? diff --git a/src/stream/tcp/tcp_stream_session.h b/src/stream/tcp/tcp_stream_session.h index f3f33c4ed..c5f85be9e 100644 --- a/src/stream/tcp/tcp_stream_session.h +++ b/src/stream/tcp/tcp_stream_session.h @@ -49,6 +49,7 @@ public: void disable_reassembly(snort::Flow*) override; uint8_t get_reassembly_direction() override; uint8_t missing_in_reassembled(uint8_t dir) override; + bool are_client_segments_queued() override; bool add_alert(snort::Packet*, uint32_t gid, uint32_t sid) override; bool check_alerted(snort::Packet*, uint32_t gid, uint32_t sid) override;