From 0e0e74e49c01ab867d430d21c149bdf27915b0c3 Mon Sep 17 00:00:00 2001 From: Jon Siwek Date: Tue, 28 Jan 2014 11:04:01 -0600 Subject: [PATCH] Improve DNS analysis. - Fix parsing of empty question sections (when QDCOUNT == 0). In this case, the DNS parser would extract two 2-byte fields for use in either "dns_query_reply" or "dns_rejected" events (dependent on value of RCODE) as qclass and qtype parameters. This is not correct, because such fields don't actually exist in the DNS message format when QDCOUNT is 0. As a result, these events are no longer raised when there's an empty question section. Scripts that depends on checking for an empty question section can do that in the "dns_message" event. - Add a new "dns_unknown_reply" event, for when Bro does not know how to fully parse a particular resource record type. This helps fix a problem in the default DNS scripts where the logic to complete request-reply pair matching doesn't work because it's waiting on more RR events to complete the reply. i.e. it expects ANCOUNT number of dns_*_reply events and will wait until it gets that many before completing a request-reply pair and logging it to dns.log. This could cause bogus replies to match a previous request if they happen to share a DNS transaction ID. --- scripts/base/protocols/dns/main.bro | 27 +++++++++++++++---- src/analyzer/protocol/dns/DNS.cc | 22 +++++++-------- src/analyzer/protocol/dns/events.bif | 20 ++++++++++++-- .../btest-doc.sphinx.using_bro#1 | 4 +-- .../conn.select | 4 +-- .../dns.log | 6 ++--- .../dns.log | 8 +++--- 7 files changed, 61 insertions(+), 30 deletions(-) diff --git a/scripts/base/protocols/dns/main.bro b/scripts/base/protocols/dns/main.bro index 0d23029ad7..f3f19d488c 100644 --- a/scripts/base/protocols/dns/main.bro +++ b/scripts/base/protocols/dns/main.bro @@ -158,12 +158,17 @@ hook set_session(c: connection, msg: dns_msg, is_query: bool) &priority=5 # If this is either a query or this is the reply but # no Info records are in the queue (we missed the query?) # we need to create an Info record and put it in the queue. - if ( is_query || - Queue::len(c$dns_state$pending[msg$id]) == 0 ) + if ( is_query ) { info = new_session(c, msg$id); Queue::put(c$dns_state$pending[msg$id], info); } + else if ( Queue::len(c$dns_state$pending[msg$id]) == 0 ) + { + info = new_session(c, msg$id); + Queue::put(c$dns_state$pending[msg$id], info); + event conn_weird("dns_unmatched_reply", c, ""); + } if ( is_query ) # If this is a query, assign the newly created info variable @@ -202,17 +207,23 @@ hook set_session(c: connection, msg: dns_msg, is_query: bool) &priority=5 event dns_message(c: connection, is_orig: bool, msg: dns_msg, len: count) &priority=5 { hook set_session(c, msg, is_orig); + + if ( msg$QR && msg$rcode != 0 && msg$num_queries == 0 ) + c$dns$rejected = T; } event DNS::do_reply(c: connection, msg: dns_msg, ans: dns_answer, reply: string) &priority=5 { + if ( ! msg$QR ) + # This is weird: the inquirer must also be providing answers in + # the request, which is not what we want to track. + return; + if ( ans$answer_type == DNS_ANS ) { if ( ! c?$dns ) - { - event conn_weird("dns_unmatched_reply", c, ""); hook set_session(c, msg, F); - } + c$dns$AA = msg$AA; c$dns$RA = msg$RA; @@ -265,6 +276,12 @@ event dns_request(c: connection, msg: dns_msg, query: string, qtype: count, qcla c$dns$query = query; } + +event dns_unknown_reply(c: connection, msg: dns_msg, ans: dns_answer) &priority=5 + { + event DNS::do_reply(c, msg, ans, fmt("", ans$qtype)); + } + event dns_A_reply(c: connection, msg: dns_msg, ans: dns_answer, a: addr) &priority=5 { event DNS::do_reply(c, msg, ans, fmt("%s", a)); diff --git a/src/analyzer/protocol/dns/DNS.cc b/src/analyzer/protocol/dns/DNS.cc index 806cb9ae75..b17a90dd61 100644 --- a/src/analyzer/protocol/dns/DNS.cc +++ b/src/analyzer/protocol/dns/DNS.cc @@ -137,18 +137,6 @@ int DNS_Interpreter::ParseQuestions(DNS_MsgInfo* msg, { int n = msg->qdcount; - if ( n == 0 ) - { - // Generate event here because we won't go into ParseQuestion. - EventHandlerPtr dns_event = - msg->rcode == DNS_CODE_OK ? - dns_query_reply : dns_rejected; - BroString* question_name = new BroString(""); - - SendReplyOrRejectEvent(msg, dns_event, data, len, question_name); - return 1; - } - while ( n > 0 && ParseQuestion(msg, data, len, msg_start) ) --n; return n == 0; @@ -299,6 +287,16 @@ int DNS_Interpreter::ParseAnswer(DNS_MsgInfo* msg, break; default: + + if ( dns_unknown_reply && ! msg->skip_event ) + { + val_list* vl = new val_list; + vl->append(analyzer->BuildConnVal()); + vl->append(msg->BuildHdrVal()); + vl->append(msg->BuildAnswerVal()); + analyzer->ConnectionEvent(dns_unknown_reply, vl); + } + analyzer->Weird("DNS_RR_unknown_type"); data += rdlength; len -= rdlength; diff --git a/src/analyzer/protocol/dns/events.bif b/src/analyzer/protocol/dns/events.bif index 95c604a8b8..b43ac95f66 100644 --- a/src/analyzer/protocol/dns/events.bif +++ b/src/analyzer/protocol/dns/events.bif @@ -50,7 +50,7 @@ event dns_message%(c: connection, is_orig: bool, msg: dns_msg, len: count%); event dns_request%(c: connection, msg: dns_msg, query: string, qtype: count, qclass: count%); ## Generated for DNS replies that reject a query. This event is raised if a DNS -## reply either indicates failure via its status code or does not pass on any +## reply indicates failure because it does not pass on any ## answers to a query. Note that all of the event's parameters are parsed out of ## the reply; there's no stateful correlation with the query. ## @@ -78,7 +78,7 @@ event dns_request%(c: connection, msg: dns_msg, query: string, qtype: count, qcl ## dns_skip_all_addl dns_skip_all_auth dns_skip_auth event dns_rejected%(c: connection, msg: dns_msg, query: string, qtype: count, qclass: count%); -## Generated for DNS replies with an *ok* status code but no question section. +## Generated for each entry in the Question section of a DNS reply. ## ## See `Wikipedia `__ for more ## information about the DNS protocol. Bro analyzes both UDP and TCP DNS @@ -401,6 +401,22 @@ event dns_TXT_reply%(c: connection, msg: dns_msg, ans: dns_answer, str: string%) ## dns_skip_addl dns_skip_all_addl dns_skip_all_auth dns_skip_auth event dns_SRV_reply%(c: connection, msg: dns_msg, ans: dns_answer%); +## Generated on DNS reply resource records when the type of record is not one +## that Bro knows how to parse and generate another more specific specific +## event. +## +## c: The connection, which may be UDP or TCP depending on the type of the +## transport-layer session being analyzed. +## +## msg: The parsed DNS message header. +## +## ans: The type-independent part of the parsed answer record. +## +## .. bro:see:: dns_AAAA_reply dns_A_reply dns_CNAME_reply dns_EDNS_addl +## dns_HINFO_reply dns_MX_reply dns_NS_reply dns_PTR_reply dns_SOA_reply +## dns_TSIG_addl dns_TXT_reply dns_WKS_reply dns_SRV_reply dns_end +event dns_unknown_reply%(c: connection, msg: dns_msg, ans: dns_answer%); + ## Generated for DNS replies of type *EDNS*. For replies with multiple answers, ## an individual event of the corresponding type is raised for each. ## diff --git a/testing/btest/Baseline/doc.sphinx.using_bro/btest-doc.sphinx.using_bro#1 b/testing/btest/Baseline/doc.sphinx.using_bro/btest-doc.sphinx.using_bro#1 index 65c802ccf2..53bcb5581d 100644 --- a/testing/btest/Baseline/doc.sphinx.using_bro/btest-doc.sphinx.using_bro#1 +++ b/testing/btest/Baseline/doc.sphinx.using_bro/btest-doc.sphinx.using_bro#1 @@ -20,8 +20,8 @@ #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto service duration orig_bytes resp_bytes conn_state local_orig missed_bytes history orig_pkts orig_ip_bytes resp_pkts resp_ip_bytes tunnel_parents #types time string addr port addr port enum string interval count count string bool count string count count count count table[string] 1300475167.096535 CXWv6p3arKYeMETxOg 141.142.220.202 5353 224.0.0.251 5353 udp dns - - - S0 - 0 D 1 73 0 0 (empty) - 1300475167.097012 CjhGID4nQcgTWjvg4c fe80::217:f2ff:fed7:cf65 5353 ff02::fb 5353 udp - - - - S0 - 0 D 1 199 0 0 (empty) - 1300475167.099816 CCvvfg3TEfuqmmG4bh 141.142.220.50 5353 224.0.0.251 5353 udp - - - - S0 - 0 D 1 179 0 0 (empty) + 1300475167.097012 CjhGID4nQcgTWjvg4c fe80::217:f2ff:fed7:cf65 5353 ff02::fb 5353 udp dns - - - S0 - 0 D 1 199 0 0 (empty) + 1300475167.099816 CCvvfg3TEfuqmmG4bh 141.142.220.50 5353 224.0.0.251 5353 udp dns - - - S0 - 0 D 1 179 0 0 (empty) 1300475168.853899 CPbrpk1qSsw6ESzHV4 141.142.220.118 43927 141.142.2.2 53 udp dns 0.000435 38 89 SF - 0 Dd 1 66 1 117 (empty) 1300475168.854378 C6pKV8GSxOnSLghOa 141.142.220.118 37676 141.142.2.2 53 udp dns 0.000420 52 99 SF - 0 Dd 1 80 1 127 (empty) 1300475168.854837 CIPOse170MGiRM1Qf4 141.142.220.118 40526 141.142.2.2 53 udp dns 0.000392 38 183 SF - 0 Dd 1 66 1 211 (empty) diff --git a/testing/btest/Baseline/scripts.base.frameworks.logging.sqlite.wikipedia/conn.select b/testing/btest/Baseline/scripts.base.frameworks.logging.sqlite.wikipedia/conn.select index bdae1a8f73..8653fd1edb 100644 --- a/testing/btest/Baseline/scripts.base.frameworks.logging.sqlite.wikipedia/conn.select +++ b/testing/btest/Baseline/scripts.base.frameworks.logging.sqlite.wikipedia/conn.select @@ -1,6 +1,6 @@ 1300475167.09653|CXWv6p3arKYeMETxOg|141.142.220.202|5353|224.0.0.251|5353|udp|dns||||S0||0|D|1|73|0|0|(empty) -1300475167.09701|CjhGID4nQcgTWjvg4c|fe80::217:f2ff:fed7:cf65|5353|ff02::fb|5353|udp|||||S0||0|D|1|199|0|0|(empty) -1300475167.09982|CCvvfg3TEfuqmmG4bh|141.142.220.50|5353|224.0.0.251|5353|udp|||||S0||0|D|1|179|0|0|(empty) +1300475167.09701|CjhGID4nQcgTWjvg4c|fe80::217:f2ff:fed7:cf65|5353|ff02::fb|5353|udp|dns||||S0||0|D|1|199|0|0|(empty) +1300475167.09982|CCvvfg3TEfuqmmG4bh|141.142.220.50|5353|224.0.0.251|5353|udp|dns||||S0||0|D|1|179|0|0|(empty) 1300475168.652|CsRx2w45OKnoww6xl4|141.142.220.118|35634|208.80.152.2|80|tcp||0.0613288879394531|463|350|OTH||0|DdA|2|567|1|402|(empty) 1300475168.72401|CRJuHdVW0XPVINV8a|141.142.220.118|48649|208.80.152.118|80|tcp|http|0.1199049949646|525|232|S1||0|ShADad|4|741|3|396|(empty) 1300475168.8539|CPbrpk1qSsw6ESzHV4|141.142.220.118|43927|141.142.2.2|53|udp|dns|0.000435113906860352|38|89|SF||0|Dd|1|66|1|117|(empty) diff --git a/testing/btest/Baseline/scripts.base.protocols.dns.dns-key/dns.log b/testing/btest/Baseline/scripts.base.protocols.dns.dns-key/dns.log index 35289b82dd..76e83452e5 100644 --- a/testing/btest/Baseline/scripts.base.protocols.dns.dns-key/dns.log +++ b/testing/btest/Baseline/scripts.base.protocols.dns.dns-key/dns.log @@ -3,8 +3,8 @@ #empty_field (empty) #unset_field - #path dns -#open 2013-08-26-19-04-08 +#open 2014-01-28-14-55-04 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto trans_id query qclass qclass_name qtype qtype_name rcode rcode_name AA TC RD RA Z answers TTLs rejected #types time string addr port addr port enum count string count string count string count string bool bool bool bool count vector[string] vector[interval] bool -1359565680.761790 CXWv6p3arKYeMETxOg 192.168.6.10 53209 192.168.129.36 53 udp 41477 paypal.com 1 C_INTERNET 48 DNSKEY 0 NOERROR F F T F 1 - - F -#close 2013-08-26-19-04-08 +1359565680.761790 CXWv6p3arKYeMETxOg 192.168.6.10 53209 192.168.129.36 53 udp 41477 paypal.com 1 C_INTERNET 48 DNSKEY 0 NOERROR F F T T 1 ,,, 455.000000,455.000000,455.000000,455.000000 F +#close 2014-01-28-14-55-04 diff --git a/testing/btest/Baseline/scripts.base.protocols.dns.duplicate-reponses/dns.log b/testing/btest/Baseline/scripts.base.protocols.dns.duplicate-reponses/dns.log index 6af017fa49..6e2a0a4699 100644 --- a/testing/btest/Baseline/scripts.base.protocols.dns.duplicate-reponses/dns.log +++ b/testing/btest/Baseline/scripts.base.protocols.dns.duplicate-reponses/dns.log @@ -3,9 +3,9 @@ #empty_field (empty) #unset_field - #path dns -#open 2013-08-26-19-04-08 +#open 2014-01-28-14-58-56 #fields ts uid id.orig_h id.orig_p id.resp_h id.resp_p proto trans_id query qclass qclass_name qtype qtype_name rcode rcode_name AA TC RD RA Z answers TTLs rejected #types time string addr port addr port enum count string count string count string count string bool bool bool bool count vector[string] vector[interval] bool -1363716396.798072 CXWv6p3arKYeMETxOg 55.247.223.174 27285 222.195.43.124 53 udp 21140 www.cmu.edu 1 C_INTERNET 1 A 0 NOERROR T F F F 1 www-cmu.andrew.cmu.edu,www-cmu-2.andrew.cmu.edu,128.2.10.163,www-cmu.andrew.cmu.edu 86400.000000,5.000000,21600.000000,86400.000000 F -1363716396.798374 CXWv6p3arKYeMETxOg 55.247.223.174 27285 222.195.43.124 53 udp 21140 - - - - - 0 NOERROR T F F F 0 www-cmu-2.andrew.cmu.edu,128.2.10.163 5.000000,21600.000000 F -#close 2013-08-26-19-04-08 +1363716396.798072 CXWv6p3arKYeMETxOg 55.247.223.174 27285 222.195.43.124 53 udp 21140 www.cmu.edu 1 C_INTERNET 1 A 0 NOERROR T F F F 1 www-cmu.andrew.cmu.edu,,www-cmu-2.andrew.cmu.edu,128.2.10.163 86400.000000,86400.000000,5.000000,21600.000000 F +1363716396.798374 CXWv6p3arKYeMETxOg 55.247.223.174 27285 222.195.43.124 53 udp 21140 - - - - - 0 NOERROR T F F F 0 www-cmu.andrew.cmu.edu,,www-cmu-2.andrew.cmu.edu,128.2.10.163 86400.000000,86400.000000,5.000000,21600.000000 F +#close 2014-01-28-14-58-56