From: Mike Stepanek (mstepane) Date: Wed, 28 Oct 2020 15:46:44 +0000 (+0000) Subject: Merge pull request #2555 in SNORT/snort3 from ~KATHARVE/snort3:h2i_pp2_rebase to... X-Git-Tag: 3.0.3-5~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0de5be34551eab2a3898243433edadcf60099c22;p=thirdparty%2Fsnort3.git Merge pull request #2555 in SNORT/snort3 from ~KATHARVE/snort3:h2i_pp2_rebase to master Squashed commit of the following: commit cc9826e066395ea0c703c29dd4572853561e24f8 Author: Katura Harvey 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 b7bc20bf0..c91003fb1 100644 --- 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 3311e90e5..35cc75134 100644 --- 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 e1b994e14..e370f0064 100644 --- 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 8b3f96281..604e56d87 100644 --- 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; -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 ec56213d4..b1351502f 100644 --- 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 07294b438..32d9460c2 100644 --- 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 940b6cc7d..8e8af4fe8 100644 --- 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 0e978bebe..c453169d7 100644 --- 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 2db7f52f8..6d0c8b03c 100644 --- 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 fd80b908a..0ccdc14cc 100644 --- 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;