Merge branch 'master' of ssh://git.bro.org/bro

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)

VERSION

@@ -1 +1 @@
-2.4-602
+2.4-613

aux/binpac

@@ -1 +1 @@
-Subproject commit a2a946d98d18693b4c38392568f5dc7876d2815c
+Subproject commit 97df41aa79344faadaf075f7fa673b87ecbc6f77

aux/bro-aux

@@ -1 +1 @@
-Subproject commit 50d33db5d12b81187ea127a08903b444a3c4bd04
+Subproject commit 4ba16fa2fcd59d90ea497965f77655d2111bc9e8

aux/broccoli

@@ -1 +1 @@
-Subproject commit b4d1686cdd3f5505e405667b1083e8335cae6928
+Subproject commit 2592077f96008f5c64b23b6fd605bfce3ec47d84

aux/broctl

@@ -1 +1 @@
-Subproject commit efffa4cc1f8d02ff748c0915cf540fb195a48b17
+Subproject commit 214682a9d4b238dc55d7ecfa7c127c3aaad750d4

aux/broker

@@ -1 +1 @@
-Subproject commit bb3f55f198f9cfd5e545345dd6425dd08ca1d45e
+Subproject commit a4f81f79cfc0d0fe3fe435d33217f5bf9c2279e1

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:

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.

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();

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;
 

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<TableVal*>(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 )
 		{

src/Val.h

@@ -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;

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,15 +287,44 @@ 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 )
 			{
+			// 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 )
+			{
+			Weird("truncated_radiotap_header");
+			return;
+			}
+
 		// 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;
+				l2_dst = pdata + 4;
 				break;
 
 			case 0x01:
@@ -320,17 +338,10 @@ void Packet::ProcessLayer2()
 				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;
-						}
-
+				l2_dst = pdata + 16;
 				break;
 		}
-			}
 
 		// skip 802.11 data header
 		pdata += len_80211;

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 '+' '-'

testing/btest/Baseline/core.recursive-event/output

@@ -0,0 +1 @@
+10

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

testing/btest/Baseline/language.expire-redef/output

@@ -0,0 +1,5 @@
+Run 0
+Run 1
+Run 2
+Expired: 0 --> some data
+Run 3

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)

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

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();
+        }

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() };
+	}
+
+
+

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() };
+	}

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";
+
+

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";
 	}