[Bro-Dev] [Bro-Commits] [git/bro] topic/bernhard/file-analysis-x509: Change x509 log - now certificates are only logged once per hour. (0d50b8b)

[Bernhard Amann]

On Mar 13, 2014, at 7:37 AM, Seth Hall <seth at icir.org> wrote:

> 
> On Mar 13, 2014, at 10:30 AM, Robin Sommer <robin at icir.org> wrote:
> 
>> On Thu, Mar 13, 2014 at 00:17 -0700, Bernhard Amann wrote:
>> 
>>>   You apparently have to be very careful which EndOfFile function of
>>>   the file analysis framework you call... otherwhise it might try
>>>   to close another file id. This took me quite a while to find.
>> 
>> Can you elaborate? I sense an opportuntity to improve our API. :-)
> 
> 
> I think he was running into the interplay between script land and the core.  Actually, I think that for the SSL/TLS analyzer, this is one of the times we don't need a file id generated in script land.  That's probably a better choice in this case.

That might be true, I am not entirely sure.

What I did was to call…

file_mgr->DataIn(reinterpret_cast<const u_char*>(cert.data()), cert.length(),
    bro_analyzer()->GetAnalyzerTag(), bro_analyzer()->Conn(), ${rec.is_orig});
file_mgr->EndOfFile(bro_analyzer()->GetAnalyzerTag(), bro_analyzer()->Conn(), ${rec.is_orig});

in exactly this order (so - directly following each other). Which does not work.

Changing it to...
 
string fid = file_mgr->DataIn(reinterpret_cast<const u_char*>(cert.data()), cert.length(),
    bro_analyzer()->GetAnalyzerTag(), bro_analyzer()->Conn(), ${rec.is_orig});
file_mgr->EndOfFile(fid);

makes it work perfectly.

Thinking about it, it makes sense that this might be due to the interplay of the script land
file ID generation and the core file ID. However… just looking at the code, it still
seems a bit non-intuitive that the first version does not work, and the second
version does.

I also do not really think this is sufficiently documented in the comments of
Manager.h. This basically is not mentioned at all there…

I have no idea if that is just me that got a wrong impression that this should work
when looking at the Interface. But - I could see other people perhaps making the same mistake.

Bernhard