From: Mike Stepanek (mstepane) <mstepane@cisco.com>
Date: Fri, 20 Mar 2020 20:25:19 +0000 (+0000)
Subject: Merge pull request #2072 in SNORT/snort3 from ~THOPETER/snort3:nhttp133 to master
X-Git-Tag: 3.0.0-270~16
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1ced7ae2ff0a51efc8ac4983244843d13377acec;p=thirdparty%2Fsnort3.git

Merge pull request #2072 in SNORT/snort3 from ~THOPETER/snort3:nhttp133 to master

Squashed commit of the following:

commit ad73c4fabe6ecbc90bb9283d52ae574288072ec9
Author: Tom Peters <thopeter@cisco.com>
Date:   Wed Feb 5 14:54:56 2020 -0500

    http_inspect: gzip detained inspection
---

diff --git a/src/service_inspectors/http_inspect/dev_notes.txt b/src/service_inspectors/http_inspect/dev_notes.txt
index a9a19c1f9..44031bbe0 100644
--- a/src/service_inspectors/http_inspect/dev_notes.txt
+++ b/src/service_inspectors/http_inspect/dev_notes.txt
@@ -14,6 +14,14 @@ one packet in a TCP stream effectively blocks all subsequent packets from being
 delivered to a target application. Once a packet is detained no attempt is made to detain
 additional packets.
 
+The core functions of detained inspection are implemented in the message body cutters. They search
+for the beginning of Javascripts by finding the string "<script". The raw packet containing the 't'
+will be detained as well the beginning of every subsequent message section. No attempt is made to
+find the end of a script--it is assumed to continue through the rest of the message body. The
+cutter will decompress gzip data to search for "<script". This unzip is unrelated and in addition
+to the unzip done in reassemble(). The decision was made to accept the performance cost of
+unzipping twice to avoid using memory to store uncompressed data waiting for reassembly.
+
 Splitter init_partial_flush() is called by Stream when a previously detained packet must be dropped
 or released immediately. It sets up reassembly and inspection of a partial message section
 containing all available data. Once inspection of this partial message section is complete, for
@@ -157,9 +165,17 @@ simplify_path.
 
 Test tool usage instructions:
 
-The HI internal test tool is the HttpTestInput class. It allows the developer to write tests that
+The HI test tool consists of two features. test_output provides extensive information about the
+inner workings of HI. It is strongly focused on showing work products (Fields) rather than being a
+tracing feature. Given a problematic pcap, the developer can see what the input is, how HI
+interprets it, and what the output to rule options will be. Several related configuration options
+(see help) allow the developer to customize the output.
+
+test_input is provided by the HttpTestInput class. It allows the developer to write tests that
 simulate HTTP messages split into TCP segments at specified points. The tests cover all of splitter
-and inspector and the impact on downstream customers such as detection and file processing.
+and inspector and the impact on downstream customers such as detection and file processing. The
+test_input option activates a modifed form of test_output. It is not necessary to also specify
+test_output.
 
 The test input comes from the file http_test_msgs.txt in the current directory. Enter HTTP test
 message text as you want it to be presented to the StreamSplitter.
@@ -175,7 +191,7 @@ must search and reassemble).
 
 Lines beginning with # are comments. Lines beginning with @ are commands. These do not apply to
 lines in the middle of a paragraph. Lines that begin with $ are insert commands - a special class of
-commands that may be used within a paragraph the insert data into the message buffer.
+commands that may be used within a paragraph to insert data into the message buffer.
 
 Commands:
   @break resets HTTP Inspect data structures and begins a new test. Use it liberally to prevent
@@ -193,6 +209,7 @@ Commands:
      beginning.
   @<decimal number> sets the test number and hence the test output file name. Applies to subsequent
      sections until changed. Don't reuse numbers.
+
 Insert commands:
   $fill <decimal number> create a paragraph consisting of <number> octets of auto-fill data
      ABCDEFGHIJABC ....
@@ -234,5 +251,6 @@ This test tool does not implement the feature of being hardened against bad inpu
 badly formatted or improper test case the program may assert or crash. The responsibility is on the
 developer to get it right.
 
-Test input is currently designed for single-threaded operation only.
+The test tool is designed for single-threaded operation only.
 
+The test tool is only available when compiled with REG_TEST.
diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc
index f6fb1eb6f..da17cc38b 100644
--- a/src/service_inspectors/http_inspect/http_cutter.cc
+++ b/src/service_inspectors/http_inspect/http_cutter.cc
@@ -22,6 +22,7 @@
 #endif
 
 #include "http_cutter.h"
+#include "http_enum.h"
 
 using namespace HttpEnums;
 
@@ -250,6 +251,36 @@ ScanResult HttpHeaderCutter::cut(const uint8_t* buffer, uint32_t length,
     return SCAN_NOT_FOUND;
 }
 
+HttpBodyCutter::HttpBodyCutter(bool detained_inspection_, CompressId compression_) :
+    detained_inspection(detained_inspection_), compression(compression_)
+{
+    if (detained_inspection && ((compression == CMP_GZIP) || (compression == CMP_DEFLATE)))
+    {
+        compress_stream = new z_stream;
+        compress_stream->zalloc = Z_NULL;
+        compress_stream->zfree = Z_NULL;
+        compress_stream->next_in = Z_NULL;
+        compress_stream->avail_in = 0;
+        const int window_bits = (compression == CMP_GZIP) ? GZIP_WINDOW_BITS : DEFLATE_WINDOW_BITS;
+        if (inflateInit2(compress_stream, window_bits) != Z_OK)
+        {
+            assert(false);
+            compression = CMP_NONE;
+            delete compress_stream;
+            compress_stream = nullptr;
+        }
+    }
+}
+
+HttpBodyCutter::~HttpBodyCutter()
+{
+    if (compress_stream != nullptr)
+    {
+        inflateEnd(compress_stream);
+        delete compress_stream;
+    }
+}
+
 ScanResult HttpBodyClCutter::cut(const uint8_t* buffer, uint32_t length, HttpInfractions*,
     HttpEventGen*, uint32_t flow_target, bool stretch)
 {
@@ -683,21 +714,55 @@ bool HttpBodyCutter::need_detained_inspection(const uint8_t* data, uint32_t leng
 // Currently we do detained inspection when we see a javascript starting
 bool HttpBodyCutter::dangerous(const uint8_t* data, uint32_t length)
 {
+    const uint8_t* input_buf = data;
+    uint32_t input_length = length;
+    uint8_t* decomp_output = nullptr;
+
+    // Zipped flows must be decompressed before we can check them. Unzipping for detained
+    // inspection is completely separate from the unzipping done later in reassemble(). 
+    if ((compression == CMP_GZIP) || (compression == CMP_DEFLATE))
+    {
+        const uint32_t decomp_buffer_size = MAX_OCTETS;
+        decomp_output = new uint8_t[decomp_buffer_size];
+
+        compress_stream->next_in = const_cast<Bytef*>(data);
+        compress_stream->avail_in = length;
+        compress_stream->next_out = decomp_output;
+        compress_stream->avail_out = decomp_buffer_size;
+
+        int ret_val = inflate(compress_stream, Z_SYNC_FLUSH);
+
+        // Not going to be subtle about this and try to fix decompression problems. If it doesn't
+        // work out we assume it could be dangerous.
+        if (((ret_val != Z_OK) && (ret_val != Z_STREAM_END)) || (compress_stream->avail_in > 0))
+        {
+            delete[] decomp_output;
+            return true;
+        }
+
+        input_buf = decomp_output;
+        input_length = decomp_buffer_size - compress_stream->avail_out;
+    }
+
     static const uint8_t match_string[] = { '<', 's', 'c', 'r', 'i', 'p', 't' };
     static const uint8_t string_length = sizeof(match_string);
-    for (uint32_t k = 0; k < length; k++)
+    for (uint32_t k = 0; k < input_length; k++)
     {
         // partial_match is persistent, enabling matches that cross data boundaries
-        if (data[k] == match_string[partial_match])
+        if (input_buf[k] == match_string[partial_match])
         {
             if (++partial_match == string_length)
+            {
+                delete[] decomp_output;
                 return true;
+            }
         }
         else
         {
             partial_match = 0;
         }
     }
+    delete[] decomp_output;
     return false;
 }
 
diff --git a/src/service_inspectors/http_inspect/http_cutter.h b/src/service_inspectors/http_inspect/http_cutter.h
index ecb45d260..a83b7f81e 100644
--- a/src/service_inspectors/http_inspect/http_cutter.h
+++ b/src/service_inspectors/http_inspect/http_cutter.h
@@ -21,6 +21,7 @@
 #define HTTP_CUTTER_H
 
 #include <cassert>
+#include <zlib.h>
 
 #include "http_enum.h"
 #include "http_event.h"
@@ -96,7 +97,8 @@ private:
 class HttpBodyCutter : public HttpCutter
 {
 public:
-    HttpBodyCutter(bool detained_inspection_) : detained_inspection(detained_inspection_) {}
+    HttpBodyCutter(bool detained_inspection_, HttpEnums::CompressId compression_);
+    ~HttpBodyCutter() override;
     void soft_reset() override { octets_seen = 0; packet_detained = false; }
     void detain_ended() { packet_detained = false; }
 
@@ -110,13 +112,16 @@ private:
     bool packet_detained = false;
     uint8_t partial_match = 0;
     bool detention_required = false;
+    HttpEnums::CompressId compression;
+    z_stream* compress_stream = nullptr;
 };
 
 class HttpBodyClCutter : public HttpBodyCutter
 {
 public:
-    HttpBodyClCutter(int64_t expected_length, bool detained_inspection) :
-        HttpBodyCutter(detained_inspection), remaining(expected_length)
+    HttpBodyClCutter(int64_t expected_length, bool detained_inspection,
+        HttpEnums::CompressId compression) :
+        HttpBodyCutter(detained_inspection, compression), remaining(expected_length)
         { assert(remaining > 0); }
     HttpEnums::ScanResult cut(const uint8_t*, uint32_t length, HttpInfractions*, HttpEventGen*,
         uint32_t flow_target, bool stretch) override;
@@ -128,7 +133,8 @@ private:
 class HttpBodyOldCutter : public HttpBodyCutter
 {
 public:
-    explicit HttpBodyOldCutter(bool detained_inspection) : HttpBodyCutter(detained_inspection) {}
+    explicit HttpBodyOldCutter(bool detained_inspection, HttpEnums::CompressId compression) :
+        HttpBodyCutter(detained_inspection, compression) {}
     HttpEnums::ScanResult cut(const uint8_t*, uint32_t, HttpInfractions*, HttpEventGen*,
         uint32_t flow_target, bool stretch) override;
 };
@@ -136,8 +142,8 @@ public:
 class HttpBodyChunkCutter : public HttpBodyCutter
 {
 public:
-    explicit HttpBodyChunkCutter(bool detained_inspection) : HttpBodyCutter(detained_inspection)
-        {}
+    explicit HttpBodyChunkCutter(bool detained_inspection, HttpEnums::CompressId compression) :
+        HttpBodyCutter(detained_inspection, compression) {}
     HttpEnums::ScanResult cut(const uint8_t* buffer, uint32_t length,
         HttpInfractions* infractions, HttpEventGen* events, uint32_t flow_target, bool stretch)
         override;
diff --git a/src/service_inspectors/http_inspect/http_msg_header.cc b/src/service_inspectors/http_inspect/http_msg_header.cc
index c8be9fa29..7cab6bef3 100644
--- a/src/service_inspectors/http_inspect/http_msg_header.cc
+++ b/src/service_inspectors/http_inspect/http_msg_header.cc
@@ -21,6 +21,8 @@
 #include "config.h"
 #endif
 
+#include <cassert>
+
 #include "http_msg_header.h"
 
 #include "decompress/file_decomp.h"
@@ -310,9 +312,8 @@ void HttpMsgHeader::prepare_body()
     setup_utf_decoding();
     setup_file_decompression();
     update_depth();
-    // Limitations on detained inspection will be lifted as the feature is built out
-    session_data->detained_inspection[source_id] = params->detained_inspection &&
-        (source_id == SRC_SERVER) && (session_data->compression[source_id] == CMP_NONE);
+    session_data->detained_inspection[source_id] =
+        params->detained_inspection && (source_id == SRC_SERVER);
     if (source_id == SRC_CLIENT)
     {
         HttpModule::increment_peg_counts(PEG_REQUEST_BODY);
@@ -427,6 +428,7 @@ void HttpMsgHeader::setup_encoding_decompression()
     const int window_bits = (compression == CMP_GZIP) ? GZIP_WINDOW_BITS : DEFLATE_WINDOW_BITS;
     if (inflateInit2(session_data->compress_stream[source_id], window_bits) != Z_OK)
     {
+        assert(false);
         session_data->compression[source_id] = CMP_NONE;
         delete session_data->compress_stream[source_id];
         session_data->compress_stream[source_id] = nullptr;
diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
index 2ddc5927c..7b21953aa 100644
--- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
+++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
@@ -377,8 +377,6 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total,
         }
     }
 
-    // FIXIT-H there is no support for partial flush with compression
-    assert((partial_buffer_length == 0) || (session_data->compression[source_id] == CMP_NONE));
     if (partial_buffer_length > 0)
     {
         assert(session_data->section_offset[source_id] == 0);
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 1316fa564..51fa92165 100644
--- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc
+++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc
@@ -72,12 +72,18 @@ HttpCutter* HttpStreamSplitter::get_cutter(SectionType type,
     case SEC_TRAILER:
         return (HttpCutter*)new HttpHeaderCutter;
     case SEC_BODY_CL:
-        return (HttpCutter*)new HttpBodyClCutter(session_data->data_length[source_id],
-            session_data->detained_inspection[source_id]);
+        return (HttpCutter*)new HttpBodyClCutter(
+            session_data->data_length[source_id],
+            session_data->detained_inspection[source_id],
+            session_data->compression[source_id]);
     case SEC_BODY_CHUNK:
-        return (HttpCutter*)new HttpBodyChunkCutter(session_data->detained_inspection[source_id]);
+        return (HttpCutter*)new HttpBodyChunkCutter(
+            session_data->detained_inspection[source_id],
+            session_data->compression[source_id]);
     case SEC_BODY_OLD:
-        return (HttpCutter*)new HttpBodyOldCutter(session_data->detained_inspection[source_id]);
+        return (HttpCutter*)new HttpBodyOldCutter(
+            session_data->detained_inspection[source_id],
+            session_data->compression[source_id]);
     default:
         assert(false);
         return nullptr;