diff --git a/CHANGES b/CHANGES index 1e4593750b..d702d85b3f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,29 @@ +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) + +2.4-609 | 2016-06-14 17:15:28 -0700 + + * 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. (Jan Grashoefer) + +2.4-606 | 2016-06-14 16:11:07 -0700 + + * Fix parsing precedence of "hook" expression. Addresses BIT-1619 + (Johanna Amann) + + * Update the "configure" usage message for --with-caf (Daniel + Thayer) + 2.4-602 | 2016-06-13 08:16:34 -0700 * Fixing Covertity warning (CID 1356391). (Robin Sommer) diff --git a/VERSION b/VERSION index c6874864e6..ac2ea9d11f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.4-602 +2.4-613 diff --git a/aux/binpac b/aux/binpac index a2a946d98d..97df41aa79 160000 --- a/aux/binpac +++ b/aux/binpac @@ -1 +1 @@ -Subproject commit a2a946d98d18693b4c38392568f5dc7876d2815c +Subproject commit 97df41aa79344faadaf075f7fa673b87ecbc6f77 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..2592077f96 160000 --- a/aux/broccoli +++ b/aux/broccoli @@ -1 +1 @@ -Subproject commit b4d1686cdd3f5505e405667b1083e8335cae6928 +Subproject commit 2592077f96008f5c64b23b6fd605bfce3ec47d84 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 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: 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/src/Serializer.h b/src/Serializer.h index 43a9943cc5..402fc5d259 100644 --- a/src/Serializer.h +++ b/src/Serializer.h @@ -125,7 +125,7 @@ protected: // This will be increased whenever there is an incompatible change // in the data format. - static const uint32 DATA_FORMAT_VERSION = 25; + static const uint32 DATA_FORMAT_VERSION = 26; ChunkedIO* io; diff --git a/src/Val.cc b/src/Val.cc index 331aff3bcf..7341aada03 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,15 +1411,17 @@ void TableVal::CheckExpireAttr(attr_tag at) if ( a ) { - Val* timeout = a->AttrExpr()->Eval(0); - if ( ! timeout ) + expire_time = a->AttrExpr(); + expire_time->Ref(); + + if ( expire_time->Type()->Tag() != TYPE_INTERVAL ) { - a->AttrExpr()->Error("value of timeout not fixed"); + if ( ! expire_time->IsError() ) + expire_time->SetError("expiration interval has wrong type"); + return; } - expire_time = timeout->AsInterval(); - if ( timer ) timer_mgr->Cancel(timer); @@ -1791,7 +1794,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 +1825,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 +1883,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); } } @@ -2176,6 +2179,13 @@ void TableVal::DoExpire(double t) PDict(TableEntryVal)* tbl = AsNonConstTable(); + double timeout = GetExpireTime(); + + if ( timeout < 0 ) + // Skip in case of unset/invalid expiration value. If it's an + // error, it has been reported already. + return; + if ( ! expire_cookie ) { expire_cookie = tbl->InitForIteration(); @@ -2197,11 +2207,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 +2231,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 +2268,27 @@ void TableVal::DoExpire(double t) InitTimer(table_expire_delay); } +double TableVal::GetExpireTime() + { + if ( ! expire_time ) + return -1; + + Val* timeout = expire_time->Eval(0); + + if ( timeout && (timeout->AsInterval() >= 0) ) + return timeout->AsInterval(); + + 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 +2313,7 @@ double TableVal::CallExpireFunc(Val* idx) try { - Val* vf = expire_expr->Eval(0); + Val* vf = expire_func->Eval(0); if ( ! vf ) { @@ -2319,11 +2347,18 @@ double TableVal::CallExpireFunc(Val* idx) void TableVal::ReadOperation(Val* index, TableEntryVal* v) { + double timeout = GetExpireTime(); + + if ( timeout < 0 ) + // Skip in case of unset/invalid expiration value. If it's an + // error, it has been reported already. + 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 +2397,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 +2524,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..160eeafe64 100644 --- a/src/Val.h +++ b/src/Val.h @@ -644,7 +644,7 @@ protected: DECLARE_SERIAL(PatternVal); }; -// ListVals are mainly used to index tables that have more than one +// ListVals are mainly used to index tables that have more than one // element in their index. class ListVal : public Val { public: @@ -862,6 +862,14 @@ protected: // Calculates default value for index. Returns 0 if none. Val* Default(Val* index); + // Returns true if item expiration is enabled. + bool ExpirationEnabled() { return expire_time != 0; } + + // Returns the expiration time defined by %{create,read,write}_expire + // attribute, or -1 for unset/invalid values. In the invalid case, an + // error will have been reported. + double GetExpireTime(); + // Calls &expire_func and returns its return interval; // takes ownership of the reference. double CallExpireFunc(Val *idx); @@ -874,8 +882,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/src/iosource/Packet.cc b/src/iosource/Packet.cc index b5a16fa69d..6b3bbe40f3 100644 --- a/src/iosource/Packet.cc +++ b/src/iosource/Packet.cc @@ -267,30 +267,19 @@ void Packet::ProcessLayer2() Weird("truncated_radiotap_header"); return; } + // Skip over the RadioTap header int rtheader_len = (pdata[3] << 8) + pdata[2]; + if ( pdata + rtheader_len >= end_of_data ) { Weird("truncated_radiotap_header"); return; } + pdata += rtheader_len; - u_char len_80211 = 0; - int type_80211 = pdata[0]; - - if ( (type_80211 >> 4) & 0x04 ) - { - //identified a null frame (we ignore for now). no weird. - return; - } - - // Look for the QoS indicator bit. - - if ( (type_80211 >> 4) & 0x08 ) - len_80211 = 26; - else - len_80211 = 24; + u_char len_80211 = 24; // minimal length of data frames if ( pdata + len_80211 >= end_of_data ) { @@ -298,40 +287,62 @@ void Packet::ProcessLayer2() return; } - // Look for data frames - if ( type_80211 & 0x08 ) + u_char fc_80211 = pdata[0]; // Frame Control field + + // Skip non-data frame types (management & control). + if ( ! ((fc_80211 >> 2) & 0x02) ) + return; + + // 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 ( (fc_80211 >> 4) & 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; + // Skip in case of A-MSDU subframes indicated by QoS + // control field. + if ( pdata[len_80211] & 0x80) + return; - case 0x01: - l2_src = pdata + 10; - l2_dst = pdata + 16; - 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; + len_80211 += 2; } + + if ( pdata + len_80211 >= end_of_data ) + { + Weird("truncated_radiotap_header"); + return; } + // 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 0x02: + l2_src = pdata + 16; + l2_dst = pdata + 4; + break; + + case 0x03: + l2_src = pdata + 24; + l2_dst = pdata + 16; + break; + } + // skip 802.11 data header pdata += len_80211; 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/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/Baseline/language.expire-expr-error/output b/testing/btest/Baseline/language.expire-expr-error/output new file mode 100644 index 0000000000..544527fe23 --- /dev/null +++ b/testing/btest/Baseline/language.expire-expr-error/output @@ -0,0 +1,2 @@ +error in /home/robin/bro/master/testing/btest/.tmp/language.expire-expr-error/expire-expr-error.bro, line 7: no such index (x[kaputt]) +received termination signal 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/Baseline/language.expire-type-error/out b/testing/btest/Baseline/language.expire-type-error/out new file mode 100644 index 0000000000..c0987a6341 --- /dev/null +++ b/testing/btest/Baseline/language.expire-type-error/out @@ -0,0 +1 @@ +error in /home/robin/bro/master/testing/btest/.tmp/language.expire-type-error/expire-type-error.bro, line 4: expiration interval has wrong type (kaputt) 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/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(); + } diff --git a/testing/btest/language/expire-expr-error.bro b/testing/btest/language/expire-expr-error.bro new file mode 100644 index 0000000000..c355bd58ed --- /dev/null +++ b/testing/btest/language/expire-expr-error.bro @@ -0,0 +1,29 @@ +# @TEST-EXEC: btest-bg-run broproc bro %INPUT +# @TEST-EXEC: btest-bg-wait -k 5 +# @TEST-EXEC: cat broproc/.stderr > output +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff output + +global x: table[string] of interval; +global data: table[int] of string &create_expire=x["kaputt"]; + +@load frameworks/communication/listen + +global runs = 0; +event do_it() + { + print fmt("Run %s", runs); + + ++runs; + if ( runs < 4 ) + schedule 1sec { do_it() }; + } + + +event bro_init() &priority=-10 + { + data[0] = "some data"; + schedule 1sec { do_it() }; + } + + + 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() }; + } diff --git a/testing/btest/language/expire-type-error.bro b/testing/btest/language/expire-type-error.bro new file mode 100644 index 0000000000..d6d807e22f --- /dev/null +++ b/testing/btest/language/expire-type-error.bro @@ -0,0 +1,6 @@ +# @TEST-EXEC-FAIL: bro -b %INPUT >out 2>&1 +# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff out + +global data: table[int] of string &write_expire="kaputt"; + + 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"; }