From: Russ Combs <rucombs@cisco.com>
Date: Fri, 8 May 2015 11:27:55 +0000 (-0400)
Subject: tom: new_http_inspect aborts on obvious non-HTTP ttraffic
X-Git-Tag: 3.0.0-233~980
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=45b3fa0bd9099c50ed1af73751e18674337da719;p=thirdparty%2Fsnort3.git

tom: new_http_inspect aborts on obvious non-HTTP ttraffic
---

diff --git a/src/service_inspectors/nhttp_inspect/nhttp_enum.h b/src/service_inspectors/nhttp_inspect/nhttp_enum.h
index 3bb549cd0..743ecb8ef 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_enum.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_enum.h
@@ -24,9 +24,9 @@
 
 namespace NHttpEnums
 {
-static const int MAXOCTETS = 63780;
-static const int DATABLOCKSIZE = 16384;
-static const int FINALBLOCKSIZE = 24576;
+static const int MAX_OCTETS = 63780;
+static const int DATA_BLOCK_SIZE = 16384;
+static const int FINAL_BLOCK_SIZE = 24576;
 static const uint32_t NHTTP_GID = 219;
 
 // Field status codes for when no valid value is present in length or integer value. Positive
@@ -121,8 +121,10 @@ enum Infraction
     INF_URI_SLASH_DOT_DOT,
     INF_URI_ROOT_TRAV,
     INF_TOO_MUCH_LEADING_WS,
+    INF_WS_BETWEEN_MSGS,
     INF_ENDLESS_HEADER,
     INF_LF_WITHOUT_CR,
+    INF_NOT_HTTP,
 };
 
 // Formats for output from a header normalization function
@@ -189,10 +191,13 @@ enum EventSid
     EVENT_PDF_CASC_COMP,
     EVENT_PDF_PARSE_FAILURE,
     EVENT_LOSS_OF_SYNC,
+    EVENT_NOT_HTTP,
+    EVENT_WS_BETWEEN_MSGS,
     EVENT_MAXVALUE
 };
 
 extern const int8_t as_hex[256];
+extern const bool token_char[256];
 } // end namespace NHttpEnums
 
 #endif
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_inspect.cc b/src/service_inspectors/nhttp_inspect/nhttp_inspect.cc
index b71699184..3c66642cf 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_inspect.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_inspect.cc
@@ -49,7 +49,7 @@ NHttpInspect::NHttpInspect(bool test_input, bool test_output)
     }
 }
 
-THREAD_LOCAL uint8_t NHttpInspect::body_buffer[MAXOCTETS];
+THREAD_LOCAL uint8_t NHttpInspect::body_buffer[MAX_OCTETS];
 
 THREAD_LOCAL NHttpMsgSection* NHttpInspect::latest_section = nullptr;
 
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_inspect.h b/src/service_inspectors/nhttp_inspect/nhttp_inspect.h
index a322672ad..3576903fc 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_inspect.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_inspect.h
@@ -35,7 +35,7 @@ class NHttpMsgSection;
 class NHttpInspect : public Inspector
 {
 public:
-    static THREAD_LOCAL uint8_t body_buffer[NHttpEnums::MAXOCTETS];
+    static THREAD_LOCAL uint8_t body_buffer[NHttpEnums::MAX_OCTETS];
 
     NHttpInspect(bool test_input, bool test_output);
 
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_splitter.cc b/src/service_inspectors/nhttp_inspect/nhttp_splitter.cc
index 6abe6d8c8..f4a3c3a3a 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_splitter.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_splitter.cc
@@ -33,6 +33,12 @@ ScanResult NHttpStartSplitter::split(const uint8_t* buffer, uint32_t length,
         {
             if ((buffer[k] == 32) || ((buffer[k] >= 9) && (buffer[k] <= 13)))
             {
+                if ((buffer[k] != 10) && (buffer[k] != 13))
+                {
+                    // tab, VT, FF, or space between messages
+                    infractions += INF_WS_BETWEEN_MSGS;
+                    events.create_event(EVENT_WS_BETWEEN_MSGS);
+                }
                 if (num_crlf < MAX_LEADING_WHITESPACE)
                 {
                     num_crlf++;
@@ -54,6 +60,21 @@ ScanResult NHttpStartSplitter::split(const uint8_t* buffer, uint32_t length,
 
         // If we get this far then the leading white space issue is behind us and num_crlf was
         // reset to zero
+        if (!validated)
+        {
+            switch (validate(buffer[k]))
+            {
+            case V_GOOD:
+                validated = true;
+                break;
+            case V_BAD:
+                infractions += INF_NOT_HTTP;
+                events.create_event(EVENT_NOT_HTTP);
+                return SCAN_ABORT;
+            case V_TBD:
+                break;
+            }
+        }
         if (buffer[k] == '\n')
         {
             num_crlf++;
@@ -73,6 +94,29 @@ ScanResult NHttpStartSplitter::split(const uint8_t* buffer, uint32_t length,
     return SCAN_NOTFOUND;
 }
 
+NHttpStartSplitter::ValidationResult NHttpRequestSplitter::validate(uint8_t octet)
+{
+    static const int max_method_length = 80;
+
+    if ((octet == ' ') || (octet == '\t'))
+        return V_GOOD;
+    if (!token_char[octet] || ++octets_checked > max_method_length)
+        return V_BAD;
+    return V_TBD;
+}
+
+NHttpStartSplitter::ValidationResult NHttpStatusSplitter::validate(uint8_t octet)
+{
+    static const int match_size = 5;
+    static const uint8_t match[match_size] = { 'H', 'T', 'T', 'P', '/' };
+
+    if (octet != match[octets_checked++])
+        return V_BAD;
+    if (octets_checked >= match_size)
+        return V_GOOD;
+    return V_TBD;
+}
+
 ScanResult NHttpHeaderSplitter::split(const uint8_t* buffer, uint32_t length,
     NHttpInfractions& infractions, NHttpEventGen& events)
 {
@@ -132,7 +176,7 @@ ScanResult NHttpBodySplitter::split(const uint8_t*, uint32_t, NHttpInfractions&,
 
     // The normal body section size is about 16K. But if there are only 24K or less remaining we
     // take the whole thing rather than leave a small final section.
-    if (remaining <= FINALBLOCKSIZE)
+    if (remaining <= FINAL_BLOCK_SIZE)
     {
         num_flush = remaining;
         remaining = 0;
@@ -141,7 +185,7 @@ ScanResult NHttpBodySplitter::split(const uint8_t*, uint32_t, NHttpInfractions&,
     else
     {
         // FIXIT-M need to implement random increments
-        num_flush = DATABLOCKSIZE;
+        num_flush = DATA_BLOCK_SIZE;
         remaining -= num_flush;
         return SCAN_FOUND_PIECE;
     }
@@ -236,14 +280,14 @@ ScanResult NHttpChunkSplitter::split(const uint8_t* buffer, uint32_t length,
         case CHUNK_DATA:
           {
             uint32_t skip_amount = (length-k <= expected) ? length-k : expected;
-            skip_amount = (skip_amount <= DATABLOCKSIZE-data_seen) ? skip_amount :
-                DATABLOCKSIZE-data_seen;
+            skip_amount = (skip_amount <= DATA_BLOCK_SIZE-data_seen) ? skip_amount :
+                DATA_BLOCK_SIZE-data_seen;
             k += skip_amount - 1;
             if ((expected -= skip_amount) == 0)
             {
                 curr_state = CHUNK_DCRLF1;
             }
-            if ((data_seen += skip_amount) == DATABLOCKSIZE)
+            if ((data_seen += skip_amount) == DATA_BLOCK_SIZE)
             {
                 // FIXIT-M need to randomize slice point
                 data_seen = 0;
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_splitter.h b/src/service_inspectors/nhttp_inspect/nhttp_splitter.h
index 53da04aea..13ce2846d 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_splitter.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_splitter.h
@@ -40,6 +40,7 @@ public:
     uint32_t get_octets_seen() const { return octets_seen; }
     virtual uint32_t get_num_excess() const { return 0; }
     virtual uint32_t get_num_head_lines() const { return 0; }
+    virtual bool valid() const { return true; }
 
 protected:
     // number of octets processed by previous split() calls that returned NOTFOUND
@@ -55,9 +56,29 @@ public:
     NHttpEnums::ScanResult split(const uint8_t* buffer, uint32_t length,
         NHttpInfractions& infractions, NHttpEventGen& events) override;
     uint32_t get_num_excess() const override { return (num_flush > 0) ? num_crlf : 0; }
+    bool valid() const override { return validated; }
+
+protected:
+    enum ValidationResult { V_GOOD, V_BAD, V_TBD };
 
 private:
     static const int MAX_LEADING_WHITESPACE = 20;
+    virtual ValidationResult validate(uint8_t octet) = 0;
+    bool validated = false;
+};
+
+class NHttpRequestSplitter : public NHttpStartSplitter
+{
+private:
+    uint32_t octets_checked = 0;
+    ValidationResult validate(uint8_t octet) override;
+};
+
+class NHttpStatusSplitter : public NHttpStartSplitter
+{
+private:
+    uint32_t octets_checked = 0;
+    ValidationResult validate(uint8_t octet) override;
 };
 
 class NHttpHeaderSplitter : public NHttpSplitter
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc
index a992b9e52..581a4110a 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc
@@ -54,8 +54,8 @@ NHttpSplitter* NHttpStreamSplitter::get_splitter(SectionType type,
 {
     switch (type)
     {
-    case SEC_REQUEST:
-    case SEC_STATUS: return (NHttpSplitter*)new NHttpStartSplitter;
+    case SEC_REQUEST: return (NHttpSplitter*)new NHttpRequestSplitter;
+    case SEC_STATUS: return (NHttpSplitter*)new NHttpStatusSplitter;
     case SEC_HEADER:
     case SEC_TRAILER: return (NHttpSplitter*)new NHttpHeaderSplitter;
     case SEC_BODY: return (NHttpSplitter*)new NHttpBodySplitter(
@@ -126,7 +126,7 @@ void NHttpStreamSplitter::chunk_spray(NHttpFlowData* session_data, uint8_t* buff
 StreamSplitter::Status NHttpStreamSplitter::scan(Flow* flow, const uint8_t* data, uint32_t length,
     uint32_t, uint32_t* flush_offset)
 {
-    assert(length <= MAXOCTETS);
+    assert(length <= MAX_OCTETS);
 
     /* FIXIT-L Temporary printf while we shake out stream interface */
     if (!NHttpTestManager::use_test_input() && NHttpTestManager::use_test_output())
@@ -168,7 +168,8 @@ StreamSplitter::Status NHttpStreamSplitter::scan(Flow* flow, const uint8_t* data
     }
     else if (NHttpTestManager::use_test_output())
     {
-        printf("Scan from flow data %p direction %d\n", (void*)session_data, source_id);
+        printf("Scan from flow data %p direction %d length %u\n", (void*)session_data, source_id,
+            length);
         fflush(stdout);
     }
 
@@ -183,13 +184,13 @@ StreamSplitter::Status NHttpStreamSplitter::scan(Flow* flow, const uint8_t* data
         splitter = get_splitter(type, session_data);
         assert(splitter != nullptr);
     }
-    const uint32_t max_length = MAXOCTETS - splitter->get_octets_seen();
+    const uint32_t max_length = MAX_OCTETS - splitter->get_octets_seen();
     const ScanResult split_result = splitter->split(data, (length <= max_length) ? length :
         max_length, session_data->infractions[source_id], session_data->events[source_id]);
     switch (split_result)
     {
     case SCAN_NOTFOUND:
-        if (splitter->get_octets_seen() == MAXOCTETS)
+        if (splitter->get_octets_seen() == MAX_OCTETS)
         {
             session_data->infractions[source_id] += INF_ENDLESS_HEADER;
             session_data->events[source_id].create_event(EVENT_LOSS_OF_SYNC);
@@ -256,11 +257,11 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total,
     copied = len;
 
     // FIXIT-M temporary workaround for high total values
-    if (total > MAXOCTETS)
+    if (total > MAX_OCTETS)
         return nullptr;
 
     assert(total >= offset + len);
-    assert(total <= MAXOCTETS);
+    assert(total <= MAX_OCTETS);
 
     /* FIXIT-L Temporary printf while we shake out stream interface */
     if (!NHttpTestManager::use_test_input() && NHttpTestManager::use_test_output())
@@ -377,7 +378,7 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total,
         {
             nhttp_buf.data = buffer;
             nhttp_buf.length = section_length;
-            assert((nhttp_buf.length <= MAXOCTETS) && (nhttp_buf.length != 0));
+            assert((nhttp_buf.length <= MAX_OCTETS) && (nhttp_buf.length != 0));
             buffer = nullptr;
             if (NHttpTestManager::use_test_output())
             {
@@ -409,6 +410,10 @@ bool NHttpStreamSplitter::finish(Flow* flow)
         (session_data->splitter[source_id]->get_octets_seen() > 0) &&
         (session_data->type_expected[source_id] != SEC_ABORT))
     {
+        if (!session_data->splitter[source_id]->valid())
+        {
+            return false;
+        }
         session_data->section_type[source_id] = session_data->type_expected[source_id];
         session_data->num_excess[source_id] = 0;
         session_data->num_head_lines[source_id] =
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.h b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.h
index bb28ccaed..6d3c96252 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.h
@@ -40,7 +40,7 @@ public:
         uint8_t* data, unsigned len, uint32_t flags, unsigned& copied) override;
     bool finish(Flow* flow) override;
     bool is_paf() override { return true; }
-    unsigned max(Flow*) override { return NHttpEnums::MAXOCTETS; }
+    unsigned max(Flow*) override { return NHttpEnums::MAX_OCTETS; }
 
 private:
     void prepare_flush(NHttpFlowData* session_data, uint32_t* flush_offset, NHttpEnums::SectionType
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_tables.cc b/src/service_inspectors/nhttp_inspect/nhttp_tables.cc
index 41683d91f..a308b8d73 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_tables.cc
+++ b/src/service_inspectors/nhttp_inspect/nhttp_tables.cc
@@ -301,6 +301,8 @@ const RuleMap NHttpModule::nhttp_events[] =
     { EVENT_PDF_CASC_COMP,              "PDF file cascaded compression" },
     { EVENT_PDF_PARSE_FAILURE,          "PDF file parse failure" },
     { EVENT_LOSS_OF_SYNC,               "HTTP misformatted or not really HTTP" },
+    { EVENT_NOT_HTTP,                   "Input apparently not HTTP" },
+    { EVENT_WS_BETWEEN_MSGS,            "White space before or between messages" },
 
     { 0, nullptr }
 };
@@ -332,3 +334,31 @@ const int8_t NHttpEnums::as_hex[256] =
     -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1
 };
 
+
+const bool NHttpEnums::token_char[256] =
+{
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false,
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false,
+
+    false,  true, false,  true,  true,  true,  true,  true, false, false,  true,  true, false,  true,  true, false,
+     true,  true,  true,  true,  true,  true,  true,  true,  true,  true, false, false, false, false, false, false,
+
+    false,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,
+     true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true, false, false, false,  true,  true,
+
+     true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true,
+     true,  true,  true,  true,  true,  true,  true,  true,  true,  true,  true, false,  true, false,  true, false,
+
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false,
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false,
+
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false,
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false,
+
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false,
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false,
+
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false,
+    false, false, false, false, false, false, false, false, false, false, false, false, false, false, false, false
+};
+
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_test_input.h b/src/service_inspectors/nhttp_inspect/nhttp_test_input.h
index 7ef62fadd..be6cbe087 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_test_input.h
+++ b/src/service_inspectors/nhttp_inspect/nhttp_test_input.h
@@ -38,7 +38,7 @@ public:
 
 private:
     FILE* test_data_file;
-    uint8_t msg_buf[2 * NHttpEnums::MAXOCTETS];
+    uint8_t msg_buf[2 * NHttpEnums::MAX_OCTETS];
 
     // data has been flushed and must be sent by reassemble() before more data may be given to
     // scan()
diff --git a/src/service_inspectors/nhttp_inspect/nhttp_test_msgs.txt b/src/service_inspectors/nhttp_inspect/nhttp_test_msgs.txt
index 0b047ec4b..823ff0a63 100644
--- a/src/service_inspectors/nhttp_inspect/nhttp_test_msgs.txt
+++ b/src/service_inspectors/nhttp_inspect/nhttp_test_msgs.txt
@@ -122,6 +122,20 @@ ghjklzxcvbnmABCDEFGHIJKLMNOPQRSTUVWXYZ,.;'<>:"qwertyuiopasdfghjklzxcvbnmABCDEFGH
 rtyuiopasdfghjklzxcvbnmABCDEFGHIJKLMNOPQRSTUVWXYZqwertyuiopasdfghjklzxcvbnmABCDEFGHIJKLMNOPQRSTUVWXYZqwertyuiopasdfghjklzxcvbnmABCDEFGHIJKLMNOPQRSTUVWXYZ
 \r\n\r\n
 
+@1011
+@break
+@response
+ HTTP/1.1 200 OK\r\n\r\n
+
+@1012
+@break
+@response
+   HTTP/1.1 200 OK\r\n\r\n
+
+@1013
+@break
+@response
+\tHTTP/1.1 200 OK\r\n\r\n
 
 # ***********************************************************************************************
 # Invalid response start lines
@@ -266,7 +280,6 @@ HTTP/1.0 401 illegal nontext character in reason\xFFphrase delete\r\n\r\n
 @response
 HTTP/1.0 401 \x08illegal nontext character in reason phrase backspace\r\n\r\n
 
-
 # ***********************************************************************************************
 # Valid request start lines
 @3001
@@ -369,6 +382,16 @@ GET http://iamahost.com/simple/example/of/a/path/ HTTP/1.1\r\n\r\n
 @request
 GET HtTpS://1.2.3.4.5.a:6/abcdef/ghijklmnop/qrstuvwxyz/?thequery?fieldcontinues?until#afragme#arrives#1234 HTTP/1.1\r\n\r\n
 
+@3021
+@break
+@request
+ MKREDIRECTREF / HTTP/2.0\r\n\r\n
+
+@3022
+@break
+@request
+\t\t\t\tBIND /1234567890?abcdef HTTP/1.1\r\n\r\n
+
 # ***********************************************************************************************
 # Invalid request start lines
 @4001
@@ -439,18 +462,17 @@ fa%6be\\..?# HTTP/1.1\r\n\r\n
 @4013
 @break
 @request
-GET http://hostname.com/?# HTTP/1.1\r\n\r\n
+GET123456789012345678901234567890123456789012345678901234567890123456789012345678 http://hostname.com/?# HTTP/1.1\r\n\r\n
 
 @4014
 @break
 @request
-GET http://hostname.com/?# HTTP/1.1\r\n\r\n
+GE;T http://hostname.com/?# HTTP/1.1\r\n\r\n
 
 @4015
 @break
 @request
-GET http://hostname.com/?# HTTP/1.1\r\n\r\n
-
+GET\xAA http://hostname.com/?# HTTP/1.1\r\n\r\n
 
 # ***********************************************************************************************
 # Valid headers without body