From: Russ Combs (rucombs) Date: Tue, 31 May 2016 19:05:44 +0000 (-0400) Subject: Merge pull request #493 in SNORT/snort3 from nhttp45 to master X-Git-Tag: 3.0.0-233~386 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4cce47e69ccbc63b212fbd21a2eb3c76152fddd5;p=thirdparty%2Fsnort3.git Merge pull request #493 in SNORT/snort3 from nhttp45 to master Squashed commit of the following: commit 83b84059b9d7c5a442b06fe39ea0e3f173588bd4 Author: Tom Peters Date: Mon May 23 13:28:07 2016 -0400 NHI alerts for abusive CL or TE --- diff --git a/src/service_inspectors/nhttp_inspect/nhttp_enum.h b/src/service_inspectors/nhttp_inspect/nhttp_enum.h index 747f144ba..e92928435 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_enum.h +++ b/src/service_inspectors/nhttp_inspect/nhttp_enum.h @@ -176,6 +176,11 @@ enum Infraction INF_U_ENCODE, INF_UNKNOWN_PERCENT, INF_DOUBLE_DECODE, + INF_MULTIPLE_CONTLEN, + INF_BOTH_CL_AND_TE, + INF_BAD_CODE_BODY_HEADER, + INF_FINAL_NOT_CHUNKED, + INF_CHUNKED_BEFORE_END, INF__MAX_VALUE }; @@ -263,6 +268,10 @@ enum EventSid EVENT_GZIP_FAILURE, EVENT_ZERO_NINE_CONTINUE, EVENT_ZERO_NINE_NOT_FIRST, + EVENT_BOTH_CL_AND_TE, + EVENT_BAD_CODE_BODY_HEADER, + EVENT_FINAL_NOT_CHUNKED, + EVENT_CHUNKED_BEFORE_END, EVENT__MAX_VALUE }; diff --git a/src/service_inspectors/nhttp_inspect/nhttp_msg_header.cc b/src/service_inspectors/nhttp_inspect/nhttp_msg_header.cc index dc32eb339..c73a89430 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_msg_header.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_msg_header.cc @@ -17,9 +17,9 @@ //-------------------------------------------------------------------------- // nhttp_msg_header.cc author Tom Peters -#include +#include +#include #include -#include #include "utils/util.h" #include "detection/detection_util.h" @@ -27,6 +27,7 @@ #include "file_api/file_flows.h" #include "nhttp_api.h" +#include "nhttp_normalizers.h" #include "nhttp_msg_request.h" #include "nhttp_msg_header.h" @@ -44,14 +45,19 @@ void NHttpMsgHeader::update_flow() { session_data->section_type[source_id] = SEC__NOT_COMPUTE; - // FIXIT-L put this test here for now. May want to integrate into the following code and - // do more careful checks for inappropriate Content-Length. if (get_header_count(HEAD_CONTENT_LENGTH) > 1) + { + infractions += INF_MULTIPLE_CONTLEN; events.create_event(EVENT_MULTIPLE_CONTLEN); + } + if ((get_header_count(HEAD_CONTENT_LENGTH) > 0) && + (get_header_count(HEAD_TRANSFER_ENCODING) > 0)) + { + infractions += INF_BOTH_CL_AND_TE; + events.create_event(EVENT_BOTH_CL_AND_TE); + } // The following logic to determine body type is by no means the last word on this topic. - // FIXIT-H need to distinguish methods such as POST that should have a body from those that - // should not. if (tcp_close) { session_data->half_reset(source_id); @@ -62,9 +68,21 @@ void NHttpMsgHeader::update_flow() if ((source_id == SRC_SERVER) && ((status_code_num <= 199) || (status_code_num == 204) || (status_code_num == 304))) { - // No body allowed by RFC for these response codes - // FIXIT-M inspect for Content-Length and Transfer-Encoding headers which should not be - // present + // No body allowed by RFC for these response codes. The message is over regardless of the + // headers. + if (get_header_count(HEAD_TRANSFER_ENCODING) > 0) + { + infractions += INF_BAD_CODE_BODY_HEADER; + events.create_event(EVENT_BAD_CODE_BODY_HEADER); + } + if (get_header_count(HEAD_CONTENT_LENGTH) > 0) + { + if (norm_decimal_integer(get_header_value_norm(HEAD_CONTENT_LENGTH)) > 0) + { + infractions += INF_BAD_CODE_BODY_HEADER; + events.create_event(EVENT_BAD_CODE_BODY_HEADER); + } + } session_data->half_reset(SRC_SERVER); return; } @@ -78,22 +96,31 @@ void NHttpMsgHeader::update_flow() } // If there is a Transfer-Encoding header, see if the last of the encoded values is "chunked". - // FIXIT-L do something with Transfer-Encoding header with chunked present but not last. - // FIXIT-L do something with Transfer-Encoding header present but no chunked at all. if (get_header_value_norm(HEAD_TRANSFER_ENCODING).length > 0) { + if (chunked_before_end(get_header_value_norm(HEAD_TRANSFER_ENCODING))) + { + infractions += INF_CHUNKED_BEFORE_END; + events.create_event(EVENT_CHUNKED_BEFORE_END); + } if (norm_last_token_code(get_header_value_norm(HEAD_TRANSFER_ENCODING), NHttpMsgHeadShared::trans_code_list) == TRANSCODE_CHUNKED) { - // FIXIT-M inspect for Content-Length header which should not be present // Chunked body session_data->type_expected[source_id] = SEC_BODY_CHUNK; prepare_body(); return; } + else + { + infractions += INF_FINAL_NOT_CHUNKED; + events.create_event(EVENT_FINAL_NOT_CHUNKED); + } } - if (get_header_value_norm(HEAD_CONTENT_LENGTH).length > 0) + // else because Transfer-Encoding header negates Content-Length header even if something was + // wrong with Transfer-Encoding header. + else if (get_header_value_norm(HEAD_CONTENT_LENGTH).length > 0) { const int64_t content_length = norm_decimal_integer(get_header_value_norm(HEAD_CONTENT_LENGTH)); diff --git a/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc b/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc index e91f96c10..1a149b6c8 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc @@ -89,3 +89,16 @@ int32_t norm_last_token_code(const Field& input, const StrCode table[]) return str_to_code(last_start, last_length, table); } +// Given a comma-separated list of words, does "chunked" appear before the last word +bool chunked_before_end(const Field& input) +{ + for (int k=0; k < (input.length - 7); k++) + { + if (((k == 0) || (input.start[k-1] == ',')) && !memcmp(input.start+k, "chunked,", 8)) + { + return true; + } + } + return false; +} + diff --git a/src/service_inspectors/nhttp_inspect/nhttp_normalizers.h b/src/service_inspectors/nhttp_inspect/nhttp_normalizers.h index eb78128f9..fdc62c0b6 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_normalizers.h +++ b/src/service_inspectors/nhttp_inspect/nhttp_normalizers.h @@ -38,6 +38,7 @@ NormFunc norm_remove_lws; // Other normalization-related utilities int64_t norm_decimal_integer(const Field& input); int32_t norm_last_token_code(const Field& input, const StrCode table[]); +bool chunked_before_end(const Field& input); #endif diff --git a/src/service_inspectors/nhttp_inspect/nhttp_tables.cc b/src/service_inspectors/nhttp_inspect/nhttp_tables.cc index 4c30cced9..105317590 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_tables.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_tables.cc @@ -327,6 +327,11 @@ const RuleMap NHttpModule::nhttp_events[] = { EVENT_GZIP_FAILURE, "Gzip decompression failed" }, { EVENT_ZERO_NINE_CONTINUE, "HTTP 0.9 requested followed by another request" }, { EVENT_ZERO_NINE_NOT_FIRST, "HTTP 0.9 request following a normal request" }, + { EVENT_BOTH_CL_AND_TE, "Message has both Content-Length and Transfer-Encoding" }, + { EVENT_BAD_CODE_BODY_HEADER, "Status code implying no body combined with Transfer-" + "Encoding or nonzero Content-Length" }, + { EVENT_FINAL_NOT_CHUNKED, "Transfer-Encoding did not end with chunked" }, + { EVENT_CHUNKED_BEFORE_END, "Transfer-Encoding with chunked not at end" }, { 0, nullptr } }; diff --git a/src/service_inspectors/nhttp_inspect/test/CMakeLists.txt b/src/service_inspectors/nhttp_inspect/test/CMakeLists.txt index 809739d96..e19c94e53 100644 --- a/src/service_inspectors/nhttp_inspect/test/CMakeLists.txt +++ b/src/service_inspectors/nhttp_inspect/test/CMakeLists.txt @@ -1,2 +1,3 @@ add_cpputest(nhttp_uri_norm_test nhttp_inspect framework) +add_cpputest(nhttp_normalizers_test nhttp_inspect framework) diff --git a/src/service_inspectors/nhttp_inspect/test/Makefile.am b/src/service_inspectors/nhttp_inspect/test/Makefile.am index 8ed6fa1e3..1628ff562 100644 --- a/src/service_inspectors/nhttp_inspect/test/Makefile.am +++ b/src/service_inspectors/nhttp_inspect/test/Makefile.am @@ -2,7 +2,8 @@ AM_DEFAULT_SOURCE_EXT = .cc check_PROGRAMS = \ -nhttp_uri_norm_test +nhttp_uri_norm_test \ +nhttp_normalizers_test TESTS = $(check_PROGRAMS) @@ -19,3 +20,9 @@ nhttp_uri_norm_test_LDADD = \ ../../../framework/module.o \ @CPPUTEST_LDFLAGS@ +nhttp_normalizers_test_CPPFLAGS = $(AM_CPPFLAGS) @CPPUTEST_CPPFLAGS@ +nhttp_normalizers_test_LDADD = \ +../nhttp_normalizers.o \ +../nhttp_field.o \ +@CPPUTEST_LDFLAGS@ + diff --git a/src/service_inspectors/nhttp_inspect/test/nhttp_normalizers_test.cc b/src/service_inspectors/nhttp_inspect/test/nhttp_normalizers_test.cc new file mode 100644 index 000000000..a05dfb6d1 --- /dev/null +++ b/src/service_inspectors/nhttp_inspect/test/nhttp_normalizers_test.cc @@ -0,0 +1,54 @@ +//-------------------------------------------------------------------------- +// Copyright (C) 2016 Cisco and/or its affiliates. All rights reserved. +// +// This program is free software; you can redistribute it and/or modify it +// under the terms of the GNU General Public License Version 2 as published +// by the Free Software Foundation. You may not use, modify or distribute +// this program under any other version of the GNU General Public License. +// +// This program is distributed in the hope that it will be useful, but +// WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +// General Public License for more details. +// +// You should have received a copy of the GNU General Public License along +// with this program; if not, write to the Free Software Foundation, Inc., +// 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +//-------------------------------------------------------------------------- + +// nhttp_normalizers_test.cc author Tom Peters +// unit test main + +#include "service_inspectors/nhttp_inspect/nhttp_msg_header.h" +#include "service_inspectors/nhttp_inspect/nhttp_test_manager.h" + +#include +#include +#include + +// Stubs whose sole purpose is to make the test code link +int32_t str_to_code(const uint8_t*, const int32_t, const StrCode []) { return 0; } +const bool NHttpEnums::is_sp_tab[256] {}; +long NHttpTestManager::print_amount {}; +bool NHttpTestManager::print_hex {}; + +TEST_GROUP(nhttp_chunked_before_end_test) {}; + +TEST(nhttp_chunked_before_end_test, examples) +{ + CHECK(!chunked_before_end(Field(11, (const uint8_t*)"foo,chunked"))); + CHECK(chunked_before_end(Field(15, (const uint8_t*)"chunked,chunked"))); + CHECK(!chunked_before_end(Field(10, (const uint8_t*)"notchunked"))); + CHECK(chunked_before_end(Field(12, (const uint8_t*)"foo,chunked,"))); + CHECK(chunked_before_end(Field(16, (const uint8_t*)"chunked,identity"))); + CHECK(chunked_before_end(Field(21, (const uint8_t*)"gzip,chunked,identity"))); + CHECK(!chunked_before_end(Field(0, (const uint8_t*)""))); + CHECK(!chunked_before_end(Field(7, (const uint8_t*)"chunked"))); + CHECK(!chunked_before_end(Field(8, (const uint8_t*)",chunked"))); +} + +int main(int argc, char** argv) +{ + return CommandLineTestRunner::RunAllTests(argc, argv); +} +