[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 bernhard at ICSI.Berkeley.EDU
Thu Mar 13 10:54:38 PDT 2014


On Mar 13, 2014, at 8:50 AM, Siwek, Jonathan Luke <jsiwek at illinois.edu> wrote:
>> 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.
> 
> It think it should work provided that matching file handles are generated at the script-layer for this type of file.  (not sure whether they are in this case, didn’t check)

Well, file handles are generated at the script layer. The current code is…

function get_file_handle(c: connection, is_orig: bool): string
	{
	set_session(c);

	local depth: count;

	if ( is_orig )
		{
		depth = c$ssl$client_depth;
		++c$ssl$client_depth;
		}
	else
		{
		depth = c$ssl$server_depth;
		++c$ssl$server_depth;
		}

	return cat(Analyzer::ANALYZER_SSL, c$start_time, is_orig, id_string(c$id), depth);
	}

I have no idea if that is “matching” or not - in any case, the above C code does not work in combination with that
file handle generation. Thinking about it, it makes sense - the EndOfFile function probably calls get_file_handle again for
that connection, gets a different file handle back (because the counters are increased), and hence removal will not work.

But….

>> I also do not really think this is sufficiently documented in the comments of
>> Manager.h. This basically is not mentioned at all there…
> 
> Yeah, it should probably at least link to [1] at least once.  Do you think it would help to link to that in each method where it matters?
> 
> [1] http://www.bro.org/development/howtos/file-analysis-file-id.html

…even after reading through the how to, I was not quite clear on the fact, that get_file_handle
has to always return the same value for the same file. Which is impossible to accomplish in cases
like this, because, several certificates are sent over a connection, and you do not get any further information
with the get_file_handle call. So - you more or less have to return differing IDs for everything.

Perhaps the EndOfFile functions should get some warning that details in which circumstances you can use
them (probably - static per-connection ID).

Also - it might be nice to throw some kind of reporter warning when EndOfFile is called for a file ID that
does not exists.

If I understood everything right, that should never happen during normal operations, should it?

Bernhard


More information about the bro-dev mailing list