Moved from https://bro-tracker.atlassian.net/browse/BIT-1521
Created by Justin Azoff at 2015-11-03T09:03:40.517-0600:
known services script does
if ( ! addr_matches_host(id$resp_h, service_tracking) ||
"ftp-data" in c$service || # don't include ftp data sessions
("DNS" in c$service && c$resp$size == 0) ) # for dns, require that the server talks.
return;
but should probably also ignore gridftp-data. Probably a good idea to add a set of services that behave like ftp for it to check.
Comment by Jeannette Dopheide at 2016-01-07T13:42:21.914-0600:
Due 1/14
Comment by Justin Azoff at 2016-01-13T08:53:46.970-0600:
Hmm, this may be a little harder than I thought. c$service is a set, and bro doesn't have a bif for comparing one set to another set.. i can do it with a loop like this, but is there a better way?
function intersects(a: set[string], b: set[string]): bool{
for (one in a) {
if (one in b) {
return T;
}
}
return F;
}
even though c$service is a set, it should only have 1 or 2 strings in it, so it is not a large loop.
Comment by Seth Hall at 2016-01-14T08:37:46.000-0600:
Yeah, it would be great to be able to do set intersections in a performant way, but I think your assessment is right that this set should be short enough in all cases to go ahead and just do the loop here.
Comment by Justin Azoff at 2016-01-14T09:07:03.193-0600:
Hmm.. it seems a little odd to put the intersects function in the known-services script, is there a better place for it?
Comment by Seth Hall at 2016-01-14T09:13:08.239-0600:
�
Don’t put a set intersection function as a general utility function. We probably shouldn’t be offering that generally right now since it should be added to the language in a way that performs better. Since you’re only using it in a single location, just do a loop.
Comment by Justin Azoff at 2016-01-14T10:06:51.126-0600:
Gah. It looks like coming up with a test case for this will be a pain. The existing pcap doesn't get detected as gridftp-data since the transfer size is too small:
jazoff@Justins-MacBook-Air /tmp/b $ bro -r ~/src/bro/testing/btest//Traces/globus-url-copy.trace local base/protocols/ftp/gridftp 'Known::service_tracking=ALL_HOSTS'
WARNING: No Site::local_nets have been defined. It's usually a good idea to define your local networks.
20000, 1
jazoff@Justins-MacBook-Air /tmp/b $ cat conn.log |bro-cut id.orig_h id.orig_p id.resp_h id.resp_p service
192.168.57.103 60108 192.168.57.101 2811 ssl,gridftp,ftp
192.168.57.103 35391 192.168.57.101 55968 ssl
jazoff@Justins-MacBook-Air /tmp/b $ cat known_services.log |bro-cut host port_num service
192.168.57.101 2811 FTP
192.168.57.101 55968 SSL
Filtering the ephemeral ssl service running on port 55968 is what I am trying to accomplish here.
Comment by Johanna Amann at 2016-01-14T12:34:51.267-0600:
If only the testcase is a problem - couldn't you just redef GridFTP::size_threshold for the test?
Comment by Justin Azoff at 2016-01-14T13:45:37.537-0600:
Ah, yes that helped the protocol detection.. though I think it shows a bug in known services in general:
$ bro -r ~/src/bro/testing/btest//Traces/globus-url-copy.trace local base/protocols/ftp/gridftp 'Known::service_tracking=ALL_HOSTS' 'GridFTP::size_threshold=1'
WARNING: No Site::local_nets have been defined. It's usually a good idea to define your local networks.
$ cat conn.log |bro-cut id.orig_h id.orig_p id.resp_h id.resp_p service192.168.57.103 60108 192.168.57.101 2811 gridftp,ssl,ftp
192.168.57.103 35391 192.168.57.101 55968 ssl,gridftp-data
$ cat known_services.log |bro-cut host port_num service
192.168.57.101 2811 FTP
192.168.57.101 55968 SSL
Some of this is due to how it keeps track of services by ip,port. Since ssl is always detected first, that is the one that gets logged.
It looks even if it was changed to ip,port,service gridftp may not show up because it never makes it into known services.
The gridftp analyzer does
add c$service["gridftp-data"];
But this doesn't trigger a protocol_confirmation (even though it would be too late anyway). since the (ip,port) would have been logged as ssl.
So, I think known-services:
- Needs to keep track of things by (ip,port,service)
- Should possibly wait until a connection is closed and it has all the facts before trying to log the service.
If I remove the protocol_confirmation event and use simply:
event connection_state_remove(c: connection) &priority=-5
{
known_services_done(c);
}
It mostly works:
$ cat known_services.log |bro-cut host port_num service
192.168.57.101 2811 FTP,SSL,gridftp
192.168.57.101 55968 gridftp-data,SSL
I'm not sure if it should log them once per line, and if we should do something about the mismatch in case.
Comment by Justin Azoff at 2016-02-17T11:13:51.009-0600:
topic/jazoff/ticket1521 contains a branch that I believe fixes most of the issues with known-services.
I think there may still be one outstanding bug (but it is something that is broken worse in the current code).
The current code tracks services by (addr,port). If no service is detected on a port it will log it as (ip, port, empty). If a service is later detected on that port, nothing will be logged.
This branch WILL log it, but it will also log twice in the opposite order, which is possibly not desired.
So, this will work and is an improvement:
ip, port, empty
# time passes
ip, port, HTTP
But it may also log
ip, port, HTTP
# time passes
ip, port, empty
To fix that it would need to keep track of a separate (ip, port) set that had a non empty service detected. Once something like HTTP was detected the (ip, port) would be added, and then it would skip logging (ip, port, empty)
Comment by Seth Hall at 2016-02-17T11:22:43.141-0600:
What if you change known_services to...
table[addr, port] of set[string]
Then you could avoid the case you laid out here, which I suspect in production would be pretty annoying.
Comment by Justin Azoff at 2016-02-17T14:19:35.172-0600:
Ok, I implemented that. I'm working on generating some pcaps for these test cases. One thing I think i ran into is I think this check (that was in the older version too) is not quite right:
# Handle the connection ending in case no protocol was ever detected.
event connection_state_remove(c: connection) &priority=-5
{
if (c$resp$state == TCP_ESTABLISHED )
event known_services_done(c);
}
It seems (in my testing) that TCP_CLOSED occurs much more often. I think this check intends to filter connections that completed a handshake, but is not really doing so. I honestly have no idea why a connection would be TCP_ESTABLISHED inside connection_state_removed.
This is a similar issue as in BIT-1535.. how to determine if a connection succeeded.
Vlad also mentioned that starting a timer for each protocol confirmation might not be the best idea. We could rely 100% on connection_state_removed if we did not mind delaying a known_service log entry until the connection closed. I think if I can fix that if statement, the timers could be removed and the entire thing could be greatly simplified.
Vlad also brought up something I was thinking about as well that c$service should probably be an ordered vector.. since now that I fixed the multiple-service-logging issue you see things like
FTP,SSL,gridftp
gridftp,FTP,SSL
SSL,gridftp,FTP
or
SSL,SMTP
but i believe chronologically they should all be
Comment by Adam Slagell at 2016-05-06T14:57:52.439-0500:
This is close enough we should push out the partial fix for 2.5
Comment by Robin Sommer at 2016-06-17T13:49:26.900-0500:
Will change quite a bit anyways with Broker integration.
Comment by Adam Slagell at 2016-06-22T10:14:12.701-0500:
Status Justin?
Comment by Justin Azoff at 2016-06-22T10:26:38.701-0500:
I believe we decided in the meeting last week that even though what I have is an improvement, since all of the known_* scripts need to be rewritten for broker it probably isn't worth changing a bunch of stuff for 2.5, only to then change it again for the next release. I will focus more on getting all the known_ scripts updated and fixing the bugs across the board.
Comment by Adam Slagell at 2016-06-22T10:37:34.730-0500:
Ah, I was not at that meeting. So are we doing anything for 2.5 here?
Comment by Adam Slagell at 2016-09-28T06:56:58.614-0500:
Justin, you fixed some of this for 2.5, but it seems like there is a broader issue than gridftp. Should this ticket be closed out, and a new one opened up for the 2.6 known_* script rewrite?
Comment by Justin Azoff at 2016-09-28T07:24:34.010-0500:
Yeah, all the known scripts will need a full rewrite once broker is integrated. I have some ideas on how to write a 'known' framework to make logging things once and only once easier.
Comment by Justin Azoff at 2017-02-21T12:53:46.306-0600:
It looks like that script is just broken. The main software script that logs new software versions does:
ts[info$name] = info;
Log::write(Software::LOG, info);
and then the version changes script is doing
But at that point old and rec are the same exact thing. It's possible to fix this, it just can't use the log_software event because at that point the "old" version has already been overwritten.
Another issue with the script is that the 'tracked' variable has a create expire of only 24h, so if the server version is only seen every 48 hours, or if bro is restarted it won't know the version changed.
Newer features in Broker should allow interesting version changes to be tracked using persistent data stores. That would really fix the issue. There are similar things that need to be re-written for better tracking known hosts/known services/known certs.
On Feb 21, 2017, at 12:50 PM, fatema bannatwala <[email protected]> wrote:
I was going through the version-changes.bro script, thinking of adding some software
to track the version changes, but realized that there is no comparison done between the
old version tracked and the version detected in "rec: Info" of log_software event.
Hence, was thinking to add a condition to check it before the notice is raised for the version
change, like following:
( or I might be missing something regarding the functionality of the script. :/)
event log_software(rec: Info)
{
local ts = tracked[rec$host];
if ( rec$name in ts )
{
local old = ts[rec$name];
# Is it a potentially interesting version change?
if ( rec$name in interesting_version_changes )
{
if (software_fmt_version(old$version) != software_fmt_version(rec$version))
{ local msg = fmt("%.6f %s switched from %s to %s (%s)",
network_time(), rec$software_type,
software_fmt_version(old$version),
software_fmt(rec), rec$software_type);
NOTICE([$note=Software_Version_Change, $src=rec$host,
$msg=msg, $sub=software_fmt(rec)]);
}
}
}
}
Any thoughts? anybody using this script to track software changes?
Comment by Jon Siwek at 2018-08-14T11:08:21.264-0500:
I removed the 2.6 tag from this since it almost seems out of scope now, but still relevant later. i.e. first step of porting to Broker for 2.6 is done, but next step for a later release is to consider doing any of the changes that Justin was had worked on. Or let me know if there's strong opinion on still needing to do any changes for 2.6.
Comment by Justin Azoff at 2018-08-14T12:06:53.846-0500:
I don't know about the timeline, but known-services still needs fixing for multi-protocol services.
topic/jazoff/ticket1521 still has the pre-broker fixed known-services.bro along with a testing/btest/Traces/ssl-and-ssh-using-sslh.trace test pcap.
I didn't try changing c$service to a vector though, it's probably the right thing to do but that may break a bunch of other scripts.
It looks like you already fixed the software version changes script at some point as part of the broker work.