Merge pull request #1629 in SNORT/snort3 from ~THOPETER/snort3:nhttp121 to master

Squashed commit of the following:

commit 1d76e71bc035d419559cdb56b39eee2c3309f39b
Author: Tom Peters <thopeter@cisco.com>
Date:   Tue Jun 4 16:49:41 2019 -0400

    http_inspect: test tool enhancement

diff --git a/src/service_inspectors/http_inspect/dev_notes.txt b/src/service_inspectors/http_inspect/dev_notes.txt
index 2cd87e624bf2b5ea9308575342434025515b31ca..3443b90a16d890cd1147cdfafc034f9d80925df6 100644 (file)
--- a/src/service_inspectors/http_inspect/dev_notes.txt
+++ b/src/service_inspectors/http_inspect/dev_notes.txt
@@ -179,7 +179,7 @@ lines in the middle of a paragraph.
 Commands:
   @break resets HTTP Inspect data structures and begins a new test. Use it liberally to prevent
      unrelated tests from interfering with each other.
-  @tcpclose simulates a half-duplex TCP close following the next paragraph of data.
+  @tcpclose simulates a half-duplex TCP close.
   @request and @response set the message direction. Applies to subsequent paragraphs until changed.
      The initial direction is always request and the break command resets the direction to request.
   @fill <decimal number> create a paragraph consisting of <number> octets of auto-fill data

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 259e075c54aeea6bcbb9947022e385f437669f4b..81d3dcab1cb6452acc615ea1df4d44af6da759ae 100644 (file)
--- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
+++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc
@@ -258,7 +258,8 @@ const snort::StreamBuffer HttpStreamSplitter::reassemble(snort::Flow* flow, unsi
             if (test_buffer == nullptr)
             {
                 // Source ID does not match test data, no test data was flushed, preparing for a
-                // partial flush, or there is no more test data
+                // partial flush, preparing for a TCP connection close, or there is no more test
+                // data
                 return http_buf;
             }
             data = test_buffer;

diff --git a/src/service_inspectors/http_inspect/http_test_input.cc b/src/service_inspectors/http_inspect/http_test_input.cc
index 867845102c585ebcf49072827a42a8477931c135..64fb798f5a44dbee5e3e6d8d00246beb5a7132a3 100644 (file)
--- a/src/service_inspectors/http_inspect/http_test_input.cc
+++ b/src/service_inspectors/http_inspect/http_test_input.cc
@@ -61,9 +61,6 @@ void HttpTestInput::reset()
     just_flushed = true;
     tcp_closed = false;
     flush_octets = 0;
-    close_pending = false;
-    close_notified = false;
-    finish_expected = false;
     need_break = false;
 
     for (int k = 0; k <= 1; k++)
@@ -218,6 +215,11 @@ void HttpTestInput::scan(uint8_t*& data, uint32_t& length, SourceId source_id, u
                     "tcpclose", strlen("tcpclose")))
                 {
                     tcp_closed = true;
+                    if (!skip_to_break)
+                    {
+                        length = 0;
+                        return;
+                    }
                 }
                 else if ((command_length > strlen("fill")) && !memcmp(command_value, "fill",
                     strlen("fill")))
@@ -449,74 +451,34 @@ void HttpTestInput::reassemble(uint8_t** buffer, unsigned& length, SourceId sour
     bool& tcp_close, bool& partial_flush)
 {
     *buffer = nullptr;
-    tcp_close = false;
     partial_flush = false;
+    tcp_close = false;
 
     // Only piggyback on data moving in the same direction.
-    // Need flushed data unless the connection is closing.
-    if ((source_id != last_source_id) || (!flushed && !tcp_closed && !partial))
+    if (source_id != last_source_id)
+        return;
+
+    if (tcp_closed)
     {
+        // Give the caller a chance to call finish()
+        tcp_close = true;
         return;
     }
 
-    if (partial)
+    // Need flushed data unless it's a partial flush.
+    if (!flushed && !partial)
     {
-        // Give the caller a chance to set up for a partial flush before giving him the data
-        partial_flush = true;
-        partial = false;
         return;
     }
 
-    // How we process TCP close situations depends on the size of the flush relative to the data
-    // buffer.
-    // 1. less than whole buffer - not the final flush, ignore pending close
-    // 2. exactly equal - process data now and signal the close next time around
-    // 3. there was no flush - signal the close now and send the leftovers next time around
-    if (tcp_closed && (!flushed || (flush_octets == end_offset[last_source_id])))
+    if (partial)
     {
-        if (close_pending)
-        {
-            // There is no more data. Clean up and notify caller about close.
-            just_flushed = true;
-            flushed = false;
-            end_offset[last_source_id] = 0;
-            previous_offset[last_source_id] = 0;
-            close_pending = false;
-            tcp_closed = false;
-            tcp_close = true;
-            finish_expected = true;
-        }
-        else if (!flushed)
-        {
-            // Failure to flush means scan() reached end of paragraph and returned PAF_SEARCH.
-            // Notify caller about close and they will do a zero-length flush().
-            previous_offset[last_source_id] = end_offset[last_source_id];
-            tcp_close = true;
-            close_notified = true;
-            finish_expected = true;
-        }
-        else
-        {
-            // The flush point is the end of the paragraph. Supply the data now and if necessary
-            // notify the caller about close next time or otherwise just clean up.
-            *buffer = msg_buf[last_source_id];
-            length = flush_octets;
-            if (close_notified)
-            {
-                just_flushed = true;
-                flushed = false;
-                close_notified = false;
-                tcp_closed = false;
-            }
-            else
-            {
-                close_pending = true;
-            }
-        }
+        // Give the caller a chance to set up for a partial flush before giving him the data
+        partial_flush = true;
+        partial = false;
         return;
     }
 
-    // Normal case with no TCP close or at least not yet
     *buffer = msg_buf[last_source_id];
     length = flush_octets;
     just_flushed = true;
@@ -525,13 +487,12 @@ void HttpTestInput::reassemble(uint8_t** buffer, unsigned& length, SourceId sour
 
 bool HttpTestInput::finish()
 {
-    if (finish_expected)
+    if (tcp_closed)
     {
-        finish_expected = false;
+        tcp_closed = false;
         return true;
     }
     return false;
 }
-
 #endif
 

diff --git a/src/service_inspectors/http_inspect/http_test_input.h b/src/service_inspectors/http_inspect/http_test_input.h
index 3b3cb20268251347b3d33d1c1fb1b4c67493f34c..2fe100f96fe6353945e3c8c4f1ca43035daa90ce 100644 (file)
--- a/src/service_inspectors/http_inspect/http_test_input.h
+++ b/src/service_inspectors/http_inspect/http_test_input.h
@@ -43,10 +43,10 @@ private:
     uint8_t msg_buf[2][2 * HttpEnums::MAX_OCTETS];
     FILE* include_file[2] = { nullptr, nullptr };
 
-    // break command has been read and we are waiting for a new flow to start
+    // break command has been read and we are waiting for a new underlying flow to start
     bool need_break = false;
 
-    // Sequence number of the flow we are currently piggybacking on
+    // Sequence number of the underlying flow we are currently piggybacking on
     uint64_t curr_seq_num = 0;
 
     // data has been flushed and must be sent by reassemble() before more data may be given to
@@ -59,7 +59,7 @@ private:
     // reassemble() just completed and all flushed octets forwarded, time to resume scan()
     bool just_flushed = true;
 
-    // TCP connection directional close at end of current paragraph
+    // TCP connection directional close
     bool tcp_closed = false;
 
     // partial flush requested, useful for testing accelerated blocking
@@ -74,15 +74,6 @@ private:
     // number of characters in the buffer
     uint32_t end_offset[2] = { 0, 0 };
 
-    // Need to send close with next pass through reassemble()
-    bool close_pending = false;
-
-    // Close notification already provided
-    bool close_notified = false;
-
-    // tcp_close notification given and we are waiting for a HttpStreamSplitter::finish() call.
-    bool finish_expected = false;
-
     void reset();
 };