Merge pull request #934 in SNORT/snort3 from nhttp75 to master

Squashed commit of the following:

commit 7f4cb724e558e1138ba028a8edd4f356c626d48e
Author: Tom Peters <thopeter@cisco.com>
Date:   Thu Jun 15 14:25:26 2017 -0400

    NHI alerts related to 100 Continue

diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h
index 1b8bafdd785158d7595cec3a9ede0adcc5ec9b00..0335bd3fe91d40cdd54e9ac305f4a077c1147ccb 100644 (file)
--- a/src/service_inspectors/http_inspect/http_enum.h
+++ b/src/service_inspectors/http_inspect/http_enum.h
@@ -218,6 +218,11 @@ enum Infraction
     INF_BAD_CHAR_IN_HEADER_NAME,
     INF_HEADER_WRAPPING,
     INF_CHUNK_BAD_SEP,
+    INF_MULTIPLE_100_RESPONSES,
+    INF_UNEXPECTED_100_RESPONSE,
+    INF_UNKNOWN_1XX_STATUS,
+    INF_EXPECT_WITHOUT_BODY_CL0,
+    INF_EXPECT_WITHOUT_BODY_NO_CL,
     INF__MAX_VALUE
 };
 
@@ -317,6 +322,10 @@ enum EventSid
     EVENT_CR_WITHOUT_LF,
     EVENT_CHUNK_BAD_SEP,
     EVENT_CHUNK_BARE_LF,
+    EVENT_MULTIPLE_100_RESPONSES,
+    EVENT_UNEXPECTED_100_RESPONSE,
+    EVENT_UNKNOWN_1XX_STATUS,
+    EVENT_EXPECT_WITHOUT_BODY,             // 90
     EVENT__MAX_VALUE
 };
 

diff --git a/src/service_inspectors/http_inspect/http_msg_header.cc b/src/service_inspectors/http_inspect/http_msg_header.cc
index 0600411f2fa4e817cce5d998c2b08dc0192b3133..2a6b81dbf666cb6455fad81e31a817613d894fa8 100644 (file)
--- a/src/service_inspectors/http_inspect/http_msg_header.cc
+++ b/src/service_inspectors/http_inspect/http_msg_header.cc
@@ -164,6 +164,11 @@ void HttpMsgHeader::update_flow()
         else if (content_length == 0)
         {
             // No body
+            if (get_header_count(HEAD_EXPECT) > 0)
+            {
+                add_infraction(INF_EXPECT_WITHOUT_BODY_CL0);
+                create_event(EVENT_EXPECT_WITHOUT_BODY);
+            }
             session_data->half_reset(source_id);
             return;
         }
@@ -186,6 +191,11 @@ void HttpMsgHeader::update_flow()
             add_infraction(INF_POST_WO_BODY);
             create_event(EVENT_UNBOUNDED_POST);
         }
+        if (get_header_count(HEAD_EXPECT) > 0)
+        {
+            add_infraction(INF_EXPECT_WITHOUT_BODY_NO_CL);
+            create_event(EVENT_EXPECT_WITHOUT_BODY);
+        }
         session_data->half_reset(source_id);
         return;
     }

diff --git a/src/service_inspectors/http_inspect/http_msg_status.cc b/src/service_inspectors/http_inspect/http_msg_status.cc
index 448e0c10e55e0a5f7f36ce9f70b81c8c20b887f9..19b3470db73d3882059ae4124cd678c0508869d5 100644 (file)
--- a/src/service_inspectors/http_inspect/http_msg_status.cc
+++ b/src/service_inspectors/http_inspect/http_msg_status.cc
@@ -24,6 +24,7 @@
 #include "http_msg_status.h"
 
 #include "http_api.h"
+#include "http_msg_header.h"
 #include "stream/stream.h"
 
 using namespace HttpEnums;
@@ -100,6 +101,11 @@ void HttpMsgStatus::derive_status_code_num()
         add_infraction(INF_BAD_STAT_CODE);
         create_event(EVENT_INVALID_STATCODE);
     }
+    if ((status_code_num >= 102) && (status_code_num <= 199))
+    {
+        add_infraction(INF_UNKNOWN_1XX_STATUS);
+        create_event(EVENT_UNKNOWN_1XX_STATUS);
+    }
 }
 
 void HttpMsgStatus::gen_events()
@@ -142,9 +148,9 @@ void HttpMsgStatus::gen_events()
         }
     }
 
-    if( !transaction->get_request() && (trans_num == 1) )
+    if (!transaction->get_request() && (trans_num == 1))
     {
-        if( flow->is_pdu_inorder(SSN_DIR_FROM_SERVER) )
+        if (flow->is_pdu_inorder(SSN_DIR_FROM_SERVER))
         {
             // HTTP response without a request. Possible ssh tunneling
             add_infraction(INF_RSP_WO_REQ);
@@ -170,7 +176,16 @@ void HttpMsgStatus::update_flow()
         // responses to all be included in the same transaction. It's not obvious whether that is
         // the best way to handle what should be a highly abnormal situation.
         if (status_code_num == 100)
-            transaction->second_response_coming();
+        {
+            // Were we "Expect"-ing this?
+            HttpMsgHeader* const req_header = transaction->get_header(SRC_CLIENT);
+            if ((req_header != nullptr) && (req_header->get_header_count(HEAD_EXPECT) == 0))
+            {
+                add_infraction(INF_UNEXPECTED_100_RESPONSE);
+                create_event(EVENT_UNEXPECTED_100_RESPONSE);
+            }
+            transaction->set_one_hundred_response();
+        }
     }
     session_data->section_type[source_id] = SEC__NOT_COMPUTE;
 }

diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc
index 9fc1d4059a0ea1bb0bcb15cde6fb6f57055fadf8..6f25541744ecc98dfba28bc27e73de708882bac1 100644 (file)
--- a/src/service_inspectors/http_inspect/http_tables.cc
+++ b/src/service_inspectors/http_inspect/http_tables.cc
@@ -344,6 +344,10 @@ const RuleMap HttpModule::http_events[] =
     { EVENT_CR_WITHOUT_LF,              "HTTP header line terminated by CR without a LF" },
     { EVENT_CHUNK_BAD_SEP,              "chunk terminated by nonstandard separator" },
     { EVENT_CHUNK_BARE_LF,              "chunk length terminated by LF without CR" },
+    { EVENT_MULTIPLE_100_RESPONSES,     "more than one response with 100 status code" },
+    { EVENT_UNEXPECTED_100_RESPONSE,    "100 status code not in response to Expect header" },
+    { EVENT_UNKNOWN_1XX_STATUS,         "1XX status code other than 100 or 101" },
+    { EVENT_EXPECT_WITHOUT_BODY,        "Expect header sent without a message body" },
     { 0, nullptr }
 };
 

diff --git a/src/service_inspectors/http_inspect/http_transaction.cc b/src/service_inspectors/http_inspect/http_transaction.cc
index 1b47373858f84db9c3d6eb64ec1cee673bc4e8ea..afc309dd7e2a9f370b68b8e554813c1de6536b79 100644 (file)
--- a/src/service_inspectors/http_inspect/http_transaction.cc
+++ b/src/service_inspectors/http_inspect/http_transaction.cc
@@ -110,7 +110,6 @@ HttpTransaction* HttpTransaction::attach_my_transaction(HttpFlowData* session_da
              (session_data->transaction[SRC_SERVER] != nullptr) &&
               session_data->transaction[SRC_SERVER]->second_response_expected)
     {
-        assert(session_data->transaction[SRC_SERVER] != nullptr);
         session_data->transaction[SRC_SERVER]->second_response_expected = false;
         delete session_data->transaction[SRC_SERVER]->status;
         session_data->transaction[SRC_SERVER]->status = nullptr;
@@ -201,3 +200,15 @@ HttpEventGen* HttpTransaction::get_events(HttpEnums::SourceId source_id)
     return events[source_id];
 }
 
+void HttpTransaction::set_one_hundred_response()
+{
+    assert(response_seen);
+    if (one_hundred_response)
+    {
+        *infractions[SRC_SERVER] += INF_MULTIPLE_100_RESPONSES;
+        events[SRC_SERVER]->create_event(EVENT_MULTIPLE_100_RESPONSES);
+    }
+    one_hundred_response = true;
+    second_response_expected = true;
+}
+

diff --git a/src/service_inspectors/http_inspect/http_transaction.h b/src/service_inspectors/http_inspect/http_transaction.h
index 50a0bb554973e852a94f77569b72191c3e84211f..9d7946d67ad8f5bd702e35ad3fc0631172074d8b 100644 (file)
--- a/src/service_inspectors/http_inspect/http_transaction.h
+++ b/src/service_inspectors/http_inspect/http_transaction.h
@@ -61,7 +61,7 @@ public:
     HttpInfractions* get_infractions(HttpEnums::SourceId source_id);
     HttpEventGen* get_events(HttpEnums::SourceId source_id);
 
-    void second_response_coming() { assert(response_seen); second_response_expected = true; }
+    void set_one_hundred_response();
     bool final_response() const { return !second_response_expected; }
 
 private:
@@ -77,6 +77,7 @@ private:
     HttpEventGen* events[2] = { nullptr, nullptr };
 
     bool response_seen = false;
+    bool one_hundred_response = false;
     bool second_response_expected = false;
 
     // This is a form of reference counting that prevents premature/double deletion of a

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 44d32ab655843cf3116bc7add5ef2f3dca9b5e06..9d6d39b10ac6629fa304f39ca8b34d50b425d09c 100644 (file)
--- a/src/service_inspectors/http_inspect/test/http_transaction_test.cc
+++ b/src/service_inspectors/http_inspect/test/http_transaction_test.cc
@@ -287,7 +287,7 @@ TEST(http_transaction_test, basic_continue)
     // Interim response
     section_type[SRC_SERVER] = SEC_STATUS;
     CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER));
-    trans->second_response_coming();
+    trans->set_one_hundred_response();
     section_type[SRC_SERVER] = SEC_HEADER;
     CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER));
 
@@ -334,7 +334,7 @@ TEST(http_transaction_test, multiple_continue)
     {
         section_type[SRC_SERVER] = SEC_STATUS;
         CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER));
-        trans->second_response_coming();
+        trans->set_one_hundred_response();
         section_type[SRC_SERVER] = SEC_HEADER;
         CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER));
     }
@@ -374,7 +374,7 @@ TEST(http_transaction_test, multiple_orphan_continue)
         section_type[SRC_SERVER] = SEC_STATUS;
         HttpTransaction* trans = HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER);
         CHECK(trans != nullptr);
-        trans->second_response_coming();
+        trans->set_one_hundred_response();
         section_type[SRC_SERVER] = SEC_HEADER;
         CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER));
         section_type[SRC_SERVER] = SEC_BODY_CHUNK;
@@ -430,7 +430,7 @@ TEST(http_transaction_test, pipeline_continue_pipeline)
     // Interim response to fourth request
     section_type[SRC_SERVER] = SEC_STATUS;
     CHECK(trans[3] == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER));
-    trans[3]->second_response_coming();
+    trans[3]->set_one_hundred_response();
     section_type[SRC_SERVER] = SEC_HEADER;
     CHECK(trans[3] == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER));