[Bro] smb_mapping log add new field from conn.log

[fatema bannatwala]

Hi Sunu,

Seth have pretty much addressed all the problems the script has.
Just wanted to add, that be careful in the types of events you choose
to log entries for.

event "file_over_new_connection" will be triggered for "every" file Bro sees
over a connection.
And event "log_conn" will be triggered for every connection Bro logs record
for.

what your code is trying to do currently, is recording protocol and
resp_ip_bytes for every connection,
and if the connection happens to have a file transfer, then, logging those
two fields in "file" record,
which doesn't seem to be your use case.

Also, I do not see any checks for "SMB" protocol specifically. Hence, your
all the if conditions will always
be true, if the connection happen to have resp_ip_bytes and proto, which
majority of Bro connections would have.

Thanks,
Fatema.

On Fri, Nov 3, 2017 at 8:09 AM, Seth Hall <seth at corelight.com> wrote:

> You have several problems that I'm seeing but I haven't actually your
> script to see if there are more.
>
> On 3 Nov 2017, at 5:59, ps sunu wrote:
>
>    redef record SMB::TreeInfo +=      {
>>     proto: transport_proto &optional &log;
>>      resp_ip_bytes: count &optional &log;
>>  };
>>
>
> You added these fields to the SMB::TreeInfo record (just keep that in
> mind)...
>
> event Conn::log_conn (rec: Conn::Info)
>> {
>> if(rec?$resp_ip_bytes) {
>> conn_resp_ip_bytes[rec$id$orig_h] = rec$resp_ip_bytes;
>> }
>>
>> if(rec?$proto) {
>> conn_name_proto[rec$id$orig_h] = rec$proto;
>> }
>> }
>>
>
> This whole section is only run when a conn log entry is being written so
> if everything only happens over a single connection you won't see your
> tables have any data since nothing will be written until the connection
> ends.
>
> event file_over_new_connection(f:fa_file; c: connection, is_orig: bool)
>> &priority=10
>> {
>>  if ( c$id$orig_h in TrackSMB::conn_resp_ip_bytes )
>>      f$info$resp_ip_bytes = TrackSMB::conn_resp_ip_bytes[c$id$orig_h];
>>
>
> You put those fields into the SMB::TreeInfo record, but f$info is the
> Files::Info record.
>
>
>     c$smb$proto = TrackSMB::conn_name_proto[c$id$orig_h];
>>     if ( c$id$resp_h in TrackSMB::conn_name_proto )
>>     f$info$proto = TrackSMB::conn_name_proto[c$id$resp_h];
>>
>
> You are jumping around a bit here, you added the proto field to
> SMB::TreeInfo, but c$smb doesn't even exist.  To access the SMB::TreeInfo
> record, it should be c$smb_state$current_tree
>
>   .Seth
>
> --
> Seth Hall * Corelight, Inc * www.corelight.com
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mailman.ICSI.Berkeley.EDU/pipermail/bro/attachments/20171103/136d2f72/attachment.html