From: Mike Stepanek (mstepane) Date: Wed, 15 Apr 2020 14:27:07 +0000 (+0000) Subject: Merge pull request #2147 in SNORT/snort3 from ~KATHARVE/snort3:connect2 to master X-Git-Tag: 3.0.1-2~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6d309ef1f68c4c238d0ea9b185e693c6c89a3784;p=thirdparty%2Fsnort3.git Merge pull request #2147 in SNORT/snort3 from ~KATHARVE/snort3:connect2 to master Squashed commit of the following: commit d885bee3d44fc6276c2df089b071a7425208ea09 Author: Katura Harvey Date: Tue Apr 7 13:35:33 2020 -0400 http_inspect: cut over to wizard on successful CONNECT response --- diff --git a/doc/http_inspect.txt b/doc/http_inspect.txt index 20768244e..afb1d2cef 100644 --- a/doc/http_inspect.txt +++ b/doc/http_inspect.txt @@ -243,6 +243,29 @@ directories to be separated by backslashes: backslash_to_slash is turned on by default. It replaces all the backslashes with slashes during normalization. +==== CONNECT processing + +The HTTP CONNECT method is used by a client to establish a tunnel to a destination via an HTTP proxy +server. If the connection is successful the server will send a 2XX success response to the client, +then proceed to blindly forward traffic between the client and destination. That traffic belongs to +a new session between the client and destination and may be of any protocol, so clearly the HTTP +inspector will be unable to continue processing traffic following the CONNECT message as if it were +just a continuation of the original HTTP/1.1 session. + +Therefore upon receiving a success response to a CONNECT request, the HTTP inspector will stop +inspecting the session. The next packet will return to the wizard, which will determine the +appropriate inspector to continue processing the flow. If the tunneled protocol happens to be +HTTP/1.1, the HTTP inspector will again start inspecting the flow, but as an entirely new session. + +There is one scenario where the cutover to the wizard will not occur despite a 2XX success response +to a CONNECT request. HTTP allows for pipelining, or sending multiple requests without waiting for a +response. If the HTTP inspector sees any further traffic from the client after a CONNECT request +before it has seen the CONNECT response, it is unclear whether this traffic should be interpreted as +a pipelined HTTP request or tunnel traffic sent in anticipation of a success response from the +server. Due to this potential evasion tactic, the HTTP inspector will not cut over to the wizard if +it sees any early client-to-server traffic, but will continue normal HTTP processing of the flow +regardless of the eventual server response. + ==== Detection rules http_inspect parses HTTP messages into their components and makes them diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h index 56b6df7b8..75745511a 100644 --- a/src/service_inspectors/http_inspect/http_enum.h +++ b/src/service_inspectors/http_inspect/http_enum.h @@ -58,7 +58,7 @@ enum PEG_COUNT { PEG_FLOW = 0, PEG_SCAN, PEG_REASSEMBLE, PEG_INSPECT, PEG_REQUES PEG_GET, PEG_HEAD, PEG_POST, PEG_PUT, PEG_DELETE, PEG_CONNECT, PEG_OPTIONS, PEG_TRACE, PEG_OTHER_METHOD, PEG_REQUEST_BODY, PEG_CHUNKED, PEG_URI_NORM, PEG_URI_PATH, PEG_URI_CODING, PEG_CONCURRENT_SESSIONS, PEG_MAX_CONCURRENT_SESSIONS, PEG_DETAINED, PEG_PARTIAL_INSPECT, - PEG_EXCESS_PARAMS, PEG_PARAMS, PEG_COUNT_MAX }; + PEG_EXCESS_PARAMS, PEG_PARAMS, PEG_CUTOVERS, PEG_COUNT_MAX }; // Result of scanning by splitter enum ScanResult { SCAN_NOT_FOUND, SCAN_NOT_FOUND_DETAIN, SCAN_FOUND, SCAN_FOUND_PIECE, @@ -235,6 +235,7 @@ enum Infraction INF_200_CONNECT_RESP_WITH_CL, INF_200_CONNECT_RESP_WITH_TE, INF_100_CONNECT_RESP, + INF_EARLY_CONNECT_RESPONSE, INF__MAX_VALUE }; @@ -360,7 +361,8 @@ enum EventSid EVENT_EARLY_C2S_TRAFFIC_AFTER_CONNECT, EVENT_200_CONNECT_RESP_WITH_CL, EVENT_200_CONNECT_RESP_WITH_TE, - EVENT_100_CONNECT_RESP, // 257 + EVENT_100_CONNECT_RESP, + EVENT_EARLY_CONNECT_RESPONSE, // 258 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 7846ba1bd..ef34a5086 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.h +++ b/src/service_inspectors/http_inspect/http_flow_data.h @@ -161,6 +161,8 @@ private: HttpEnums::VERS__NOT_PRESENT }; HttpEnums::MethodId method_id = HttpEnums::METH__NOT_PRESENT; + bool cutover_on_clear = false; + // *** Transaction management including pipelining static const int MAX_PIPELINE = 100; // requests seen - responses seen <= MAX_PIPELINE HttpTransaction* transaction[2] = { nullptr, nullptr }; diff --git a/src/service_inspectors/http_inspect/http_inspect.cc b/src/service_inspectors/http_inspect/http_inspect.cc index 458f8669e..544110994 100644 --- a/src/service_inspectors/http_inspect/http_inspect.cc +++ b/src/service_inspectors/http_inspect/http_inspect.cc @@ -572,5 +572,12 @@ void HttpInspect::clear(Packet* p) current_transaction->garbage_collect(); session_data->garbage_collect(); + + if (session_data->cutover_on_clear) + { + Flow* flow = p->flow; + flow->set_service(p, nullptr); + flow->free_flow_data(HttpFlowData::inspector_id); + } } diff --git a/src/service_inspectors/http_inspect/http_msg_header.cc b/src/service_inspectors/http_inspect/http_msg_header.cc index e8f0def5c..e40bbd96e 100644 --- a/src/service_inspectors/http_inspect/http_msg_header.cc +++ b/src/service_inspectors/http_inspect/http_msg_header.cc @@ -171,8 +171,7 @@ void HttpMsgHeader::update_flow() // Successful CONNECT responses (2XX) switch to tunneled traffic immediately following the // header. Transfer-Encoding and Content-Length headers are not allowed in successful // responses by the RFC. - if(((session_data->last_connect_trans_w_early_traffic == 0) || - (trans_num > session_data->last_connect_trans_w_early_traffic)) && + if((trans_num > session_data->last_connect_trans_w_early_traffic) && ((status_code_num >= 200) && (status_code_num < 300))) { if ((get_header_count(HEAD_TRANSFER_ENCODING) > 0)) @@ -185,9 +184,27 @@ void HttpMsgHeader::update_flow() add_infraction(INF_200_CONNECT_RESP_WITH_CL); create_event(EVENT_200_CONNECT_RESP_WITH_CL); } - // Temporary - Further traffic on this flow is tunneled traffic, and will get sent back - // to the wizard to determine the appropriate inspector. For now just abort. - session_data->type_expected[source_id] = SEC_ABORT; + + // FIXIT-E This case addresses the scenario where Snort sees a success response to a + // CONNECT request before the request message is complete. Currently this will trigger + // an alert then proceed to cutover as usual, meaning the remaining request message will + // be processed as part of the tunnel session. There may be a better solution. + if (session_data->type_expected[SRC_CLIENT] != SEC_REQUEST) + { + add_infraction(INF_EARLY_CONNECT_RESPONSE); + create_event(EVENT_EARLY_CONNECT_RESPONSE); + } + session_data->cutover_on_clear = true; + HttpModule::increment_peg_counts(PEG_CUTOVERS); + +#ifdef REG_TEST + if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP)) + { + fprintf(HttpTestManager::get_output_file(), "2XX CONNECT response triggered flow " + "cutover to wizard\n"); + } +#endif + return; } if ((status_code_num >= 100) && (status_code_num < 200)) diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc index b48c53214..c14767c81 100644 --- a/src/service_inspectors/http_inspect/http_tables.cc +++ b/src/service_inspectors/http_inspect/http_tables.cc @@ -391,6 +391,7 @@ const RuleMap HttpModule::http_events[] = { EVENT_200_CONNECT_RESP_WITH_CL, "HTTP CONNECT 2XX response with Content-Length header" }, { EVENT_200_CONNECT_RESP_WITH_TE, "HTTP CONNECT 2XX response with Transfer-Encoding header" }, { EVENT_100_CONNECT_RESP, "HTTP CONNECT response with 1XX status code" }, + { EVENT_EARLY_CONNECT_RESPONSE, "HTTP CONNECT response before request message completed" }, { 0, nullptr } }; @@ -422,6 +423,7 @@ const PegInfo HttpModule::peg_names[PEG_COUNT_MAX+1] = { CountType::SUM, "partial_inspections", "pre-inspections for detained inspection" }, { CountType::SUM, "excess_parameters", "repeat parameters exceeding max" }, { CountType::SUM, "parameters", "HTTP parameters inspected" }, + { CountType::SUM, "connect_tunnel_cutovers", "CONNECT tunnel flow cutovers to wizard" }, { CountType::END, nullptr, nullptr } };