I have a TCP analyzer that I wrote for my master thesis which I’m trying to update to the latest version of Bro. After rebasing to the trunk, I observed only a few collisions. I resolved the collisions and but something seems to have changed with how the logs are written. Are there changes in the logging framework between Bro 2.2 and the current master which could influence how events are generated? Could this be a change in how packets are delivered to TCP child/support/application analyzers?
I am only guessing at things as I haven’t had much time to debug why the logs aren’t being generated. From some quick debug, I can see that the analyzer is still being added to TCP as a child analyzer, so it seems related to either delivery or event generation.
I know this is little information to go on. I can provide more information as needed.
Just a guess, but it could be related to this: https://github.com/bro/bro/blob/master/CHANGES#L1578
ints changed to uint64s. As an example, you can see how the HTTP analyzer was modified here: https://github.com/bro/bro/commit/96bcc2d69d72c21f5f4eff0c88cd8d43613bee22#diff-978a30a2ac40a10fbf3c8b5500d3a9f3
The other big change was moving to plugins, but if you’re seeing it added as a child analyzer, that doesn’t sound like it’d be the issue.
Was this analyzer written in BinPAC, or in C++?
–Vlad
Well, what I meant with that change was that the functions used for data
delivery changed. Specifically:
Analyzer::{NextPacket, NextUndelivered, ForwardPacket, ForwardUndelivered,
DeliverPacket, Undelivered} were modified to change the int seq parameter
to a uint64. If your functions aren't updated, and are expecting a plain
old int for the sequence number, I've seen the scenario you describe: the
analyzer attaches, but doesn't function.
--Vlad
If I understand the patch correctly, it would only cause problems for connections with over 2GB of data payload, but I think it should work fine for a small trace of say 200KB. I’m not seeing any events at all, nor am I seeing the log files that should be created when using the analyzer.
I’ll correct the functions and test it out though.
In Analyzer.cc, there is a quick check for ‘if (skip)’ . How does this variable get set?
1285862632.803262/1434571577.132267 [dpd] TCPRS[101422] DeliverPacket(0, T, 9005, 0x7fff0d41bf80, 0) []
1285862632.803262/1434571577.132274 [dpd] TCP_ApplicationAnalyzer ignoring DeliverPacket(0, T, 9005, 0x7fff0d41bf80, 0) []
Are these two lines related? I’m stuck. I’ve run bro with GDB attached using a simple trace file and TCPRS_Analyzer::DeliverPacket never seems to be entered.
If I understand the patch correctly, it would only cause problems for
connections with over 2GB of data payload, but I think it should work fine
for a small trace of say 200KB. I'm not seeing any events at all, nor am I
seeing the log files that should be created when using the analyzer.
That was the point of that change, yes, but the breaking modification was
that the function signatures are now different. Specifically, those are
virtual functions that you're inheriting. If your parameters don't match
exactly, you're just defining a new virtual function as opposed to
redefining the existing function.
I'll correct the functions and test it out though.
When you said that you're not seeing DeliverPacket be entered, was that
after making the uint64 change?
--Vlad
If I understand the patch correctly, it would only cause problems for connections with over 2GB of data payload, but I think it should work fine for a small trace of say 200KB. I’m not seeing any events at all, nor am I seeing the log files that should be created when using the analyzer.
That was the point of that change, yes, but the breaking modification was that the function signatures are now different. Specifically, those are virtual functions that you’re inheriting. If your parameters don’t match exactly, you’re just defining a new virtual function as opposed to redefining the existing function.
Good point.
I’ll correct the functions and test it out though.
When you said that you’re not seeing DeliverPacket be entered, was that after making the uint64 change?
I thought I had corrected it, but seems like the definition was still slightly off. I’m getting all of my logs now as expected. Thanks.