Merge pull request #2478 in SNORT/snort3 from ~THOPETER/snort3:h2i4 to master

Squashed commit of the following:

commit 5fb3446f7c55d1061ccda7b7566a437a08d702b7
Author: Tom Peters <thopeter@cisco.com>
Date:   Tue Sep 15 19:49:55 2020 -0400

    http2_inspect: refactor HI interactions out of frame constructors

diff --git a/src/pub_sub/test/pub_sub_http_event_test.cc b/src/pub_sub/test/pub_sub_http_event_test.cc
index fac9e4b3738f70c3783d4be988d8816c6106350f..84ce96afaa0087d5d6517febc8dbda97ab03adde 100644 (file)
--- a/src/pub_sub/test/pub_sub_http_event_test.cc
+++ b/src/pub_sub/test/pub_sub_http_event_test.cc
@@ -37,6 +37,10 @@ using namespace snort;
 using namespace HttpCommon;
 
 // Stubs to make the code link
+Field::Field(int32_t length, const uint8_t* start, bool own_the_buffer_) :
+    strt(start), len(length), own_the_buffer(own_the_buffer_)
+{}
+
 void Field::set(const Field& input)
 {
     strt = input.strt;

diff --git a/src/service_inspectors/http2_inspect/http2_frame.h b/src/service_inspectors/http2_inspect/http2_frame.h
index 526d2beae26965bf978399b0f115a95b9793f029..73380eeb8c232b9e352d199b27da5aa4926739cb 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_frame.h
@@ -23,6 +23,8 @@
 #include "service_inspectors/http_inspect/http_common.h"
 #include "service_inspectors/http_inspect/http_field.h"
 
+#include "http2_enum.h"
+
 /* This class is called Http2Frame, but an object of this class may not represent exactly one
  * HTTP/2 frame as received on the wire. For HEADERS frames, the Http2Frame object contains the
  * initial HEADERS frame plus any following CONTINUATION frames grouped together. For DATA frames,
@@ -41,6 +43,9 @@ public:
     static Http2Frame* new_frame(const uint8_t* header_buffer, const int32_t header_len,
         const uint8_t* data_buffer, const int32_t data_len, Http2FlowData* session_data,
         HttpCommon::SourceId source_id, Http2Stream* stream);
+    virtual bool valid_sequence(Http2Enums::StreamState state)
+        { return state != Http2Enums::STREAM_ERROR; }
+    virtual void analyze_http1() { }
     virtual void clear() { }
     virtual const Field& get_buf(unsigned id);
     virtual uint32_t get_xtradata_mask() { return 0; }

diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc b/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
index 6036abc4a9f2355031f40083120891ed1e49d91f..573466a53b3aa136933cdd33405c79eaa7788bf5 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
@@ -66,9 +66,18 @@ Http2HeadersFrameHeader::Http2HeadersFrameHeader(const uint8_t* header_buffer, c
         // FIXIT-E set stream state to error
         return;
     }
-    
-    StreamBuffer stream_buf;
-    HttpFlowData* http_flow;
+}
+
+Http2HeadersFrameHeader::~Http2HeadersFrameHeader()
+{
+    delete start_line;
+    delete start_line_generator;
+}
+
+void Http2HeadersFrameHeader::analyze_http1()
+{
+    if (!process_frame || start_line == nullptr)
+        return;
 
     // http_inspect scan() of start line
     {
@@ -84,6 +93,8 @@ Http2HeadersFrameHeader::Http2HeadersFrameHeader(const uint8_t* header_buffer, c
         assert((int64_t)flush_offset == start_line->length());
     }
 
+    StreamBuffer stream_buf;
+
     // http_inspect reassemble() of start line
     {
         unsigned copied;
@@ -94,7 +105,8 @@ Http2HeadersFrameHeader::Http2HeadersFrameHeader(const uint8_t* header_buffer, c
         assert(copied == (unsigned)start_line->length());
     }
 
-    http_flow = session_data->get_current_stream(source_id)->get_hi_flow_data();
+    HttpFlowData* const http_flow =
+        session_data->get_current_stream(source_id)->get_hi_flow_data();
     // http_inspect eval() and clear() of start line
     {
         Http2DummyPacket dummy_pkt;
@@ -116,12 +128,6 @@ Http2HeadersFrameHeader::Http2HeadersFrameHeader(const uint8_t* header_buffer, c
     process_decoded_headers(http_flow);
 }
 
-Http2HeadersFrameHeader::~Http2HeadersFrameHeader()
-{
-    delete start_line;
-    delete start_line_generator;
-}
-
 #ifdef REG_TEST
 void Http2HeadersFrameHeader::print_frame(FILE* output)
 {

diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_header.h b/src/service_inspectors/http2_inspect/http2_headers_frame_header.h
index 1bb0b3d4ecbddbce0e4b22e392a5fb632addaaa1..28d4778f0ed42cb8491b0226bddb9c02b49d5c11 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_header.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_header.h
@@ -28,11 +28,13 @@ class Http2StartLine;
 class Http2HeadersFrameHeader : public Http2HeadersFrame
 {
 public:
-    ~Http2HeadersFrameHeader();
+    ~Http2HeadersFrameHeader() override;
     
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const int32_t, const uint8_t*,
         const int32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
 
+    void analyze_http1() override;
+
 #ifdef REG_TEST
     void print_frame(FILE* output) override;
 #endif

diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc
index 2c539ab93d580ecb654cecbd71638fdcb36cf853..622af5138f099a5d3b0aaf08c0e08ec6bcc7f362 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.cc
@@ -48,11 +48,24 @@ Http2HeadersFrameTrailer::Http2HeadersFrameTrailer(const uint8_t* header_buffer,
     if (!process_frame)
         return;
 
-    StreamBuffer stream_buf;
-    HttpFlowData* http_flow;
+    // Decode trailers
+    if (!hpack_decoder->decode_headers((data.start() + hpack_headers_offset), data.length() -
+        hpack_headers_offset, decoded_headers, nullptr, true))
+    {
+        session_data->frame_type[source_id] = FT__ABORT;
+        error_during_decode = true;
+    }
+}
 
-    http_flow = session_data->get_current_stream(source_id)->get_hi_flow_data();
+void Http2HeadersFrameTrailer::analyze_http1()
+{
+    if (!process_frame)
+        return;
+
+    HttpFlowData* const http_flow =
+        session_data->get_current_stream(source_id)->get_hi_flow_data();
     assert(http_flow);
+
     if (http_flow->get_type_expected(source_id) != HttpEnums::SEC_TRAILER)
     {
         // If there was no unflushed data on this stream when the trailers arrived, http_inspect
@@ -62,7 +75,8 @@ Http2HeadersFrameTrailer::Http2HeadersFrameTrailer(const uint8_t* header_buffer,
         stream->finish_msg_body(source_id, true, true); // calls http_inspect scan()
 
         unsigned copied;
-        stream_buf = session_data->hi_ss[source_id]->reassemble(session_data->flow,
+        const StreamBuffer stream_buf =
+            session_data->hi_ss[source_id]->reassemble(session_data->flow,
             0, 0, nullptr, 0, PKT_PDU_TAIL, copied);
         assert(stream_buf.data != nullptr);
         assert(copied == 0);
@@ -82,14 +96,6 @@ Http2HeadersFrameTrailer::Http2HeadersFrameTrailer(const uint8_t* header_buffer,
         session_data->hi->clear(&dummy_pkt);
     }
 
-    // Decode headers
-    if (!hpack_decoder->decode_headers((data.start() + hpack_headers_offset), data.length() -
-        hpack_headers_offset, decoded_headers, nullptr, true))
-    {
-        session_data->frame_type[source_id] = FT__ABORT;
-        error_during_decode = true;
-    }
-
     process_decoded_headers(http_flow);
 }
 

diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h
index d8b0aca0f8b577e2603bfc0b0bbc1abf82248db7..1c12ba6bb4b0a2560c1f008eb76bef5e85f20892 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_trailer.h
@@ -29,6 +29,8 @@ public:
     friend Http2Frame* Http2Frame::new_frame(const uint8_t*, const int32_t, const uint8_t*,
         const int32_t, Http2FlowData*, HttpCommon::SourceId, Http2Stream* stream);
 
+    void analyze_http1() override;
+
 #ifdef REG_TEST
     void print_frame(FILE* output) override;
 #endif

diff --git a/src/service_inspectors/http2_inspect/http2_stream.cc b/src/service_inspectors/http2_inspect/http2_stream.cc
index 1cfa5610126bded917689db9c061c05506eb9a5f..44ffc05b6441eca35d9c0642d0c054f3556debc2 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream.cc
@@ -57,6 +57,9 @@ void Http2Stream::eval_frame(const uint8_t* header_buffer, int32_t header_len,
     assert(current_frame == nullptr);
     current_frame = Http2Frame::new_frame(header_buffer, header_len, data_buffer,
         data_len, session_data, source_id, this);
+    valid_frame_order[source_id] = valid_frame_order[source_id] &&
+        current_frame->valid_sequence(state[source_id]);
+    current_frame->analyze_http1();
     current_frame->update_stream_state();
 }
 

diff --git a/src/service_inspectors/http2_inspect/http2_stream.h b/src/service_inspectors/http2_inspect/http2_stream.h
index 3aec9cd6832c8756a435c559fac9b63257884e70..33413687b35492ed9024bd1fa78668936bfb3126 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream.h
+++ b/src/service_inspectors/http2_inspect/http2_stream.h
@@ -78,6 +78,7 @@ public:
 private:
     const uint32_t stream_id;
     Http2FlowData* const session_data;
+    bool valid_frame_order[2] = { true, true };
     Http2Frame* current_frame = nullptr;
     HttpFlowData* hi_flow_data = nullptr;
     HttpMsgSection* hi_msg_section = nullptr;

diff --git a/src/service_inspectors/http_inspect/http_field.cc b/src/service_inspectors/http_inspect/http_field.cc
index a174af2d356a4f246626b0bbee80ad854d3eeda3..2fe4798603c828b0f0b70073903de99a8c27ad58 100644 (file)
--- a/src/service_inspectors/http_inspect/http_field.cc
+++ b/src/service_inspectors/http_inspect/http_field.cc
@@ -32,6 +32,13 @@ using namespace HttpEnums;
 
 const Field Field::FIELD_NULL { STAT_NO_SOURCE };
 
+Field::Field(int32_t length, const uint8_t* start, bool own_the_buffer_) :
+    strt(start), len(length), own_the_buffer(own_the_buffer_)
+{
+    assert(!((start == nullptr) && (length > 0)));
+    assert(!((start != nullptr) && (length < 0)));
+}
+
 void Field::set(int32_t length, const uint8_t* start, bool own_the_buffer_)
 {
     assert(len == STAT_NOT_COMPUTE);

diff --git a/src/service_inspectors/http_inspect/http_field.h b/src/service_inspectors/http_inspect/http_field.h
index 193ab835714e49e3d03075fbe1f8c5f157880bb3..9d42f957c51571221da9881c76d6904061d1a625 100644 (file)
--- a/src/service_inspectors/http_inspect/http_field.h
+++ b/src/service_inspectors/http_inspect/http_field.h
@@ -35,8 +35,7 @@ class Field
 public:
     static const Field FIELD_NULL;
 
-    Field(int32_t length, const uint8_t* start, bool own_the_buffer_ = false) : strt(start),
-        len(length), own_the_buffer(own_the_buffer_) { }
+    Field(int32_t length, const uint8_t* start, bool own_the_buffer_ = false);
     explicit Field(int32_t length) : len(length) { assert(length<=0); }
     Field() = default;
     ~Field() { if (own_the_buffer) delete[] strt; }