From ae9a02140e0acaf803f5606448b1dfdeb31a5310 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Fri, 17 May 2013 10:35:08 -0400 Subject: [PATCH 01/13] Fix the issue with transaction ID reuse in a single DNS connection. - Each transaction ID within a connection is now maintained as a queue of DNS::Info logging records. - New function added to the queue.bro script to support peeking at the new gettable item in the queue without removing it. --- scripts/base/protocols/dns/main.bro | 103 +++++++++++------- scripts/base/utils/queue.bro | 20 +++- testing/btest/Baseline/core.ipv6-frag/dns.log | 5 +- 3 files changed, 83 insertions(+), 45 deletions(-) diff --git a/scripts/base/protocols/dns/main.bro b/scripts/base/protocols/dns/main.bro index ee0e4166da..7d69d2f9ee 100644 --- a/scripts/base/protocols/dns/main.bro +++ b/scripts/base/protocols/dns/main.bro @@ -1,6 +1,7 @@ ##! Base DNS analysis script which tracks and logs DNS queries along with ##! their responses. +@load base/utils/queue @load ./consts module DNS; @@ -73,19 +74,6 @@ export { total_replies: count &optional; }; - ## A record type which tracks the status of DNS queries for a given - ## :bro:type:`connection`. - type State: record { - ## Indexed by query id, returns Info record corresponding to - ## query/response which haven't completed yet. - pending: table[count] of Info &optional; - - ## This is the list of DNS responses that have completed based on the - ## number of responses declared and the number received. The contents - ## of the set are transaction IDs. - finished_answers: set[count] &optional; - }; - ## An event that can be handled to access the :bro:type:`DNS::Info` ## record as it is sent to the logging framework. global log_dns: event(rec: Info); @@ -102,8 +90,32 @@ export { ## ## reply: The specific response information according to RR type/class. global do_reply: event(c: connection, msg: dns_msg, ans: dns_answer, reply: string); + + ## A hook that is called whenever a session is being set. + ## This can be used if additional initialization logic needs to happen + ## when creating a new session value. + ## + ## c: The connection involved in the new session + ## + ## msg: The DNS message header information. + ## + ## is_query: Indicator for if this is being called for a query or a response. + global set_session: hook(c: connection, msg: dns_msg, is_query: bool); } +## A record type which tracks the status of DNS queries for a given +## :bro:type:`connection`. +type State: record { + ## Indexed by query id, returns Info record corresponding to + ## query/response which haven't completed yet. + pending: table[count] of Queue::Queue; + + ## This is the list of DNS responses that have completed based on the + ## number of responses declared and the number received. The contents + ## of the set are transaction IDs. + finished_answers: set[count]; +}; + redef record connection += { dns: Info &optional; dns_state: State &optional; @@ -134,14 +146,6 @@ event bro_init() &priority=5 function new_session(c: connection, trans_id: count): Info { - if ( ! c?$dns_state ) - { - local state: State; - state$pending=table(); - state$finished_answers=set(); - c$dns_state = state; - } - local info: Info; info$ts = network_time(); info$id = c$id; @@ -151,18 +155,37 @@ function new_session(c: connection, trans_id: count): Info return info; } -function set_session(c: connection, msg: dns_msg, is_query: bool) +hook set_session(c: connection, msg: dns_msg, is_query: bool) &priority=5 { - if ( ! c?$dns_state || msg$id !in c$dns_state$pending ) + if ( ! c?$dns_state ) { - c$dns_state$pending[msg$id] = new_session(c, msg$id); - # Try deleting this transaction id from the set of finished answers. - # Sometimes hosts will reuse ports and transaction ids and this should - # be considered to be a legit scenario (although bad practice). - delete c$dns_state$finished_answers[msg$id]; + local state: State; + c$dns_state = state; } - c$dns = c$dns_state$pending[msg$id]; + if ( msg$id !in c$dns_state$pending ) + c$dns_state$pending[msg$id] = Queue::init(); + + local info: Info; + # If this is either a query or this is the reply but + # no Info records are in the queue (we missed the query?) + # we need to create an Info record and put it in the queue. + if ( is_query || + Queue::len(c$dns_state$pending[msg$id]) == 0 ) + { + info = new_session(c, msg$id); + Queue::put(c$dns_state$pending[msg$id], info); + } + + if ( is_query ) + # If this is a query, assign the newly created info variable + # so that the world looks correct to anything else handling + # this query. + c$dns = info; + else + # Peek at the next item in the queue for this trans_id and + # assign it to c$dns since this is a response. + c$dns = Queue::peek(c$dns_state$pending[msg$id]); if ( ! is_query ) { @@ -190,7 +213,7 @@ function set_session(c: connection, msg: dns_msg, is_query: bool) event dns_message(c: connection, is_orig: bool, msg: dns_msg, len: count) &priority=5 { - set_session(c, msg, is_orig); + hook set_session(c, msg, is_orig); } event DNS::do_reply(c: connection, msg: dns_msg, ans: dns_answer, reply: string) &priority=5 @@ -200,9 +223,6 @@ event DNS::do_reply(c: connection, msg: dns_msg, ans: dns_answer, reply: string) c$dns$AA = msg$AA; c$dns$RA = msg$RA; - if ( msg$id in c$dns_state$finished_answers ) - event conn_weird("dns_reply_seen_after_done", c, ""); - if ( reply != "" ) { if ( ! c$dns?$answers ) @@ -217,7 +237,6 @@ event DNS::do_reply(c: connection, msg: dns_msg, ans: dns_answer, reply: string) if ( c$dns?$answers && c$dns?$total_answers && |c$dns$answers| == c$dns$total_answers ) { - add c$dns_state$finished_answers[c$dns$trans_id]; # Indicate this request/reply pair is ready to be logged. c$dns$ready = T; } @@ -230,7 +249,7 @@ event DNS::do_reply(c: connection, msg: dns_msg, ans: dns_answer, reply: string) { Log::write(DNS::LOG, c$dns); # This record is logged and no longer pending. - delete c$dns_state$pending[c$dns$trans_id]; + Queue::get(c$dns_state$pending[c$dns$trans_id]); delete c$dns; } } @@ -243,15 +262,14 @@ event dns_request(c: connection, msg: dns_msg, query: string, qtype: count, qcla c$dns$qclass_name = classes[qclass]; c$dns$qtype = qtype; c$dns$qtype_name = query_types[qtype]; + c$dns$Z = msg$Z; # Decode netbios name queries # Note: I'm ignoring the name type for now. Not sure if this should be # worked into the query/response in some fashion. if ( c$id$resp_p == 137/udp ) query = decode_netbios_name(query); - c$dns$query = query; - - c$dns$Z = msg$Z; + c$dns$query = query; } event dns_A_reply(c: connection, msg: dns_msg, ans: dns_answer, a: addr) &priority=5 @@ -339,6 +357,13 @@ event connection_state_remove(c: connection) &priority=-5 # If Bro is expiring state, we should go ahead and log all unlogged # request/response pairs now. for ( trans_id in c$dns_state$pending ) - Log::write(DNS::LOG, c$dns_state$pending[trans_id]); + { + local infos: vector of Info; + Queue::get_vector(c$dns_state$pending[trans_id], infos); + for ( i in infos ) + { + Log::write(DNS::LOG, infos[i]); + } + } } diff --git a/scripts/base/utils/queue.bro b/scripts/base/utils/queue.bro index 11e85f229d..eb4f69a08e 100644 --- a/scripts/base/utils/queue.bro +++ b/scripts/base/utils/queue.bro @@ -19,22 +19,29 @@ export { ## s: A :bro:record:`Settings` record configuring the queue. ## ## Returns: An opaque queue record. - global init: function(s: Settings): Queue; + global init: function(s: Settings &default=[]): Queue; - ## Put a string onto the beginning of a queue. + ## Put a value onto the beginning of a queue. ## ## q: The queue to put the value into. ## ## val: The value to insert into the queue. global put: function(q: Queue, val: any); - ## Get a string from the end of a queue. + ## Get a value from the end of a queue. ## - ## q: The queue to get the string from. + ## q: The queue to get the value from. ## ## Returns: The value gotten from the queue. global get: function(q: Queue): any; + ## Peek at the value at the end of the queue without removing it. + ## + ## q: The queue to get the value from. + ## + ## Returns: The value at the end of the queue. + global peek: function(q: Queue): any; + ## Merge two queue's together. If any settings are applied ## to the queues, the settings from q1 are used for the new ## merged queue. @@ -103,6 +110,11 @@ function get(q: Queue): any return ret; } +function peek(q: Queue): any + { + return q$vals[q$bottom]; + } + function merge(q1: Queue, q2: Queue): Queue { local ret = init(q1$settings); diff --git a/testing/btest/Baseline/core.ipv6-frag/dns.log b/testing/btest/Baseline/core.ipv6-frag/dns.log index de027644e8..97fb552c0d 100644 --- a/testing/btest/Baseline/core.ipv6-frag/dns.log +++ b/testing/btest/Baseline/core.ipv6-frag/dns.log @@ -3,9 +3,10 @@ #empty_field (empty) #unset_field - #path dns -#open 2012-10-05-17-47-27 +#open 2013-05-17-14-28-17 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto trans_id query qclass qclass_name qtype qtype_name rcode rcode_name AA TC RD RA Z answers TTLs rejected #types time string addr port addr port enum count string count string count string count string bool bool bool bool count vector[string] vector[interval] bool 1331084278.438444 UWkUyAuUGXf 2001:470:1f11:81f:d138:5f55:6d4:1fe2 51850 2607:f740:b::f93 53 udp 3903 txtpadding_323.n1.netalyzr.icsi.berkeley.edu 1 C_INTERNET 16 TXT 0 NOERROR T F T F 0 This TXT record should be ignored 1.000000 F 1331084293.592245 arKYeMETxOg 2001:470:1f11:81f:d138:5f55:6d4:1fe2 51851 2607:f740:b::f93 53 udp 40849 txtpadding_3230.n1.netalyzr.icsi.berkeley.edu 1 C_INTERNET 16 TXT 0 NOERROR T F T F 0 This TXT record should be ignored 1.000000 F -#close 2012-10-05-17-47-27 +1331084298.593081 arKYeMETxOg 2001:470:1f11:81f:d138:5f55:6d4:1fe2 51851 2607:f740:b::f93 53 udp 40849 txtpadding_3230.n1.netalyzr.icsi.berkeley.edu 1 C_INTERNET 16 TXT - - F F T F 0 - - F +#close 2013-05-17-14-28-17 From 85e3eb4c579f6890049f13e6b16663ff415c3d4b Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Fri, 17 May 2013 07:44:01 -0700 Subject: [PATCH 02/13] Fixing Broxygen generation. Needs to have BROMAGIC set to find the magic database. --- doc/scripts/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/scripts/CMakeLists.txt b/doc/scripts/CMakeLists.txt index 33d473b005..315f751cd1 100644 --- a/doc/scripts/CMakeLists.txt +++ b/doc/scripts/CMakeLists.txt @@ -107,7 +107,7 @@ macro(REST_TARGET srcDir broInput) COMMAND "${CMAKE_COMMAND}" ARGS -E remove_directory .state # generate the reST documentation using bro - COMMAND BROPATH=${BROPATH}:${srcDir} ${CMAKE_BINARY_DIR}/src/bro + COMMAND BROPATH=${BROPATH}:${srcDir} BROMAGIC=${CMAKE_SOURCE_DIR}/magic ${CMAKE_BINARY_DIR}/src/bro ARGS -b -Z ${broInput} || (rm -rf .state *.log *.rst && exit 1) # move generated doc into a new directory tree that # defines the final structure of documents From 31f94b8f371a768fb2f449c6efbc9b8af1b577b7 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Fri, 17 May 2013 07:47:14 -0700 Subject: [PATCH 03/13] Updating submodule(s). [nomail] --- CHANGES | 9 +++++++++ VERSION | 2 +- aux/bro-aux | 2 +- aux/broctl | 2 +- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 36c1ee63cb..66a10b0fc7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,13 @@ +2.1-647 | 2013-05-17 07:47:14 -0700 + + * Fixing Broxygen generation to have BROMAGIC set. (Robin Sommer) + + * Fix for 'fchmod undeclared here' on FreeBSD. (Robin Sommer) + + * CMake policy fix to avoid errors with older versions. (Robin + Sommer) + 2.1-641 | 2013-05-15 18:15:09 -0700 * Test update. (Robin Sommer) diff --git a/VERSION b/VERSION index 66bc7b0865..a5b5bd372c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.1-641 +2.1-647 diff --git a/aux/bro-aux b/aux/bro-aux index 18c454981e..cfaf4eea78 160000 --- a/aux/bro-aux +++ b/aux/bro-aux @@ -1 +1 @@ -Subproject commit 18c454981e3b0903811c541f6e728d4ef6cee2c5 +Subproject commit cfaf4eea788bdac4ebfe9e46e3de2cd74b0bc068 diff --git a/aux/broctl b/aux/broctl index 3ea30f7a14..ee213040f0 160000 --- a/aux/broctl +++ b/aux/broctl @@ -1 +1 @@ -Subproject commit 3ea30f7a146343f054a3846a61ee5c67259b2de2 +Subproject commit ee213040f0c0c632bef9775f06615d53015a629f From 945aa8a5508156c260783ec74440d2bb9f3dedd4 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Fri, 17 May 2013 14:09:24 -0500 Subject: [PATCH 04/13] Fix uninitialized DPM member. Was seeing crashes due to this primarily on Ubuntu 12.04 when generating reST docs. --- src/DPM.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DPM.cc b/src/DPM.cc index d7e5cd25ef..fdba458dd7 100644 --- a/src/DPM.cc +++ b/src/DPM.cc @@ -34,7 +34,7 @@ ExpectedConn::ExpectedConn(const ExpectedConn& c) } DPM::DPM() -: expected_conns_queue(AssignedAnalyzer::compare) +: active_analyzers(0), expected_conns_queue(AssignedAnalyzer::compare) { } From bd02da8a0c7e9b3d05d0b9d6d3889a5ac43fb785 Mon Sep 17 00:00:00 2001 From: Bernhard Amann Date: Fri, 17 May 2013 13:38:26 -0700 Subject: [PATCH 05/13] change sqlite3 default threading mode to no-mutex, disable memory statistics, finalize prepared statement before exitting logger. This might fix the deadlock issue, at least it did not happen for me on my tried on the test system where it happened quite regularly before. --- src/3rdparty/sqlite3.c | 3 +++ src/logging/writers/SQLite.cc | 5 ++++- src/threading/Queue.h | 5 +++-- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/3rdparty/sqlite3.c b/src/3rdparty/sqlite3.c index 77f14da90d..ba6a30e132 100644 --- a/src/3rdparty/sqlite3.c +++ b/src/3rdparty/sqlite3.c @@ -1,3 +1,6 @@ +# define SQLITE_THREADSAFE 2 +# define SQLITE_DEFAULT_MEMSTATUS 0 + /****************************************************************************** ** This file is an amalgamation of many separate C source files from SQLite ** version 3.7.16.2. By combining all the individual C code files into this diff --git a/src/logging/writers/SQLite.cc b/src/logging/writers/SQLite.cc index 22037f029e..c395f02b86 100644 --- a/src/logging/writers/SQLite.cc +++ b/src/logging/writers/SQLite.cc @@ -35,13 +35,16 @@ SQLite::SQLite(WriterFrontend* frontend) : WriterBackend(frontend) db = 0; io = new AsciiFormatter(this, AsciiFormatter::SeparatorInfo(set_separator, unset_field, empty_field)); + st = 0; } SQLite::~SQLite() { if ( db != 0 ) { - sqlite3_close(db); + sqlite3_finalize(st); + if ( !sqlite3_close(db) ) + Error("Sqlite could not close connection"); db = 0; } diff --git a/src/threading/Queue.h b/src/threading/Queue.h index 5988c94042..792fb63f9c 100644 --- a/src/threading/Queue.h +++ b/src/threading/Queue.h @@ -113,8 +113,9 @@ private: inline static void safe_lock(pthread_mutex_t* mutex) { - if ( pthread_mutex_lock(mutex) != 0 ) - reporter->FatalErrorWithCore("cannot lock mutex"); + int res = pthread_mutex_lock(mutex); + if ( res != 0 ) + reporter->FatalErrorWithCore("cannot lock mutex: %d(%s)", res, strerror(res)); } inline static void safe_unlock(pthread_mutex_t* mutex) From 14abcc52fa7b69c598f560444db5d45caee48233 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Fri, 17 May 2013 13:38:29 -0700 Subject: [PATCH 06/13] Updating submodule(s). [nomail] --- aux/broctl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aux/broctl b/aux/broctl index ee213040f0..c25a64b173 160000 --- a/aux/broctl +++ b/aux/broctl @@ -1 +1 @@ -Subproject commit ee213040f0c0c632bef9775f06615d53015a629f +Subproject commit c25a64b173652e934bcd7b88e8573b306bf59ac5 From 65b56479d2c0ee89231f2b3e9c21533f410bdddb Mon Sep 17 00:00:00 2001 From: Bernhard Amann Date: Fri, 17 May 2013 14:08:43 -0700 Subject: [PATCH 07/13] (hopefully) fix mutex lock problem. log writers were removed on shutdown while frontends still had pointers to it. A similar fix will be necessary for the input framework (tomorrow :) ) --- src/logging/Manager.cc | 10 ++-------- src/logging/Manager.h | 6 ------ 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/logging/Manager.cc b/src/logging/Manager.cc index 37cc90cd78..61e15a334f 100644 --- a/src/logging/Manager.cc +++ b/src/logging/Manager.cc @@ -1271,9 +1271,8 @@ bool Manager::Flush(EnumVal* id) return true; } -void Manager::FlushBuffers() +void Manager::Terminate() { - // Flush out cached entries in Frontend for ( vector::iterator s = streams.begin(); s != streams.end(); ++s ) { if ( ! *s ) @@ -1281,15 +1280,10 @@ void Manager::FlushBuffers() for ( Stream::WriterMap::iterator i = (*s)->writers.begin(); i != (*s)->writers.end(); i++ ) - i->second->writer->FlushWriteBuffer(); + i->second->writer->Stop(); } } -void Manager::Terminate() - { - FlushBuffers(); - } - // Timer which on dispatching rotates the filter. class RotationTimer : public Timer { public: diff --git a/src/logging/Manager.h b/src/logging/Manager.h index 5ee4318f65..61f6dcd8a7 100644 --- a/src/logging/Manager.h +++ b/src/logging/Manager.h @@ -149,12 +149,6 @@ public: */ bool Flush(EnumVal* id); - /** - * Flushes all buffers that are currently held by writer frontends - * out to the threads. Does not call the thread flush operation. - */ - void FlushBuffers(); - /** * Signals the manager to shutdown at Bro's termination. */ From e46300a724d07353fa2ddb18f50b6b43ab296ffa Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Sat, 18 May 2013 16:47:04 -0700 Subject: [PATCH 08/13] Fixing test that would fail without ES/curl support. It used to special-case an error message produced in the case that ES isn't available, however with scripts/test-all-policy.bro now explicitly disabling ES output, that doesn't seem necessary anymore. --- .../coverage.bare-mode-errors/unique_errors_no_elasticsearch | 1 - testing/btest/coverage/bare-mode-errors.test | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) delete mode 100644 testing/btest/Baseline/coverage.bare-mode-errors/unique_errors_no_elasticsearch diff --git a/testing/btest/Baseline/coverage.bare-mode-errors/unique_errors_no_elasticsearch b/testing/btest/Baseline/coverage.bare-mode-errors/unique_errors_no_elasticsearch deleted file mode 100644 index e95f88e74b..0000000000 --- a/testing/btest/Baseline/coverage.bare-mode-errors/unique_errors_no_elasticsearch +++ /dev/null @@ -1 +0,0 @@ -error: unknown writer type requested diff --git a/testing/btest/coverage/bare-mode-errors.test b/testing/btest/coverage/bare-mode-errors.test index da968d5601..34ba063081 100644 --- a/testing/btest/coverage/bare-mode-errors.test +++ b/testing/btest/coverage/bare-mode-errors.test @@ -11,5 +11,4 @@ # @TEST-EXEC: test -d $DIST/scripts # @TEST-EXEC: for script in `find $DIST/scripts/ -name \*\.bro -not -path '*/site/*'`; do echo "=== $script" >>allerrors; if echo "$script" | egrep -q 'communication/listen|controllee'; then rm -rf load_attempt .bgprocs; btest-bg-run load_attempt bro -b $script; btest-bg-wait -k 2; cat load_attempt/.stderr >>allerrors; else bro -b $script 2>>allerrors; fi done || exit 0 # @TEST-EXEC: cat allerrors | grep -v "received termination signal" | grep -v '===' | sort | uniq > unique_errors -# @TEST-EXEC: if [ $(grep -c LibCURL_INCLUDE_DIR-NOTFOUND $BUILD/CMakeCache.txt) -ne 0 ]; then cp unique_errors unique_errors_no_elasticsearch; fi -# @TEST-EXEC: if [ $(grep -c LibCURL_INCLUDE_DIR-NOTFOUND $BUILD/CMakeCache.txt) -ne 0 ]; then btest-diff unique_errors_no_elasticsearch; else btest-diff unique_errors; fi +# @TEST-EXEC: btest-diff unique_errors From e45933562e15a1af1ecd50bf0e8c0a4de02b4c3f Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Thu, 23 May 2013 16:53:42 -0500 Subject: [PATCH 09/13] Fix broken/missing documentation. --- doc/logging.rst | 3 +-- doc/notice.rst | 26 +++++++++---------- doc/scripts/builtins.rst | 25 ++++++++++++++++++ scripts/base/frameworks/logging/main.bro | 2 +- scripts/base/frameworks/notice/main.bro | 3 --- scripts/base/frameworks/sumstats/main.bro | 2 +- .../base/frameworks/sumstats/plugins/last.bro | 3 ++- scripts/base/protocols/dns/main.bro | 25 +++++++++--------- scripts/base/utils/queue.bro | 2 +- .../policy/misc/detect-traceroute/main.bro | 4 +-- scripts/policy/misc/scan.bro | 13 +++++----- src/bro.bif | 8 +++--- src/event.bif | 1 + 13 files changed, 71 insertions(+), 46 deletions(-) diff --git a/doc/logging.rst b/doc/logging.rst index 7fb4205b9a..b982206e85 100644 --- a/doc/logging.rst +++ b/doc/logging.rst @@ -89,8 +89,7 @@ Note the fields that are set for the filter: are generated by taking the stream's ID and munging it slightly. :bro:enum:`Conn::LOG` is converted into ``conn``, :bro:enum:`PacketFilter::LOG` is converted into - ``packet_filter``, and :bro:enum:`Notice::POLICY_LOG` is - converted into ``notice_policy``. + ``packet_filter``. ``include`` A set limiting the fields to the ones given. The names diff --git a/doc/notice.rst b/doc/notice.rst index e6d4326db1..76d5bcdecb 100644 --- a/doc/notice.rst +++ b/doc/notice.rst @@ -86,21 +86,21 @@ directly make modifications to the :bro:see:`Notice::Info` record given as the argument to the hook. Here's a simple example which tells Bro to send an email for all notices of -type :bro:see:`SSH::Login` if the server is 10.0.0.1: +type :bro:see:`SSH::Password_Guessing` if the server is 10.0.0.1: .. code:: bro hook Notice::policy(n: Notice::Info) { - if ( n$note == SSH::Login && n$id$resp_h == 10.0.0.1 ) + if ( n$note == SSH::Password_Guessing && n$id$resp_h == 10.0.0.1 ) add n$actions[Notice::ACTION_EMAIL]; } .. note:: - Keep in mind that the semantics of the SSH::Login notice are - such that it is only raised when Bro heuristically detects a successful - login. No apparently failed logins will raise this notice. + Keep in mind that the semantics of the SSH::Password_Guessing notice are + such that it is only raised when Bro heuristically detects a failed + login. Hooks can also have priorities applied to order their execution like events with a default priority of 0. Greater values are executed first. Setting @@ -110,7 +110,7 @@ a hook body to run before default hook bodies might look like this: hook Notice::policy(n: Notice::Info) &priority=5 { - if ( n$note == SSH::Login && n$id$resp_h == 10.0.0.1 ) + if ( n$note == SSH::Password_Guessing && n$id$resp_h == 10.0.0.1 ) add n$actions[Notice::ACTION_EMAIL]; } @@ -173,16 +173,16 @@ Raising Notices A script should raise a notice for any occurrence that a user may want to be notified about or take action on. For example, whenever the base -SSH analysis scripts sees an SSH session where it is heuristically -guessed to be a successful login, it raises a Notice of the type -:bro:see:`SSH::Login`. The code in the base SSH analysis script looks -like this: +SSH analysis scripts sees enough failed logins to a given host, it +raises a notice of the type :bro:see:`SSH::Password_Guessing`. The code +in the base SSH analysis script which raises the notice looks like this: .. code:: bro - NOTICE([$note=SSH::Login, - $msg="Heuristically detected successful SSH login.", - $conn=c]); + NOTICE([$note=Password_Guessing, + $msg=fmt("%s appears to be guessing SSH passwords (seen in %d connections).", key$host, r$num), + $src=key$host, + $identifier=cat(key$host)]); :bro:see:`NOTICE` is a normal function in the global namespace which wraps a function within the ``Notice`` namespace. It takes a single diff --git a/doc/scripts/builtins.rst b/doc/scripts/builtins.rst index 06d61232ad..369f38c9eb 100644 --- a/doc/scripts/builtins.rst +++ b/doc/scripts/builtins.rst @@ -402,6 +402,31 @@ The Bro scripting language supports the following built-in types. if ( r?$s ) ... +.. bro:type:: opaque + + A data type whose actual representation/implementation is + intentionally hidden, but whose values may be passed to certain + functions that can actually access the internal/hidden resources. + Opaque types are differentiated from each other by qualifying them + like ``opaque of md5`` or ``opaque of sha1``. Any valid identifier + can be used as the type qualifier. + + An example use of this type is the set of built-in functions which + perform hashing: + + .. code:: bro + + local handle: opaque of md5 = md5_hash_init(); + md5_hash_update(handle, "test"); + md5_hash_update(handle, "testing"); + print md5_hash_finish(handle); + + Here the opaque type is used to provide a handle to a particular + resource which is calculating an MD5 checksum incrementally over + time, but the details of that resource aren't relevant, it's only + necessary to have a handle as a way of identifying it and + distinguishing it from other such resources. + .. bro:type:: file Bro supports writing to files, but not reading from them. For diff --git a/scripts/base/frameworks/logging/main.bro b/scripts/base/frameworks/logging/main.bro index 82d3fa043b..b1d76cfb62 100644 --- a/scripts/base/frameworks/logging/main.bro +++ b/scripts/base/frameworks/logging/main.bro @@ -195,7 +195,7 @@ export { ## ## Returns: True if a new stream was successfully removed. ## - ## .. bro:see:: Log:create_stream + ## .. bro:see:: Log::create_stream global remove_stream: function(id: ID) : bool; ## Enables a previously disabled logging stream. Disabled streams diff --git a/scripts/base/frameworks/notice/main.bro b/scripts/base/frameworks/notice/main.bro index 71071df9ab..30e0013517 100644 --- a/scripts/base/frameworks/notice/main.bro +++ b/scripts/base/frameworks/notice/main.bro @@ -431,9 +431,6 @@ hook Notice::notice(n: Notice::Info) &priority=-5 } } -## This determines if a notice is being suppressed. It is only used -## internally as part of the mechanics for the global :bro:id:`NOTICE` -## function. function is_being_suppressed(n: Notice::Info): bool { if ( n?$identifier && [n$note, n$identifier] in suppressing ) diff --git a/scripts/base/frameworks/sumstats/main.bro b/scripts/base/frameworks/sumstats/main.bro index 6864966766..cc2aba2362 100644 --- a/scripts/base/frameworks/sumstats/main.bro +++ b/scripts/base/frameworks/sumstats/main.bro @@ -99,7 +99,7 @@ export { reducers: set[Reducer]; ## Provide a function to calculate a value from the - ## :bro:see:`Result` structure which will be used + ## :bro:see:`SumStats::Result` structure which will be used ## for thresholding. ## This is required if a $threshold value is given. threshold_val: function(key: SumStats::Key, result: SumStats::Result): count &optional; diff --git a/scripts/base/frameworks/sumstats/plugins/last.bro b/scripts/base/frameworks/sumstats/plugins/last.bro index e2cf31c902..daebe30cf5 100644 --- a/scripts/base/frameworks/sumstats/plugins/last.bro +++ b/scripts/base/frameworks/sumstats/plugins/last.bro @@ -16,7 +16,8 @@ export { redef record ResultVal += { ## This is the queue where elements are maintained. Use the - ## :bro:see:`SumStats::get_elements` function to get a vector of the current element values. + ## :bro:see:`SumStats::get_last` function to get a vector of + ## the current element values. last_elements: Queue::Queue &optional; }; diff --git a/scripts/base/protocols/dns/main.bro b/scripts/base/protocols/dns/main.bro index 7d69d2f9ee..fd524b49cf 100644 --- a/scripts/base/protocols/dns/main.bro +++ b/scripts/base/protocols/dns/main.bro @@ -101,20 +101,21 @@ export { ## ## is_query: Indicator for if this is being called for a query or a response. global set_session: hook(c: connection, msg: dns_msg, is_query: bool); + + ## A record type which tracks the status of DNS queries for a given + ## :bro:type:`connection`. + type State: record { + ## Indexed by query id, returns Info record corresponding to + ## query/response which haven't completed yet. + pending: table[count] of Queue::Queue; + + ## This is the list of DNS responses that have completed based on the + ## number of responses declared and the number received. The contents + ## of the set are transaction IDs. + finished_answers: set[count]; + }; } -## A record type which tracks the status of DNS queries for a given -## :bro:type:`connection`. -type State: record { - ## Indexed by query id, returns Info record corresponding to - ## query/response which haven't completed yet. - pending: table[count] of Queue::Queue; - - ## This is the list of DNS responses that have completed based on the - ## number of responses declared and the number received. The contents - ## of the set are transaction IDs. - finished_answers: set[count]; -}; redef record connection += { dns: Info &optional; diff --git a/scripts/base/utils/queue.bro b/scripts/base/utils/queue.bro index eb4f69a08e..64202c54bc 100644 --- a/scripts/base/utils/queue.bro +++ b/scripts/base/utils/queue.bro @@ -16,7 +16,7 @@ export { ## Initialize a queue record structure. ## - ## s: A :bro:record:`Settings` record configuring the queue. + ## s: A record which configures the queue. ## ## Returns: An opaque queue record. global init: function(s: Settings &default=[]): Queue; diff --git a/scripts/policy/misc/detect-traceroute/main.bro b/scripts/policy/misc/detect-traceroute/main.bro index c194d03e13..3ed315746f 100644 --- a/scripts/policy/misc/detect-traceroute/main.bro +++ b/scripts/policy/misc/detect-traceroute/main.bro @@ -32,8 +32,8 @@ export { const icmp_time_exceeded_threshold = 3 &redef; ## Interval at which to watch for the - ## :bro:id:`ICMPTimeExceeded::icmp_time_exceeded_threshold` variable to be crossed. - ## At the end of each interval the counter is reset. + ## :bro:id:`Traceroute::icmp_time_exceeded_threshold` variable to be + ## crossed. At the end of each interval the counter is reset. const icmp_time_exceeded_interval = 3min &redef; ## The log record for the traceroute log. diff --git a/scripts/policy/misc/scan.bro b/scripts/policy/misc/scan.bro index f3dcaf2291..31caf527b7 100644 --- a/scripts/policy/misc/scan.bro +++ b/scripts/policy/misc/scan.bro @@ -13,17 +13,18 @@ module Scan; export { redef enum Notice::Type += { - ## Address scans detect that a host appears to be scanning some number of - ## destinations on a single port. This notice is generated when more than - ## :bro:id:`addr_scan_threshold` unique hosts are seen over the previous - ## :bro:id:`addr_scan_interval` time range. + ## Address scans detect that a host appears to be scanning some number + ## of destinations on a single port. This notice is generated when more + ## than :bro:id:`Scan::addr_scan_threshold` unique hosts are seen over + ## the previous :bro:id:`Scan::addr_scan_interval` time range. Address_Scan, ## Port scans detect that an attacking host appears to be scanning a ## single victim host on several ports. This notice is generated when - ## an attacking host attempts to connect to :bro:id:`port_scan_threshold` + ## an attacking host attempts to connect to + ## :bro:id:`Scan::port_scan_threshold` ## unique ports on a single host over the previous - ## :bro:id:`port_scan_interval` time range. + ## :bro:id:`Scan::port_scan_interval` time range. Port_Scan, }; diff --git a/src/bro.bif b/src/bro.bif index d9558106a7..26fe16d821 100644 --- a/src/bro.bif +++ b/src/bro.bif @@ -2923,7 +2923,7 @@ function bytestring_to_hexstr%(bytestring: string%): string ## ## Returns: The encoded version of *s*. ## -## .. bro:see:: encode_base64_custom, decode_base64 +## .. bro:see:: encode_base64_custom decode_base64 function encode_base64%(s: string%): string %{ BroString* t = encode_base64(s->AsString()); @@ -2946,7 +2946,7 @@ function encode_base64%(s: string%): string ## ## Returns: The encoded version of *s*. ## -## .. bro:see:: encode_base64, decode_base64_custom +## .. bro:see:: encode_base64 decode_base64_custom function encode_base64_custom%(s: string, a: string%): string %{ BroString* t = encode_base64(s->AsString(), a->AsString()); @@ -2965,7 +2965,7 @@ function encode_base64_custom%(s: string, a: string%): string ## ## Returns: The decoded version of *s*. ## -## .. bro:see:: decode_base64_custom, encode_base64 +## .. bro:see:: decode_base64_custom encode_base64 function decode_base64%(s: string%): string %{ BroString* t = decode_base64(s->AsString()); @@ -2988,7 +2988,7 @@ function decode_base64%(s: string%): string ## ## Returns: The decoded version of *s*. ## -## .. bro:see:: decode_base64, encode_base64_custom +## .. bro:see:: decode_base64 encode_base64_custom function decode_base64_custom%(s: string, a: string%): string %{ BroString* t = decode_base64(s->AsString(), a->AsString()); diff --git a/src/event.bif b/src/event.bif index 0fcbd1cb5d..2263412699 100644 --- a/src/event.bif +++ b/src/event.bif @@ -7042,6 +7042,7 @@ event file_gap%(f: fa_file, offset: count, len: count%); ## This event is generated each time file analysis is ending for a given file. ## ## f: The file. +## ## .. bro:see:: file_new file_over_new_connection file_timeout file_gap event file_state_remove%(f: fa_file%); From d67123d0c300c6207a83f46930c8a78bde2f3b02 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Fri, 24 May 2013 18:07:36 -0700 Subject: [PATCH 10/13] Updating submodule(s). [nomail] --- aux/broctl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aux/broctl b/aux/broctl index c25a64b173..4d0b75afad 160000 --- a/aux/broctl +++ b/aux/broctl @@ -1 +1 @@ -Subproject commit c25a64b173652e934bcd7b88e8573b306bf59ac5 +Subproject commit 4d0b75afadd6a3c6507e8ca18cb1913faa93a3b0 From 04dd363279283c56d205f0979aaa09aa70409391 Mon Sep 17 00:00:00 2001 From: Bernhard Amann Date: Mon, 27 May 2013 20:30:03 -0700 Subject: [PATCH 11/13] accept libmagic starting from 5.03 --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b95b637770..284cd0dfa2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -74,7 +74,7 @@ if (MISSING_PREREQS) message(FATAL_ERROR "Configuration aborted due to missing prerequisites") endif () -set(libmagic_req 5.04) +set(libmagic_req 5.03) if ( LibMagic_VERSION VERSION_LESS ${libmagic_req} ) message(FATAL_ERROR "libmagic of at least version ${libmagic_req} required " "(found ${LibMagic_VERSION})") From bcc81a1a143c2715240e9822c0c52760bc9b5006 Mon Sep 17 00:00:00 2001 From: Bernhard Amann Date: Mon, 27 May 2013 21:10:51 -0700 Subject: [PATCH 12/13] Sorry, that libmagic version actually might have some problems - at least on the linux distribution I have access to. So... it was a bad idea. Revert "accept libmagic starting from 5.03" This reverts commit 04dd363279283c56d205f0979aaa09aa70409391. --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 284cd0dfa2..b95b637770 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -74,7 +74,7 @@ if (MISSING_PREREQS) message(FATAL_ERROR "Configuration aborted due to missing prerequisites") endif () -set(libmagic_req 5.03) +set(libmagic_req 5.04) if ( LibMagic_VERSION VERSION_LESS ${libmagic_req} ) message(FATAL_ERROR "libmagic of at least version ${libmagic_req} required " "(found ${LibMagic_VERSION})") From 22a4113ac3e0a9c977fd51f429c385ba0f2ea1a2 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 28 May 2013 16:21:29 -0500 Subject: [PATCH 13/13] Dangling pointer fix. Addresses #1004. --- src/Sessions.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Sessions.cc b/src/Sessions.cc index 2e5a6ded30..00f3bd539c 100644 --- a/src/Sessions.cc +++ b/src/Sessions.cc @@ -1159,12 +1159,12 @@ Connection* NetSessions::NewConn(HashKey* k, double t, const ConnID* id, if ( ! WantConnection(src_h, dst_h, tproto, flags, flip) ) return 0; + ConnID flip_id = *id; + if ( flip ) { // Make a guess that we're seeing the tail half of // an analyzable connection. - ConnID flip_id = *id; - const IPAddr ta = flip_id.src_addr; flip_id.src_addr = flip_id.dst_addr; flip_id.dst_addr = ta;