Pull request #3139: BUG #705517 Http2HeadersFrame::clear is looking at server side...

Merge in SNORT/snort3 from ~MDAGON/snort3:push_promise2 to master

Squashed commit of the following:

commit f57c8f53f1fdfef5a73320471b8ef4369fba6f70
Author: Maya Dagon <mdagon@cisco.com>
Date:   Mon Oct 25 14:52:44 2021 -0400

    http2_inspect: push promise error state check

diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame.cc b/src/service_inspectors/http2_inspect/http2_headers_frame.cc
index c451f7c0c9e14fdeb838be99fb363717033bdc6b..6ec1fd3098bbea1489dc3b9a553ace3770c4553a 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_headers_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame.cc
@@ -53,9 +53,14 @@ Http2HeadersFrame::Http2HeadersFrame(const uint8_t* header_buffer, const uint32_
     hpack_decoder = &session_data->hpack_decoder[source_id];
 }
 
+bool Http2HeadersFrame::in_error_state() const
+{
+    return stream->get_state(source_id) == STREAM_ERROR;
+}
+
 void Http2HeadersFrame::clear()
 {
-    if (session_data->abort_flow[source_id] || stream->get_state(source_id) == STREAM_ERROR)
+    if (session_data->abort_flow[source_id] || in_error_state())
         return;
     Packet dummy_pkt(false);
     dummy_pkt.flow = session_data->flow;

diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame.h b/src/service_inspectors/http2_inspect/http2_headers_frame.h
index 5235e6bc474e078ce0df75d3c646502b2d2a3c63..b400992097ba48752f182bcfa43c1b7d6b782794 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_headers_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame.h
@@ -49,6 +49,7 @@ protected:
     bool decode_headers(Http2StartLine* start_line_generator, bool trailers);
     void process_decoded_headers(HttpFlowData* http_flow, HttpCommon::SourceId hi_source_id);
     uint8_t get_flags_mask() const override;
+    virtual bool in_error_state() const;
 
     Field http1_header;                 // finalized headers to be passed to http_inspect
     uint32_t xtradata_mask = 0;

diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc
index e47fa964eb5f6929c0bf6e8de9196cb61e8d51cd..d77584d204f9b5d2f7982f33eeb28f987b5efc53 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_inspect.cc
+++ b/src/service_inspectors/http2_inspect/http2_inspect.cc
@@ -204,6 +204,7 @@ void Http2Inspect::clear(Packet* p)
     session_data->stream_in_hi = NO_STREAM_ID;
     session_data->processing_stream_id = NO_STREAM_ID;
     session_data->processing_partial_header = false;
+    session_data->set_hi_msg_section(nullptr);
 }
 
 void Http2Inspect::show(const SnortConfig*) const

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 a47578d54ba5455595cdcdea41f3b508737faa40..91b90e10e35f0c9e534db5cccad342eeeec33e86 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_push_promise_frame.cc
@@ -150,6 +150,17 @@ uint32_t Http2PushPromiseFrame::get_promised_stream_id(Http2EventGen* const even
 uint8_t Http2PushPromiseFrame::get_flags_mask() const
 { return (FLAG_END_HEADERS|FLAG_PADDED); }
 
+bool Http2PushPromiseFrame::in_error_state() const
+{
+    // valid_sequence failures set error on source_id side.
+    // Header processing errors set error on client side.
+    // If client side was already in error state, valid_sequence
+    // would have failed.
+    return stream->get_state(SRC_CLIENT) == STREAM_ERROR ||
+        stream->get_state(source_id) == STREAM_ERROR;
+}
+
+
 #ifdef REG_TEST
 void Http2PushPromiseFrame::print_frame(FILE* output)
 {

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 5cb58f933bac8a2c5d1191b829acb6b45745b520..d64f9392ef7beffb79fbc65a7ad4e234e5467c6e 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_push_promise_frame.h
@@ -59,6 +59,8 @@ private:
         HttpCommon::SourceId src_id, Http2Stream* stream);
     uint8_t get_flags_mask() const override;
 
+    bool in_error_state() const override;
+
     static const int32_t PROMISED_ID_LENGTH = 4;
 };
 #endif

diff --git a/src/service_inspectors/http_inspect/http_inspect.cc b/src/service_inspectors/http_inspect/http_inspect.cc
index c5218aee5b2ea14c559a791470e3f859a3939049..9d06c35d652b15e31f6035392f52f5ea65c5475b 100755 (executable)
--- a/src/service_inspectors/http_inspect/http_inspect.cc
+++ b/src/service_inspectors/http_inspect/http_inspect.cc
@@ -618,7 +618,7 @@ void HttpInspect::clear(Packet* p)
 
     if (session_data == nullptr)
     {
-        // assert(false); // FIXIT-M something wrong with H2I Push Promise triggers this.
+        assert(false);
         return;
     }