From: Mike Stepanek (mstepane) Date: Tue, 3 Sep 2019 12:42:25 +0000 (-0400) Subject: Merge pull request #1720 in SNORT/snort3 from ~KATHARVE/snort3:http2_framework to... X-Git-Tag: 3.0.0-261~14 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=48183d3bdb81f168d29a058eb4d2f0159e7c889e;p=thirdparty%2Fsnort3.git Merge pull request #1720 in SNORT/snort3 from ~KATHARVE/snort3:http2_framework to master Squashed commit of the following: commit 513ce97b05f7efc8d49df200bf7f26bd4bc3afb2 Author: Katura Harvey Date: Fri Aug 30 14:17:29 2019 -0400 modify event enum names and correctly handle preface split multile across tcp packets commit f209fca6eaa6825f627d87f76321c41cc95a0ec7 Author: Katura Harvey Date: Fri Aug 30 11:37:22 2019 -0400 fix data length in unit test commit b1da12736d0576d1004d8320dcdda6e9e66fccb0 Author: Katura Harvey Date: Thu Aug 29 22:59:47 2019 -0400 update unit test to avoid adding another spelling exception commit 687d7c7f9e827c283962d991ef25a738f9c25c82 Author: Katura Harvey Date: Thu Aug 29 17:19:14 2019 -0400 address more comments commit 5ee375bae4390516802cef80e69b2da16df1726f Author: Katura Harvey Date: Thu Aug 29 17:15:49 2019 -0400 fix bug in scan - it wasn't actually searching until the end of data frames commit 039c6513104af4116d51e3e72ddf570f581eda90 Author: Katura Harvey Date: Thu Aug 29 10:36:10 2019 -0400 fix comment commit b7f2c09c64a7c6db49351dd53bb2c5f2ebed0215 Author: Katura Harvey Date: Wed Aug 28 10:48:57 2019 -0400 address first batch of comments commit 559e6de2c803bb2bd09179624ac7b35d59b060f1 Author: Katura Harvey Date: Tue Aug 27 10:42:42 2019 -0400 code cleanup commit 918fb7e2de8533fb3e9f14f3c5488757abd1be95 Author: Katura Harvey Date: Mon Aug 26 21:19:03 2019 -0400 http2_inspect: send raw encoded headers to detection --- diff --git a/src/service_inspectors/http2_inspect/http2_api.cc b/src/service_inspectors/http2_inspect/http2_api.cc index 02f32bd14..cbfd8f10b 100644 --- a/src/service_inspectors/http2_inspect/http2_api.cc +++ b/src/service_inspectors/http2_inspect/http2_api.cc @@ -40,6 +40,7 @@ const char* Http2Api::classic_buffer_names[] = { "http2_frame_type", "http2_raw_frame", + "http2_decoded_header", nullptr }; @@ -73,6 +74,7 @@ const InspectApi Http2Api::http2_api = extern const BaseApi* ips_http2_frame_header; extern const BaseApi* ips_http2_frame_data; +extern const BaseApi* ips_http2_decoded_header; #ifdef BUILDING_SO SO_PUBLIC const BaseApi* snort_plugins[] = @@ -83,6 +85,7 @@ const BaseApi* sin_http2[] = &Http2Api::http2_api.base, ips_http2_frame_header, ips_http2_frame_data, + ips_http2_decoded_header, nullptr }; diff --git a/src/service_inspectors/http2_inspect/http2_enum.h b/src/service_inspectors/http2_inspect/http2_enum.h index f67920435..55590f832 100644 --- a/src/service_inspectors/http2_inspect/http2_enum.h +++ b/src/service_inspectors/http2_inspect/http2_enum.h @@ -36,7 +36,8 @@ enum FrameType { FT_DATA=0, FT_HEADERS=1, FT_PRIORITY=2, FT_RST_STREAM=3, FT_SET // Message buffers available to clients // This enum must remain synchronized with Http2Api::classic_buffer_names[] -enum HTTP2_BUFFER { HTTP2_BUFFER_FRAME_HEADER = 1, HTTP2_BUFFER_FRAME_DATA, HTTP2_BUFFER_MAX }; +enum HTTP2_BUFFER { HTTP2_BUFFER_FRAME_HEADER = 1, HTTP2_BUFFER_FRAME_DATA, HTTP2_BUFFER_DECODED_HEADER, + HTTP2_BUFFER_MAX }; // Peg counts // This enum must remain synchronized with Http2Module::peg_names[] in http2_tables.cc @@ -49,6 +50,8 @@ enum EventSid EVENT_INT_DECODE_FAILURE = 1, EVENT_INT_LEADING_ZEROS = 2, EVENT_STRING_DECODE_FAILURE = 3, + EVENT_MISSING_CONTINUATION = 4, + EVENT_UNEXPECTED_CONTINUATION = 5, EVENT__MAX_VALUE }; @@ -66,8 +69,19 @@ enum Infraction INF_HUFFMAN_BAD_PADDING = 7, INF_HUFFMAN_DECODED_EOS = 8, INF_HUFFMAN_INCOMPLETE_CODE_PADDING = 9, + INF_MISSING_CONTINUATION = 10, + INF_UNEXPECTED_CONTINUATION = 11, INF__MAX_VALUE -}; +}; + +enum HeaderFrameFlags +{ + END_STREAM = 0x1, + END_HEADERS = 0x4, + PADDED = 0x8, + PRIORITY = 0x20, +}; + } // end namespace Http2Enums diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.cc b/src/service_inspectors/http2_inspect/http2_flow_data.cc index 5656c150f..c074b58d2 100644 --- a/src/service_inspectors/http2_inspect/http2_flow_data.cc +++ b/src/service_inspectors/http2_inspect/http2_flow_data.cc @@ -30,6 +30,7 @@ using namespace snort; using namespace Http2Enums; +using namespace HttpCommon; unsigned Http2FlowData::inspector_id = 0; @@ -71,6 +72,27 @@ Http2FlowData::~Http2FlowData() { delete[] frame_header[k]; delete[] frame[k]; + delete[] http2_decoded_header[k]; + delete[] header_frame_header[k]; + delete infractions[k]; + delete events[k]; } } +int Http2FlowData::get_frame_type(HttpCommon::SourceId source_id) +{ + const int frame_type_index = 3; + if (frame_header[source_id]) + return frame_header[source_id][frame_type_index]; + else + return STAT_NO_SOURCE; +} + +int Http2FlowData::get_frame_flags(HttpCommon::SourceId source_id) +{ + const int frame_flags_index = 4; + if (frame_header[source_id]) + return frame_header[source_id][frame_flags_index]; + else + return STAT_NO_SOURCE; +} diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h index 3edecd93a..fd181b8d9 100644 --- a/src/service_inspectors/http2_inspect/http2_flow_data.h +++ b/src/service_inspectors/http2_inspect/http2_flow_data.h @@ -20,11 +20,23 @@ #ifndef HTTP2_FLOW_DATA_H #define HTTP2_FLOW_DATA_H +#include + +#include "main/snort_types.h" +#include "utils/event_gen.h" +#include "utils/infractions.h" #include "flow/flow.h" #include "service_inspectors/http_inspect/http_common.h" +#include "service_inspectors/http_inspect/http_field.h" #include "stream/stream_splitter.h" + #include "http2_enum.h" +using Http2Infractions = Infractions; + +using Http2EventGen = EventGen; + class Http2FlowData : public snort::FlowData { public: @@ -54,9 +66,28 @@ protected: uint32_t frame_size[2] = { 0, 0 }; uint8_t* frame_data[2] = { nullptr, nullptr }; uint32_t frame_data_size[2] = { 0, 0 }; + uint8_t* http2_decoded_header[2] = { nullptr, nullptr }; + uint32_t http2_decoded_header_size[2] = { 0, 0 }; uint32_t leftover_data[2] = { 0, 0 }; uint32_t octets_seen[2] = { 0, 0 }; + uint32_t header_octets_seen[2] = { 0, 0 }; + uint32_t inspection_section_length[2] = { 0, 0 }; bool frame_in_detection = false; + + int32_t get_frame_type(HttpCommon::SourceId source_id); + int32_t get_frame_flags(HttpCommon::SourceId source_id); + + bool continuation_expected = false; + //FIXIT-M Most of this will need to change when we handle multiple streams, so this vector is + //not intended to be the best long-term solution + std::vector continuation_frame_lengths; + uint8_t* header_frame_header[2] = { nullptr, nullptr }; + + // These will eventually be moved over to the frame/stream object, as they are moved to the + // transaction in NHI. Also as in NHI accessor methods will need to be added. + Http2Infractions* infractions[2] = { new Http2Infractions, new Http2Infractions }; + Http2EventGen* events[2] = { new Http2EventGen, new Http2EventGen }; + #ifdef REG_TEST static uint64_t instance_count; diff --git a/src/service_inspectors/http2_inspect/http2_hpack_int_decode.h b/src/service_inspectors/http2_inspect/http2_hpack_int_decode.h index 48ef44557..ce0e17f59 100644 --- a/src/service_inspectors/http2_inspect/http2_hpack_int_decode.h +++ b/src/service_inspectors/http2_inspect/http2_hpack_int_decode.h @@ -39,6 +39,8 @@ public: private: const uint8_t prefix_mask; + // FIXIT-M These will get merged into the corresponding frame/stream object infractions and + // events Http2EventGen* const events; Http2Infractions* const infractions; }; diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc index 436167b3a..e8b84060e 100644 --- a/src/service_inspectors/http2_inspect/http2_inspect.cc +++ b/src/service_inspectors/http2_inspect/http2_inspect.cc @@ -131,6 +131,16 @@ void Http2Inspect::clear(Packet* p) session_data->frame_header[source_id] = nullptr; delete[] session_data->frame[source_id]; session_data->frame[source_id] = nullptr; + session_data->frame_data[source_id] = nullptr; session_data->frame_in_detection = false; + delete[] session_data->http2_decoded_header[source_id]; + session_data->http2_decoded_header[source_id] = nullptr; + session_data->continuation_expected = false; + if (session_data->header_frame_header[source_id]) + { + delete[] session_data->header_frame_header[source_id]; + session_data->header_frame_header[source_id] = nullptr; + } + session_data->continuation_frame_lengths.clear(); } diff --git a/src/service_inspectors/http2_inspect/http2_inspect_impl.cc b/src/service_inspectors/http2_inspect/http2_inspect_impl.cc index d1fe19176..e8f5135c0 100644 --- a/src/service_inspectors/http2_inspect/http2_inspect_impl.cc +++ b/src/service_inspectors/http2_inspect/http2_inspect_impl.cc @@ -47,6 +47,12 @@ bool implement_get_buf(unsigned id, Http2FlowData* session_data, SourceId source b.data = session_data->frame_data[source_id]; b.len = session_data->frame_data_size[source_id]; break; + case HTTP2_BUFFER_DECODED_HEADER: + if (session_data->http2_decoded_header[source_id] == nullptr) + return false; + b.data = session_data->http2_decoded_header[source_id]; + b.len = session_data->http2_decoded_header_size[source_id]; + break; default: return false; } 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 caa0080e8..91d5efe5d 100644 --- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc +++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc @@ -26,19 +26,34 @@ #include "http2_stream_splitter.h" #include "protocols/packet.h" #include "service_inspectors/http_inspect/http_common.h" + #include "http2_flow_data.h" using namespace snort; using namespace HttpCommon; using namespace Http2Enums; +static uint32_t get_frame_length(const uint8_t *frame_buffer) +{ + return (frame_buffer[0] << 16) + (frame_buffer[1] << 8) + frame_buffer[2]; +} + StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t* data, uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id) { + StreamSplitter::Status status = StreamSplitter::FLUSH; if (session_data->preface[source_id]) { // 24-byte preface, not a real frame, no frame header - *flush_offset = 24; + const uint32_t preface_length = 24; + if (session_data->octets_seen[source_id] + length < preface_length) + { + session_data->octets_seen[source_id] += length; + return status = StreamSplitter::SEARCH; + } + + //FIXIT-M verify preface is correct, else generate loss of sync event and abort + *flush_offset = 24 - session_data->octets_seen[source_id]; session_data->header_coming[source_id] = false; session_data->preface[source_id] = false; } @@ -46,52 +61,148 @@ StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t { // Continuation of ongoing data frame session_data->header_coming[source_id] = false; - *flush_offset = (session_data->leftover_data[source_id] < DATA_SECTION_SIZE) ? - session_data->leftover_data[source_id] : DATA_SECTION_SIZE; - session_data->leftover_data[source_id] -= *flush_offset; + + // If this is a new section, update next inspection_section_length + if (session_data->octets_seen[source_id] == 0) + { + if (session_data->leftover_data[source_id] > DATA_SECTION_SIZE) + session_data->inspection_section_length[source_id] = DATA_SECTION_SIZE; + else + session_data->inspection_section_length[source_id] = + session_data->leftover_data[source_id]; + } + + // Don't have full inspection section, keep scanning + if (session_data->octets_seen[source_id] + length < + session_data->inspection_section_length[source_id]) + { + session_data->octets_seen[source_id] += length; + return status = StreamSplitter::SEARCH; + } + + // Have full inspection section, flush and update leftover + *flush_offset = session_data->inspection_section_length[source_id]; + session_data->leftover_data[source_id] -= session_data->inspection_section_length[source_id]; + session_data->octets_seen[source_id] = 0; } else { // frame with header - if (session_data->frame_header[source_id] == nullptr) + // If there is a header frame followed by a continuation frame in the same tcp segment, + // need to process multiple frames in a single scan + *flush_offset = 0; + do { - session_data->header_coming[source_id] = true; - session_data->frame_header[source_id] = new uint8_t[FRAME_HEADER_LENGTH]; - session_data->octets_seen[source_id] = 0; - } + if (session_data->frame_header[source_id] == nullptr) + { + session_data->header_coming[source_id] = true; + session_data->frame_header[source_id] = new uint8_t[FRAME_HEADER_LENGTH]; + session_data->octets_seen[source_id] = 0; + session_data->header_octets_seen[source_id] = 0; + } - // The first nine bytes are the frame header. But all nine might not all be present in the - // first TCP segment we receive. - for (uint32_t k = 0; (k < length) && (session_data->octets_seen[source_id] < - FRAME_HEADER_LENGTH); k++, session_data->octets_seen[source_id]++) - { - session_data->frame_header[source_id][session_data->octets_seen[source_id]] = data[k]; - } - if (session_data->octets_seen[source_id] < FRAME_HEADER_LENGTH) - return StreamSplitter::SEARCH; - - uint32_t const frame_length = (session_data->frame_header[source_id][0] << 16) + - (session_data->frame_header[source_id][1] << 8) + - session_data->frame_header[source_id][2]; - if ((session_data->frame_header[source_id][3] == FT_DATA) && - (frame_length > DATA_SECTION_SIZE)) - { - // Long data frame is cut into pieces - *flush_offset = DATA_SECTION_SIZE + FRAME_HEADER_LENGTH; - session_data->leftover_data[source_id] = frame_length - DATA_SECTION_SIZE; - } - else if (frame_length + FRAME_HEADER_LENGTH > MAX_OCTETS) - { - // FIXIT-M long non-data frame needs to be supported - return StreamSplitter::ABORT; - } - else - { - // Normal case - *flush_offset = frame_length + FRAME_HEADER_LENGTH; - } + // The first nine bytes are the frame header. But all nine might not all be present in the + // first TCP segment we receive. + for (uint32_t k = 0; (k < length) && (session_data->header_octets_seen[source_id] < + FRAME_HEADER_LENGTH); k++, session_data->header_octets_seen[source_id]++) + { + session_data->frame_header[source_id][session_data->header_octets_seen[source_id]] = data[k]; + } + if (session_data->header_octets_seen[source_id] < FRAME_HEADER_LENGTH) + { + session_data->octets_seen[source_id] += length; + status = StreamSplitter::SEARCH; + break; + } + int type = session_data->get_frame_type(source_id); + + // Frame length does not include the frame header + uint32_t const frame_length = get_frame_length(session_data->frame_header[source_id]); + + // For non-data frames, send a full frame to detection + session_data->inspection_section_length[source_id] = frame_length + FRAME_HEADER_LENGTH; + + if ((type == FT_DATA) && (frame_length > DATA_SECTION_SIZE)) + { + // Break up long data frames into pieces for detection + session_data->inspection_section_length[source_id] = DATA_SECTION_SIZE + FRAME_HEADER_LENGTH; + } + else if (frame_length + FRAME_HEADER_LENGTH > MAX_OCTETS) + { + // FIXIT-M long non-data frame needs to be supported + status = StreamSplitter::ABORT; + break; + } + + if (length + session_data->octets_seen[source_id] < + session_data->inspection_section_length[source_id]) + { + // If we don't have the full inspection length, keep scanning + session_data->octets_seen[source_id] += length; + status = StreamSplitter::SEARCH; + break; + } + else + { + // we have the full frame section to flush to detection + *flush_offset += session_data->inspection_section_length[source_id] - session_data->octets_seen[source_id]; + session_data->leftover_data[source_id] = frame_length + FRAME_HEADER_LENGTH + - session_data->inspection_section_length[source_id]; + session_data->octets_seen[source_id] = 0; + } + + // Process all header frames as one unit - if the END_HEADERS flag is not set and scan + // is out of data, tell stream to keep searching + if (type == FT_HEADERS && !(session_data->get_frame_flags(source_id) & END_HEADERS)) + { + session_data->continuation_expected = true; + // We need to save the header frame header for reassembly + session_data->header_frame_header[source_id] = new uint8_t[FRAME_HEADER_LENGTH]; + memcpy(session_data->header_frame_header[source_id], + session_data->frame_header[source_id], FRAME_HEADER_LENGTH); + + delete[] session_data->frame_header[source_id]; + session_data->frame_header[source_id] = nullptr; + status = StreamSplitter::SEARCH; + data += frame_length + FRAME_HEADER_LENGTH; + } + else if ( type == FT_CONTINUATION && session_data->continuation_expected) + { + if (!(session_data->get_frame_flags(source_id) & END_HEADERS)) + { + // For continuation frames we only need the frame length + // FIXIT-M Need to verify that continuation frame has correct stream id + session_data->continuation_frame_lengths.push_back(frame_length); + + delete[] session_data->frame_header[source_id]; + session_data->frame_header[source_id] = nullptr; + status = StreamSplitter::SEARCH; + data += frame_length + FRAME_HEADER_LENGTH; + } + else + { + // continuation frame ending headers + status = StreamSplitter::FLUSH; + session_data->continuation_expected = false; + } + } + //FIXIT-M CONTINUATION frames can also follow PUSH_PROMISE frames, which is not + //currently supported + else if (type == FT_CONTINUATION) + { + *session_data->infractions[source_id] += INF_UNEXPECTED_CONTINUATION; + session_data->events[source_id]->create_event(EVENT_UNEXPECTED_CONTINUATION); + status = StreamSplitter::ABORT; + } + else if (session_data->continuation_expected) + { + *session_data->infractions[source_id] += INF_MISSING_CONTINUATION; + session_data->events[source_id]->create_event(EVENT_MISSING_CONTINUATION); + status = StreamSplitter::ABORT; + } + } while (status != StreamSplitter::FLUSH && *flush_offset < length); } - return StreamSplitter::FLUSH; + return status; } const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned total, @@ -100,7 +211,7 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to { assert(offset+len <= total); assert(total >= FRAME_HEADER_LENGTH); - assert(total <= Http2Enums::MAX_OCTETS); + assert(total <= MAX_OCTETS); StreamBuffer frame_buf { nullptr, 0 }; @@ -110,8 +221,9 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to session_data->frame_size[source_id] = total; } assert(session_data->frame_size[source_id] == total); - + memcpy(session_data->frame[source_id]+offset, data, len); + if (flags & PKT_PDU_TAIL) { assert(offset+len == total); @@ -132,6 +244,77 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to session_data->frame[source_id] + FRAME_HEADER_LENGTH; session_data->frame_data_size[source_id] = session_data->frame_size[source_id] - FRAME_HEADER_LENGTH; + + const int type = session_data->get_frame_type(source_id); + + if (type == FT_HEADERS || type == FT_CONTINUATION) + { + assert(session_data->http2_decoded_header[source_id] == nullptr); + session_data->http2_decoded_header[source_id] = new uint8_t[MAX_OCTETS]; + uint8_t header_payload_offset = 0; + uint8_t pad_len = 0; + + uint8_t frame_flags; + uint32_t header_frame_length; + if (type == FT_HEADERS) + { + frame_flags = session_data->get_frame_flags(source_id); + header_frame_length = get_frame_length(session_data->frame_header[source_id]); + } + else + { + assert(session_data->header_frame_header[source_id] != nullptr); + frame_flags = session_data->header_frame_header[source_id][4]; + header_frame_length = get_frame_length(session_data->header_frame_header[source_id]); + } + + if (frame_flags & PADDED) + { + header_payload_offset += 1; + pad_len = session_data->frame_data[source_id][0]; + } + //FIXIT-M handle stream dependency and weight. For now just skip over + if (frame_flags & PRIORITY) + { + header_payload_offset += 5; + } + + //FIXIT-H This will eventually be the decoded header buffer. For now just copy directly + uint32_t header_payload_len = header_frame_length - header_payload_offset - pad_len; + memcpy(session_data->http2_decoded_header[source_id], session_data->frame_data[source_id] + + header_payload_offset, header_payload_len); + session_data->http2_decoded_header_size[source_id] = header_payload_len; + + // check for continuation frames, skipping over frame headers + if (type == FT_CONTINUATION) + { + header_payload_offset += header_payload_len + pad_len + FRAME_HEADER_LENGTH; + for (uint32_t continuation_length : session_data->continuation_frame_lengths) + { + assert(header_payload_offset + continuation_length < total); + assert(session_data->http2_decoded_header_size[source_id] + continuation_length < + MAX_OCTETS); + memcpy(session_data->http2_decoded_header[source_id] + + session_data->http2_decoded_header_size[source_id], + session_data->frame_data[source_id] + header_payload_offset, + continuation_length); + session_data->http2_decoded_header_size[source_id] += continuation_length; + header_payload_offset += continuation_length + FRAME_HEADER_LENGTH; + } + + // The last continuation frame header is stored in the frame_header buffer + uint32_t final_continuation_length = + get_frame_length(session_data->frame_header[source_id]); + assert(header_payload_offset + final_continuation_length < total); + assert(session_data->http2_decoded_header_size[source_id] + + final_continuation_length < MAX_OCTETS); + memcpy(session_data->http2_decoded_header[source_id] + + session_data->http2_decoded_header_size[source_id], + session_data->frame_data[source_id] + header_payload_offset, + final_continuation_length); + session_data->http2_decoded_header_size[source_id] += final_continuation_length; + } + } } // Return 0-length non-null buffer to stream which signals detection required, but don't // create pkt_data buffer diff --git a/src/service_inspectors/http2_inspect/http2_tables.cc b/src/service_inspectors/http2_inspect/http2_tables.cc index b50303848..bf1686e8a 100644 --- a/src/service_inspectors/http2_inspect/http2_tables.cc +++ b/src/service_inspectors/http2_inspect/http2_tables.cc @@ -30,9 +30,11 @@ using namespace Http2Enums; const snort::RuleMap Http2Module::http2_events[] = { - { EVENT_INT_DECODE_FAILURE, "Error in HPACK integer value" }, - { EVENT_INT_LEADING_ZEROS, "Integer value has leading zeros" }, - { EVENT_STRING_DECODE_FAILURE, "Error in HPACK string value" }, + { EVENT_INT_DECODE_FAILURE, "error in HPACK integer value" }, + { EVENT_INT_LEADING_ZEROS, "integer value has leading zeros" }, + { EVENT_STRING_DECODE_FAILURE, "error in HPACK string value" }, + { EVENT_MISSING_CONTINUATION, "missing continuation frame"}, + { EVENT_UNEXPECTED_CONTINUATION, "unexpected continuation frame"}, { 0, nullptr } }; diff --git a/src/service_inspectors/http2_inspect/ips_http2.cc b/src/service_inspectors/http2_inspect/ips_http2.cc index 50088bf62..7cb00a98e 100644 --- a/src/service_inspectors/http2_inspect/ips_http2.cc +++ b/src/service_inspectors/http2_inspect/ips_http2.cc @@ -88,7 +88,7 @@ IpsOption::EvalStatus Http2IpsOption::eval(Cursor& c, Packet* p) #undef IPS_OPT #define IPS_OPT "http2_frame_data" #undef IPS_HELP -#define IPS_HELP "rule option to see HTTP/2 frame body" +#define IPS_HELP "rule option to set detection cursor to the HTTP/2 frame body" static Module* frame_data_mod_ctor() { @@ -128,7 +128,7 @@ static const IpsApi frame_data_api = #undef IPS_OPT #define IPS_OPT "http2_frame_header" #undef IPS_HELP -#define IPS_HELP "rule option to see 9-octet HTTP/2 frame header" +#define IPS_HELP "rule option to set detection cursor to the 9-octet HTTP/2 frame header" static Module* frame_header_mod_ctor() { @@ -161,10 +161,51 @@ static const IpsApi frame_header_api = nullptr }; +//------------------------------------------------------------------------- +// http2_decoded_header +//------------------------------------------------------------------------- + +#undef IPS_OPT +#define IPS_OPT "http2_decoded_header" +#undef IPS_HELP +#define IPS_HELP "rule option to set detection cursor to the decoded HTTP/2 header" + +static Module* decoded_header_mod_ctor() +{ + return new Http2CursorModule(IPS_OPT, IPS_HELP, HTTP2_BUFFER_DECODED_HEADER, CAT_SET_OTHER, + PSI_DECODED_HEADER); +} + +static const IpsApi decoded_header_api = +{ + { + PT_IPS_OPTION, + sizeof(IpsApi), + IPSAPI_VERSION, + 1, + API_RESERVED, + API_OPTIONS, + IPS_OPT, + IPS_HELP, + decoded_header_mod_ctor, + Http2CursorModule::mod_dtor + }, + OPT_TYPE_DETECTION, + 0, PROTO_BIT__TCP, + nullptr, + nullptr, + nullptr, + nullptr, + Http2IpsOption::opt_ctor, + Http2IpsOption::opt_dtor, + nullptr +}; + //------------------------------------------------------------------------- // plugins //------------------------------------------------------------------------- const BaseApi* ips_http2_frame_data = &frame_data_api.base; const BaseApi* ips_http2_frame_header = &frame_header_api.base; +const BaseApi* ips_http2_decoded_header = &decoded_header_api.base; diff --git a/src/service_inspectors/http2_inspect/ips_http2.h b/src/service_inspectors/http2_inspect/ips_http2.h index 7609ec2c0..1a80d07aa 100644 --- a/src/service_inspectors/http2_inspect/ips_http2.h +++ b/src/service_inspectors/http2_inspect/ips_http2.h @@ -28,7 +28,7 @@ #include "http2_enum.h" -enum PsIdx { PSI_FRAME_DATA, PSI_FRAME_HEADER, PSI_MAX }; +enum PsIdx { PSI_FRAME_DATA, PSI_FRAME_HEADER, PSI_DECODED_HEADER, PSI_MAX }; class Http2CursorModule : public snort::Module { diff --git a/src/service_inspectors/http2_inspect/test/http2_hpack_string_decode_test.cc b/src/service_inspectors/http2_inspect/test/http2_hpack_string_decode_test.cc index 1cb648e64..16e4eff08 100644 --- a/src/service_inspectors/http2_inspect/test/http2_hpack_string_decode_test.cc +++ b/src/service_inspectors/http2_inspect/test/http2_hpack_string_decode_test.cc @@ -25,6 +25,7 @@ #include "../http2_enum.h" #include "../http2_huffman_state_machine.h" #include "../http2_hpack_string_decode.h" +#include "../../http_inspect/http_common.h" #include "../../http_inspect/http_enum.h" #include @@ -38,6 +39,7 @@ int DetectionEngine::queue_event(unsigned int, unsigned int, Actions::Type) { re } using namespace Http2Enums; +using namespace HttpCommon; // // The following tests should result in a successful decode, no infractions/events @@ -137,21 +139,21 @@ TEST(http2_hpack_string_decode_success, string_len_1) TEST(http2_hpack_string_decode_success, max_field_length) { // prepare buf to decode - int + string == MAX_OCTETS (Field limitation) - uint8_t buf[HttpEnums::MAX_OCTETS]; + uint8_t buf[MAX_OCTETS]; buf[0] = 0x7F; buf[1] = 0xA1; buf[2] = 0xF1; buf[3]= 0x3; - memset(&buf[4], 'A', HttpEnums::MAX_OCTETS-4); + memset(&buf[4], 'A', MAX_OCTETS-4); // decode uint32_t bytes_processed = 0, bytes_written = 0; - uint8_t res[HttpEnums::MAX_OCTETS]; - bool success = decode->translate(buf, HttpEnums::MAX_OCTETS, bytes_processed, res, - HttpEnums::MAX_OCTETS, bytes_written); + uint8_t res[MAX_OCTETS]; + bool success = decode->translate(buf, MAX_OCTETS, bytes_processed, res, + MAX_OCTETS, bytes_written); // check results CHECK(success == true); - CHECK(bytes_processed == HttpEnums::MAX_OCTETS); - CHECK(bytes_written == (HttpEnums::MAX_OCTETS-4)); + CHECK(bytes_processed == MAX_OCTETS); + CHECK(bytes_written == (MAX_OCTETS-4)); CHECK(memcmp(res, &buf[4], bytes_written) == 0); } @@ -626,17 +628,17 @@ TEST(http2_hpack_string_decode_infractions, max_field_length_plus_1) Http2Infractions local_inf; Http2HpackStringDecode local_decode(&local_events, &local_inf); // prepare buf to decode - int + string == MAX_OCTETS+1 (Field limitation + 1) - uint8_t buf[HttpEnums::MAX_OCTETS]; + uint8_t buf[MAX_OCTETS]; buf[0] = 0x7F; buf[1] = 0xA2; buf[2] = 0xF1; buf[3]= 0x3; - memset(&buf[4], 'A', HttpEnums::MAX_OCTETS-4); + memset(&buf[4], 'A', MAX_OCTETS-4); // decode uint32_t bytes_processed = 0, bytes_written = 0; - uint8_t res[HttpEnums::MAX_OCTETS]; - bool success = local_decode.translate(buf, HttpEnums::MAX_OCTETS, bytes_processed, res, - HttpEnums::MAX_OCTETS, bytes_written); + uint8_t res[MAX_OCTETS]; + bool success = local_decode.translate(buf, MAX_OCTETS, bytes_processed, res, + MAX_OCTETS, bytes_written); // check results CHECK(success == false); CHECK(bytes_processed == 4); @@ -652,17 +654,17 @@ TEST(http2_hpack_string_decode_infractions, out_buf_out_of_space) Http2Infractions local_inf; Http2HpackStringDecode local_decode(&local_events, &local_inf); // prepare buf to decode - uint8_t buf[HttpEnums::MAX_OCTETS]; + uint8_t buf[MAX_OCTETS]; buf[0] = 0x7F; buf[1] = 0xA1; buf[2] = 0xF1; buf[3]= 0x3; - memset(&buf[4], 'A', HttpEnums::MAX_OCTETS-4); + memset(&buf[4], 'A', MAX_OCTETS-4); // decode uint32_t bytes_processed = 0, bytes_written = 0; - uint8_t res[HttpEnums::MAX_OCTETS-5]; - bool success = local_decode.translate(buf, HttpEnums::MAX_OCTETS, bytes_processed, res, - HttpEnums::MAX_OCTETS-5, bytes_written); + uint8_t res[MAX_OCTETS-5]; + bool success = local_decode.translate(buf, MAX_OCTETS, bytes_processed, res, + MAX_OCTETS-5, bytes_written); // check results CHECK(success == false); CHECK(bytes_processed == 4); diff --git a/src/service_inspectors/http2_inspect/test/http2_inspect_impl_test.cc b/src/service_inspectors/http2_inspect/test/http2_inspect_impl_test.cc index baab0dac5..62efd5ec7 100644 --- a/src/service_inspectors/http2_inspect/test/http2_inspect_impl_test.cc +++ b/src/service_inspectors/http2_inspect/test/http2_inspect_impl_test.cc @@ -43,6 +43,7 @@ using namespace Http2Enums; // Stubs whose sole purpose is to make the test code link unsigned HttpTestManager::test_input = IN_NONE; unsigned HttpTestManager::test_output = IN_NONE; +int DetectionEngine::queue_event(unsigned int, unsigned int, Actions::Type) { return 0; } TEST_GROUP(http2_get_buf_test) { diff --git a/src/service_inspectors/http2_inspect/test/http2_stream_splitter_impl_test.cc b/src/service_inspectors/http2_inspect/test/http2_stream_splitter_impl_test.cc index 72313c15e..2a0440fc3 100644 --- a/src/service_inspectors/http2_inspect/test/http2_stream_splitter_impl_test.cc +++ b/src/service_inspectors/http2_inspect/test/http2_stream_splitter_impl_test.cc @@ -29,6 +29,7 @@ #include "service_inspectors/http_inspect/http_common.h" #include "service_inspectors/http_inspect/http_test_manager.h" #include "service_inspectors/http2_inspect/http2_enum.h" +#include "service_inspectors/http2_inspect/http2_flow_data.h" #include "http2_flow_data_test.h" @@ -43,6 +44,7 @@ using namespace Http2Enums; // Stubs whose sole purpose is to make the test code link unsigned HttpTestManager::test_input = IN_NONE; unsigned HttpTestManager::test_output = IN_NONE; +int DetectionEngine::queue_event(unsigned int, unsigned int, Actions::Type) { return 0; } TEST_GROUP(http2_scan_test) { @@ -88,8 +90,8 @@ TEST(http2_scan_test, header_without_data) const StreamSplitter::Status result = implement_scan(session_data, (const uint8_t*)"\x00\x00\x0A\x02\x00\x00\x00\x00\x00", 9, &flush_offset, SRC_CLIENT); - CHECK(result == StreamSplitter::FLUSH); - CHECK(flush_offset == 19); + CHECK(result == StreamSplitter::SEARCH); + CHECK(flush_offset == 0); CHECK(session_data->get_header_coming(SRC_CLIENT)); } @@ -123,10 +125,10 @@ TEST(http2_scan_test, short_input) result = implement_scan(session_data, (const uint8_t*)"\x04\x05\x06", 3, &flush_offset, SRC_SERVER); CHECK(result == StreamSplitter::SEARCH); - result = implement_scan(session_data, (const uint8_t*)"\x07\x08\x09YZ", 5, &flush_offset, + result = implement_scan(session_data, (const uint8_t*)"\x07\x08\x09" "0123456789ABCDEF", 19, &flush_offset, SRC_SERVER); CHECK(result == StreamSplitter::FLUSH); - CHECK(flush_offset == 25); + CHECK(flush_offset == 19); CHECK(session_data->get_header_coming(SRC_SERVER)); CHECK(memcmp(session_data->get_frame_header(SRC_SERVER), "\x00\x00\x10\x04\x05\x06\x07\x08\x09", 9) == 0); @@ -143,9 +145,10 @@ TEST(http2_scan_test, oversize_non_data_frame) TEST(http2_scan_test, maximum_frame) { + uint8_t data[63870] = { 'A' }; + memcpy(data,"\x00\xF9\x1B\x04", 4); const StreamSplitter::Status result = implement_scan(session_data, - (const uint8_t*)"\x00\xF9\x1B" "12345678901234567", - 20, &flush_offset, SRC_SERVER); + data, 63780, &flush_offset, SRC_SERVER); CHECK(result == StreamSplitter::FLUSH); CHECK(flush_offset == 63780); CHECK(session_data->get_header_coming(SRC_SERVER)); @@ -153,40 +156,35 @@ TEST(http2_scan_test, maximum_frame) TEST(http2_scan_test, data_sections) { + uint8_t data[DATA_SECTION_SIZE+9] ={ 'A' }; + memcpy(data,"\x01\x21\x3C\x00\x00\x00\x00\x00\x00", 9); StreamSplitter::Status result = implement_scan(session_data, - (const uint8_t*)"\x01\x21\x3C\x00\x00\x00\x00\x00\x00" "abcdefghij", - 19, &flush_offset, SRC_SERVER); + data, DATA_SECTION_SIZE + 9, &flush_offset, SRC_SERVER); CHECK(result == StreamSplitter::FLUSH); CHECK(flush_offset == DATA_SECTION_SIZE + 9); CHECK(session_data->get_header_coming(SRC_SERVER)); CHECK(session_data->get_leftover_data(SRC_SERVER) == 0xE13C); result = implement_scan(session_data, - (const uint8_t*)"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstu" - "vwxyz+-", - 80, &flush_offset, SRC_SERVER); + data, DATA_SECTION_SIZE, &flush_offset, SRC_SERVER); CHECK(result == StreamSplitter::FLUSH); CHECK(flush_offset == DATA_SECTION_SIZE); CHECK(!session_data->get_header_coming(SRC_SERVER)); result = implement_scan(session_data, - (const uint8_t*)"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstu" - "vwxyz+-=", - 81, &flush_offset, SRC_SERVER); + data, DATA_SECTION_SIZE, &flush_offset, SRC_SERVER); CHECK(result == StreamSplitter::FLUSH); CHECK(flush_offset == DATA_SECTION_SIZE); CHECK(!session_data->get_header_coming(SRC_SERVER)); result = implement_scan(session_data, - (const uint8_t*)"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstu" - "vwxyz+-=*", - 82, &flush_offset, SRC_SERVER); + data, DATA_SECTION_SIZE, &flush_offset, SRC_SERVER); CHECK(result == StreamSplitter::FLUSH); CHECK(flush_offset == DATA_SECTION_SIZE); CHECK(!session_data->get_header_coming(SRC_SERVER)); result = implement_scan(session_data, - (const uint8_t*)"!", - 1, &flush_offset, SRC_SERVER); + data, 8508, &flush_offset, SRC_SERVER); CHECK(result == StreamSplitter::FLUSH); - CHECK(flush_offset == 0x213C); + CHECK(flush_offset == 8508); CHECK(!session_data->get_header_coming(SRC_SERVER)); + CHECK(session_data->get_leftover_data(SRC_SERVER) == 0); } TEST_GROUP(http2_reassemble_test) diff --git a/src/service_inspectors/http_inspect/http_field.h b/src/service_inspectors/http_inspect/http_field.h index 8bf1b87cf..13958faef 100644 --- a/src/service_inspectors/http_inspect/http_field.h +++ b/src/service_inspectors/http_inspect/http_field.h @@ -36,7 +36,7 @@ public: static const Field FIELD_NULL; Field(int32_t length, const uint8_t* start, bool own_the_buffer_ = false) : strt(start), - len(length), own_the_buffer(own_the_buffer_) { assert(length <= HttpEnums::MAX_OCTETS); } + len(length), own_the_buffer(own_the_buffer_) { } explicit Field(int32_t length) : len(length) { assert(length<=0); } Field() = default; ~Field() { if (own_the_buffer) delete[] strt; } diff --git a/src/service_inspectors/http_inspect/http_test_input.h b/src/service_inspectors/http_inspect/http_test_input.h index 1b01adac9..be628f0e6 100644 --- a/src/service_inspectors/http_inspect/http_test_input.h +++ b/src/service_inspectors/http_inspect/http_test_input.h @@ -41,6 +41,7 @@ public: private: FILE* test_data_file; + // FIXIT-L Figure out how big this buf needs to be and revise value uint8_t msg_buf[2][2 * HttpEnums::MAX_OCTETS]; FILE* include_file[2] = { nullptr, nullptr };