From: Tom Peters (thopeter) Date: Thu, 23 Jun 2022 16:26:28 +0000 (+0000) Subject: Pull request #3461: http_inspect: uniform alerts when splitter aborts X-Git-Tag: 3.1.33.0~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9b131eb00b41fb359905b683324bd5e88fbf51fd;p=thirdparty%2Fsnort3.git Pull request #3461: http_inspect: uniform alerts when splitter aborts Merge in SNORT/snort3 from ~ADMAMOLE/snort3:uniform_alerts_for_abort to master Squashed commit of the following: commit 9a69be6c333453ce2cac6e9df8d06b4008a69653 Author: Adrian Mamolea Date: Thu May 26 14:59:09 2022 -0400 http_inspect: uniform alerts when splitter aborts --- diff --git a/doc/reference/builtin_stubs.txt b/doc/reference/builtin_stubs.txt index 6bb4ab0b8..7fca9a4da 100644 --- a/doc/reference/builtin_stubs.txt +++ b/doc/reference/builtin_stubs.txt @@ -1017,11 +1017,6 @@ to the message body so that the length of the message body can be determined by An HTTP message includes a Transfer-Encoding header value that specifies other encodings before "chunked." -119:224 - -The traffic contains an HTTP version, but does not contain a recognizable start line. This -conclusion applies only to one direction of the flow. The opposite direction may be OK. - 119:225 The HTTP Content-Encoding header contains a coding other than gzip and deflate @@ -1299,6 +1294,38 @@ does not apply to HTTP/2 or HTTP/3 traffic. The HTTP message body is gzip encoded and the FEXTRA flag is set in the gzip header. +119:279 + +HTTP Status-Line failed validation checks. Checks include minimum length, format, characters used, etc. + +119:280 + +HTTP message headers longer than 63780 bytes + +119:281 + +HTTP Request-Line failed validation checks. Checks include minimum length, format, characters used, etc. + +119:282 + +Packet with more than 20 white space characters when an HTTP Start-Line is required. + +119:283 + +HTTP message Status-Line longer than 63780 bytes + +119:284 + +Connection closed in the middle of a Request-Line or Status-Line. + +119:285 + +HTTP message Request-Line longer than 63780 bytes + +119:286 + +HTTP/2 preface received instead of an HTTP/1 method + 121:1 Invalid flag set on HTTP/2 frame header @@ -1463,6 +1490,12 @@ Nonempty HTTP/2 Data frame where a message body was not expected. HTTP/2 non-Data frame longer than 63780 bytes +121:39 + +HTTP/2 inspector is unable to parse this flow. Either the connection is not actually using HTTP/2 or +some sort of unrecoverable HTTP/2 protocol error has occurred. This conclusion applies only to one +direction of the flow. The opposite direction may be OK. + 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 466291fce..1f36c9a2e 100644 --- a/src/service_inspectors/http2_inspect/http2_enum.h +++ b/src/service_inspectors/http2_inspect/http2_enum.h @@ -94,6 +94,7 @@ enum EventSid EVENT_HPACK_TABLE_SIZE_UPDATE_EXCEEDS_MAX = 36, EVENT_UNEXPECTED_DATA_FRAME = 37, EVENT_NON_DATA_FRAME_TOO_LONG = 38, + EVENT_LOSS_OF_SYNC = 39, EVENT__MAX_VALUE }; diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.cc b/src/service_inspectors/http2_inspect/http2_flow_data.cc index c34e5e72a..6a6599eb9 100644 --- a/src/service_inspectors/http2_inspect/http2_flow_data.cc +++ b/src/service_inspectors/http2_inspect/http2_flow_data.cc @@ -134,6 +134,7 @@ Http2Stream* Http2FlowData::get_processing_stream(const SourceId source_id, uint { *infractions[source_id] += INF_TOO_MANY_STREAMS; events[source_id]->create_event(EVENT_TOO_MANY_STREAMS); + events[source_id]->create_event(EVENT_LOSS_OF_SYNC); Http2Module::increment_peg_counts(PEG_FLOWS_OVER_STREAM_LIMIT); abort_flow[SRC_CLIENT] = true; abort_flow[SRC_SERVER] = true; diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame.cc b/src/service_inspectors/http2_inspect/http2_headers_frame.cc index 8c1f99576..239e44633 100644 --- a/src/service_inspectors/http2_inspect/http2_headers_frame.cc +++ b/src/service_inspectors/http2_inspect/http2_headers_frame.cc @@ -77,6 +77,7 @@ bool Http2HeadersFrame::decode_headers(Http2StartLine* start_line_generator, boo { session_data->abort_flow[source_id] = true; session_data->events[source_id]->create_event(EVENT_MISFORMATTED_HTTP2); + session_data->events[source_id]->create_event(EVENT_LOSS_OF_SYNC); http1_header.set(STAT_PROBLEMATIC); hpack_decoder->cleanup(); return false; diff --git a/src/service_inspectors/http2_inspect/http2_headers_frame_with_startline.cc b/src/service_inspectors/http2_inspect/http2_headers_frame_with_startline.cc index 5574dbf8d..057ed1031 100644 --- a/src/service_inspectors/http2_inspect/http2_headers_frame_with_startline.cc +++ b/src/service_inspectors/http2_inspect/http2_headers_frame_with_startline.cc @@ -62,8 +62,11 @@ bool Http2HeadersFrameWithStartline::process_start_line(HttpFlowData*& http_flow const StreamSplitter::Status start_scan_result = session_data->hi_ss[hi_source_id]->scan(&dummy_pkt, start_line.start(), start_line.length(), unused, &flush_offset); - assert(start_scan_result == StreamSplitter::FLUSH); - UNUSED(start_scan_result); + if (start_scan_result != StreamSplitter::FLUSH) + { + stream->set_state(hi_source_id, STREAM_ERROR); + return false; + } assert((int64_t)flush_offset == start_line.length()); } diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc index f87cfe814..6243ad9f3 100644 --- a/src/service_inspectors/http2_inspect/http2_inspect.cc +++ b/src/service_inspectors/http2_inspect/http2_inspect.cc @@ -219,6 +219,6 @@ static void print_flow_issues(FILE* output, Http2Infractions* const infractions, Http2EventGen* const events) { fprintf(output, "Infractions: %016" PRIx64 ", Events: %016" PRIx64 "\n\n", - infractions->get_raw(), events->get_raw()); + infractions->get_raw(0), events->get_raw(0)); } #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 54f31faa2..d987953de 100644 --- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc +++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc @@ -150,6 +150,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio return StreamSplitter::FLUSH; case V_BAD: session_data->events[source_id]->create_event(EVENT_PREFACE_MATCH_FAILURE); + session_data->events[source_id]->create_event(EVENT_LOSS_OF_SYNC); return StreamSplitter::ABORT; case V_TBD: session_data->preface_octets_seen += length; @@ -203,6 +204,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio { *session_data->infractions[source_id] += INF_MISSING_CONTINUATION; session_data->events[source_id]->create_event(EVENT_MISSING_CONTINUATION); + session_data->events[source_id]->create_event(EVENT_LOSS_OF_SYNC); return StreamSplitter::ABORT; } @@ -224,6 +226,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio { *session_data->infractions[source_id] += INF_UNEXPECTED_CONTINUATION; session_data->events[source_id]->create_event(EVENT_UNEXPECTED_CONTINUATION); + session_data->events[source_id]->create_event(EVENT_LOSS_OF_SYNC); return StreamSplitter::ABORT; } // Do flags check for continuation frame, since it is not saved @@ -240,6 +243,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio // FIXIT-E long non-data frames may need to be supported *session_data->infractions[source_id] += INF_NON_DATA_FRAME_TOO_LONG; session_data->events[source_id]->create_event(EVENT_NON_DATA_FRAME_TOO_LONG); + session_data->events[source_id]->create_event(EVENT_LOSS_OF_SYNC); return StreamSplitter::ABORT; } @@ -254,6 +258,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio *session_data->infractions[source_id] += INF_PADDING_ON_EMPTY_FRAME; session_data->events[source_id]->create_event( EVENT_PADDING_ON_EMPTY_FRAME); + session_data->events[source_id]->create_event(EVENT_LOSS_OF_SYNC); return StreamSplitter::ABORT; } session_data->scan_state[source_id] = SCAN_PADDING_LENGTH; @@ -287,6 +292,7 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio { *session_data->infractions[source_id] += INF_PADDING_LEN; session_data->events[source_id]->create_event(EVENT_PADDING_LEN); + session_data->events[source_id]->create_event(EVENT_LOSS_OF_SYNC); return StreamSplitter::ABORT; } data_offset++; diff --git a/src/service_inspectors/http2_inspect/http2_tables.cc b/src/service_inspectors/http2_inspect/http2_tables.cc index 0c724274c..f4e3caca4 100644 --- a/src/service_inspectors/http2_inspect/http2_tables.cc +++ b/src/service_inspectors/http2_inspect/http2_tables.cc @@ -72,6 +72,7 @@ const RuleMap Http2Module::http2_events[] = "HTTP/2 HPACK table size update exceeds max value set by decoder in SETTINGS frame" }, { EVENT_UNEXPECTED_DATA_FRAME, "Nonempty HTTP/2 Data frame where message body not expected" }, { EVENT_NON_DATA_FRAME_TOO_LONG, "HTTP/2 non-Data frame longer than 63780 bytes" }, + { EVENT_LOSS_OF_SYNC, "not HTTP/2 traffic or unrecoverable HTTP/2 protocol error" }, { 0, nullptr } }; @@ -81,11 +82,9 @@ const PegInfo Http2Module::peg_names[PEG_COUNT__MAX+1] = { CountType::NOW, "concurrent_sessions", "total concurrent HTTP/2 sessions" }, { CountType::MAX, "max_concurrent_sessions", "maximum concurrent HTTP/2 sessions" }, { CountType::MAX, "max_table_entries", "maximum entries in an HTTP/2 dynamic table" }, - { CountType::MAX, "max_concurrent_files", "maximum concurrent file transfers per HTTP/2 " - "connection" }, + { CountType::MAX, "max_concurrent_files", "maximum concurrent file transfers per HTTP/2 connection" }, { CountType::SUM, "total_bytes", "total HTTP/2 data bytes inspected" }, - { CountType::MAX, "max_concurrent_streams", "maximum concurrent streams per HTTP/2 " - "connection" }, + { CountType::MAX, "max_concurrent_streams", "maximum concurrent streams per HTTP/2 connection" }, { CountType::SUM, "flows_over_stream_limit", "HTTP/2 flows exceeding 100 concurrent streams" }, { CountType::END, nullptr, nullptr } }; diff --git a/src/service_inspectors/http2_inspect/test/http2_hpack_int_decode_test.cc b/src/service_inspectors/http2_inspect/test/http2_hpack_int_decode_test.cc index c96e99509..975a3c090 100644 --- a/src/service_inspectors/http2_inspect/test/http2_hpack_int_decode_test.cc +++ b/src/service_inspectors/http2_inspect/test/http2_hpack_int_decode_test.cc @@ -195,7 +195,7 @@ TEST(http2_hpack_int_decode_failure, 0_len_field) // check results CHECK(success == false); CHECK(bytes_processed == 0); - CHECK(local_inf.get_raw() == (1<generate_misformatted_http(buffer, length); + events->create_event(HttpEnums::EVENT_TOO_MUCH_LEADING_WS); return SCAN_ABORT; } } @@ -82,7 +82,6 @@ ScanResult HttpStartCutter::cut(const uint8_t* buffer, uint32_t length, break; case V_BAD: *infractions += INF_NOT_HTTP; - events->generate_misformatted_http(buffer, length); return SCAN_ABORT; case V_TBD: break; @@ -117,7 +116,7 @@ ScanResult HttpStartCutter::cut(const uint8_t* buffer, uint32_t length, } HttpStartCutter::ValidationResult HttpRequestCutter::validate(uint8_t octet, - HttpInfractions* infractions, HttpEventGen*) + HttpInfractions* infractions, HttpEventGen* events) { // Request line must begin with a method. There is no list of all possible methods because // extension is allowed, so there is no absolute way to tell whether something is a method. @@ -141,6 +140,7 @@ HttpStartCutter::ValidationResult HttpRequestCutter::validate(uint8_t octet, if (octets_checked >= preface_len) { *infractions += INF_HTTP2_IN_HI; + events->create_event(HttpEnums::EVENT_UNEXPECTED_H2_PREFACE); return V_BAD; } return V_TBD; @@ -155,7 +155,10 @@ HttpStartCutter::ValidationResult HttpRequestCutter::validate(uint8_t octet, if ((octet == ' ') || (octet == '\t')) return V_GOOD; if (!token_char[octet] || ++octets_checked > max_method_length) + { + events->create_event(HttpEnums::EVENT_BAD_REQ_LINE); return V_BAD; + } return V_TBD; } @@ -176,7 +179,10 @@ HttpStartCutter::ValidationResult HttpStatusCutter::validate(uint8_t octet, events->create_event(EVENT_VERSION_NOT_UPPERCASE); } else + { + events->create_event(HttpEnums::EVENT_BAD_STAT_LINE); return V_BAD; + } } if (++octets_checked >= match_size) return V_GOOD; diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h index 23c25426c..759d7ccf7 100755 --- a/src/service_inspectors/http_inspect/http_enum.h +++ b/src/service_inspectors/http_inspect/http_enum.h @@ -381,7 +381,7 @@ enum EventSid EVENT_BAD_CODE_BODY_HEADER = 221, EVENT_BAD_TE_HEADER = 222, EVENT_PADDED_TE_HEADER = 223, - EVENT_MISFORMATTED_HTTP = 224, + // EVENT_MISFORMATTED_HTTP = 224, // Retired. Do not reuse this number EVENT_UNSUPPORTED_ENCODING = 225, EVENT_UNKNOWN_ENCODING = 226, EVENT_STACKED_ENCODINGS = 227, @@ -436,6 +436,14 @@ enum EventSid EVENT_VERSION_0 = 276, EVENT_VERSION_HIGHER_THAN_1 = 277, EVENT_GZIP_FEXTRA = 278, + EVENT_BAD_STAT_LINE = 279, + EVENT_HEADERS_TOO_LONG = 280, + EVENT_BAD_REQ_LINE = 281, + EVENT_TOO_MUCH_LEADING_WS = 282, + EVENT_STAT_TOO_LONG = 283, + EVENT_PARTIAL_START = 284, + EVENT_REQ_TOO_LONG = 285, + EVENT_UNEXPECTED_H2_PREFACE = 286, EVENT__MAX_VALUE }; diff --git a/src/service_inspectors/http_inspect/http_event.h b/src/service_inspectors/http_inspect/http_event.h index 18a5a6feb..e35b9b492 100644 --- a/src/service_inspectors/http_inspect/http_event.h +++ b/src/service_inspectors/http_inspect/http_event.h @@ -20,9 +20,6 @@ #ifndef HTTP_EVENT_H #define HTTP_EVENT_H -#include - -#include "events/event_queue.h" #include "utils/event_gen.h" #include "utils/infractions.h" #include "utils/util_cstring.h" @@ -30,36 +27,13 @@ #include "http_enum.h" //------------------------------------------------------------------------- -// HTTP Event generator class +// HTTP Event generator //------------------------------------------------------------------------- -class HttpEventGen : public EventGen -{ -public: - void generate_misformatted_http(const uint8_t* buffer, uint32_t length) - { - if ( snort::SnortStrnStr((const char*)buffer, length, "HTTP/") != nullptr ) - create_event(HttpEnums::EVENT_MISFORMATTED_HTTP); - else - create_event(HttpEnums::EVENT_LOSS_OF_SYNC); - } - - // The following methods are for convenience of debug and test output only! - uint64_t get_raw2() const { return - ((events_generated >> BASE_1XX_EVENTS) & bitmask).to_ulong(); } - - uint64_t get_raw3() const { return - ((events_generated >> BASE_2XX_EVENTS) & bitmask).to_ulong(); } - - uint64_t get_raw4() const { return - ((events_generated >> (BASE_2XX_EVENTS + 64)) & bitmask).to_ulong(); } - -private: - static const unsigned BASE_1XX_EVENTS = 100; - static const unsigned BASE_2XX_EVENTS = 200; -}; +using HttpEventGen = EventGen; +static const unsigned BASE_1XX_EVENTS = 100; +static const unsigned BASE_2XX_EVENTS = 200; //------------------------------------------------------------------------- // Http Infractions diff --git a/src/service_inspectors/http_inspect/http_flow_data.cc b/src/service_inspectors/http_inspect/http_flow_data.cc index 4f55e8c72..2c74979a1 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.cc +++ b/src/service_inspectors/http_inspect/http_flow_data.cc @@ -77,6 +77,8 @@ HttpFlowData::HttpFlowData(Flow* flow) : FlowData(inspector_id) { for_http2 = true; h2_stream_id = h2i_flow_data->get_processing_stream_id(); + events[0]->suppress_event(HttpEnums::EVENT_LOSS_OF_SYNC); + events[1]->suppress_event(HttpEnums::EVENT_LOSS_OF_SYNC); } } diff --git a/src/service_inspectors/http_inspect/http_msg_request.cc b/src/service_inspectors/http_inspect/http_msg_request.cc index adaf6e29a..afc591d2f 100644 --- a/src/service_inspectors/http_inspect/http_msg_request.cc +++ b/src/service_inspectors/http_inspect/http_msg_request.cc @@ -74,8 +74,8 @@ void HttpMsgRequest::parse_start_line() else { add_infraction(INF_BAD_REQ_LINE); - session_data->events[source_id]->generate_misformatted_http(start_line.start(), - start_line.length()); + session_data->events[source_id]->create_event(HttpEnums::EVENT_BAD_REQ_LINE); + session_data->events[source_id]->create_event(HttpEnums::EVENT_LOSS_OF_SYNC); return; } } diff --git a/src/service_inspectors/http_inspect/http_msg_section.cc b/src/service_inspectors/http_inspect/http_msg_section.cc index 3522fd3d2..9743732af 100644 --- a/src/service_inspectors/http_inspect/http_msg_section.cc +++ b/src/service_inspectors/http_inspect/http_msg_section.cc @@ -452,13 +452,13 @@ void HttpMsgSection::print_section_wrapup(FILE* output) const { fprintf(output, "Infractions: %016" PRIx64 " %016" PRIx64 " %016" PRIx64 ", Events: %016" PRIx64 " %016" PRIx64 " %016" PRIx64 " %016" PRIx64 ", TCP Close: %s\n\n", - transaction->get_infractions(source_id)->get_raw3(), - transaction->get_infractions(source_id)->get_raw2(), - transaction->get_infractions(source_id)->get_raw(), - session_data->events[source_id]->get_raw4(), - session_data->events[source_id]->get_raw3(), - session_data->events[source_id]->get_raw2(), - session_data->events[source_id]->get_raw(), + transaction->get_infractions(source_id)->get_raw(128), + transaction->get_infractions(source_id)->get_raw(64), + transaction->get_infractions(source_id)->get_raw(0), + session_data->events[source_id]->get_raw(BASE_2XX_EVENTS + 64), + session_data->events[source_id]->get_raw(BASE_2XX_EVENTS), + session_data->events[source_id]->get_raw(BASE_1XX_EVENTS), + session_data->events[source_id]->get_raw(0), tcp_close ? "True" : "False"); if (HttpTestManager::get_show_pegs()) { diff --git a/src/service_inspectors/http_inspect/http_msg_status.cc b/src/service_inspectors/http_inspect/http_msg_status.cc index c0bd97fff..9a67d247c 100644 --- a/src/service_inspectors/http_inspect/http_msg_status.cc +++ b/src/service_inspectors/http_inspect/http_msg_status.cc @@ -49,7 +49,8 @@ void HttpMsgStatus::parse_start_line() if ((start_line.length() < 12) || !is_sp_tab[start_line.start()[8]]) { add_infraction(INF_BAD_STAT_LINE); - create_event(EVENT_MISFORMATTED_HTTP); + create_event(EVENT_BAD_STAT_LINE); + create_event(EVENT_LOSS_OF_SYNC); return; } @@ -61,7 +62,8 @@ void HttpMsgStatus::parse_start_line() if (start_line.length() < first_end + 4) { add_infraction(INF_BAD_STAT_LINE); - create_event(EVENT_MISFORMATTED_HTTP); + create_event(EVENT_BAD_STAT_LINE); + create_event(EVENT_LOSS_OF_SYNC); return; } @@ -70,7 +72,8 @@ void HttpMsgStatus::parse_start_line() // FIXIT-M This should not be fatal. HI supports something like "HTTP/1.1 200\\OK\r\n" as // seen in a status line test. add_infraction(INF_BAD_STAT_LINE); - create_event(EVENT_MISFORMATTED_HTTP); + create_event(EVENT_BAD_STAT_LINE); + create_event(EVENT_LOSS_OF_SYNC); return; } diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc b/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc index ec2140574..b9f6ebfea 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc @@ -107,7 +107,7 @@ bool HttpStreamSplitter::finish(Flow* flow) (session_data->type_expected[source_id] == SEC_STATUS)) { *session_data->get_infractions(source_id) += INF_PARTIAL_START; - // FIXIT-M why not use generate_misformatted_http()? + session_data->events[source_id]->create_event(EVENT_PARTIAL_START); session_data->events[source_id]->create_event(EVENT_LOSS_OF_SYNC); return false; } 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 3155adb25..549164dd3 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc @@ -242,6 +242,7 @@ StreamSplitter::Status HttpStreamSplitter::scan(Packet* pkt, const uint8_t* data { cutter = get_cutter(type, session_data); } + const uint32_t max_length = MAX_OCTETS - cutter->get_octets_seen(); const ScanResult cut_result = cutter->cut(data, (length <= max_length) ? length : max_length, session_data->get_infractions(source_id), session_data->events[source_id], @@ -255,10 +256,13 @@ StreamSplitter::Status HttpStreamSplitter::scan(Packet* pkt, const uint8_t* data if (cutter->get_octets_seen() == MAX_OCTETS) { *session_data->get_infractions(source_id) += INF_ENDLESS_HEADER; - // FIXIT-L the following call seems inappropriate for headers and trailers. Those cases - // should be an unconditional EVENT_LOSS_OF_SYNC. - session_data->events[source_id]->generate_misformatted_http(data, length); - // FIXIT-E need to process this data not just discard it. + auto event = HttpEnums::EVENT_HEADERS_TOO_LONG; + if (session_data ->type_expected[source_id] == HttpCommon::SEC_REQUEST) + event = HttpEnums::EVENT_REQ_TOO_LONG; + else if (session_data ->type_expected[source_id] == HttpCommon::SEC_STATUS) + event = HttpEnums::EVENT_STAT_TOO_LONG; + session_data->events[source_id]->create_event(event); + session_data->events[source_id]->create_event(HttpEnums::EVENT_LOSS_OF_SYNC); type = SEC_ABORT; delete cutter; cutter = nullptr; @@ -281,6 +285,7 @@ StreamSplitter::Status HttpStreamSplitter::scan(Packet* pkt, const uint8_t* data type = SEC_ABORT; delete cutter; cutter = nullptr; + session_data->events[source_id]->create_event(HttpEnums::EVENT_LOSS_OF_SYNC); return status_value(StreamSplitter::ABORT); case SCAN_DISCARD: case SCAN_DISCARD_PIECE: @@ -312,6 +317,9 @@ StreamSplitter::Status HttpStreamSplitter::scan(Packet* pkt, const uint8_t* data } default: assert(false); + type = SEC_ABORT; + delete cutter; + cutter = nullptr; return status_value(StreamSplitter::ABORT); } } diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc index 05cfabe65..35881af73 100755 --- a/src/service_inspectors/http_inspect/http_tables.cc +++ b/src/service_inspectors/http_inspect/http_tables.cc @@ -227,50 +227,36 @@ const RuleMap HttpModule::http_events[] = { EVENT_ASCII, "URI has percent-encoding of an unreserved character" }, { EVENT_DOUBLE_DECODE, "URI contains double-encoded hexadecimal characters" }, { EVENT_U_ENCODE, "URI has non-standard %u-style Unicode encoding" }, - { EVENT_BARE_BYTE, "URI has Unicode encodings containing bytes that were not " - "percent-encoded" }, + { EVENT_BARE_BYTE, "URI has Unicode encodings containing bytes that were not percent-encoded" }, { EVENT_UTF_8, "URI has two-byte or three-byte UTF-8 encoding" }, { EVENT_CODE_POINT_IN_URI, "URI has unicode map code point encoding" }, { EVENT_MULTI_SLASH, "URI path contains consecutive slash characters" }, - { EVENT_BACKSLASH_IN_URI, "backslash character appears in the path portion of a URI" - }, - { EVENT_SELF_DIR_TRAV, "URI path contains /./ pattern repeating the current " - "directory" }, + { EVENT_BACKSLASH_IN_URI, "backslash character appears in the path portion of a URI" }, + { EVENT_SELF_DIR_TRAV, "URI path contains /./ pattern repeating the current directory" }, { EVENT_DIR_TRAV, "URI path contains /../ pattern moving up a directory" }, { EVENT_APACHE_WS, "Tab character in HTTP start line" }, - { EVENT_LF_WITHOUT_CR, "HTTP start line or header line terminated by LF without " - "a CR" }, - { EVENT_NON_RFC_CHAR, "Normalized URI includes character from bad_characters " - "list" }, + { EVENT_LF_WITHOUT_CR, "HTTP start line or header line terminated by LF without a CR" }, + { EVENT_NON_RFC_CHAR, "Normalized URI includes character from bad_characters list" }, { EVENT_OVERSIZE_DIR, "URI path contains a segment that is longer than the " "oversize_dir_length parameter" }, { EVENT_LARGE_CHUNK, "chunk length exceeds configured maximum_chunk_length" }, - { EVENT_WEBROOT_DIR, "URI path includes /../ that goes above the root directory" - }, + { EVENT_WEBROOT_DIR, "URI path includes /../ that goes above the root directory" }, { EVENT_LONG_HDR, "HTTP header line exceeds 4096 bytes" }, { EVENT_MAX_HEADERS, "HTTP message has more than 200 header fields" }, - { EVENT_MULTIPLE_CONTLEN, "HTTP message has more than one Content-Length header " - "value" }, - { EVENT_MULTIPLE_HOST_HDRS, "Host header field appears more than once or has multiple " - "values" }, - { EVENT_LONG_HOSTNAME, "length of HTTP Host header field value exceeds " - "maximum_host_length option" }, - { EVENT_UNBOUNDED_POST, "HTTP POST or PUT request without content-length or chunks" - }, + { EVENT_MULTIPLE_CONTLEN, "HTTP message has more than one Content-Length header value" }, + { EVENT_MULTIPLE_HOST_HDRS, "Host header field appears more than once or has multiple values" }, + { EVENT_LONG_HOSTNAME, "length of HTTP Host header field value exceeds maximum_host_length option" }, + { EVENT_UNBOUNDED_POST, "HTTP POST or PUT request without content-length or chunks" }, { EVENT_UNKNOWN_METHOD, "HTTP request method is not known to Snort" }, - { EVENT_SIMPLE_REQUEST, "HTTP request uses primitive HTTP format known as HTTP/0.9" - }, - { EVENT_UNESCAPED_SPACE_URI, "HTTP request URI has space character that is not " - "percent-encoded" }, + { EVENT_SIMPLE_REQUEST, "HTTP request uses primitive HTTP format known as HTTP/0.9" }, + { EVENT_UNESCAPED_SPACE_URI, "HTTP request URI has space character that is not percent-encoded" }, { EVENT_PIPELINE_MAX, "HTTP connection has more than 100 simultaneous pipelined " "requests that have not been answered" }, { EVENT_INVALID_STATCODE, "invalid status code in HTTP response" }, - { EVENT_UTF_NORM_FAIL, "HTTP response has UTF character set that failed to " - "normalize" }, + { EVENT_UTF_NORM_FAIL, "HTTP response has UTF character set that failed to normalize" }, { EVENT_UTF7, "HTTP response has UTF-7 character set" }, { EVENT_JS_OBFUSCATION_EXCD, "more than one level of JavaScript obfuscation" }, - { EVENT_JS_EXCESS_WS, "consecutive JavaScript whitespaces exceed maximum allowed" - }, + { EVENT_JS_EXCESS_WS, "consecutive JavaScript whitespaces exceed maximum allowed" }, { EVENT_MIXED_ENCODINGS, "multiple encodings within JavaScript obfuscated data" }, { EVENT_SWF_ZLIB_FAILURE, "SWF file zlib decompression failure" }, { EVENT_SWF_LZMA_FAILURE, "SWF file LZMA decompression failure" }, @@ -301,7 +287,6 @@ const RuleMap HttpModule::http_events[] = "Encoding or nonzero Content-Length" }, { EVENT_BAD_TE_HEADER, "Transfer-Encoding not ending with chunked" }, { EVENT_PADDED_TE_HEADER, "Transfer-Encoding with encodings before chunked" }, - { EVENT_MISFORMATTED_HTTP, "misformatted HTTP traffic" }, { EVENT_UNSUPPORTED_ENCODING, "unsupported Content-Encoding used" }, { EVENT_UNKNOWN_ENCODING, "unknown Content-Encoding used" }, { EVENT_STACKED_ENCODINGS, "multiple Content-Encodings applied" }, @@ -320,20 +305,16 @@ const RuleMap HttpModule::http_events[] = { EVENT_CHUNKED_ONE_POINT_ZERO, "HTTP 1.0 message with Transfer-Encoding header" }, { EVENT_CTE_HEADER, "Content-Transfer-Encoding used as HTTP header" }, { EVENT_ILLEGAL_TRAILER, "illegal field in chunked message trailers" }, - { EVENT_REPEATED_HEADER, "header field inappropriately appears twice or has two " - "values" }, + { EVENT_REPEATED_HEADER, "header field inappropriately appears twice or has two values" }, { EVENT_CONTENT_ENCODING_CHUNKED, "invalid value chunked in Content-Encoding header" }, { EVENT_206_WITHOUT_RANGE, "206 response sent to a request without a Range header" }, { EVENT_VERSION_NOT_UPPERCASE, "'HTTP' in version field not all upper case" }, { EVENT_BAD_HEADER_WHITESPACE, "white space embedded in critical header value" }, - { EVENT_GZIP_EARLY_END, "gzip compressed data followed by unexpected non-gzip " - "data" }, + { 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" }, + { 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" }, { EVENT_CONNECT_REQUEST_BODY, "HTTP CONNECT request with a message body" }, { EVENT_EARLY_C2S_TRAFFIC_AFTER_CONNECT, "HTTP client-to-server traffic after CONNECT request " "but before CONNECT response" }, @@ -354,14 +335,21 @@ const RuleMap HttpModule::http_events[] = { EVENT_JS_SHORTENED_TAG, "script opening tag in a short form" }, { EVENT_JS_IDENTIFIER_OVERFLOW, "max number of unique JavaScript identifiers reached" }, { EVENT_JS_BRACKET_NEST_OVERFLOW, "excessive JavaScript bracket nesting" }, - { EVENT_ACCEPT_ENCODING_CONSECUTIVE_COMMAS, "Consecutive commas in HTTP Accept-Encoding " - "header" }, + { EVENT_ACCEPT_ENCODING_CONSECUTIVE_COMMAS, "Consecutive commas in HTTP Accept-Encoding header" }, { EVENT_JS_DATA_LOST, "data gaps during JavaScript normalization" }, { EVENT_JS_SCOPE_NEST_OVERFLOW, "excessive JavaScript scope nesting" }, { EVENT_INVALID_SUBVERSION, "HTTP/1 version other than 1.0 or 1.1" }, { EVENT_VERSION_0, "HTTP version in start line is 0" }, { EVENT_VERSION_HIGHER_THAN_1, "HTTP version in start line is higher than 1" }, { EVENT_GZIP_FEXTRA, "HTTP gzip body with the FEXTRA flag set" }, + { EVENT_BAD_STAT_LINE, "invalid status line" }, + { EVENT_HEADERS_TOO_LONG, "HTTP message headers longer than 63780 bytes" }, + { EVENT_BAD_REQ_LINE, "invalid request line" }, + { EVENT_TOO_MUCH_LEADING_WS, "too many white space characters when start line is expected" }, + { EVENT_STAT_TOO_LONG, "HTTP message status line longer than 63780 bytes" }, + { EVENT_PARTIAL_START, "partial start line" }, + { EVENT_REQ_TOO_LONG, "HTTP message request line longer than 63780 bytes" }, + { EVENT_UNEXPECTED_H2_PREFACE, "HTTP/2 preface received instead of an HTTP/1 method" }, { 0, nullptr } }; @@ -402,8 +390,7 @@ const PegInfo HttpModule::peg_names[PEG_COUNT_MAX+1] = { CountType::SUM, "js_external_scripts", "total number of external JavaScripts processed" }, { CountType::SUM, "js_bytes", "total number of JavaScript bytes processed" }, { CountType::SUM, "js_identifiers", "total number of unique JavaScript identifiers processed" }, - { CountType::SUM, "js_identifier_overflows", "total number of unique JavaScript identifier " - "limit overflows" }, + { CountType::SUM, "js_identifier_overflows", "total number of unique JavaScript identifier limit overflows" }, { CountType::END, nullptr, nullptr } }; diff --git a/src/service_inspectors/http_inspect/http_uri_norm.cc b/src/service_inspectors/http_inspect/http_uri_norm.cc index 5e1c75a0d..003d5f02c 100644 --- a/src/service_inspectors/http_inspect/http_uri_norm.cc +++ b/src/service_inspectors/http_inspect/http_uri_norm.cc @@ -31,6 +31,8 @@ using namespace HttpEnums; using namespace snort; +static HttpEventGen events_sink(std::bitset().set()); + void UriNormalizer::normalize(const Field& input, Field& result, bool do_path, uint8_t* buffer, const HttpParaList::UriParam& uri_param, HttpInfractions* infractions, HttpEventGen* events, bool own_the_buffer) @@ -457,12 +459,11 @@ void UriNormalizer::classic_normalize(const Field& input, Field& result, // that we can conveniently modify it as requirements are better understood. HttpInfractions unused; - HttpDummyEventGen dummy_ev; uint8_t* const buffer = new uint8_t[input.length() + URI_NORM_EXPANSION]; // Normalize character escape sequences - int32_t data_length = norm_char_clean(input, buffer, uri_param, &unused, &dummy_ev); + int32_t data_length = norm_char_clean(input, buffer, uri_param, &unused, &events_sink); if (do_path && uri_param.simplify_path) { @@ -473,9 +474,9 @@ void UriNormalizer::classic_normalize(const Field& input, Field& result, { const int32_t uri_offset = first_slash - buffer; norm_substitute(buffer + uri_offset, data_length - uri_offset, uri_param, &unused, - &dummy_ev); + &events_sink); data_length = uri_offset + - norm_path_clean(buffer + uri_offset, data_length - uri_offset, &unused, &dummy_ev); + norm_path_clean(buffer + uri_offset, data_length - uri_offset, &unused, &events_sink); } } @@ -486,9 +487,8 @@ bool UriNormalizer::classic_need_norm(const Field& uri_component, bool do_path, const HttpParaList::UriParam& uri_param) { HttpInfractions unused; - HttpDummyEventGen dummy_ev; - return need_norm(uri_component, do_path, uri_param, &unused, &dummy_ev); + return need_norm(uri_component, do_path, uri_param, &unused, &events_sink); } void UriNormalizer::load_default_unicode_map(uint8_t map[65536]) diff --git a/src/service_inspectors/http_inspect/http_uri_norm.h b/src/service_inspectors/http_inspect/http_uri_norm.h index 91ba6c246..4533600c0 100644 --- a/src/service_inspectors/http_inspect/http_uri_norm.h +++ b/src/service_inspectors/http_inspect/http_uri_norm.h @@ -81,12 +81,6 @@ private: static inline uint8_t extract_percent_encoding(const Field& input, int32_t index); static inline bool is_u_encoding(const Field& input, int32_t index); static inline uint16_t extract_u_encoding(const Field& input, int32_t index); - - // An artifice used by the classic normalization methods to disable event generation - class HttpDummyEventGen : public HttpEventGen - { - void create_event(int) override {} - }; }; bool UriNormalizer::is_percent_encoding(const Field& input, int32_t index) diff --git a/src/utils/event_gen.h b/src/utils/event_gen.h index 29a559e1e..eb6f3d33b 100644 --- a/src/utils/event_gen.h +++ b/src/utils/event_gen.h @@ -33,29 +33,45 @@ template class EventGen { public: - virtual ~EventGen() = default; + EventGen() = default; - virtual void create_event(int sid) + EventGen(const std::bitset& suppressed) : events_suppressed(suppressed) {} + + void create_event(int sid) { if (sid == EVENT_NONE) return; assert((sid > 0) && (sid <= EVENT_MAX)); - if (!events_generated[sid-1]) + if (!events_suppressed[sid-1]) { snort::DetectionEngine::queue_event(GID, (uint32_t)sid); + events_suppressed[sid-1] = true; +#ifdef REG_TEST events_generated[sid-1] = true; +#endif } } + void suppress_event(int sid) + { + assert((sid > 0) && (sid <= EVENT_MAX)); + events_suppressed[sid-1] = true; + } + +#ifdef REG_TEST bool none_found() const { return events_generated == 0; } - // The following method is for convenience of debug and test output only! - uint64_t get_raw() const { return - (events_generated & bitmask).to_ulong(); } + uint64_t get_raw(unsigned base) const { return + ((events_generated >> base) & bitmask).to_ulong(); } +#endif protected: + std::bitset events_suppressed = 0; + +#ifdef REG_TEST std::bitset events_generated = 0; const std::bitset bitmask = 0xFFFFFFFFFFFFFFFF; +#endif }; #endif diff --git a/src/utils/infractions.h b/src/utils/infractions.h index 7e6aa6189..95039276b 100644 --- a/src/utils/infractions.h +++ b/src/utils/infractions.h @@ -39,7 +39,6 @@ public: assert((inf >= 0) && (inf < MAX)); infractions[inf] = true; } - bool none_found() const { return infractions == 0; } Infractions& operator+=(const Infractions& rhs) { infractions |= rhs.infractions; return *this; } friend Infractions operator+(Infractions lhs, const Infractions& rhs) @@ -47,13 +46,12 @@ public: friend bool operator&(const Infractions& lhs, const Infractions& rhs) { return (lhs.infractions & rhs.infractions) != 0; } - // The following methods are for convenience of debug and test output only! - uint64_t get_raw() const { return - (infractions & std::bitset(0xFFFFFFFFFFFFFFFF)).to_ulong(); } - uint64_t get_raw2() const { return - ((infractions >> 64) & std::bitset(0xFFFFFFFFFFFFFFFF)).to_ulong(); } - uint64_t get_raw3() const { return - ((infractions >> 128) & std::bitset(0xFFFFFFFFFFFFFFFF)).to_ulong(); } +#ifdef REG_TEST + bool none_found() const { return infractions == 0; } + + uint64_t get_raw(unsigned base) const { return + ((infractions >> base) & std::bitset(0xFFFFFFFFFFFFFFFF)).to_ulong(); } +#endif private: std::bitset infractions = 0;