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 <admamole@cisco.com>
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 8f60f6c5dd8653deddc623a083a1fecf937b599c..407c639c6c243625a4f6adfef8b973a47fd89d52 100644 (file)
--- 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 b550f35138e0d362e1bcf6f9e8e6e4c1f98d75f3..5b9aa5dd05166755913ebf4d6ff13d820776b6f3 100755 (executable)
--- 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 2d779588061aa62b496ccffdaae85807565cec67..4671fa1addf3b3cd9b82450bdded2a99cd9dbb98 100755 (executable)
--- 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 73cc6dce58f5c5fe0a714c674ca4bd3996cf6577..60f05c4850bb134b847731ce7159fe8270db8f02 100644 (file)
--- 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 e0f4a3c4a09abbefbbd33cbda525d8626904b198..203c08c5cd4a00ac37f574a69c0848e0c1613c86 100644 (file)
--- 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 69fa99861c9d19bfe303b97f24ee81c22e39dcbe..28153a072462a72536bbad53e12572ed1096bb18 100755 (executable)
--- 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 2ccb7da9086c787d960321cb9ed1b8172412bf14..46d8834d1c3e7791f45b66c64b3e5bfb05fd707d 100755 (executable)
--- 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 d790ff5ad87a5c7dcdc572c8a78e778889197e27..3fdabc5d244dde9ff453a9f085a1bb6b2a6f8756 100644 (file)
--- 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 5a044501fbd3abca9f9b1becabf175e1f9c1b760..99001b840fd87ac835660923df1e5ff5287288dc 100755 (executable)
--- 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 9bc8db7c280fcb43f14273559435535d52fcc758..8ffe1655960ca5cecdb58c52835cbf8c54f32e1c 100644 (file)
--- 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 86a24c9ea2dd85661c24e7cffb47c000c52edac8..9515bf2b07b0f6ec880367817642562c8849193a 100644 (file)
--- 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, &params);
     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 4df122144e7f3f13de3acedddc231d06e30d3f1b..1e4875e8beed29d9661345d10192456c6a6fa66c 100644 (file)
--- 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<std::string, bool> MethodMap; //Method Name => Negated