From a8404f47c8f975e604864c7a14a0eeebdd304ed8 Mon Sep 17 00:00:00 2001 From: Daniel Thayer Date: Thu, 9 Jun 2016 14:03:05 -0500 Subject: [PATCH 1/7] Update the configure script usage message for --with-caf --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 8859a6fa9b..1df97334c9 100755 --- a/configure +++ b/configure @@ -57,7 +57,7 @@ Usage: $0 [OPTION]... [VAR=VALUE]... --with-flex=PATH path to flex executable --with-bison=PATH path to bison executable --with-python=PATH path to Python executable - --with-libcaf=PATH path to C++ Actor Framework installation + --with-caf=PATH path to C++ Actor Framework installation (a required Broker dependency) Optional Packages in Non-Standard Locations: From 8a87055fccbbdfe8630d02d30932ab6f212d2f7c Mon Sep 17 00:00:00 2001 From: Jan Grashoefer Date: Mon, 13 Jun 2016 21:01:46 +0200 Subject: [PATCH 2/7] Fixed table expiration evaluation. The expiration attribute expression is now evaluated for every use. Thus later adjustments of the value (e.g. by redefining a const) will now take effect. Values less than 0 will disable expiration. --- src/Val.cc | 77 ++++++++++++------- src/Val.h | 9 ++- .../Baseline/language.expire-redef/output | 5 ++ testing/btest/language/expire-redef.bro | 38 +++++++++ 4 files changed, 98 insertions(+), 31 deletions(-) create mode 100644 testing/btest/Baseline/language.expire-redef/output create mode 100644 testing/btest/language/expire-redef.bro diff --git a/src/Val.cc b/src/Val.cc index 331aff3bcf..19fa56cd37 100644 --- a/src/Val.cc +++ b/src/Val.cc @@ -1323,7 +1323,7 @@ void TableVal::Init(TableType* t) { ::Ref(t); table_type = t; - expire_expr = 0; + expire_func = 0; expire_time = 0; expire_cookie = 0; timer = 0; @@ -1350,7 +1350,8 @@ TableVal::~TableVal() delete subnets; Unref(attrs); Unref(def_val); - Unref(expire_expr); + Unref(expire_func); + Unref(expire_time); } void TableVal::RemoveAll() @@ -1399,8 +1400,8 @@ void TableVal::SetAttrs(Attributes* a) Attr* ef = attrs->FindAttr(ATTR_EXPIRE_FUNC); if ( ef ) { - expire_expr = ef->AttrExpr(); - expire_expr->Ref(); + expire_func = ef->AttrExpr(); + expire_func->Ref(); } } @@ -1410,14 +1411,8 @@ void TableVal::CheckExpireAttr(attr_tag at) if ( a ) { - Val* timeout = a->AttrExpr()->Eval(0); - if ( ! timeout ) - { - a->AttrExpr()->Error("value of timeout not fixed"); - return; - } - - expire_time = timeout->AsInterval(); + expire_time = a->AttrExpr(); + expire_time->Ref(); if ( timer ) timer_mgr->Cancel(timer); @@ -1791,7 +1786,7 @@ Val* TableVal::Lookup(Val* index, bool use_default_val) if ( attrs && attrs->FindAttr(ATTR_EXPIRE_READ) ) { v->SetExpireAccess(network_time); - if ( LoggingAccess() && expire_time ) + if ( LoggingAccess() && ExpirationEnabled() ) ReadOperation(index, v); } @@ -1822,7 +1817,7 @@ Val* TableVal::Lookup(Val* index, bool use_default_val) if ( attrs && attrs->FindAttr(ATTR_EXPIRE_READ) ) { v->SetExpireAccess(network_time); - if ( LoggingAccess() && expire_time ) + if ( LoggingAccess() && ExpirationEnabled() ) ReadOperation(index, v); } @@ -1880,7 +1875,7 @@ TableVal* TableVal::LookupSubnetValues(const SubNetVal* search) if ( attrs && attrs->FindAttr(ATTR_EXPIRE_READ) ) { entry->SetExpireAccess(network_time); - if ( LoggingAccess() && expire_time ) + if ( LoggingAccess() && ExpirationEnabled() ) ReadOperation(s, entry); } } @@ -2174,6 +2169,10 @@ void TableVal::DoExpire(double t) if ( ! type ) return; // FIX ME ### + double timeout = GetExpireTime(); + if ( timeout < 0 ) + return; // Skip in case of invalid expiration value + PDict(TableEntryVal)* tbl = AsNonConstTable(); if ( ! expire_cookie ) @@ -2197,11 +2196,11 @@ void TableVal::DoExpire(double t) // correct, so we just need to wait. } - else if ( v->ExpireAccessTime() + expire_time < t ) + else if ( v->ExpireAccessTime() + timeout < t ) { Val* val = v->Value(); - if ( expire_expr ) + if ( expire_func ) { Val* idx = RecoverIndex(k); double secs = CallExpireFunc(idx); @@ -2221,7 +2220,7 @@ void TableVal::DoExpire(double t) { // User doesn't want us to expire // this now. - v->SetExpireAccess(network_time - expire_time + secs); + v->SetExpireAccess(network_time - timeout + secs); delete k; continue; } @@ -2258,9 +2257,29 @@ void TableVal::DoExpire(double t) InitTimer(table_expire_delay); } +double TableVal::GetExpireTime() + { + if ( expire_time ) + { + Val* timeout = expire_time->Eval(0); + if ( timeout && (timeout->AsInterval() >= 0) ) + { + return timeout->AsInterval(); + } + else + { + // invalid expiration interval + expire_time = 0; + if ( timer ) + timer_mgr->Cancel(timer); + } + } + return -1; + } + double TableVal::CallExpireFunc(Val* idx) { - if ( ! expire_expr ) + if ( ! expire_func ) { Unref(idx); return 0; @@ -2285,7 +2304,7 @@ double TableVal::CallExpireFunc(Val* idx) try { - Val* vf = expire_expr->Eval(0); + Val* vf = expire_func->Eval(0); if ( ! vf ) { @@ -2319,11 +2338,15 @@ double TableVal::CallExpireFunc(Val* idx) void TableVal::ReadOperation(Val* index, TableEntryVal* v) { + // Skip in case of invalid expiration value + double timeout = GetExpireTime(); + if ( timeout < 0 ) + return; // In theory we need to only propagate one update per &read_expire // interval to prevent peers from expiring intervals. To account for // practical issues such as latency, we send one update every half // &read_expire. - if ( network_time - v->LastReadUpdate() > expire_time / 2 ) + if ( network_time - v->LastReadUpdate() > timeout / 2 ) { StateAccess::Log(new StateAccess(OP_READ_IDX, this, index)); v->SetLastReadUpdate(network_time); @@ -2362,11 +2385,9 @@ bool TableVal::DoSerialize(SerialInfo* info) const state->did_index = false; info->s->WriteOpenTag(table_type->IsSet() ? "set" : "table"); - if ( ! SERIALIZE(expire_time) ) - return false; - SERIALIZE_OPTIONAL(attrs); - SERIALIZE_OPTIONAL(expire_expr); + SERIALIZE_OPTIONAL(expire_time); + SERIALIZE_OPTIONAL(expire_func); // Make sure nobody kills us in between. const_cast(this)->Ref(); @@ -2491,13 +2512,11 @@ bool TableVal::DoUnserialize(UnserialInfo* info) { DO_UNSERIALIZE(MutableVal); - if ( ! UNSERIALIZE(&expire_time) ) - return false; - Init((TableType*) type); UNSERIALIZE_OPTIONAL(attrs, Attributes::Unserialize(info)); - UNSERIALIZE_OPTIONAL(expire_expr, Expr::Unserialize(info)); + UNSERIALIZE_OPTIONAL(expire_time, Expr::Unserialize(info)); + UNSERIALIZE_OPTIONAL(expire_func, Expr::Unserialize(info)); while ( true ) { diff --git a/src/Val.h b/src/Val.h index a49a2e2235..542d18e09a 100644 --- a/src/Val.h +++ b/src/Val.h @@ -862,6 +862,11 @@ protected: // Calculates default value for index. Returns 0 if none. Val* Default(Val* index); + // Returns true if item expiration is defined. + bool ExpirationEnabled() { return expire_time != 0; } + // Returns the expiration time defined by create, read + // or write expire attribute or -1 for invalid values. + double GetExpireTime(); // Calls &expire_func and returns its return interval; // takes ownership of the reference. double CallExpireFunc(Val *idx); @@ -874,8 +879,8 @@ protected: TableType* table_type; CompositeHash* table_hash; Attributes* attrs; - double expire_time; - Expr* expire_expr; + Expr* expire_time; + Expr* expire_func; TableValTimer* timer; IterCookie* expire_cookie; PrefixTable* subnets; diff --git a/testing/btest/Baseline/language.expire-redef/output b/testing/btest/Baseline/language.expire-redef/output new file mode 100644 index 0000000000..d0a3a5dac1 --- /dev/null +++ b/testing/btest/Baseline/language.expire-redef/output @@ -0,0 +1,5 @@ +Run 0 +Run 1 +Run 2 +Expired: 0 --> some data +Run 3 diff --git a/testing/btest/language/expire-redef.bro b/testing/btest/language/expire-redef.bro new file mode 100644 index 0000000000..044e6bb950 --- /dev/null +++ b/testing/btest/language/expire-redef.bro @@ -0,0 +1,38 @@ +# @TEST-EXEC: btest-bg-run broproc bro %INPUT +# @TEST-EXEC: btest-bg-wait -k 5 +# @TEST-EXEC: cat broproc/.stdout > output +# @TEST-EXEC: btest-diff output + + +@load frameworks/communication/listen + +const exp_val = -1sec &redef; + +global expired: function(tbl: table[int] of string, idx: int): interval; +global data: table[int] of string &write_expire=exp_val &expire_func=expired; + +redef table_expire_interval = 1sec; +redef exp_val = 3sec; + +global runs = 0; +event do_it() + { + print fmt("Run %s", runs); + + ++runs; + if ( runs < 4 ) + schedule 1sec { do_it() }; + } + + +function expired(tbl: table[int] of string, idx: int): interval + { + print fmt("Expired: %s --> %s", idx, tbl[idx]); + return 0sec; + } + +event bro_init() &priority=-10 + { + data[0] = "some data"; + schedule 1sec { do_it() }; + } From a72112accaceb43636375bb7ae8da7fdc5e8a165 Mon Sep 17 00:00:00 2001 From: Johanna Amann Date: Mon, 13 Jun 2016 16:02:06 -0700 Subject: [PATCH 3/7] Fix precedence of hook The precedence is now lower than the precedence of &&/|| so that expressions like hook a() && doSomething() work. Addresses BIT-1619 --- src/parse.y | 2 +- testing/btest/Baseline/language.hook/out | 4 ++++ testing/btest/language/hook.bro | 25 ++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/parse.y b/src/parse.y index f9eb7cbe9b..facd7e55ed 100644 --- a/src/parse.y +++ b/src/parse.y @@ -31,12 +31,12 @@ %token TOK_NO_TEST -%nonassoc TOK_HOOK %left ',' '|' %right '=' TOK_ADD_TO TOK_REMOVE_FROM %right '?' ':' %left TOK_OR %left TOK_AND +%nonassoc TOK_HOOK %nonassoc '<' '>' TOK_LE TOK_GE TOK_EQ TOK_NE %left TOK_IN TOK_NOT_IN %left '+' '-' diff --git a/testing/btest/Baseline/language.hook/out b/testing/btest/Baseline/language.hook/out index d4f367f875..28543d5320 100644 --- a/testing/btest/Baseline/language.hook/out +++ b/testing/btest/Baseline/language.hook/out @@ -17,3 +17,7 @@ myhook return F myhook return T myhook, &priority=5, [a=37, b=goobye world] F +myhook5, test +second part ran +myhook5 ran +myhook6, test diff --git a/testing/btest/language/hook.bro b/testing/btest/language/hook.bro index 9c9ab30c18..3edfd9556c 100644 --- a/testing/btest/language/hook.bro +++ b/testing/btest/language/hook.bro @@ -10,6 +10,8 @@ global myhook: hook(r: rec); global myhook2: hook(s: string); # a hook doesn't have to take any arguments global myhook4: hook(); +global myhook5: hook(s: string); +global myhook6: hook(s: string); hook myhook(r: rec) &priority=5 { @@ -72,6 +74,23 @@ hook myhook4() &priority=2 print "myhook4", 2; } +hook myhook5(s: string) + { + print "myhook5", s; + } + +hook myhook6(s: string) + { + print "myhook6", s; + break; + } + +function printMe(s: string): bool + { + print s; + return T; + } + event bro_init() { print hook myhook([$a=1156, $b="hello world"]); @@ -90,4 +109,10 @@ event bro_init() # invoked directly by name. local h = myhook; print hook h([$a=2, $b="it works"]); + + if ( hook myhook5("test") && printMe("second part ran") ) + print "myhook5 ran"; + + if ( ( hook myhook6("test") ) && printMe("second part ran") ) + print "myhook6 ran"; } From 75849f8fe245b245573f874eb370449086c71923 Mon Sep 17 00:00:00 2001 From: Jan Grashoefer Date: Tue, 14 Jun 2016 17:52:34 +0200 Subject: [PATCH 4/7] Improved handling of 802.11 headers. Frame types except data and frames subtypes without payload are skipped. Header length is determined based on presence of QoS and flags indicating the use of the 4th address field. Handling of aggregated MSDUs is explicitly prevented. --- src/iosource/Packet.cc | 84 +++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 41 deletions(-) diff --git a/src/iosource/Packet.cc b/src/iosource/Packet.cc index b5a16fa69d..88c1aeee79 100644 --- a/src/iosource/Packet.cc +++ b/src/iosource/Packet.cc @@ -276,21 +276,33 @@ void Packet::ProcessLayer2() } pdata += rtheader_len; - u_char len_80211 = 0; - int type_80211 = pdata[0]; + u_char len_80211 = 0; // Frame header length + u_char fc_80211 = pdata[0]; // Frame Control field - if ( (type_80211 >> 4) & 0x04 ) - { - //identified a null frame (we ignore for now). no weird. + // Skip non data frame types (management & control). + if ( !((fc_80211 >> 2) & 0x02) ) return; - } + + len_80211 = 24; // minimal length of data frames + + // Skip subtypes without data. + if ( (fc_80211 >> 4) & 0x04 ) + return; + + // 'To DS' and 'From DS' flags set indicate + // use of the 4th address field + if ( (pdata[1] & 0x03) == 0x03 ) + len_80211 += l2_addr_len; // Look for the QoS indicator bit. - - if ( (type_80211 >> 4) & 0x08 ) - len_80211 = 26; - else - len_80211 = 24; + if ( (fc_80211 >> 4) & 0x08 ) + { + // Skip in case of A-MSDU subframes + // indicated by QoS control field + if ( pdata[len_80211] & 0x80) + return; + len_80211 += 2; + } if ( pdata + len_80211 >= end_of_data ) { @@ -298,39 +310,29 @@ void Packet::ProcessLayer2() return; } - // Look for data frames - if ( type_80211 & 0x08 ) - { - // Determine link-layer addresses based - // on 'To DS' and 'From DS' flags - switch ( pdata[1] & 0x03 ) { - case 0x00: - l2_dst = pdata + 4; - l2_src = pdata + 10; - break; + // Determine link-layer addresses based + // on 'To DS' and 'From DS' flags + switch ( pdata[1] & 0x03 ) { + case 0x00: + l2_src = pdata + 10; + l2_dst = pdata + 4; + break; - case 0x01: - l2_src = pdata + 10; - l2_dst = pdata + 16; - break; + case 0x01: + l2_src = pdata + 10; + l2_dst = pdata + 16; + break; - case 0x02: - l2_src = pdata + 16; - l2_dst = pdata + 4; - break; + case 0x02: + l2_src = pdata + 16; + l2_dst = pdata + 4; + break; - case 0x03: - // TODO: We should integrate this - // test into the length check above. - if ( pdata + 24 + l2_addr_len >= end_of_data ) - { - l2_dst = pdata + 16; - l2_src = pdata + 24; - } - - break; - } - } + case 0x03: + l2_src = pdata + 24; + l2_dst = pdata + 16; + break; + } // skip 802.11 data header pdata += len_80211; From 0c3cbf385265bc58ed64b3bcd9e72f30773257f5 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Tue, 14 Jun 2016 16:19:10 -0700 Subject: [PATCH 5/7] Updating submodule(s). [nomail] --- aux/binpac | 2 +- aux/bro-aux | 2 +- aux/broccoli | 2 +- aux/broctl | 2 +- aux/broker | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/aux/binpac b/aux/binpac index a2a946d98d..580184dd96 160000 --- a/aux/binpac +++ b/aux/binpac @@ -1 +1 @@ -Subproject commit a2a946d98d18693b4c38392568f5dc7876d2815c +Subproject commit 580184dd96ba8e17509d837d2024b7ece860f3a5 diff --git a/aux/bro-aux b/aux/bro-aux index 50d33db5d1..4ba16fa2fc 160000 --- a/aux/bro-aux +++ b/aux/bro-aux @@ -1 +1 @@ -Subproject commit 50d33db5d12b81187ea127a08903b444a3c4bd04 +Subproject commit 4ba16fa2fcd59d90ea497965f77655d2111bc9e8 diff --git a/aux/broccoli b/aux/broccoli index b4d1686cdd..65cf386961 160000 --- a/aux/broccoli +++ b/aux/broccoli @@ -1 +1 @@ -Subproject commit b4d1686cdd3f5505e405667b1083e8335cae6928 +Subproject commit 65cf386961f5e37f6898d372dd7a5a949b5eaa28 diff --git a/aux/broctl b/aux/broctl index efffa4cc1f..214682a9d4 160000 --- a/aux/broctl +++ b/aux/broctl @@ -1 +1 @@ -Subproject commit efffa4cc1f8d02ff748c0915cf540fb195a48b17 +Subproject commit 214682a9d4b238dc55d7ecfa7c127c3aaad750d4 diff --git a/aux/broker b/aux/broker index bb3f55f198..a4f81f79cf 160000 --- a/aux/broker +++ b/aux/broker @@ -1 +1 @@ -Subproject commit bb3f55f198f9cfd5e545345dd6425dd08ca1d45e +Subproject commit a4f81f79cfc0d0fe3fe435d33217f5bf9c2279e1 From ddabd1309796a16e2ead716911ac9b9188acd799 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Tue, 14 Jun 2016 17:42:52 -0700 Subject: [PATCH 6/7] Updating submodule(s). [nomail] --- CHANGES | 4 ++++ VERSION | 2 +- aux/binpac | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGES b/CHANGES index a8402c6cfd..94ba4ddf4e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,8 @@ +2.4-612 | 2016-06-14 17:42:52 -0700 + + * Improved handling of 802.11 headers. (Jan Grashoefer) + 2.4-609 | 2016-06-14 17:15:28 -0700 * Fixed table expiration evaluation. The expiration attribute diff --git a/VERSION b/VERSION index de037e8cd9..4f4e5c1be1 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.4-609 +2.4-612 diff --git a/aux/binpac b/aux/binpac index 580184dd96..97df41aa79 160000 --- a/aux/binpac +++ b/aux/binpac @@ -1 +1 @@ -Subproject commit 580184dd96ba8e17509d837d2024b7ece860f3a5 +Subproject commit 97df41aa79344faadaf075f7fa673b87ecbc6f77 From 2335a62a075b551e6bd1bca2bdc02be605bc6987 Mon Sep 17 00:00:00 2001 From: Robin Sommer Date: Tue, 14 Jun 2016 18:10:37 -0700 Subject: [PATCH 7/7] Preventing the event processing from looping endlessly when an event reraised itself during execution of its handlers. --- CHANGES | 6 +++ VERSION | 2 +- src/Event.cc | 50 +++++++++++-------- src/Event.h | 2 - .../Baseline/core.recursive-event/output | 1 + testing/btest/core/recursive-event.bro | 31 ++++++++++++ 6 files changed, 67 insertions(+), 25 deletions(-) create mode 100644 testing/btest/Baseline/core.recursive-event/output create mode 100644 testing/btest/core/recursive-event.bro diff --git a/CHANGES b/CHANGES index 94ba4ddf4e..d702d85b3f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,10 @@ +2.4-613 | 2016-06-14 18:10:37 -0700 + + * Preventing the event processing from looping endlessly when an + event reraised itself during execution of its handlers. (Robin + Sommer) + 2.4-612 | 2016-06-14 17:42:52 -0700 * Improved handling of 802.11 headers. (Jan Grashoefer) diff --git a/VERSION b/VERSION index 4f4e5c1be1..ac2ea9d11f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.4-612 +2.4-613 diff --git a/src/Event.cc b/src/Event.cc index 5d54752a5a..6371a69248 100644 --- a/src/Event.cc +++ b/src/Event.cc @@ -94,26 +94,6 @@ void EventMgr::QueueEvent(Event* event) ++num_events_queued; } -void EventMgr::Dispatch() - { - if ( ! head ) - reporter->InternalError("EventMgr::Dispatch underflow"); - - Event* current = head; - - head = head->NextEvent(); - if ( ! head ) - tail = head; - - current_src = current->Source(); - current_mgr = current->Mgr(); - current_aid = current->Analyzer(); - current->Dispatch(); - Unref(current); - - ++num_events_dispatched; - } - void EventMgr::Drain() { if ( event_queue_flush_point ) @@ -124,8 +104,34 @@ void EventMgr::Drain() PLUGIN_HOOK_VOID(HOOK_DRAIN_EVENTS, HookDrainEvents()); draining = true; - while ( head ) - Dispatch(); + + // Past Bro versions drained as long as there events, including when + // a handler queued new events during its execution. This could lead + // to endless loops in case a handler kept triggering its own event. + // We now limit this to just a couple of rounds. We do more than + // just one round to make it less likley to break existing scripts + // that expect the old behavior to trigger something quickly. + + for ( int round = 0; head && round < 2; round++ ) + { + Event* current = head; + head = 0; + tail = 0; + + while ( current ) + { + Event* next = current->NextEvent(); + + current_src = current->Source(); + current_mgr = current->Mgr(); + current_aid = current->Analyzer(); + current->Dispatch(); + Unref(current); + + ++num_events_dispatched; + current = next; + } + } // Note: we might eventually need a general way to specify things to // do after draining events. diff --git a/src/Event.h b/src/Event.h index 0d004d526c..1b76928f10 100644 --- a/src/Event.h +++ b/src/Event.h @@ -90,8 +90,6 @@ public: delete_vals(vl); } - void Dispatch(); - void Dispatch(Event* event, bool no_remote = false) { current_src = event->Source(); diff --git a/testing/btest/Baseline/core.recursive-event/output b/testing/btest/Baseline/core.recursive-event/output new file mode 100644 index 0000000000..f599e28b8a --- /dev/null +++ b/testing/btest/Baseline/core.recursive-event/output @@ -0,0 +1 @@ +10 diff --git a/testing/btest/core/recursive-event.bro b/testing/btest/core/recursive-event.bro new file mode 100644 index 0000000000..cc7a2be8eb --- /dev/null +++ b/testing/btest/core/recursive-event.bro @@ -0,0 +1,31 @@ +# @TEST-EXEC: bro %INPUT 2>&1 | grep -v termination | sort | uniq | wc -l >output +# @TEST-EXEC: btest-diff output + +# In old version, the event would keep triggering endlessely, with the network +# time not moving forward and Bro not terminating. +# +# Note that the output will be 10 (not 20) because we still execute two rounds +# of events every time we drain. + +redef exit_only_after_terminate=T; + +global c = 0; + +event test() + { + c += 1; + + if ( c == 20 ) + { + terminate(); + return; + } + + print network_time(); + event test(); + } + +event bro_init() + { + event test(); + }