Merge pull request #2555 in SNORT/snort3 from ~KATHARVE/snort3:h2i_pp2_rebase to...

Squashed commit of the following:

commit cc9826e066395ea0c703c29dd4572853561e24f8
Author: Katura Harvey <katharve@cisco.com>
Date:   Wed Oct 14 10:46:52 2020 -0400

    http2_inspect: perform hpack decoding on push_promise frames

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 b7bc20bf0e4956d1caea40c0cf2460606a4781cd..c91003fb1f0eec8261066f154b93abc135e0d865 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame_header.cc
@@ -33,7 +33,9 @@
 #include "http2_enum.h"
 #include "http2_flow_data.h"
 #include "http2_hpack.h"
+#include "http2_request_line.h"
 #include "http2_start_line.h"
+#include "http2_status_line.h"
 #include "http2_stream.h"
 
 using namespace snort;
@@ -49,8 +51,12 @@ Http2HeadersFrameHeader::Http2HeadersFrameHeader(const uint8_t* header_buffer,
     if (!process_frame)
         return;
 
-    start_line_generator = Http2StartLine::new_start_line_generator(source_id,
-        session_data->events[source_id], session_data->infractions[source_id]);
+    if (source_id == SRC_CLIENT)
+        start_line_generator = new Http2RequestLine(session_data->events[source_id],
+            session_data->infractions[source_id]);
+    else
+        start_line_generator = new Http2StatusLine(session_data->events[source_id],
+            session_data->infractions[source_id]);
 
     // Decode headers
     if (!hpack_decoder->decode_headers((data.start() + hpack_headers_offset), data.length() -

diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc
index 3311e90e5785d8447e31d8a1b357c092dba0e5a7..35cc75134ca480e9f4c6f46fdeb909e5c0daa26f 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_inspect.cc
+++ b/src/service_inspectors/http2_inspect/http2_inspect.cc
@@ -87,7 +87,7 @@ bool Http2Inspect::get_buf(unsigned id, Packet* p, InspectionBuffer& b)
         return false;
 
     const SourceId source_id = p->is_from_client() ? SRC_CLIENT : SRC_SERVER;
-    Http2Stream* const stream = session_data->get_current_stream(source_id);
+    Http2Stream* const stream = session_data->get_processing_stream(source_id);
     const Field& buffer = stream->get_buf(id);
     if (buffer.length() <= 0)
         return false;

diff --git a/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc b/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc
index e1b994e14ab430c890feb0878a9b53154373b1c0..e370f006440c5358babafe04f2090d701bd9774c 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc
@@ -24,6 +24,9 @@
 #include "http2_push_promise_frame.h"
 
 #include "http2_flow_data.h"
+#include "http2_hpack.h"
+#include "http2_request_line.h"
+#include "http2_start_line.h"
 #include "http2_stream.h"
 #include "http2_utils.h"
 
@@ -33,7 +36,8 @@ using namespace Http2Enums;
 Http2PushPromiseFrame::Http2PushPromiseFrame(const uint8_t* header_buffer,
     const uint32_t header_len, const uint8_t* data_buffer, const uint32_t data_len,
     Http2FlowData* session_data_, HttpCommon::SourceId source_id_, Http2Stream* stream_) :
-    Http2Frame(header_buffer, header_len, data_buffer, data_len, session_data_, source_id_, stream_)
+    Http2HeadersFrame(header_buffer, header_len, data_buffer, data_len, session_data_, source_id_,
+        stream_)
 {
     // If this was a short frame, it's being processed by the stream that sent it. We've already
     // alerted
@@ -59,6 +63,25 @@ Http2PushPromiseFrame::Http2PushPromiseFrame(const uint8_t* header_buffer,
         session_data->events[source_id]->create_event(EVENT_INVALID_FLAG);
         *session_data->infractions[source_id] += INF_INVALID_FLAG;
     }
+
+    start_line_generator = new Http2RequestLine(session_data->events[source_id],
+        session_data->infractions[source_id]);
+
+    hpack_headers_offset += PROMISED_ID_LENGTH;
+
+    // Decode headers
+    if (!hpack_decoder->decode_headers((data.start() + hpack_headers_offset), data.length() -
+        hpack_headers_offset, decoded_headers, start_line_generator, false))
+    {
+        session_data->abort_flow[source_id] = true;
+        session_data->events[source_id]->create_event(EVENT_MISFORMATTED_HTTP2);
+        return;
+    }
+}
+
+Http2PushPromiseFrame::~Http2PushPromiseFrame()
+{
+    delete start_line_generator;
 }
 
 bool Http2PushPromiseFrame::valid_sequence(Http2Enums::StreamState)
@@ -93,18 +116,34 @@ bool Http2PushPromiseFrame::valid_sequence(Http2Enums::StreamState)
     return true;
 }
 
-void Http2PushPromiseFrame::update_stream_state()
+// FIXIT-E current implementation for testing purposes only. Headers are not yet being sent to
+// http_inspect.
+void Http2PushPromiseFrame::analyze_http1()
 {
-    switch (stream->get_state(source_id))
+    if (session_data->abort_flow[source_id])
+        return;
+    
+    detection_required = true;
+
+    if (!start_line_generator->generate_start_line(start_line))
     {
-        case STREAM_EXPECT_HEADERS:
-            stream->set_state(SRC_CLIENT, STREAM_COMPLETE);
-            break;
-        default:
-            //only STREAM_EXPECT_HEADERS is valid so should never get here
-            assert(false);
-            stream->set_state(source_id, STREAM_ERROR);
+        // can't send request or push-promise headers to http_inspect, but response will still
+        // be processed
+        stream->set_state(SRC_CLIENT, STREAM_ERROR);
+        return;
     }
+
+    http1_header = hpack_decoder->get_decoded_headers(decoded_headers);
+}
+
+void Http2PushPromiseFrame::update_stream_state()
+{
+    if (stream->get_state(SRC_CLIENT) == STREAM_EXPECT_HEADERS)
+        stream->set_state(SRC_CLIENT, STREAM_COMPLETE);
+
+    assert(stream->get_state(SRC_SERVER) == STREAM_EXPECT_HEADERS);
+    assert((stream->get_state(SRC_CLIENT) == STREAM_COMPLETE) or
+        (stream->get_state(SRC_CLIENT) == STREAM_ERROR));
 }
 
 uint32_t Http2PushPromiseFrame::get_promised_stream_id(Http2EventGen* const events,
@@ -125,6 +164,7 @@ uint32_t Http2PushPromiseFrame::get_promised_stream_id(Http2EventGen* const even
 void Http2PushPromiseFrame::print_frame(FILE* output)
 {
     fprintf(output, "Push_Promise frame\n");
-    Http2Frame::print_frame(output);
+    start_line.print(output, "Decoded start-line");
+    Http2HeadersFrame::print_frame(output);
 }
 #endif

diff --git a/src/service_inspectors/http2_inspect/http2_push_promise_frame.h b/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
index 8b3f962819cadb38cd22d8502614b8cdab213898..604e56d879b0898671bcbbe8040eff3c1ee1a0eb 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
@@ -26,6 +26,7 @@
 
 #include "http2_enum.h"
 #include "http2_frame.h"
+#include "http2_headers_frame.h"
 
 class Field;
 class Http2Frame;
@@ -36,10 +37,12 @@ using Http2Infractions = Infractions<Http2Enums::INF__MAX_VALUE, Http2Enums::INF
 using Http2EventGen = EventGen<Http2Enums::EVENT__MAX_VALUE, Http2Enums::EVENT__NONE,
     Http2Enums::HTTP2_GID>;
 
-class Http2PushPromiseFrame : public Http2Frame
+class Http2PushPromiseFrame : public Http2HeadersFrame
 {
 public:
+    ~Http2PushPromiseFrame() override;
     bool valid_sequence(Http2Enums::StreamState state) override;
+    void analyze_http1() override;
     void update_stream_state() override;
     static uint32_t get_promised_stream_id(Http2EventGen* const events,
         Http2Infractions* const infractions, const uint8_t* data_buffer, uint32_t data_len);
@@ -56,5 +59,7 @@ private:
         const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* ssn_data,
         HttpCommon::SourceId src_id, Http2Stream* stream);
     static const int32_t PROMISED_ID_LENGTH = 4;
+    Http2StartLine* start_line_generator = nullptr;
+    Field start_line;
 };
 #endif

diff --git a/src/service_inspectors/http2_inspect/http2_request_line.cc b/src/service_inspectors/http2_inspect/http2_request_line.cc
index ec56213d455a9a7d1c2af23fc9ae9312f785a6fb..b1351502ffdb9fdf61fcd92838a876c75d2320c2 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_request_line.cc
+++ b/src/service_inspectors/http2_inspect/http2_request_line.cc
@@ -76,8 +76,7 @@ void Http2RequestLine::process_pseudo_header(const Field& name, const Field& val
     field->set(value.length(), value_str, true);
 }
 
-// This is called on the first non-pseudo-header. Select the appropriate URI form based on the
-// provided pseudo-headers and generate the start line
+// Select the appropriate URI form based on the provided pseudo-headers and generate the start line
 bool Http2RequestLine::generate_start_line(Field& start_line)
 {
     uint32_t bytes_written = 0;
@@ -190,7 +189,7 @@ bool Http2RequestLine::generate_start_line(Field& start_line)
     {
         // FIXIT-E May want to be more lenient than RFC on generating start line
         *infractions += INF_PSEUDO_HEADER_URI_FORM_MISMATCH;
-        events->create_event(EVENT_MISFORMATTED_HTTP2);
+        events->create_event(EVENT_REQUEST_WITHOUT_REQUIRED_FIELD);
         return false;
     }
 

diff --git a/src/service_inspectors/http2_inspect/http2_request_line.h b/src/service_inspectors/http2_inspect/http2_request_line.h
index 07294b438dfab42f918e745c006b256b53d44da7..32d9460c2bbc17eae95ba3bc571946098c4dd764 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_request_line.h
+++ b/src/service_inspectors/http2_inspect/http2_request_line.h
@@ -29,16 +29,12 @@
 class Http2RequestLine : public Http2StartLine
 {
 public:
+    Http2RequestLine(Http2EventGen* const evs, Http2Infractions* const infrs) :
+        Http2StartLine(evs, infrs) { }
     void process_pseudo_header(const Field& name, const Field& value) override;
     bool generate_start_line(Field& start_line) override;
 
-    friend Http2StartLine* Http2StartLine::new_start_line_generator(HttpCommon::SourceId source_id,
-        Http2EventGen* const events, Http2Infractions* const infractions);
-
 private:
-    Http2RequestLine(Http2EventGen* const evs, Http2Infractions* const infrs) :
-        Http2StartLine(evs, infrs) { }
-
     Field method;
     Field path;
     Field scheme;

diff --git a/src/service_inspectors/http2_inspect/http2_start_line.cc b/src/service_inspectors/http2_inspect/http2_start_line.cc
index 940b6cc7d918d64b7ce9051b526669ea62c0b7d1..8e8af4fe8e881e59599746571c30a57a14122133 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_start_line.cc
+++ b/src/service_inspectors/http2_inspect/http2_start_line.cc
@@ -40,13 +40,3 @@ Http2StartLine::~Http2StartLine()
 {
     delete[] start_line_buffer;
 }
-
-Http2StartLine* Http2StartLine::new_start_line_generator(SourceId source_id,
-    Http2EventGen* const events, Http2Infractions* const infractions)
-{
-    if (source_id == SRC_CLIENT)
-        return new Http2RequestLine(events, infractions);
-    else
-        return new Http2StatusLine(events, infractions);
-}
-

diff --git a/src/service_inspectors/http2_inspect/http2_start_line.h b/src/service_inspectors/http2_inspect/http2_start_line.h
index 0e978bebeee7ce5a7fd1f138ed5b8bc32f000fcd..c453169d7f9ed0df037a9eebe94676d4f3a1c1bd 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_start_line.h
+++ b/src/service_inspectors/http2_inspect/http2_start_line.h
@@ -36,9 +36,6 @@ class Http2FlowData;
 class Http2StartLine
 {
 public:
-    static Http2StartLine* new_start_line_generator(HttpCommon::SourceId source_id,
-        Http2EventGen* const events, Http2Infractions* const infractions);
-
     virtual ~Http2StartLine();
 
     friend class Http2Hpack;

diff --git a/src/service_inspectors/http2_inspect/http2_status_line.cc b/src/service_inspectors/http2_inspect/http2_status_line.cc
index 2db7f52f823facbdc18dcbf7b22b525951ab5b44..6d0c8b03c039f9f1c9a0d3ef6af994a7f0bf7c1c 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_status_line.cc
+++ b/src/service_inspectors/http2_inspect/http2_status_line.cc
@@ -52,7 +52,6 @@ void Http2StatusLine::process_pseudo_header(const Field& name, const Field& valu
     }
 }
 
-// This is called on the first non-pseudo-header.
 bool Http2StatusLine::generate_start_line(Field& start_line)
 {
     uint32_t bytes_written = 0;

diff --git a/src/service_inspectors/http2_inspect/http2_status_line.h b/src/service_inspectors/http2_inspect/http2_status_line.h
index fd80b908afdda30d6fbac7983e7a21c8cf64f6a6..0ccdc14cc5b3925c86aee99529f6ca0fd15dee54 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_status_line.h
+++ b/src/service_inspectors/http2_inspect/http2_status_line.h
@@ -28,16 +28,13 @@
 class Http2StatusLine : public Http2StartLine
 {
 public:
+    Http2StatusLine(Http2EventGen* const evs, Http2Infractions* const infrs) :
+        Http2StartLine(evs, infrs) { }
+
     void process_pseudo_header(const Field& name, const Field& value) override;
     bool generate_start_line(Field& start_line) override;
 
-    friend Http2StartLine* Http2StartLine::new_start_line_generator(HttpCommon::SourceId source_id,
-        Http2EventGen* const events, Http2Infractions* const infractions);
-
 private:
-    Http2StatusLine(Http2EventGen* const evs, Http2Infractions* const infrs) :
-        Http2StartLine(evs, infrs) { }
-
     Field status;
 
     static const char* STATUS_NAME;