diff --git a/CHANGES b/CHANGES index 73c1788832..d0dcf69713 100644 --- a/CHANGES +++ b/CHANGES @@ -1,4 +1,21 @@ +2.2-213 | 2014-03-09 08:57:37 -0700 + + * No longer accidentally attempting to parse NBSTAT RRs as SRV RRs + in DNS analyzer. (Seth Hall) + + * Fix DNS SRV responses and a small issue with NBNS queries and + label length. (Seth Hall) + + - DNS SRV responses never had the code written to actually + generate the dns_SRV_reply event. Adding this required + extending the event a bit to add extra information. SRV responses + now appear in the dns.log file correctly. + + - Fixed an issue where some Microsoft NetBIOS Name Service lookups + would exceed the max label length for DNS and cause an incorrect + "DNS_label_too_long" weird. + 2.2-210 | 2014-03-06 22:52:36 -0500 * Improve SSL logging so that connections are logged even when the diff --git a/NEWS b/NEWS index 54ee916d5b..1c7cf66f1c 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,9 @@ New Functionality parsing past the GRE header in between the delivery and payload IP packets. +- The DNS analyzer now actually generates the dns_SRV_reply() event. + It had been documented before, yet was never raised. + Changed Functionality --------------------- diff --git a/VERSION b/VERSION index b997c5d8db..d8c47c6b7b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -2.2-210 +2.2-213 diff --git a/scripts/base/protocols/dns/main.bro b/scripts/base/protocols/dns/main.bro index 294220d1f2..348c361311 100644 --- a/scripts/base/protocols/dns/main.bro +++ b/scripts/base/protocols/dns/main.bro @@ -360,7 +360,15 @@ event dns_request(c: connection, msg: dns_msg, query: string, qtype: count, qcla # Note: I'm ignoring the name type for now. Not sure if this should be # worked into the query/response in some fashion. if ( c$id$resp_p == 137/udp ) + { query = decode_netbios_name(query); + if ( c$dns$qtype_name == "SRV" ) + { + # The SRV RFC used the ID used for NetBios Status RRs. + # So if this is NetBios Name Service we name it correctly. + c$dns$qtype_name = "NBSTAT"; + } + } c$dns$query = query; } @@ -421,9 +429,9 @@ event dns_WKS_reply(c: connection, msg: dns_msg, ans: dns_answer) &priority=5 hook DNS::do_reply(c, msg, ans, ""); } -event dns_SRV_reply(c: connection, msg: dns_msg, ans: dns_answer) &priority=5 +event dns_SRV_reply(c: connection, msg: dns_msg, ans: dns_answer, target: string, priority: count, weight: count, p: count) &priority=5 { - hook DNS::do_reply(c, msg, ans, ""); + hook DNS::do_reply(c, msg, ans, target); } # TODO: figure out how to handle these diff --git a/src/analyzer/protocol/dns/DNS.cc b/src/analyzer/protocol/dns/DNS.cc index b17a90dd61..a97c7b8632 100644 --- a/src/analyzer/protocol/dns/DNS.cc +++ b/src/analyzer/protocol/dns/DNS.cc @@ -208,6 +208,7 @@ int DNS_Interpreter::ParseAnswer(DNS_MsgInfo* msg, int name_len = sizeof(name) - 1; u_char* name_end = ExtractName(data, len, name, name_len, msg_start); + if ( ! name_end ) return 0; @@ -275,7 +276,17 @@ int DNS_Interpreter::ParseAnswer(DNS_MsgInfo* msg, break; case TYPE_SRV: - status = ParseRR_SRV(msg, data, len, rdlength, msg_start); + if ( ntohs(analyzer->Conn()->RespPort()) == 137 ) + { + // This is an NBSTAT (NetBIOS NODE STATUS) record. + // The SRV RFC reused the value that was already being + // used for this. + // We aren't parsing this yet. + status = 1; + } + else + status = ParseRR_SRV(msg, data, len, rdlength, msg_start); + break; case TYPE_EDNS: @@ -400,7 +411,9 @@ int DNS_Interpreter::ExtractLabel(const u_char*& data, int& len, return 0; } - if ( label_len > 63 ) + if ( label_len > 63 && + // NetBIOS name service look ups can use longer labels. + ntohs(analyzer->Conn()->RespPort()) != 137 ) { analyzer->Weird("DNS_label_too_long"); return 0; @@ -633,15 +646,24 @@ int DNS_Interpreter::ParseRR_SRV(DNS_MsgInfo* msg, u_char* name_end = ExtractName(data, len, name, name_len, msg_start); if ( ! name_end ) return 0; - *name_end = 0; // terminate name so we can use it in snprintf() if ( data - data_start != rdlength ) analyzer->Weird("DNS_RR_length_mismatch"); - // The following is just a placeholder. - char buf[2048]; - safe_snprintf(buf, sizeof(buf), "SRV %s priority=%d weight=%d port=%d", - name, priority, weight, port); + if ( dns_SRV_reply && ! msg->skip_event ) + { + val_list* vl = new val_list; + vl->append(analyzer->BuildConnVal()); + vl->append(msg->BuildHdrVal()); + vl->append(msg->BuildAnswerVal()); + vl->append(new StringVal(new BroString(name, name_end - name, 1))); + vl->append(new Val(priority, TYPE_COUNT)); + vl->append(new Val(weight, TYPE_COUNT)); + vl->append(new Val(port, TYPE_COUNT)); + + analyzer->ConnectionEvent(dns_SRV_reply, vl); + } + return 1; } diff --git a/src/analyzer/protocol/dns/events.bif b/src/analyzer/protocol/dns/events.bif index b43ac95f66..520e32fe6d 100644 --- a/src/analyzer/protocol/dns/events.bif +++ b/src/analyzer/protocol/dns/events.bif @@ -392,6 +392,12 @@ event dns_TXT_reply%(c: connection, msg: dns_msg, ans: dns_answer, str: string%) ## ## ans: The type-independent part of the parsed answer record. ## +## priority: Priority of the SRV response. +## +## weight: Weight of the SRV response. +## +## p: Port of the SRV response. +## ## .. 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_end dns_full_request @@ -399,7 +405,7 @@ event dns_TXT_reply%(c: connection, msg: dns_msg, ans: dns_answer, str: string%) ## dns_mapping_unverified dns_mapping_valid dns_message dns_query_reply ## dns_rejected dns_request non_dns_request dns_max_queries dns_session_timeout ## 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%); +event dns_SRV_reply%(c: connection, msg: dns_msg, ans: dns_answer, target: string, priority: count, weight: count, p: count%); ## 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