From: Tom Peters (thopeter) Date: Tue, 18 Oct 2022 19:51:00 +0000 (+0000) Subject: Pull request #3616: http_inspect: maximum_pipelined_requests X-Git-Tag: 3.1.45.0~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0fd283b530dcdc9a191dcbff7df0dae7335b217b;p=thirdparty%2Fsnort3.git Pull request #3616: http_inspect: maximum_pipelined_requests Merge in SNORT/snort3 from ~ADMAMOLE/snort3:pipeline to master Squashed commit of the following: commit fb53e1c4acacf776a7c20658dd638318c4ecfd2a Author: Adrian Mamolea Date: Fri Sep 30 16:41:01 2022 -0400 http_inspect: maximum_pipelined_requests --- diff --git a/doc/reference/builtin_stubs.txt b/doc/reference/builtin_stubs.txt index 8f60f6c5d..407c639c6 100644 --- a/doc/reference/builtin_stubs.txt +++ b/doc/reference/builtin_stubs.txt @@ -849,7 +849,8 @@ HTTP request URI has space character that is not percent-encoded. 119:34 -HTTP connection has more than 100 simultaneous pipelined requests that have not been answered. +HTTP connection has more than maximum_pipelined_requests simultaneous pipelined requests that have +not been answered. 119:102 diff --git a/doc/user/http_inspect.txt b/doc/user/http_inspect.txt index b550f3513..5b9aa5dd0 100755 --- a/doc/user/http_inspect.txt +++ b/doc/user/http_inspect.txt @@ -382,6 +382,16 @@ maximum_header_length = N {0 : 65535} (default 4096). http_inspect generates 119:20 when the number of headers exceeds maximum_headers = N {0 : 65535} (default 200). +===== maximum_pipelined_requests + +http_inspect generates 119:34 when the number of pipelined requests exceeds +maximum_pipelined_requests = N {0 : 99} (default 99). This number does +not include the first request in a sequence of requests. Setting +maximum_pipelined_requests = 0, will not trigger an alert in the case +of an alternating sequence of requests and responses. It will trigger the +alert once the client issue a request before getting the response to a +previous request. + ===== URI processing Normalization and inspection of the URI in the HTTP request message is a diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h index 2d7795880..4671fa1ad 100755 --- a/src/service_inspectors/http_inspect/http_enum.h +++ b/src/service_inspectors/http_inspect/http_enum.h @@ -301,6 +301,7 @@ enum Infraction INF_GZIP_FEXTRA = 135, INF_METHOD_NOT_ON_ALLOWED_LIST = 136, INF_METHOD_ON_DISALLOWED_LIST = 137, + INF_PIPELINE_MAX = 138, INF__MAX_VALUE }; diff --git a/src/service_inspectors/http_inspect/http_flow_data.cc b/src/service_inspectors/http_inspect/http_flow_data.cc index 73cc6dce5..60f05c485 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.cc +++ b/src/service_inspectors/http_inspect/http_flow_data.cc @@ -51,7 +51,8 @@ unsigned HttpFlowData::inspector_id = 0; uint64_t HttpFlowData::instance_count = 0; #endif -HttpFlowData::HttpFlowData(Flow* flow) : FlowData(inspector_id) +HttpFlowData::HttpFlowData(Flow* flow, const HttpParaList* params_) : + FlowData(inspector_id), params(params_) { static HttpFlowStreamIntf h1_stream; #ifdef REG_TEST @@ -323,6 +324,14 @@ bool HttpFlowData::add_to_pipeline(HttpTransaction* latest) return true; } +int HttpFlowData::pipeline_length() +{ + int size = pipeline_back - pipeline_front; + if (size < 0) + size += MAX_PIPELINE; + return size; +} + HttpTransaction* HttpFlowData::take_from_pipeline() { assert(!pipeline_underflow); diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h index e0f4a3c4a..203c08c5c 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.h +++ b/src/service_inspectors/http_inspect/http_flow_data.h @@ -51,7 +51,7 @@ class MimeSession; class HttpFlowData : public snort::FlowData { public: - HttpFlowData(snort::Flow* flow); + HttpFlowData(snort::Flow* flow, const HttpParaList* params_); ~HttpFlowData() override; static unsigned inspector_id; static void init() { inspector_id = snort::FlowData::create_flow_data_id(); } @@ -92,6 +92,8 @@ public: bool is_for_httpx() const { return for_httpx; } private: + const HttpParaList* const params; + // Convenience routines void half_reset(HttpCommon::SourceId source_id); void trailer_prep(HttpCommon::SourceId source_id); @@ -203,6 +205,7 @@ private: bool pipeline_overflow = false; bool pipeline_underflow = false; bool add_to_pipeline(HttpTransaction* latest); + int pipeline_length(); HttpTransaction* take_from_pipeline(); void delete_pipeline(); diff --git a/src/service_inspectors/http_inspect/http_module.cc b/src/service_inspectors/http_inspect/http_module.cc index 69fa99861..28153a072 100755 --- a/src/service_inspectors/http_inspect/http_module.cc +++ b/src/service_inspectors/http_inspect/http_module.cc @@ -80,6 +80,9 @@ const Parameter HttpModule::http_params[] = { "maximum_headers", Parameter::PT_INT, "0:65535", "200", "alert when the number of headers in a message exceeds this value" }, + { "maximum_pipelined_requests", Parameter::PT_INT, "0:99", "99", + "alert when the number of pipelined requests exceeds this value" }, + { "normalize_utf", Parameter::PT_BOOL, nullptr, "true", "normalize charset utf encodings in response bodies" }, @@ -285,6 +288,10 @@ bool HttpModule::set(const char*, Value& val, SnortConfig*) { params->maximum_headers = val.get_uint16(); } + else if (val.is("maximum_pipelined_requests")) + { + params->maximum_pipelined_requests = val.get_uint16(); + } else if (val.is("decompress_pdf")) { params->decompress_pdf = val.get_bool(); diff --git a/src/service_inspectors/http_inspect/http_module.h b/src/service_inspectors/http_inspect/http_module.h index 2ccb7da90..46d8834d1 100755 --- a/src/service_inspectors/http_inspect/http_module.h +++ b/src/service_inspectors/http_inspect/http_module.h @@ -57,6 +57,7 @@ public: int64_t maximum_chunk_length = 0xFFFFFFFF; uint16_t maximum_header_length = 4096; uint16_t maximum_headers = 200; + uint16_t maximum_pipelined_requests = 99; bool decompress_pdf = false; bool decompress_swf = false; bool decompress_zip = false; diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc index d790ff5ad..3fdabc5d2 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc @@ -146,7 +146,7 @@ StreamSplitter::Status HttpStreamSplitter::scan(Flow* flow, const uint8_t* data, if (session_data == nullptr) { - HttpInspect::http_set_flow_data(flow, session_data = new HttpFlowData(flow)); + HttpInspect::http_set_flow_data(flow, session_data = new HttpFlowData(flow, my_inspector->params)); HttpModule::increment_peg_counts(PEG_FLOW); } diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc index 5a044501f..99001b840 100755 --- a/src/service_inspectors/http_inspect/http_tables.cc +++ b/src/service_inspectors/http_inspect/http_tables.cc @@ -250,8 +250,8 @@ const RuleMap HttpModule::http_events[] = { EVENT_UNKNOWN_METHOD, "HTTP request method is not known to Snort" }, { EVENT_SIMPLE_REQUEST, "HTTP request uses primitive HTTP format known as HTTP/0.9" }, { EVENT_UNESCAPED_SPACE_URI, "HTTP request URI has space character that is not percent-encoded" }, - { EVENT_PIPELINE_MAX, "HTTP connection has more than 100 simultaneous pipelined " - "requests that have not been answered" }, + { EVENT_PIPELINE_MAX, "HTTP connection has more than maximum_pipelined_requests simultaneous " + "pipelined requests that have not been answered" }, { EVENT_INVALID_STATCODE, "invalid status code in HTTP response" }, { EVENT_UTF_NORM_FAIL, "HTTP response has UTF character set that failed to normalize" }, { EVENT_UTF7, "HTTP response has UTF-7 character set" }, diff --git a/src/service_inspectors/http_inspect/http_transaction.cc b/src/service_inspectors/http_inspect/http_transaction.cc index 9bc8db7c2..8ffe16559 100644 --- a/src/service_inspectors/http_inspect/http_transaction.cc +++ b/src/service_inspectors/http_inspect/http_transaction.cc @@ -107,12 +107,23 @@ HttpTransaction* HttpTransaction::attach_my_transaction(HttpFlowData* session_da // now on. We just throw things away when we are done with them. delete_transaction(session_data->transaction[SRC_CLIENT], session_data); } - else if (!session_data->add_to_pipeline(session_data->transaction[SRC_CLIENT])) + else { - // The pipeline is full and just overflowed. - *session_data->infractions[source_id] += INF_PIPELINE_OVERFLOW; - session_data->events[source_id]->create_event(EVENT_PIPELINE_MAX); - delete_transaction(session_data->transaction[SRC_CLIENT], session_data); + if (!session_data->add_to_pipeline(session_data->transaction[SRC_CLIENT])) + { + // The pipeline is full and just overflowed. + *session_data->infractions[source_id] += INF_PIPELINE_OVERFLOW; + delete_transaction(session_data->transaction[SRC_CLIENT], session_data); + // When overflow occurs the length of the pipeline is unchanged + // next code ensures that the alert is still raised + *session_data->infractions[source_id] += INF_PIPELINE_MAX; + session_data->events[source_id]->create_event(EVENT_PIPELINE_MAX); + } + if (session_data->pipeline_length() > session_data->params->maximum_pipelined_requests) + { + *session_data->infractions[source_id] += INF_PIPELINE_MAX; + session_data->events[source_id]->create_event(EVENT_PIPELINE_MAX); + } } } session_data->transaction[SRC_CLIENT] = new HttpTransaction(session_data); diff --git a/src/service_inspectors/http_inspect/test/http_transaction_test.cc b/src/service_inspectors/http_inspect/test/http_transaction_test.cc index 86a24c9ea..9515bf2b0 100644 --- a/src/service_inspectors/http_inspect/test/http_transaction_test.cc +++ b/src/service_inspectors/http_inspect/test/http_transaction_test.cc @@ -53,6 +53,10 @@ Flow::Flow() { stream_intf = nullptr; } Flow::~Flow() = default; } +HttpParaList::UriParam::UriParam() {} +HttpParaList::JsNormParam::~JsNormParam() {} +HttpParaList::~HttpParaList() {} + unsigned Http2FlowData::inspector_id = 0; uint32_t Http2FlowData::get_processing_stream_id() const { return 0; } @@ -70,7 +74,8 @@ public: TEST_GROUP(http_transaction_test) { Flow* const flow = new Flow(); - HttpFlowData* const flow_data = new HttpFlowData(flow); + HttpParaList params; + HttpFlowData* flow_data = new HttpFlowData(flow, ¶ms); SectionType* const section_type = HttpUnitTestSetup::get_section_type(flow_data); SectionType* const type_expected = HttpUnitTestSetup::get_type_expected(flow_data); diff --git a/src/service_inspectors/sip/ips_sip_method.cc b/src/service_inspectors/sip/ips_sip_method.cc index 4df122144..1e4875e8b 100644 --- a/src/service_inspectors/sip/ips_sip_method.cc +++ b/src/service_inspectors/sip/ips_sip_method.cc @@ -45,7 +45,7 @@ using namespace snort; #define s_name "sip_method" #define s_help \ - "detection option for sip stat code" + "detection option for sip method" typedef std::unordered_map MethodMap; //Method Name => Negated