]> git.ipfire.org Git - thirdparty/snort3.git/commitdiff
code review in progress
authorTom Peters <thopeter@cisco.com>
Thu, 16 Oct 2014 18:58:49 +0000 (14:58 -0400)
committerTom Peters <thopeter@cisco.com>
Thu, 16 Oct 2014 18:58:49 +0000 (14:58 -0400)
src/service_inspectors/nhttp_inspect/nhttp_inspect.cc
src/service_inspectors/nhttp_inspect/nhttp_msg_start.cc
src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc
src/service_inspectors/nhttp_inspect/nhttp_test_input.cc
src/service_inspectors/nhttp_inspect/nhttp_test_input.h
src/service_inspectors/nhttp_inspect/nhttp_test_msgs.txt

index ded9080cfc116b3dad6b500d9bc624b01a1a0e0e..c40e5bd5ef69fbe040d00b07f04a4ab8c61bbd54 100644 (file)
@@ -75,7 +75,8 @@ bool NHttpInspect::get_buf(unsigned id, Packet*, InspectionBuffer& b)
     return true;
 }
 
-ProcessResult NHttpInspect::process(const uint8_t* data, const uint16_t dsize, Flow* const flow, SourceId source_id, bool buf_owner)
+ProcessResult NHttpInspect::process(const uint8_t* data, const uint16_t dsize, Flow* const flow, SourceId source_id,
+   bool buf_owner)
 {
     NHttpFlowData* session_data = (NHttpFlowData*)flow->get_application_data(NHttpFlowData::nhttp_flow_id);
     assert(session_data != nullptr);
index c66d2c5fe1f679a79770f4769dc4eb855e9735c9..5e37fd80e6c3b3f6420cb3e57385d63c129141b2 100644 (file)
@@ -75,10 +75,10 @@ void NHttpMsgStart::gen_events() {}
 
 ProcessResult NHttpMsgStart::worth_detection() {
     // We combine the start line with the headers for sending to detection if they are already available and we will
-    // not exceed paf_max.
+    // not exceed maximum size.
     if ((session_data->header_octets_visible[source_id] > 0) &&
         (session_data->type_expected[source_id] == SEC_HEADER) &&
-        (msg_text.length + session_data->header_octets_visible[source_id]) <= 63780) {
+        (msg_text.length + session_data->header_octets_visible[source_id]) <= MAXOCTETS) {
         return RES_AGGREGATE;
     }
     else {
index 82db37c55fb109e92201816b98edb5dad6809692..687f76824e49e9bd562bd92ba959f78a5787df08 100644 (file)
@@ -177,7 +177,7 @@ StreamSplitter::Status NHttpStreamSplitter::scan (Flow* flow, const uint8_t* dat
         }
       }
       case SEC_BODY: {
-        prepare_flush(session_data, flush_offset, source_id, type,
+        prepare_flush(session_data, flush_offset, source_id, SEC_BODY,
            tcp_close && (length <= session_data->data_length[source_id]),
            0, session_data->data_length[source_id], length, 0);
         return StreamSplitter::FLUSH;
@@ -236,7 +236,7 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned /* tota
         return nullptr;
     }
 
-    bool is_chunk = session_data->section_type[source_id] == SEC_CHUNK;
+    bool is_chunk = (session_data->section_type[source_id] == SEC_CHUNK);
 
     uint8_t*& chunk_buffer = session_data->chunk_buffer[source_id];
     int32_t& chunk_buffer_length = session_data->chunk_buffer_length[source_id];
@@ -258,8 +258,8 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned /* tota
         if (!is_chunk) {
             // start line/headers/body individual section processing with aggregation prior to being sent to detection
             // only the last section added to the buffer goes to the inspector
-            send_to_detection = my_inspector->process(buffer + buffer_length, offset + len - num_excess, flow, source_id,
-               buffer_length == 0);
+            send_to_detection = my_inspector->process(buffer + buffer_length, offset + len - num_excess, flow,
+               source_id, buffer_length == 0);
         }
         else {
             // small chunks are aggregated before processing and are kept here until the buffer is full (paf_max)
index ba1b9a8ae38c97fcb1270d9d10eccfa3e82da2eb..437cd2dc6bf05f9a86ba31f81021c85575803bcb 100644 (file)
 // nhttp_test_input.cc author Tom Peters <thopeter@cisco.com>
 
 #include <assert.h>
-#include <string.h>
-#include <stdio.h>
 #include <stdexcept>
-#include <stdint.h>
 
 #include "nhttp_test_manager.h"
 #include "nhttp_test_input.h"
 
 using namespace NHttpEnums;
 
-NHttpTestInput::NHttpTestInput(const char *file_name) {
+NHttpTestInput::NHttpTestInput(const charfile_name) {
     if ((test_data_file = fopen(file_name, "r")) == nullptr) throw std::runtime_error("Cannot open test input file");
 }
 
@@ -37,7 +34,7 @@ NHttpTestInput::~NHttpTestInput() {
     fclose(test_data_file);
 }
 
-// Read from the test data file and present to PAF.
+// Read from the test data file and present to StreamSplitter.
 // In the process we may need to skip comments, execute simple commands, and handle escape sequences.
 // The best way to understand this function is to read the comments at the top of the file of test cases.
 void NHttpTestInput::scan(uint8_t*& data, uint32_t &length, SourceId &source_id, bool &tcp_close, bool &need_break) {
@@ -52,27 +49,27 @@ void NHttpTestInput::scan(uint8_t*& data, uint32_t &length, SourceId &source_id,
     need_break = false;
 
     if (just_flushed) {
-        // PAF just flushed and it has all been sent to inspection. There may or may not be leftover data from the
-        // last segment that was not flushed.
+        // StreamSplitter just flushed and it has all been sent by reassemble. There may or may not be leftover data
+        // from the last paragraph that was not flushed.
         just_flushed = false;
         data = msg_buf;
         length = end_offset - flush_octets;  // this is the leftover data
         previous_offset = 0;
         end_offset = length;
         if (length > 0) {
-            // Must present unflushed leftovers to PAF again.
-            // If we don't take this opportunity to left justify our data in the buffer we may "walk" to the right until we run out of buffer space
+            // Must present unflushed leftovers to StreamSplitter again. If we don't take this opportunity to left
+            // justify our data in the buffer we may "walk" to the right until we run out of buffer space.
             memmove(msg_buf, msg_buf+flush_octets, length);
             tcp_close = tcp_closed;
             flush_octets = 0;
             return;
         }
-        // If we reach here then PAF has already flushed all the data we have read so far.
+        // If we reach here then StreamSplitter has already flushed all the data we have read so far.
         tcp_closed = false;
         flush_octets = 0;
     }
     else {
-        // The data we gave PAF last time was not flushed
+        // The data we gave StreamSplitter last time was not flushed
         length = 0;
         previous_offset = end_offset;
         data = msg_buf + previous_offset;
@@ -80,7 +77,7 @@ void NHttpTestInput::scan(uint8_t*& data, uint32_t &length, SourceId &source_id,
 
     // Now we need to move forward by reading more data from the file
     int new_char;
-    typedef enum { WAITING, COMMENT, COMMAND, SECTION, ESCAPE, HEXVAL } State;
+    typedef enum { WAITING, COMMENT, COMMAND, PARAGRAPH, ESCAPE, HEXVAL } State;
     State state = WAITING;
     bool ending = false;
     int command_length = 0;
@@ -104,7 +101,7 @@ void NHttpTestInput::scan(uint8_t*& data, uint32_t &length, SourceId &source_id,
                 ending = false;
             }
             else if (new_char != '\n') {
-                state = SECTION;
+                state = PARAGRAPH;
                 ending = false;
                 data[length++] = (uint8_t) new_char;
             }
@@ -117,6 +114,8 @@ void NHttpTestInput::scan(uint8_t*& data, uint32_t &length, SourceId &source_id,
           case COMMAND:
             if (new_char == '\n') {
                 state = WAITING;
+                // FIXIT-L should not change direction with unflushed data remaining from previous paragraph. At the
+                // minimum need to test for this and assert.
                 if ((command_length == strlen("request")) && !memcmp(command_value, "request", strlen("request"))) {
                     source_id = last_source_id = SRC_CLIENT;
                 }
@@ -158,14 +157,14 @@ void NHttpTestInput::scan(uint8_t*& data, uint32_t &length, SourceId &source_id,
                 }
             }
             break;
-          case SECTION:
+          case PARAGRAPH:
             if (new_char == '\\') {
                 state = ESCAPE;
                 ending = false;
             }
             else if (new_char == '\n') {
                 if (ending) {
-                    // Found the blank line that ends the section.
+                    // Found the second consecutive blank line that ends the paragraph.
                     end_offset = previous_offset + length;
                     return;
                 }
@@ -178,15 +177,15 @@ void NHttpTestInput::scan(uint8_t*& data, uint32_t &length, SourceId &source_id,
             break;
           case ESCAPE:
             switch (new_char) {
-              case 'n':  state = SECTION; data[length++] = '\n'; break;
-              case 'r':  state = SECTION; data[length++] = '\r'; break;
-              case 't':  state = SECTION; data[length++] = '\t'; break;
-              case '#':  state = SECTION; data[length++] = '#';  break;
-              case '@':  state = SECTION; data[length++] = '@';  break;
-              case '\\': state = SECTION; data[length++] = '\\'; break;
+              case 'n':  state = PARAGRAPH; data[length++] = '\n'; break;
+              case 'r':  state = PARAGRAPH; data[length++] = '\r'; break;
+              case 't':  state = PARAGRAPH; data[length++] = '\t'; break;
+              case '#':  state = PARAGRAPH; data[length++] = '#';  break;
+              case '@':  state = PARAGRAPH; data[length++] = '@';  break;
+              case '\\': state = PARAGRAPH; data[length++] = '\\'; break;
               case 'x':
               case 'X':  state = HEXVAL; hex_val = 0; num_digits = 0; break;
-              default:   assert(0); state = SECTION; break;
+              default:   assert(0); state = PARAGRAPH; break;
             }
             break;
           case HEXVAL:
@@ -196,12 +195,12 @@ void NHttpTestInput::scan(uint8_t*& data, uint32_t &length, SourceId &source_id,
             else assert(0);
             if (++num_digits == 2) {
                 data[length++] = hex_val;
-                state = SECTION;
+                state = PARAGRAPH;
             }
             break;
         }
         // Don't allow a buffer overrun.
-        if (previous_offset + length >= sizeof(msg_buf)) assert(0);
+        assert(previous_offset + length < sizeof(msg_buf));
     }
     // End-of-file. Return everything we have so far.
     end_offset = previous_offset + length;
@@ -213,8 +212,7 @@ void NHttpTestInput::flush(uint32_t length) {
     flushed = true;
 }
 
-
-void NHttpTestInput::reassemble(uint8_t **buffer, unsigned &length, SourceId source_id, const NHttpFlowData* session_data,
+void NHttpTestInput::reassemble(uint8_t** buffer, unsigned& length, SourceId source_id, const NHttpFlowData* session_data,
    bool& tcp_close) {
     if (!flushed || (source_id != last_source_id)) {
         *buffer = nullptr;
@@ -232,8 +230,8 @@ void NHttpTestInput::reassemble(uint8_t **buffer, unsigned &length, SourceId sou
     else {
         // We need to generate additional data to fill out the body or chunk section. We may come through here
         // multiple times as we generate all the maximum size body sections needed for a single flush.
-        unsigned paf_max = 16384 - session_data->chunk_buffer_length[source_id];
         tcp_close = false;
+        const unsigned paf_max = DATABLOCKSIZE - session_data->chunk_buffer_length[source_id];
         length = (flush_octets <= paf_max) ? flush_octets : paf_max;
         for (uint32_t k = end_offset; k < length; k++) {
             msg_buf[k] = 'A' + k % 26;
@@ -247,6 +245,3 @@ void NHttpTestInput::reassemble(uint8_t **buffer, unsigned &length, SourceId sou
     }
 }
 
-
-
-
index 175256a9a4ee30cd559cd96732ffc8e2457793c6..a2ecb8dfc46c622053a97ce6b06b17dafb0174e6 100644 (file)
@@ -21,6 +21,8 @@
 #ifndef NHTTP_TEST_INPUT_H
 #define NHTTP_TEST_INPUT_H
 
+#include <stdio.h>
+
 #include "nhttp_enum.h"
 #include "nhttp_flow_data.h"
 
@@ -36,13 +38,27 @@ public:
 private:
     FILE* test_data_file;
     uint8_t msg_buf[2 * NHttpEnums::MAXOCTETS];
+
+    // data has been flushed and must be sent by reassemble() before more data may be given to scan()
     bool flushed = false;
-    NHttpEnums::SourceId last_source_id = NHttpEnums::SRC_CLIENT;   // current direction of traffic flow. Toggled by commands in file.
-    bool just_flushed = true;   // all octets sent to inspection and must resume reading the file
-    bool tcp_closed = false;  // so we can keep presenting a TCP close to PAF until all the remaining octets are consumed and flushed
-    uint32_t flush_octets = 0;  // number of octets that have been flushed and must go to inspection
-    uint32_t previous_offset = 0;   // last character in the buffer shown to PAF but not flushed yet
-    uint32_t end_offset = 0;   // last read character in the buffer
+
+    // current direction of traffic flow. Toggled by commands in file.
+    NHttpEnums::SourceId last_source_id = NHttpEnums::SRC_CLIENT;
+
+    // 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
+    bool tcp_closed = false;
+
+    // number of octets that have been flushed and must be sent by reassemble
+    uint32_t flush_octets = 0;
+
+    // last character in the buffer previously shown to PAF but not flushed yet
+    uint32_t previous_offset = 0;
+
+    // last read character in the buffer
+    uint32_t end_offset = 0;
 };
 
 #endif
index 3b7b40d891affe0f7981e401b0134dc440e09ac4..c4e86aec37b6b55ae094a2bb984c9549fbe40a29 100644 (file)
@@ -805,6 +805,25 @@ Transfer-Encoding: chunked\r\n
 
 0\r\n\r\n
 
+@13006
+@break
+@request
+
+GET /wall/fruitwall_file.txt HTTP/1.1
+
+\r\nHost: www.hektik.org\r\n\r\n
+
+@response
+
+HTTP/1.1 200 OK\r\n
+Date: Fri, 01 Aug 2003 21:22:16 GMT\r\n
+Server: Apache/1.3.27 (Unix) mod_ssl/2.8.12 OpenSSL/0.9.7-beta3 PHP/4.3.0\r\n
+Last-Modified: Fri, 01 Aug 2003 18:08:10 GMT\r\n
+ETag: "295682-9-3f2aac8a"\r\n
+Accept-Ranges: bytes\r\n
+Content-Length: 9\r\n
+Content-Type: text/plain\r\n\r\n
+596254978
 
 # ***********************************************************************************************
 # Alerts