A while ago we've been discussing adding a new "async" keyword to run
certain functions asynchronously; Bro would proceed with other script
code until they complete. Summary is here:
https://www.bro.org/development/projects/broker-lang-ext.html#asynchronous-…
After my original proof-of-concept version, I now have a 2nd-gen
implementation mostly ready that internally unifies the machinery for
"when" and "async". That simplifies the code and, more importantly,
makes the two work together more smoothly. The branch for that work is
topic/robin/sync, I'll clean that up a bit more and would then be
interested in seeing somebody test drive it.
In the meantime I want to propose a slight change to the original
plan. In earlier discussions, we ended up concluding that introducing
a new keyword to trigger the asynchronous behaviour is useful for the
script writer, as it signals that semantics are different for these
calls. Example using the syntax we arrived at:
event bro_init()
{
local x = async lookup_hostname("www.icir.org"); # A
print "IP of www.icir.org is", x; # B
}
Once the DNS request is issued in line A, the event handler will be
suspended until the answer arrives. That means that other event
handlers may execute before line B, i.e., execution order isn't fully
deterministic anymore. The use of "async" is pointing out that
possibility.
However, look at the following example. Let's say we want to outsource
such DNS functionality into a separate framework, like in this toy
example:
# cat test.bro
@load my-dns
event bro_init()
{
local x = MyCoolDNSFramework::lookup("www.icir.org"); # A
print "IP of www.icir.org is", x; # B
}
# cat my-dns.bro
module MyCoolDNSFramework;
export {
global lookup: function(name: string) : set[addr];
}
function lookup(name: string) : set[addr] {
local addrs = async lookup_hostname(name); # C
return addrs; # D
}
That example behaves exactly as the 1st: execution may suspend between
lines A and B because the call to MyCoolDNSFramework::lookup()
executes an asynchronous function call internally (it will hold
between C and D). But in this 2nd example that behaviour is not
apparent at the call site in line A.
We could require using "async" in line A as well but that would be
extremely painful: whenever calling some function, one would need to
know whether internally the callee may end up using "async" somewhere
(potentially buried further deep inside its call stack).
I think we should instead just skip the "async" keyword altogether.
Requiring it at some places, but not others, hurts more than it helps
in my opinion. The 1st example would then just go back to look like
this:
event bro_init()
{
local x = lookup_hostname("www.icir.org"); # A
print "IP of www.icir.org is", x; # B
}
This would still behave the same as before: potential suspension
between A and B.
I don't think skipping "async" this would be a big deal for anything,
as the cases where the new behaviour may actually lead to significant
differences should be rare.
Thoughts?
Robin
--
Robin Sommer * ICSI/LBNL * robin(a)icir.org * www.icir.org/robin
Hi,
I am going to delete these (merged) branches thursday, unless someone
feels especially attached to them:
topic/dnthayer/ticket1821
topic/dnthayer/ticket1836
topic/dnthayer/ticket1863
topic/jazoff/contentline-limit
topic/jazoff/fix-gridftp
topic/jazoff/fix-intel-error
topic/jazoff/speedup-for
topic/robin/broker-logging
topic/robin/event-args
topic/robin/plugin-version-check
topic/seth/add-file-lookup-functions
topic/seth/input-thread-behavior
topic/seth/remove-dns-weird
topic/vladg/bit-1838
Johanna
Bro-Dev Group,
I am doing a little research into using Bro to log and analyze specific Microsoft DCE-RPC interfaces and methods. I notice that the Bro events for 'dce_rpc_request' and 'dce_rpc_response' provide the length of the RCP data stub (aka 'stub_len'). I found reference that these events previously provided a byte string containing the stub data itself, but at some point it was reduced to just the stub_len instead. I have a few questions that I hope you could answer:
1. What was the reason you decided to remove the stub data from the events and pass only the stub length?
1. On github, I see a BinPAC file for the RPC 'At' service (bro/src/analyzerprotocol/dce-rpc/endpoint-atsvc.pac), but there are no events generated by it. I think this would be very useful for my project. What is the reason that you have the analyzer, but no events for scriptland?
1. I have a use case, for a very few, limited number of RPC interfaces/methods, where I need to receive the stub data in scriptland for logging and analysis. How do you recommend I approach this scenario? I see a couple options:
1. I could customize the DCE-RPC analyzer to pass the sub data for *ALL* 'dce_rpc_request' and 'dce_rpc_response' events; or
2. I could customize the DCE-RPC analyzer to create new events specifically for the interfaces/methods (aka UUIDs/OpNums) that I care about.
3. Other ideas?
I think both (a) and (b) will achieve the desired result; but there are trade-offs, pros and cons. I wonder which option would have a more negative impact on Bro performance? I imagine the reason you stopped passing stub data for all events was due to the performance hit, so I want to approach this in the best way possible. I appreciate your feedback.
Cheers!
Mark
I have been using the input framework with great success as a tool to
read and parse structured text logs. Unfortunately I have reached a
performance impasse and was looking for a little advice.
The data source is a log file that grows at ~7-9k records/sec and
consists of small text lines of < 512 bytes, newline delimited.
The primary symptom here is a steadily growing memory footprint even
though the back end analyzer seems to be processing the events in near
real time - i.e. there is obviously some buffering going on but the data
is being consumed. The footprint for script side variables is not to
blame as it is always << 1% of the total.
I tried modifying Raw::block_size to better fit the line size, but that
made it worse. Increasing it to 16k seemed to be the sweet spot, but
the problem is still there.
Any thoughts on what might help here (besides lower data rates)?
thanks!
scott
Hi all:
I was tinkering with the sumstats code, and inadvertantly deleted the final
"}" closing out the last function. When running the code, the misleading
error message is received:
error in
/Users/melland/traces/bro/share/bro/base/frameworks/tunnels/./main.bro,
line 8: syntax error, at or near "module"
presumably due to the function still being open when the next policy script
is loaded. Wouldn't it be more reasonable to check at the end of each
script when loaded that there are no dangling functions, expressions, etc.
????
Jim Mellander
ESNet
Hi all:
The attached brogram demonstrates that the bro sort() function does not
sort correctly under certain circumstances (tested on OS/X & Linux). The
behavior also occurs when using the common function idiom of sort(myvec,
function(a: int, b: int): int { return a-b;});
I haven't examined bro source code, but since some of the test values are
larger than 32 bits, I surmise that there is a casting from 64 to 32 bits
that could change the sign of the comparison, thus causing this problem.
Mitigation is to use a function that returns the sign of subtraction
results, rather than the actual subtraction results, something like
sort(myvec, function(a: int, b: int): int { return a<b ? -1 : (a>b ? 1 :
0);});
Cheers,
Jim Mellander
ESNet
Hi,
packaging some POC-seen scripts for the intel framework I was wondering
what would be the preferred granularity of Bro packages. In case of seen
scripts, it feels extreme to generate a package for every script.
So one approach would be to group them into a single package and let the
user load the single scripts on demand. But, some of the scripts might
depend on other packages. These packages would be suggested during
install. Assuming a minimal install this could lead to a couple of
scripts, that spit errors if loaded. So if someone decides to load the
scripts later, he or she might forgot about the dependencies. In that
case it would be nice if one could check either for the availability of
certain identifiers (lookup_ID didn't work for me due to type clash in
comparison) or a package.
What would be the preferred way?
Thanks,
Jan
In a previous email, I asked the question: Does anyone use sumstats outside
of a cluster context?
In my case, the answer is yes, as I perform development on a laptop, and
run bro standalone to test new bro policies.
I found several different bugs in
share/bro/base/frameworks/sumstats/non-cluster.bro,
specifically SumStats::process_epoch_results
1. The policy returns sumstats results 50 at a time, and then reschedules
itself for the next 50 after 0.01 seconds.. Unfortunately, the reschedule
is:
schedule 0.01 secs { process_epoch_result(ss, now, data1) };
instead of:
schedule 0.01 secs { *SumStats::*process_epoch_result(ss, now, data1) };
so it silently fails after the first 50 results. Would be nice to have a
warning if a script schedules an event that doesn't exist.
2.The serious issue with the policy, though, is that the 'for' loop over
the result table is the main loop, with up to 50 items processed and
deleted within the loop, the expectation being that the iteration will not
thus be disturbed.
The attached program (hash_test.bro) demonstrates that this is not the case
(should output 1000, 0, but the 2nd value comes back non-zero), in line
with the documented caveat: "Currently, modifying a container’s membership
while iterating over it may result in undefined behavior, so do not add or
remove elements inside the loop." I didn't examine bro source code to
appreciate the reason, but surmise that table resizing and rehashing would
account for the issue.
The consequences of this issue are that, under certain circumstances:
* Not all data will be returned by SumStats at the epoch
* SumStats::finish_epoch may not be run.
To address the issue can be done via a rearrangement of the code, along the
lines of the following pseudocode (boundary conditions, etc. ignored)
original (modifies table inside 'for' loop):
i=50;
for (foo in bar)
{
process bar[foo];
delete bar[foo];
--i;
if (i == 0) break;
}
to (modifies table outside 'for' loop):
i=50;
while (i >0)
{
for (foo in bar)
{
break;
}
process bar[foo];
delete bar[foo]
--i;
}
... there are a few other subtleties in the code (keeping a closure on the
result table so that sumstats can clear the original table & proceed to the
next epoch, and not running SumStats::finish_epoch if the result table was
empty to begin with).
A bit of rearrangement fixes the bugs while preserving the original
behavior, with the help of a wrapper event that checks for an empty result
table, and if not makes an explicit copy for further processing by the
actual event doing the work.
An additional 'for' loop around the result table could be used to keep it
all in one event, but looks too much like black magic (and still, albeit
probably in a safe way, depending on undefined behavior) - I prefer clear,
understandable code (ha!), rather than "dark corner" features. Six months
later, when I look at the code, I won't be able to remember the clever
trick I was using :-)
Attached please find hash_test.bro & (patched) non-cluster.bro
Jim Mellander
ESNet
Hi all:
I'm working on an interesting Bro policy, and I want to be able to begin a
'for loop' at some point where it previously left off.
Pseudo-code:
for (foo in bar)
{
if (foo == "baz") break;
.
process bar[foo]
.
}
.
.
Do other work (not changing bar)
.
.
first_time = T;
for (foo in bar)
{
if (first_time)
{
first_time = F;
foo = "baz";
}
.
process bar[foo]
.
}
....
If the loop variable can be reassigned in the loop, and the loop continued
from that point, it would facilitate some of the processing I'm doing. The
above synthetic code could be refactored to avoid the issue, but....
My real-world issue is that I have a large table to process, and want to
amortize the processing of it on the time domain:
A. Process first N items in table
B. Schedule processing of next N items via an event
C. When event triggers, pick up where we left off, and process next N
items, etc.
(There are inefficient ways of processing these that solve some, but not
all issues, such as putting the indices in a vector, then going thru that -
wont go into the problems with that right now)
I haven't checked whether my desired behavior works, but since its not
documented, I wouldn't want to rely on it in any event.
I would be interested in hearing comments or suggestions on this issue.
Jim
Hi,
after a few months I finally made to pack my contribution proposal as a
pull request available at
https://github.com/bro/bro/pull/121
The patch introduces new options types for DHCP protocol and extends
dhcp event including new parameters that I believe are useful in network
forensics analysis.
The options are the following:
55 Parameters Request List;
58 Renewal time;
59 Rebinding time;
61 Client Identifier;
82 Relay Agent Information.
while the following are the extended events:
dhcp_discover exports client identifier and parameters request list;
dhcp_request exports client_identifier and parameters request list;
dhcp_ack exports rebinding time, renewal time and list of suboptions
value of
dhcp relay agent information option;
dhcp_inform exports parameters request list.
Looking forward to receving feedbacks!
best,
Valerio
Il 14/06/2017 01:28, Robin Sommer ha scritto:
>
>
> On Wed, Jun 14, 2017 at 01:04 +0200, Valerio wrote:
>
>> What would be the best procedure (and format) to submit such a patch?
>
> Easiest is to prepare a pull request on GitHub. We have some
> guidelines here:
> https://www.bro.org/development/contribute.html#submitting-patches
>
> Looking forward to your patches!
>
> Robin
>
