From: Tom Peters (thopeter) Date: Tue, 20 Jun 2017 17:38:17 +0000 (-0400) Subject: Merge pull request #934 in SNORT/snort3 from nhttp75 to master X-Git-Tag: 3.0.0-239~40 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9bee70cbf2d1f5ad021d32eaf0911b8a52a4cadc;p=thirdparty%2Fsnort3.git Merge pull request #934 in SNORT/snort3 from nhttp75 to master Squashed commit of the following: commit 7f4cb724e558e1138ba028a8edd4f356c626d48e Author: Tom Peters Date: Thu Jun 15 14:25:26 2017 -0400 NHI alerts related to 100 Continue --- diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h index 1b8bafdd7..0335bd3fe 100644 --- a/src/service_inspectors/http_inspect/http_enum.h +++ b/src/service_inspectors/http_inspect/http_enum.h @@ -218,6 +218,11 @@ enum Infraction INF_BAD_CHAR_IN_HEADER_NAME, INF_HEADER_WRAPPING, INF_CHUNK_BAD_SEP, + INF_MULTIPLE_100_RESPONSES, + INF_UNEXPECTED_100_RESPONSE, + INF_UNKNOWN_1XX_STATUS, + INF_EXPECT_WITHOUT_BODY_CL0, + INF_EXPECT_WITHOUT_BODY_NO_CL, INF__MAX_VALUE }; @@ -317,6 +322,10 @@ enum EventSid EVENT_CR_WITHOUT_LF, EVENT_CHUNK_BAD_SEP, EVENT_CHUNK_BARE_LF, + EVENT_MULTIPLE_100_RESPONSES, + EVENT_UNEXPECTED_100_RESPONSE, + EVENT_UNKNOWN_1XX_STATUS, + EVENT_EXPECT_WITHOUT_BODY, // 90 EVENT__MAX_VALUE }; diff --git a/src/service_inspectors/http_inspect/http_msg_header.cc b/src/service_inspectors/http_inspect/http_msg_header.cc index 0600411f2..2a6b81dbf 100644 --- a/src/service_inspectors/http_inspect/http_msg_header.cc +++ b/src/service_inspectors/http_inspect/http_msg_header.cc @@ -164,6 +164,11 @@ void HttpMsgHeader::update_flow() else if (content_length == 0) { // No body + if (get_header_count(HEAD_EXPECT) > 0) + { + add_infraction(INF_EXPECT_WITHOUT_BODY_CL0); + create_event(EVENT_EXPECT_WITHOUT_BODY); + } session_data->half_reset(source_id); return; } @@ -186,6 +191,11 @@ void HttpMsgHeader::update_flow() add_infraction(INF_POST_WO_BODY); create_event(EVENT_UNBOUNDED_POST); } + if (get_header_count(HEAD_EXPECT) > 0) + { + add_infraction(INF_EXPECT_WITHOUT_BODY_NO_CL); + create_event(EVENT_EXPECT_WITHOUT_BODY); + } session_data->half_reset(source_id); return; } diff --git a/src/service_inspectors/http_inspect/http_msg_status.cc b/src/service_inspectors/http_inspect/http_msg_status.cc index 448e0c10e..19b3470db 100644 --- a/src/service_inspectors/http_inspect/http_msg_status.cc +++ b/src/service_inspectors/http_inspect/http_msg_status.cc @@ -24,6 +24,7 @@ #include "http_msg_status.h" #include "http_api.h" +#include "http_msg_header.h" #include "stream/stream.h" using namespace HttpEnums; @@ -100,6 +101,11 @@ void HttpMsgStatus::derive_status_code_num() add_infraction(INF_BAD_STAT_CODE); create_event(EVENT_INVALID_STATCODE); } + if ((status_code_num >= 102) && (status_code_num <= 199)) + { + add_infraction(INF_UNKNOWN_1XX_STATUS); + create_event(EVENT_UNKNOWN_1XX_STATUS); + } } void HttpMsgStatus::gen_events() @@ -142,9 +148,9 @@ void HttpMsgStatus::gen_events() } } - if( !transaction->get_request() && (trans_num == 1) ) + if (!transaction->get_request() && (trans_num == 1)) { - if( flow->is_pdu_inorder(SSN_DIR_FROM_SERVER) ) + if (flow->is_pdu_inorder(SSN_DIR_FROM_SERVER)) { // HTTP response without a request. Possible ssh tunneling add_infraction(INF_RSP_WO_REQ); @@ -170,7 +176,16 @@ void HttpMsgStatus::update_flow() // responses to all be included in the same transaction. It's not obvious whether that is // the best way to handle what should be a highly abnormal situation. if (status_code_num == 100) - transaction->second_response_coming(); + { + // Were we "Expect"-ing this? + HttpMsgHeader* const req_header = transaction->get_header(SRC_CLIENT); + if ((req_header != nullptr) && (req_header->get_header_count(HEAD_EXPECT) == 0)) + { + add_infraction(INF_UNEXPECTED_100_RESPONSE); + create_event(EVENT_UNEXPECTED_100_RESPONSE); + } + transaction->set_one_hundred_response(); + } } session_data->section_type[source_id] = SEC__NOT_COMPUTE; } diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc index 9fc1d4059..6f2554174 100644 --- a/src/service_inspectors/http_inspect/http_tables.cc +++ b/src/service_inspectors/http_inspect/http_tables.cc @@ -344,6 +344,10 @@ const RuleMap HttpModule::http_events[] = { EVENT_CR_WITHOUT_LF, "HTTP header line terminated by CR without a LF" }, { EVENT_CHUNK_BAD_SEP, "chunk terminated by nonstandard separator" }, { EVENT_CHUNK_BARE_LF, "chunk length terminated by LF without CR" }, + { EVENT_MULTIPLE_100_RESPONSES, "more than one response with 100 status code" }, + { EVENT_UNEXPECTED_100_RESPONSE, "100 status code not in response to Expect header" }, + { EVENT_UNKNOWN_1XX_STATUS, "1XX status code other than 100 or 101" }, + { EVENT_EXPECT_WITHOUT_BODY, "Expect header sent without a message body" }, { 0, nullptr } }; diff --git a/src/service_inspectors/http_inspect/http_transaction.cc b/src/service_inspectors/http_inspect/http_transaction.cc index 1b4737385..afc309dd7 100644 --- a/src/service_inspectors/http_inspect/http_transaction.cc +++ b/src/service_inspectors/http_inspect/http_transaction.cc @@ -110,7 +110,6 @@ HttpTransaction* HttpTransaction::attach_my_transaction(HttpFlowData* session_da (session_data->transaction[SRC_SERVER] != nullptr) && session_data->transaction[SRC_SERVER]->second_response_expected) { - assert(session_data->transaction[SRC_SERVER] != nullptr); session_data->transaction[SRC_SERVER]->second_response_expected = false; delete session_data->transaction[SRC_SERVER]->status; session_data->transaction[SRC_SERVER]->status = nullptr; @@ -201,3 +200,15 @@ HttpEventGen* HttpTransaction::get_events(HttpEnums::SourceId source_id) return events[source_id]; } +void HttpTransaction::set_one_hundred_response() +{ + assert(response_seen); + if (one_hundred_response) + { + *infractions[SRC_SERVER] += INF_MULTIPLE_100_RESPONSES; + events[SRC_SERVER]->create_event(EVENT_MULTIPLE_100_RESPONSES); + } + one_hundred_response = true; + second_response_expected = true; +} + diff --git a/src/service_inspectors/http_inspect/http_transaction.h b/src/service_inspectors/http_inspect/http_transaction.h index 50a0bb554..9d7946d67 100644 --- a/src/service_inspectors/http_inspect/http_transaction.h +++ b/src/service_inspectors/http_inspect/http_transaction.h @@ -61,7 +61,7 @@ public: HttpInfractions* get_infractions(HttpEnums::SourceId source_id); HttpEventGen* get_events(HttpEnums::SourceId source_id); - void second_response_coming() { assert(response_seen); second_response_expected = true; } + void set_one_hundred_response(); bool final_response() const { return !second_response_expected; } private: @@ -77,6 +77,7 @@ private: HttpEventGen* events[2] = { nullptr, nullptr }; bool response_seen = false; + bool one_hundred_response = false; bool second_response_expected = false; // This is a form of reference counting that prevents premature/double deletion of a diff --git a/src/service_inspectors/http_inspect/test/http_transaction_test.cc b/src/service_inspectors/http_inspect/test/http_transaction_test.cc index 44d32ab65..9d6d39b10 100644 --- a/src/service_inspectors/http_inspect/test/http_transaction_test.cc +++ b/src/service_inspectors/http_inspect/test/http_transaction_test.cc @@ -287,7 +287,7 @@ TEST(http_transaction_test, basic_continue) // Interim response section_type[SRC_SERVER] = SEC_STATUS; CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER)); - trans->second_response_coming(); + trans->set_one_hundred_response(); section_type[SRC_SERVER] = SEC_HEADER; CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER)); @@ -334,7 +334,7 @@ TEST(http_transaction_test, multiple_continue) { section_type[SRC_SERVER] = SEC_STATUS; CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER)); - trans->second_response_coming(); + trans->set_one_hundred_response(); section_type[SRC_SERVER] = SEC_HEADER; CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER)); } @@ -374,7 +374,7 @@ TEST(http_transaction_test, multiple_orphan_continue) section_type[SRC_SERVER] = SEC_STATUS; HttpTransaction* trans = HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER); CHECK(trans != nullptr); - trans->second_response_coming(); + trans->set_one_hundred_response(); section_type[SRC_SERVER] = SEC_HEADER; CHECK(trans == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER)); section_type[SRC_SERVER] = SEC_BODY_CHUNK; @@ -430,7 +430,7 @@ TEST(http_transaction_test, pipeline_continue_pipeline) // Interim response to fourth request section_type[SRC_SERVER] = SEC_STATUS; CHECK(trans[3] == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER)); - trans[3]->second_response_coming(); + trans[3]->set_one_hundred_response(); section_type[SRC_SERVER] = SEC_HEADER; CHECK(trans[3] == HttpTransaction::attach_my_transaction(flow_data, SRC_SERVER));