DNS TXT Queries and the Cache File

Development
1

Hello,

I'm working on wrapping up some code for adding DNS TXT query support to Bro. I have code that works, I'm just doing a final review to make sure the support has been added in everywhere it needs to be.

The other DNS query types (host and address) support saving the cached queries/responses to and from a DNS cache file. While I would like to see this added for completeness, it would mean making larger-scale changes, as a couple classes would need to be tweaked as well.

Is this something that people think is necessary? Should I throw up the code I have for now, and hopefully it'll get added down the road - either by myself, or the next poor soul to dive into the DNS code :-).

Thanks,

  --Vlad

2

As the previous poor soul to touch that code, I wouldn't mind looking at what you've got so far and then attempting to add the caching support.

     Jon

3

Cool, thanks for working on this, Vlad.

As the previous poor soul to touch that code, I wouldn't mind looking at
what you've got so far and then attempting to add the caching support.

If the caching is trikcy to get in (or makes the code even worse ...),
we can indeed skip it. The main reason for having the caching at all
is DNS names embedded in scripts (e.g., code of the form "set[addr] =
{ foo.bar }"). Bro looks these up once at startup and that can
potentially take a while if there are a lot or responses are coming in
slowly. So what one can do is "prime" the cache first, so that the
next time Bro starts up, it doesn't need to do the lookups. That was
more important in the Old Days though when people restarted Bro once a
day to flush state and that had to be fast.

This is all not relevant to TXT records. And, in fact, I've already
been wondering if we can get rid of the cache altogether to simplify
the DNS code.

Robin

4

Sorry for the huge delay in getting this out there - it just fell on the back burner.

I've put my code up at <https://github.com/grigorescu/bro/tree/topic/vladg/dns_txt_queries&gt;\. The changes weren't terribly significant. It adds lookup_hostname_txt:

when (local result = lookup_hostname_txt("733a48a9cb49651d72fe824ca91e8d00.malware.hash.cymru.com"))
    print result;

Please let me know if anyone sees any issues. There is a save TXT function, but there is no capability to read the data back from a file, as I mentioned. If someone wants to take a stab to getting that working properly, please feel free. Otherwise, let me know and I'll remove the save function.

Thanks,

  --Vlad

5

Thanks, I've merged this in, can you try master and see if it works
for you?

e let me know if anyone sees any issues. There is a save TXT
function, but there is no capability to read the data back from a
file, as I mentioned. If someone wants to take a stab to getting that
working properly, please feel free. Otherwise, let me know and I'll
remove the save function.

I've removed it, don't think we need it. If we wanted to bring it
back, that's easy to do.

Robin

6

Thanks, Robin.

I tested master, and it wasn't working properly for me. I traced the problem to the simplification of DNS_Mgr::AsyncLookupName. For both lookups, it was using AsyncRequestNameMap. I split off the processing of TXT queries so that it'd use AsyncRequestTextMap. That was committed in: <https://github.com/grigorescu/bro/commit/a1c0b853fe180ee0dc97880653d2d274b7e08e21>.

I also have a couple of other small commits, which are ready in <https://github.com/grigorescu/bro/commits/topic/vladg/dns_txt_queries>. One has some fixes for the BIF, and one switches the MHR script to use TXT queries, with a newly introduced threshold.

I tested these manually, and they work for me. Unfortunately, I don't have any good ideas on how we'd go about writing some tests for the TXT queries - this was the one case where being able to load the cache from a file did seem useful.

  --Vlad

7

I tested master, and it wasn't working properly for me. I traced the
problem to the simplification of DNS_Mgr::AsyncLookupName.

Ah, I wasn't looking right it seems. With that, joining the functions
actually doesn't look like a good change, so I'm going to move this
back to your original implementation. Sorry about the confusion.

on how we'd go about writing some tests for the TXT queries - this was
the one case where being able to load the cache from a file did seem
useful.

Yeah, but even that would only test a part of the chain, not the
requests themselves.

Robin