Merge pull request #2111 in SNORT/snort3 from ~KATHARVE/snort3:h2_headers to master

Squashed commit of the following:

commit b076d151ec56be77b27a72904e68c9eae18e887b
Author: Katura Harvey <katharve@cisco.com>
Date:   Wed Mar 25 19:06:59 2020 -0400

    http2_inspect: handle Cl and TE headers, and end_stream flags set on headers frames

diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.cc b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
index f97e483cf42a44a5e123a14ea5217e89fb12bb08..c98bc7d6a14ecb743d301ca4a3fcfd3cacb8c63b 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_data_cutter.cc
+++ b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
@@ -154,7 +154,7 @@ StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t*
             if (frame_flags & END_STREAM)
             {
                 session_data->get_current_stream(source_id)->get_hi_flow_data()->
-                    set_http2_end_stream(source_id);
+                    finish_h2_body(source_id);
                 scan_result = session_data->hi_ss[source_id]->scan(&dummy_pkt, nullptr, 0, unused,
                     &http_flush_offset);
                 assert(scan_result == StreamSplitter::FLUSH);

diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame.cc b/src/service_inspectors/http2_inspect/http2_headers_frame.cc
index c2e4743057e9f09f015df4447dace8ad1903d10a..78c5a3e9e3b6dfbff1fb932b90cae3c594d52f0b 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_headers_frame.cc
+++ b/src/service_inspectors/http2_inspect/http2_headers_frame.cc
@@ -44,6 +44,10 @@ Http2HeadersFrame::Http2HeadersFrame(const uint8_t* header_buffer, const int32_t
     HttpCommon::SourceId source_id_) :
     Http2Frame(header_buffer, header_len, data_buffer, data_len, session_data_, source_id_)
 {
+    Http2Stream* const stream = session_data->find_stream(get_stream_id());
+    if (get_flags() & END_STREAM)
+        stream->set_end_stream(source_id);
+
     uint8_t hpack_headers_offset = 0;
 
     // Remove stream dependency if present

diff --git a/src/service_inspectors/http2_inspect/http2_stream.h b/src/service_inspectors/http2_inspect/http2_stream.h
index 5938614cfef7430149da9584c15b64bc7f25f07e..e026ae97cd1a4b898888111a7d00c6a7769dc923 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream.h
+++ b/src/service_inspectors/http2_inspect/http2_stream.h
@@ -51,8 +51,10 @@ public:
     
     Http2DataCutter* get_data_cutter(HttpCommon::SourceId source_id);
     void set_data_cutter(Http2DataCutter* cutter, HttpCommon::SourceId source_id)
-    { data_cutter[source_id] = cutter; }
+        { data_cutter[source_id] = cutter; }
 
+    void set_end_stream(HttpCommon::SourceId source_id) { end_stream_set[source_id] = true; }
+    bool end_stream_is_set(HttpCommon::SourceId source_id) { return end_stream_set[source_id]; }
 #ifdef REG_TEST
     void print_frame(FILE* output);
 #endif
@@ -64,6 +66,7 @@ private:
     HttpFlowData* hi_flow_data = nullptr;
     HttpMsgSection* hi_msg_section = nullptr;
     Http2DataCutter* data_cutter[2] = { nullptr, nullptr};
+    bool end_stream_set[2] = { false, false };
 };
 
 #endif

diff --git a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
index 3401d0ecf90e5a1c5c068edb5276e58bee444a9e..16f7a064a68a6634c52c158e19de33407bc1dcd5 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -48,8 +48,8 @@ StreamSplitter::Status data_scan(Http2FlowData* session_data, const uint8_t* dat
     if (stream)
         http_flow = (HttpFlowData*)stream->get_hi_flow_data();
 
-    if (!stream || !http_flow || (frame_length > 0 and
-        (http_flow->get_type_expected(source_id) != HttpEnums::SEC_BODY_H2)))
+    if (!stream || !http_flow || stream->end_stream_is_set(source_id) ||
+        (frame_length > 0 and (http_flow->get_type_expected(source_id) != HttpEnums::SEC_BODY_H2)))
     {
         *session_data->infractions[source_id] += INF_FRAME_SEQUENCE;
         session_data->events[source_id]->create_event(EVENT_FRAME_SEQUENCE);
@@ -90,6 +90,14 @@ StreamSplitter::Status non_data_scan(Http2FlowData* session_data,
         session_data->scan_remaining_frame_octets[source_id] = frame_length;
         session_data->total_bytes_in_split[source_id] += FRAME_HEADER_LENGTH +
             frame_length;
+
+        // If the stream object exists and the end_stream flag is set, save that state in the stream
+        // object. If this is the first headers frame in the current stream,the stream object has
+        // not been created yet. The end_stream flag will be handled in the headers frame processing
+        Http2Stream* const stream = session_data->find_stream(
+            session_data->current_stream[source_id]);
+        if (stream and frame_flags & END_STREAM)
+            stream->set_end_stream(source_id);
     }
 
     // If we don't have the full frame, keep scanning

diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc
index 30c9bc2734cdb6523b61f0285be423a768258dde..0c4f0069d55159d3e7b0586341179efdb786c551 100644 (file)
--- a/src/service_inspectors/http_inspect/http_cutter.cc
+++ b/src/service_inspectors/http_inspect/http_cutter.cc
@@ -21,6 +21,7 @@
 #include "config.h"
 #endif
 
+#include "http_common.h"
 #include "http_cutter.h"
 #include "http_enum.h"
 
@@ -687,37 +688,60 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length,
     return detain_this_packet ? SCAN_NOT_FOUND_DETAIN : SCAN_NOT_FOUND;
 }
 
-ScanResult HttpBodyH2Cutter::cut(const uint8_t* buffer, uint32_t length, HttpInfractions*,
-    HttpEventGen*, uint32_t flow_target, bool stretch, bool h2_end_stream)
+ScanResult HttpBodyH2Cutter::cut(const uint8_t* buffer, uint32_t length,
+    HttpInfractions* infractions, HttpEventGen* events, uint32_t flow_target, bool stretch,
+    bool h2_body_finished)
 {
-    //FIXIT-M detained inspection not yet supported for http2
+    //FIXIT-E detained inspection not yet supported for http2
     UNUSED(buffer);
 
-    // FIXIT-M stretch not yet supported for http2 message bodies
+    // FIXIT-E stretch not yet supported for http2 message bodies
     UNUSED(stretch);
 
+    // If the headers included a content length header (expected length >= 0), check it against the
+    // actual message body length. Alert if it does not match at the end of the message body, or if
+    // it overflows during the body (alert once then stop computing).
+    if (expected_body_length >= 0)
+    {
+        if ((total_octets_scanned + length) > expected_body_length)
+        {
+            *infractions += INF_H2_DATA_OVERRUNS_CL;
+            events->create_event(EVENT_H2_DATA_OVERRUNS_CL);
+            expected_body_length = HttpCommon::STAT_NOT_COMPUTE;
+        }
+        else if (h2_body_finished and ((total_octets_scanned + length) < expected_body_length))
+        {
+            *infractions += INF_H2_DATA_UNDERRUNS_CL;
+            events->create_event(EVENT_H2_DATA_UNDERRUNS_CL);
+        }
+    }
+
     if (flow_target == 0)
     {
         num_flush = length;
+        total_octets_scanned += length;
         return SCAN_DISCARD_PIECE;
     }
-    if (!h2_end_stream)
+    if (!h2_body_finished)
     {
         if (octets_seen + length < flow_target)
         {
             // Not enough data yet to create a message section
             octets_seen += length;
+            total_octets_scanned += length;
             return SCAN_NOT_FOUND;
         }
         else
         {
             num_flush = flow_target - octets_seen;
+            total_octets_scanned += num_flush;
             return SCAN_FOUND_PIECE;
         }
     }
     else
     {
         // For now if end_stream is set for scan, a zero-length buffer is always sent to flush
+        assert(length == 0);
         num_flush = 0;
         return SCAN_FOUND;
     }

diff --git a/src/service_inspectors/http_inspect/http_cutter.h b/src/service_inspectors/http_inspect/http_cutter.h
index 1ea351b241ca1d8248b99b8bdce29eb00c9452d6..ae3197829f5a03fea817dfc232eb28672a0a9987 100644 (file)
--- a/src/service_inspectors/http_inspect/http_cutter.h
+++ b/src/service_inspectors/http_inspect/http_cutter.h
@@ -36,7 +36,7 @@ public:
     virtual ~HttpCutter() = default;
     virtual HttpEnums::ScanResult cut(const uint8_t* buffer, uint32_t length,
         HttpInfractions* infractions, HttpEventGen* events, uint32_t flow_target, bool stretch,
-        bool h2_end_stream) = 0;
+        bool h2_body_finished) = 0;
     uint32_t get_num_flush() const { return num_flush; }
     uint32_t get_octets_seen() const { return octets_seen; }
     uint32_t get_num_excess() const { return num_crlf; }
@@ -164,10 +164,14 @@ private:
 class HttpBodyH2Cutter : public HttpBodyCutter
 {
 public:
-    explicit HttpBodyH2Cutter(bool detained_inspection, HttpEnums::CompressId compression) :
-        HttpBodyCutter(detained_inspection, compression) {}
+    explicit HttpBodyH2Cutter(int64_t expected_length, bool detained_inspection,
+        HttpEnums::CompressId compression) :
+        HttpBodyCutter(detained_inspection, compression), expected_body_length(expected_length) {}
     HttpEnums::ScanResult cut(const uint8_t*, uint32_t, HttpInfractions*, HttpEventGen*,
-        uint32_t flow_target, bool stretch, bool h2_end_stream) override;
+        uint32_t flow_target, bool stretch, bool h2_body_finished) override;
+private:
+    int64_t expected_body_length;
+    uint32_t total_octets_scanned = 0;
 };
 
 

diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h
index cbfce493f2d7781d6813fdb8855093f336d95bfe..3fc8a0106eda25c701b585da58ce4d539b121496 100644 (file)
--- a/src/service_inspectors/http_inspect/http_enum.h
+++ b/src/service_inspectors/http_inspect/http_enum.h
@@ -227,6 +227,9 @@ enum Infraction
     INF_VERSION_NOT_UPPERCASE,
     INF_CHUNK_LEADING_WS,
     INF_BAD_HEADER_WHITESPACE,
+    INF_H2_NON_IDENTITY_TE,
+    INF_H2_DATA_OVERRUNS_CL,
+    INF_H2_DATA_UNDERRUNS_CL,
     INF__MAX_VALUE
 };
 
@@ -345,6 +348,9 @@ enum EventSid
     EVENT_BAD_HEADER_WHITESPACE,
     EVENT_GZIP_EARLY_END,                  // 248
     EVENT_EXCESS_REPEAT_PARAMS,
+    EVENT_H2_NON_IDENTITY_TE,
+    EVENT_H2_DATA_OVERRUNS_CL,
+    EVENT_H2_DATA_UNDERRUNS_CL,
     EVENT__MAX_VALUE
 };
 

diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h
index a4527e472e323f714e75e4da7a96f4aa58ad1615..e0068cb997b6bcf9206c440c4a3e662bd36c0cbc 100644 (file)
--- a/src/service_inspectors/http_inspect/http_flow_data.h
+++ b/src/service_inspectors/http_inspect/http_flow_data.h
@@ -71,13 +71,13 @@ public:
     HttpEnums::SectionType get_type_expected(HttpCommon::SourceId source_id)
     { return type_expected[source_id]; }
 
-    void set_http2_end_stream(HttpCommon::SourceId source_id)
-    { http2_end_stream[source_id] = true; }
+    void finish_h2_body(HttpCommon::SourceId source_id)
+    { h2_body_finished[source_id] = true; }
 
 private:
     // HTTP/2 handling
     bool for_http2 = false;
-    bool http2_end_stream[2] = { false, false };
+    bool h2_body_finished[2] = { false, false };
 
     // Convenience routines
     void half_reset(HttpCommon::SourceId source_id);

diff --git a/src/service_inspectors/http_inspect/http_msg_body_h2.cc b/src/service_inspectors/http_inspect/http_msg_body_h2.cc
index 1f57873e7452f2797b3eca5646b3f7800b8fdec2..191f2eceed1c59ff7fad65941806909f3806cce0 100644 (file)
--- a/src/service_inspectors/http_inspect/http_msg_body_h2.cc
+++ b/src/service_inspectors/http_inspect/http_msg_body_h2.cc
@@ -26,18 +26,13 @@
 void HttpMsgBodyH2::update_flow()
 {
     session_data->body_octets[source_id] = body_octets;
-    if (session_data->http2_end_stream[source_id])
+    if (session_data->h2_body_finished[source_id])
     {
-        // FIXIT-E check content length header against bytes received
-
         session_data->trailer_prep(source_id);
-        session_data->http2_end_stream[source_id] = false;
+        session_data->h2_body_finished[source_id] = false;
     }
     else
-    {
-        // FIXIT-E check have not exceeded content length
         update_depth();
-    }
 }
 
 #ifdef REG_TEST

diff --git a/src/service_inspectors/http_inspect/http_msg_header.cc b/src/service_inspectors/http_inspect/http_msg_header.cc
index 362b386d5f5ff1cd1ab9f3b54dc3dad684c9dbfb..6a558715890de502273df2e4d3d2419e2c280983 100644 (file)
--- a/src/service_inspectors/http_inspect/http_msg_header.cc
+++ b/src/service_inspectors/http_inspect/http_msg_header.cc
@@ -141,7 +141,7 @@ const Field& HttpMsgHeader::get_true_ip_addr()
 void HttpMsgHeader::gen_events()
 {
     if ((get_header_count(HEAD_CONTENT_LENGTH) > 0) &&
-        (get_header_count(HEAD_TRANSFER_ENCODING) > 0))
+        (get_header_count(HEAD_TRANSFER_ENCODING) > 0) && !session_data->for_http2)
     {
         add_infraction(INF_BOTH_CL_AND_TE);
         create_event(EVENT_BOTH_CL_AND_TE);
@@ -196,15 +196,35 @@ void HttpMsgHeader::update_flow()
         return;
     }
 
+    const Field& te_header = get_header_value_norm(HEAD_TRANSFER_ENCODING);
+
     if (session_data->for_http2)
     {
-        // FIXIT-E check for transfer-encoding and content-length headers
+        // The only transfer-encoding header we should see for HTTP/2 traffic is "identity"
+        const int IDENTITY_SIZE = 8;
+        if ((te_header.length() > 0) && ( (te_header.length() != IDENTITY_SIZE) ||
+            memcmp(te_header.start(), "identity", IDENTITY_SIZE) != 0))
+        {
+            add_infraction(INF_H2_NON_IDENTITY_TE);
+            create_event(EVENT_H2_NON_IDENTITY_TE);
+        }
+        if (get_header_value_norm(HEAD_CONTENT_LENGTH).length() > 0)
+        {
+            const int64_t content_length =
+                norm_decimal_integer(get_header_value_norm(HEAD_CONTENT_LENGTH));
+            if (content_length >= 0)
+                session_data->data_length[source_id] = content_length;
+            else
+            {
+                add_infraction(INF_BAD_CONTENT_LENGTH);
+                create_event(EVENT_BAD_CONTENT_LENGTH);
+            }
+        }
         session_data->type_expected[source_id] = SEC_BODY_H2;
         prepare_body();
         return;
     }
 
-    const Field& te_header = get_header_value_norm(HEAD_TRANSFER_ENCODING);
     if ((te_header.length() > 0) && (version_id == VERS_1_0))
     {
         // HTTP 1.0 should not be chunked and many browsers will ignore the TE header

diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc
index 1e6f193672f18738bb021d56ad26f375273c8407..85c600bafb65465da1873229936432c5ac3700ca 100644 (file)
--- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc
+++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc
@@ -87,6 +87,7 @@ HttpCutter* HttpStreamSplitter::get_cutter(SectionType type,
             session_data->compression[source_id]);
     case SEC_BODY_H2:
         return (HttpCutter*)new HttpBodyH2Cutter(
+            session_data->data_length[source_id],
             session_data->detained_inspection[source_id],
             session_data->compression[source_id]);
     default:
@@ -227,7 +228,8 @@ StreamSplitter::Status HttpStreamSplitter::scan(Packet* pkt, const uint8_t* data
     const ScanResult cut_result = cutter->cut(data, (length <= max_length) ? length :
         max_length, session_data->get_infractions(source_id), session_data->events[source_id],
         session_data->section_size_target[source_id],
-        session_data->stretch_section_to_packet[source_id], session_data->http2_end_stream[source_id]);
+        session_data->stretch_section_to_packet[source_id],
+        session_data->h2_body_finished[source_id]);
     switch (cut_result)
     {
     case SCAN_NOT_FOUND:

diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc
index d13fe4ff07cf4fca13006a477a02ee5e2a52566f..145579d332d34d802cddff829cb920f7ae022cf2 100644 (file)
--- a/src/service_inspectors/http_inspect/http_tables.cc
+++ b/src/service_inspectors/http_inspect/http_tables.cc
@@ -380,6 +380,11 @@ const RuleMap HttpModule::http_events[] =
     { EVENT_GZIP_EARLY_END,             "gzip compressed data followed by unexpected non-gzip "
                                         "data" },
     { EVENT_EXCESS_REPEAT_PARAMS,       "excessive HTTP parameter key repeats" },
+    { EVENT_H2_NON_IDENTITY_TE,         "HTTP/2 Transfer-Encoding header other than identity" },
+    { EVENT_H2_DATA_OVERRUNS_CL,        "HTTP/2 message body overruns Content-Length header "
+                                        "value" },
+    { EVENT_H2_DATA_UNDERRUNS_CL,       "HTTP/2 message body smaller than Content-Length header "
+                                        "value" },
     { 0, nullptr }
 };