Pull request #3946: http2_inspect: update connection settings on ack

Merge in SNORT/snort3 from ~ADMAMOLE/snort3:settings_ack to master

Squashed commit of the following:

commit 28a58b0433ba324da53fcf14398c2cdd205dd0b3
Author: Adrian Mamolea <admamole@cisco.com>
Date:   Tue Jul 25 16:03:20 2023 -0400

    http2_inspect: update connection settings on ack

diff --git a/doc/reference/builtin_stubs.txt b/doc/reference/builtin_stubs.txt
index 1cef38c446a9fbf60b1a36005852f2d17eaa71a0..ec9e52da79b45caba24fcdd11b519677abc8d878 100644 (file)
--- a/doc/reference/builtin_stubs.txt
+++ b/doc/reference/builtin_stubs.txt
@@ -1458,6 +1458,14 @@ Invalid HTTP/2 PRIORITY frame. Stream ID is 0 or length is not 5.
 
 Invalid HTTP/2 GOAWAY frame. R bit is set or stream ID is not 0 or length is less than 8.
 
+121:42
+
+More than 6 unacknowledged settings frames.
+
+121:43
+
+Unexpected settings ACK.
+
 122:1
 
 Basic one host to one host TCP portscan where multiple TCP ports are scanned on

diff --git a/src/service_inspectors/http2_inspect/http2_enum.h b/src/service_inspectors/http2_inspect/http2_enum.h
index b84f1d72599a57c3a922c06fe27263da1ef696fb..586e57730bb50ee2b8cf4a54bc60441b52ef2407 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_enum.h
+++ b/src/service_inspectors/http2_inspect/http2_enum.h
@@ -97,6 +97,8 @@ enum EventSid
     EVENT_LOSS_OF_SYNC = 39,
     EVENT_INVALID_PRIORITY_FRAME = 40,
     EVENT_INVALID_GOAWAY_FRAME = 41,
+    EVENT_SETTINGS_QUEUE_OVERFLOW = 42,
+    EVENT_SETTINGS_QUEUE_UNDERFLOW = 43,
     EVENT__MAX_VALUE
 };
 
@@ -159,6 +161,8 @@ enum Infraction
     INF_BAD_GOAWAY_FRAME_STREAM_ID = 52,
     INF_BAD_GOAWAY_FRAME_LENGTH = 53,
     INF_BAD_GOAWAY_FRAME_R_BIT = 54,
+    INF_SETTINGS_QUEUE_OVERFLOW = 55,
+    INF_SETTINGS_QUEUE_UNDERFLOW = 56,
     INF__MAX_VALUE
 };
 

diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h
index 01fa764d9c1936c8f41df58315206e989d8b11e6..7d3d43d5fc3f6cbff53d12271d9cbf3ae12a6205 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_flow_data.h
+++ b/src/service_inspectors/http2_inspect/http2_flow_data.h
@@ -149,6 +149,7 @@ protected:
 
     // Used in eval()
     Http2ConnectionSettings connection_settings[2];
+    Http2ConnectionSettingsQueue settings_queue[2];
     Http2HpackDecoder hpack_decoder[2];
     std::list<Http2Stream> streams;
     uint32_t concurrent_files = 0;

diff --git a/src/service_inspectors/http2_inspect/http2_settings_frame.cc b/src/service_inspectors/http2_inspect/http2_settings_frame.cc
index 9927fd5685663e718522ad265fd1eba031a61a22..60669d839efd1bc756d9edf83a4a3f9089292180 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_settings_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_settings_frame.cc
@@ -59,16 +59,29 @@ Http2SettingsFrame::Http2SettingsFrame(const uint8_t* header_buffer, const uint3
     }
 
     if (FLAG_ACK & get_flags())
-        return;
-
-    if (src_id == HttpCommon::SRC_SERVER && !ssn_data->was_server_settings_received())
-        ssn_data->set_server_settings_received();
+        apply_settings();
+    else
+    {
+        if (src_id == HttpCommon::SRC_SERVER && !ssn_data->was_server_settings_received())
+            ssn_data->set_server_settings_received();
 
-    parse_settings_frame();
+        queue_settings();
+    }
 }
 
-void Http2SettingsFrame::parse_settings_frame()
+void Http2SettingsFrame::queue_settings()
 {
+    auto& settings_queue = session_data->settings_queue[source_id];
+    // Insert new settings in the queue (duplicating latest queued or current)
+    if (not settings_queue.extend(session_data->connection_settings[source_id]))
+    {
+        session_data->events[source_id]->create_event(EVENT_SETTINGS_QUEUE_OVERFLOW);
+        *session_data->infractions[source_id] += INF_SETTINGS_QUEUE_OVERFLOW;
+    }
+
+    // Update new settings values based on received frame
+    Http2ConnectionSettings& settings = settings_queue.back();
+
     int32_t data_pos = 0;
 
     while (data_pos < data.length())
@@ -82,11 +95,14 @@ void Http2SettingsFrame::parse_settings_frame()
         {
             session_data->events[source_id]->create_event(EVENT_SETTINGS_FRAME_UNKN_PARAM);
             *session_data->infractions[source_id] += INF_SETTINGS_FRAME_UNKN_PARAM;
-            continue;
         }
-
-        if (handle_update(parameter_id, parameter_value))
-            session_data->connection_settings[source_id].set_param(parameter_id, parameter_value);
+        else if (parameter_id == SFID_ENABLE_PUSH and parameter_value > 1)
+        {
+            session_data->events[source_id]->create_event(EVENT_BAD_SETTINGS_VALUE);
+            *session_data->infractions[source_id] += INF_BAD_SETTINGS_PUSH_VALUE;
+        }
+        else
+            settings.set_param(parameter_id, parameter_value);
     }
 }
 
@@ -103,25 +119,40 @@ bool Http2SettingsFrame::sanity_check()
     return !(bad_frame);
 }
 
+void Http2SettingsFrame::apply_settings()
+{
+    // Apply settings to direction opposite to current ACK frame.
+    auto settings_source_id = 1 - source_id;
+    assert(settings_source_id == HttpCommon::SRC_CLIENT || settings_source_id == HttpCommon::SRC_SERVER);
+    auto& settings_queue = session_data->settings_queue[settings_source_id];
+    if (settings_queue.size() == 0)
+    {
+        session_data->events[source_id]->create_event(EVENT_SETTINGS_QUEUE_UNDERFLOW);
+        *session_data->infractions[source_id] += INF_SETTINGS_QUEUE_UNDERFLOW;
+        return;
+    }
+
+    auto& next_settings = settings_queue.front();
+    auto& current_settings = session_data->connection_settings[settings_source_id];
+
+    for (uint16_t parameter_id = SFID_HEADER_TABLE_SIZE; parameter_id <= SFID_MAX_HEADER_LIST_SIZE; ++parameter_id)
+        if (next_settings.get_param(parameter_id) != current_settings.get_param(parameter_id))
+            handle_update(parameter_id, next_settings.get_param(parameter_id));
+
+    current_settings = next_settings;
+    settings_queue.pop();
+}
+
 bool Http2SettingsFrame::handle_update(uint16_t id, uint32_t value)
 {
     switch (id)
     {
         case SFID_HEADER_TABLE_SIZE:
             // Sending a table size parameter informs the receiver the maximum hpack dynamic
-            // table size they may use.
-            session_data->get_hpack_decoder((HttpCommon::SourceId) (1 - source_id))->
+            // table size they may use. The receiver is the sender of this ack.
+            session_data->get_hpack_decoder((HttpCommon::SourceId) (source_id))->
                 settings_table_size_update(value);
             break;
-        case SFID_ENABLE_PUSH:
-            // Only values of 0 or 1 are allowed
-            if (!(value == 0 or value == 1))
-            {
-                session_data->events[source_id]->create_event(EVENT_BAD_SETTINGS_VALUE);
-                *session_data->infractions[source_id] += INF_BAD_SETTINGS_PUSH_VALUE;
-                return false;
-            }
-            break;
         default:
             break;
     }
@@ -136,7 +167,8 @@ void Http2SettingsFrame::print_frame(FILE* output)
     if (bad_frame)
         fprintf(output, " Error in settings frame.");
     else if (FLAG_ACK & get_flags())
-        fprintf(output, " ACK");
+        fprintf(output, " ACK, Header Table Size: %d.",
+                session_data->connection_settings[1 - source_id].get_param(SFID_HEADER_TABLE_SIZE));
     else
         fprintf(output, " Parameters in current frame - %d.", (data.length()/6)) ;
 
@@ -160,3 +192,33 @@ void Http2ConnectionSettings::set_param(uint16_t id, uint32_t value)
 
     parameters[id - 1] = value;
 }
+
+void Http2ConnectionSettingsQueue::pop()
+{
+    assert(size());
+    queue->erase(queue->begin());
+    if (queue->size() == 0)
+    {
+        delete queue;
+        queue = nullptr;
+    }
+}
+
+bool Http2ConnectionSettingsQueue::init(Http2ConnectionSettings& item)
+{
+    queue = new std::vector<Http2ConnectionSettings>();
+    queue->reserve(SETTINGS_QUEUE_MAX);
+    queue->push_back(item);
+    return true;
+}
+
+bool Http2ConnectionSettingsQueue::extend()
+{
+    if (size() == SETTINGS_QUEUE_MAX)
+        // to stay in sync, do an implicit tail drop
+        return false;
+
+    auto& item = back();
+    queue->push_back(item);
+    return true;
+}

diff --git a/src/service_inspectors/http2_inspect/http2_settings_frame.h b/src/service_inspectors/http2_inspect/http2_settings_frame.h
index fd546d861c14a09289451c5887d5d1f3ae074b72..13ddd09a77d07c3e9e55082f311564299ab16132 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_settings_frame.h
+++ b/src/service_inspectors/http2_inspect/http2_settings_frame.h
@@ -42,8 +42,9 @@ private:
         const uint8_t* data_buffer, const uint32_t data_len, Http2FlowData* ssn_data,
         HttpCommon::SourceId src_id, Http2Stream* stream);
 
-    void parse_settings_frame();
+    void queue_settings();
     bool sanity_check();
+    void apply_settings();
     bool handle_update(uint16_t id, uint32_t value);
 
     uint8_t get_flags_mask() const override
@@ -71,4 +72,22 @@ private:
        4294967295  // Max header list size
     };
 };
+
+class Http2ConnectionSettingsQueue
+{
+public:
+    ~Http2ConnectionSettingsQueue() { delete queue; }
+    auto size() { return queue ? queue->size() : 0; }
+    bool extend(Http2ConnectionSettings& item) { return size() ? extend() : init(item); }
+    void pop();
+    auto& front() { assert(size()); return queue->front(); }
+    auto& back() { assert(size()); return queue->back(); }
+
+private:
+    bool init(Http2ConnectionSettings& item);
+    bool extend();
+
+    static const uint8_t SETTINGS_QUEUE_MAX = 6;
+    std::vector<Http2ConnectionSettings>* queue = nullptr;
+};
 #endif

diff --git a/src/service_inspectors/http2_inspect/http2_tables.cc b/src/service_inspectors/http2_inspect/http2_tables.cc
index 674dd6e19a5cb048d08fff696d52f0c8b25f08e9..f9574c5fea80f53a613111ceb3869553e95f096f 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_tables.cc
+++ b/src/service_inspectors/http2_inspect/http2_tables.cc
@@ -75,6 +75,8 @@ const RuleMap Http2Module::http2_events[] =
     { EVENT_LOSS_OF_SYNC,  "not HTTP/2 traffic or unrecoverable HTTP/2 protocol error" },
     { EVENT_INVALID_PRIORITY_FRAME, "invalid HTTP/2 PRIORITY frame" },
     { EVENT_INVALID_GOAWAY_FRAME, "invalid HTTP/2 GOAWAY frame" },
+    { EVENT_SETTINGS_QUEUE_OVERFLOW, "too many unacknowledged settings" },
+    { EVENT_SETTINGS_QUEUE_UNDERFLOW, "setting acknowledgment without actual settings" },
     { 0, nullptr }
 };