DNS over TCP does not use/validate the length header
Phillipmartin opened this issue · 4 comments
DNS over TCP includes a length header that we should use to ensure we have all the data packets before we parse and log the entries.
https://github.com/google/gopacket/blob/master/examples/httpassembly/main.go has some examples of HTTP stream re-assembly. It seems a little heavy weight, especially as we're already doing a lot of the state tracking stuff in conntable.
So, if we do this with the gopacket tcpreassemble stuff, we're really going to re-write https://github.com/Phillipmartin/gopassivedns/blob/master/main.go#L277 and the follow-on clauses to essentially be:
if TCP:
reassemble(TCP)
and then implement a custom dnsStream and Factory a la https://github.com/google/gopacket/blob/master/examples/httpassembly/main.go. The problem is that the reassembled packet appears in the dnsStream instance, not in handlePacket. in a perfect world, I could hand a channel to the reassemble stuff, which would then pass it on to the various places that actually see the reassembled data...but that isn't a thing with gopacket as it stands. If we could do something like that, I'd refactor the main doCapture loop to use a select and bounce between reading packets and reassembled data. Maybe we can do that with a, ugh, global channel for the reassemble callbacks to shove the reassembled data into...although maybe that is actually the right choice here. I will play around with that idea.
initial refactor for support of this feature was just pushed to https://github.com/Phillipmartin/gopassivedns/tree/feature/tcp_dns (commit 20bed65). I also added some processing benchmarks so I can track how these changes impact processing times.
done and merged to master in 33637c9