diff --git a/CHANGES b/CHANGES index a1173284c7..f033a9fd1c 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,47 @@ +3.2.0-dev.486 | 2020-05-11 11:11:51 -0700 + + * Limit rate at which MMDB error/status messages are emitted + + If there's some bad state we can be in where MMDB lookup/open operations + consistently fail, then the volume of associated reporter messages can + get overwhelmingly large especially if a lookup operation is being done + for each network connection. + + This adds a limit of an arbitrary 20 messages every 5 minutes, which + should be enough information to understand the overall + open/close/lookup-failure pattern. (Jon Siwek, Corelight) + +3.2.0-dev.484 | 2020-05-08 11:50:54 -0700 + + * Change timer_list in BroList to be an unordered list. (Tim Wojtulewicz, Corelight) + + This type is used by Conn and Analyzer to hold onto timers being added and + removed. We don't expect the elements in those lists to maintain an order + as the list is being modified. + + * Add ability for List to be ordered/unordered (Tim Wojtulewicz, Corelight) + + This fixes a "bug" with List where remove_nth() can be an O(n) operation + when it doesn't need to be. remove_nth for lists that don't necessarily + need to keep an order can be an O(1) operation instead. + + * Add unit testing for List (Tim Wojtulewicz, Corelight) + + * Fix bug with List where replace() doesn't work with non-pointer types (Tim Wojtulewicz, Corelight) + +3.2.0-dev.478 | 2020-05-08 11:47:38 -0700 + + * Added examples to set_to_regex comments (James Lagermann, Corelight) + + * Unbreak build on Fedora 32 (gcc 10.0.1) (Johanna Amann, Corelight) + + It requires cstdint in a few more headers. + +3.2.0-dev.475 | 2020-05-07 17:15:23 -0700 + + * GH-958: Fix crash when trying to redef non-existing enum (Johanna Amann, Corelight) + 3.2.0-dev.473 | 2020-05-06 10:40:09 -0700 * Revert addition of final modifier to JSON formatter (Tim Wojtulewicz, Corelight) diff --git a/VERSION b/VERSION index c039298b41..fef583e48a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0-dev.473 +3.2.0-dev.486 diff --git a/doc b/doc index 850c5bea87..73cce33f7a 160000 --- a/doc +++ b/doc @@ -1 +1 @@ -Subproject commit 850c5bea8787c315cddc9079a29a17d89db055ec +Subproject commit 73cce33f7ab3a08ee8c92bf7815a092eae7ad031 diff --git a/scripts/base/utils/patterns.zeek b/scripts/base/utils/patterns.zeek index fc2ee6ba61..7890a6e111 100644 --- a/scripts/base/utils/patterns.zeek +++ b/scripts/base/utils/patterns.zeek @@ -4,7 +4,14 @@ module GLOBAL; ## Given a pattern as a string with two tildes (~~) contained in it, it will ## return a pattern with string set's elements OR'd together where the -## double-tilde was given. +## double-tilde was given. Examples: +## +## .. sourcecode:: zeek +## +## global r1 = set_to_regex(set("a", "b", "c"), "~~"); +## # r1 = /^?(a|b|c)$?/ +## global r2 = set_to_regex(set("a.com", "b.com", "c.com"), "\\.(~~)"); +## # r2 = /^?(\.(a\.com|b\.com|c\.com))$?/ ## ## ss: a set of strings to OR together. ## diff --git a/src/Anon.h b/src/Anon.h index 3b2e704083..ecda53f023 100644 --- a/src/Anon.h +++ b/src/Anon.h @@ -12,6 +12,7 @@ #include #include +#include // TODO: Anon.h may not be the right place to put these functions ... diff --git a/src/BroList.h b/src/BroList.h index 48e9ebee04..a37f40d84a 100644 --- a/src/BroList.h +++ b/src/BroList.h @@ -23,4 +23,4 @@ class Attr; typedef PList attr_list; class Timer; -typedef PList timer_list; +typedef PList timer_list; diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0e529c0aac..de2146418e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -246,6 +246,7 @@ set(MAIN_SRCS IntSet.cc IP.cc IPAddr.cc + List.cc Reporter.cc NFA.cc Net.cc diff --git a/src/List.cc b/src/List.cc new file mode 100644 index 0000000000..c19353b341 --- /dev/null +++ b/src/List.cc @@ -0,0 +1,130 @@ +#include +#include <3rdparty/doctest.h> + +TEST_CASE("list construction") + { + List list; + CHECK(list.empty()); + + List list2(10); + CHECK(list2.empty()); + CHECK(list2.max() == 10); + } + +TEST_CASE("list operation") + { + List list({ 1, 2, 3 }); + CHECK(list.size() == 3); + CHECK(list.max() == 3); + CHECK(list[0] == 1); + CHECK(list[1] == 2); + CHECK(list[2] == 3); + + // push_back forces a resize of the list here, which grows the list + // by a growth factor. That makes the max elements equal to 6. + list.push_back(4); + CHECK(list.size() == 4); + CHECK(list.max() == 6); + CHECK(list[3] == 4); + + CHECK(list.front() == 1); + CHECK(list.back() == 4); + + list.pop_front(); + CHECK(list.size() == 3); + CHECK(list.front() == 2); + + list.pop_back(); + CHECK(list.size() == 2); + CHECK(list.back() == 3); + + list.push_back(4); + CHECK(list.is_member(2)); + CHECK(list.member_pos(2) == 0); + + list.remove(2); + CHECK(list.size() == 2); + CHECK(list[0] == 3); + CHECK(list[1] == 4); + + // Squash the list down to the existing elements. + list.resize(); + CHECK(list.size() == 2); + CHECK(list.max() == 2); + + // Attempt replacing a known position. + int old = list.replace(0, 10); + CHECK(list.size() == 2); + CHECK(list.max() == 2); + CHECK(old == 3); + CHECK(list[0] == 10); + CHECK(list[1] == 4); + + // Attempt replacing an element off the end of the list, which + // causes a resize. + old = list.replace(3, 5); + CHECK(list.size() == 4); + CHECK(list.max() == 4); + CHECK(old == 0); + CHECK(list[0] == 10); + CHECK(list[1] == 4); + CHECK(list[2] == 0); + CHECK(list[3] == 5); + + // Attempt replacing an element with a negative index, which returns the + // default value for the list type. + old = list.replace(-1, 50); + CHECK(list.size() == 4); + CHECK(list.max() == 4); + CHECK(old == 0); + + list.clear(); + CHECK(list.size() == 0); + CHECK(list.max() == 0); + } + +TEST_CASE("list iteration") + { + List list({ 1, 2, 3, 4}); + + int index = 1; + for ( int v : list ) + CHECK(v == index++); + + index = 1; + for ( auto it = list.begin(); it != list.end(); index++, ++it) + CHECK(*it == index); + } + +TEST_CASE("plists") + { + PList list; + list.push_back(new int(1)); + list.push_back(new int(2)); + list.push_back(new int(3)); + + CHECK(*list[0] == 1); + + int* new_val = new int(5); + auto old = list.replace(-1, new_val); + delete new_val; + CHECK(old == nullptr); + + for ( auto v : list ) + delete v; + list.clear(); + } + +TEST_CASE("unordered list operation") + { + List list({1, 2, 3, 4}); + CHECK(list.size() == 4); + + // An unordered list doesn't maintain the ordering of the elements when + // one is removed. It just swaps the last element into the hole. + list.remove(2); + CHECK(list.size() == 3); + CHECK(list[0] == 1); + CHECK(list[1] == 4); + CHECK(list[2] == 3); + } diff --git a/src/List.h b/src/List.h index d4b9798e4d..c6d0902013 100644 --- a/src/List.h +++ b/src/List.h @@ -29,7 +29,9 @@ // TODO: this can be removed in v3.1 when List::sort() is removed typedef int (*list_cmp_func)(const void* v1, const void* v2); -template +enum class ListOrder : int { ORDERED, UNORDERED }; + +template class List { public: @@ -195,13 +197,11 @@ public: bool remove(const T& a) // delete entry from list { - for ( int i = 0; i < num_entries; ++i ) + int pos = member_pos(a); + if ( pos != -1 ) { - if ( a == entries[i] ) - { - remove_nth(i); - return true; - } + remove_nth(pos); + return true; } return false; @@ -212,10 +212,22 @@ public: assert(n >=0 && n < num_entries); T old_ent = entries[n]; - --num_entries; - for ( ; n < num_entries; ++n ) - entries[n] = entries[n+1]; + // For data where we don't care about ordering, we don't care about keeping + // the list in the same order when removing an element. Just swap the last + // element with the element being removed. + if constexpr ( Order == ListOrder::ORDERED ) + { + --num_entries; + + for ( ; n < num_entries; ++n ) + entries[n] = entries[n+1]; + } + else + { + entries[n] = entries[num_entries - 1]; + --num_entries; + } return old_ent; } @@ -240,16 +252,16 @@ public: T replace(int ent_index, const T& new_ent) // replace entry #i with a new value { if ( ent_index < 0 ) - return 0; + return T{}; - T old_ent = nullptr; + T old_ent{}; if ( ent_index > num_entries - 1 ) { // replacement beyond the end of the list resize(ent_index + 1); for ( int i = num_entries; i < max_entries; ++i ) - entries[i] = nullptr; + entries[i] = T{}; num_entries = max_entries; } else @@ -318,8 +330,8 @@ protected: // Specialization of the List class to store pointers of a type. -template -using PList = List; +template +using PList = List; // Popular type of list: list of strings. typedef PList name_list; diff --git a/src/Notifier.h b/src/Notifier.h index a3ae5188b7..d6618a5e5f 100644 --- a/src/Notifier.h +++ b/src/Notifier.h @@ -8,6 +8,7 @@ #pragma once #include +#include namespace notifier { diff --git a/src/Reassem.h b/src/Reassem.h index 4166f4dccb..079de77584 100644 --- a/src/Reassem.h +++ b/src/Reassem.h @@ -9,6 +9,7 @@ #include #include #include // for u_char +#include // Whenever subclassing the Reassembler class // you should add to this for known subclasses. diff --git a/src/parse.y b/src/parse.y index 54761ca9df..928340f20b 100644 --- a/src/parse.y +++ b/src/parse.y @@ -155,13 +155,14 @@ static void parser_redef_enum (ID *id) /* Redef an enum. id points to the enum to be redefined. Let cur_enum_type point to it. */ assert(cur_enum_type == NULL); + // abort on errors; enums need to be accessible to continue parsing if ( ! id->Type() ) - id->Error("unknown identifier"); + reporter->FatalError("unknown enum identifier \"%s\"", id->Name()); else { + if ( ! id->Type() || id->Type()->Tag() != TYPE_ENUM ) + reporter->FatalError("identifier \"%s\" is not an enum", id->Name()); cur_enum_type = id->Type()->AsEnumType(); - if ( ! cur_enum_type ) - id->Error("not an enum"); } } diff --git a/src/zeek.bif b/src/zeek.bif index 3721185545..77d3a11838 100644 --- a/src/zeek.bif +++ b/src/zeek.bif @@ -3640,6 +3640,32 @@ extern "C" { #include } +static int mmdb_msg_count = 0; +static constexpr int mmdb_msg_limit = 20; +static double mmdb_msg_suppression_time = 0; +static constexpr double mmdb_msg_suppression_duration = 300; + +static void report_mmdb_msg(const char* format, ...) + { + if ( network_time > mmdb_msg_suppression_time + mmdb_msg_suppression_duration ) + { + mmdb_msg_count = 0; + mmdb_msg_suppression_time = network_time; + } + + if ( mmdb_msg_count >= mmdb_msg_limit ) + return; + + ++mmdb_msg_count; + + va_list al; + va_start(al, format); + std::string msg = fmt(format, al); + va_end(al); + + reporter->Info("%s", msg.data()); + } + class MMDB { public: MMDB(const char* filename, struct stat info); @@ -3712,8 +3738,8 @@ bool MMDB::StaleDB() if ( buf.st_ino != file_info.st_ino || buf.st_mtime != file_info.st_mtime ) { - reporter->Info("Inode change detected for MaxMind DB [%s]", - mmdb.filename); + report_mmdb_msg("Inode change detected for MaxMind DB [%s]", + mmdb.filename); return true; } @@ -3758,8 +3784,8 @@ static bool mmdb_open(const char* filename, bool asn) else did_mmdb_loc_db_error = false; - reporter->Info("Failed to open MaxMind DB: %s [%s]", filename, - e.what()); + report_mmdb_msg("Failed to open MaxMind DB: %s [%s]", filename, + e.what()); return false; } @@ -3780,7 +3806,7 @@ static void mmdb_check_loc() { if ( mmdb_loc && mmdb_loc->StaleDB() ) { - reporter->Info("Closing stale MaxMind DB [%s]", mmdb_loc->Filename()); + report_mmdb_msg("Closing stale MaxMind DB [%s]", mmdb_loc->Filename()); did_mmdb_loc_db_error = false; mmdb_loc.release(); } @@ -3790,7 +3816,7 @@ static void mmdb_check_asn() { if ( mmdb_asn && mmdb_asn->StaleDB() ) { - reporter->Info("Closing stale MaxMind DB [%s]", mmdb_asn->Filename()); + report_mmdb_msg("Closing stale MaxMind DB [%s]", mmdb_asn->Filename()); did_mmdb_asn_db_error = false; mmdb_asn.release(); } @@ -3823,8 +3849,7 @@ static bool mmdb_lookup(const IPAddr& addr, MMDB_lookup_result_s& result, catch ( const std::exception& e ) { - reporter->Info("MaxMind DB lookup location error [%s]", - e.what()); + report_mmdb_msg("MaxMind DB lookup location error [%s]", e.what()); return false; } @@ -3874,7 +3899,7 @@ static Val* mmdb_getvalue(MMDB_entry_data_s* entry_data, int status, break; default: - reporter->Info("MaxMind DB error [%s]", MMDB_strerror(status)); + report_mmdb_msg("MaxMind DB error [%s]", MMDB_strerror(status)); break; } diff --git a/testing/btest/Baseline/language.enum-nonexisting/output b/testing/btest/Baseline/language.enum-nonexisting/output new file mode 100644 index 0000000000..fa32be3d46 --- /dev/null +++ b/testing/btest/Baseline/language.enum-nonexisting/output @@ -0,0 +1 @@ +fatal error in /path/to/zeek/testing/infrastructure/.tmp/language.enum-nonexisting/enum-nonexisting.zeek, line 4: unknown enum identifier "notexisting" diff --git a/testing/btest/language/enum-nonexisting.zeek b/testing/btest/language/enum-nonexisting.zeek new file mode 100644 index 0000000000..5dda65e664 --- /dev/null +++ b/testing/btest/language/enum-nonexisting.zeek @@ -0,0 +1,6 @@ +# @TEST-EXEC-FAIL: zeek -b %INPUT >output 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER="$SCRIPTS/diff-remove-abspath" btest-diff output + +redef enum notexisting += { + This_Causes_a_Segfault +};