Author: zwoop
Date: Thu Aug 26 16:41:40 2010
New Revision: 989815
URL: http://svn.apache.org/viewvc?rev=989815&view=rev
Log:
TS-425 Improve verification of DNS response packets.
Reviewed: Vijay + Steve + other in the community
Modified:
trafficserver/traffic/trunk/iocore/dns/DNS.cc
trafficserver/traffic/trunk/proxy/config/records.config.in
trafficserver/traffic/trunk/proxy/mgmt2/RecordsConfig.cc
Modified: trafficserver/traffic/trunk/iocore/dns/DNS.cc
URL:
http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/dns/DNS.cc?rev=989815&r1=989814&r2=989815&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/dns/DNS.cc (original)
+++ trafficserver/traffic/trunk/iocore/dns/DNS.cc Thu Aug 26 16:41:40 2010
@@ -44,6 +44,7 @@ int dns_failover_number = DEFAULT_FAILOV
int dns_failover_period = DEFAULT_FAILOVER_PERIOD;
int dns_failover_try_period = DEFAULT_FAILOVER_TRY_PERIOD;
int dns_max_dns_in_flight = MAX_DNS_IN_FLIGHT;
+int dns_validate_qname = 0;
unsigned int dns_sequence_number = 0;
unsigned int dns_handler_initialized = 0;
int dns_ns_rr = 0;
@@ -117,6 +118,7 @@ DNSProcessor::start(int)
IOCORE_EstablishStaticConfigInt32(dns_failover_number,
"proxy.config.dns.failover_number");
IOCORE_EstablishStaticConfigInt32(dns_failover_period,
"proxy.config.dns.failover_period");
IOCORE_EstablishStaticConfigInt32(dns_max_dns_in_flight,
"proxy.config.dns.max_dns_in_flight");
+ IOCORE_EstablishStaticConfigInt32(dns_validate_qname,
"proxy.config.dns.validate_query_name");
IOCORE_EstablishStaticConfigInt32(dns_ns_rr,
"proxy.config.dns.round_robin_nameservers");
IOCORE_ReadConfigStringAlloc(dns_ns_list, "proxy.config.dns.nameservers");
@@ -1267,15 +1269,42 @@ dns_process(DNSHandler * handler, HostEn
int n;
unsigned char *srv[50];
int num_srv = 0;
+ int rname_len = -1;
//
// Expand name
//
if ((n = ink_dn_expand((u_char *) h, eom, cp, bp, buflen)) < 0)
goto Lerror;
+
+ // Should we validate the query name?
+ if (dns_validate_qname) {
+ int qlen = e->qname_len;
+ int rlen = strlen((char *)bp);
+
+ rname_len = rlen; // Save for later use
+ if ((qlen > 0) && ('.' == e->qname[qlen-1]))
+ --qlen;
+ if ((rlen > 0) && ('.' == bp[rlen-1]))
+ --rlen;
+ // TODO: At some point, we might want to care about the case here, and
use an algorithm
+ // to randomly pick upper case characters in the query, and validate the
response with
+ // case sensitivity.
+ if ((qlen != rlen) || (strncasecmp(e->qname, (const char*)bp, qlen) !=
0)) {
+ // Bad mojo, forged?
+ Warning("received DNS response with query name of %s, but response
query name is %s", e->qname, bp);
+ goto Lerror;
+ } else {
+ Debug("dns", "query name validated properly for %s", e->qname);
+ }
+ }
+
cp += n + QFIXEDSZ;
if (e->qtype == T_A) {
- n = strlen((char *) bp) + 1;
+ if (-1 == rname_len)
+ n = strlen((char *)bp) + 1;
+ else
+ n = rname_len + 1;
buf->ent.h_name = (char *) bp;
bp += n;
buflen -= n;
@@ -1296,6 +1325,8 @@ dns_process(DNSHandler * handler, HostEn
// Once it's full, a new entry get inputted into try_server_names round-
// robin style every 50 success dns response.
+ // TODO: Why do we do strlen(e->qname) ? That should be available in
+ // e->qname_len, no ?
if (local_num_entries >= DEFAULT_NUM_TRY_SERVER) {
if ((attempt_num_entries % 50) == 0) {
try_servers = (try_servers + 1) % SIZE(try_server_names);
@@ -1503,24 +1534,6 @@ ink_dns_init(ModuleVersion v)
// create a stat block for HostDBStats
dns_rsb = RecAllocateRawStatBlock((int) DNS_Stat_Count);
- IOCORE_RegisterConfigInteger(RECT_CONFIG, "proxy.config.dns.retries", 3,
RECU_DYNAMIC, RECC_NULL, NULL);
-
- IOCORE_RegisterConfigInteger(RECT_CONFIG, "proxy.config.dns.lookup_timeout",
30, RECU_DYNAMIC, RECC_NULL, NULL);
-
- IOCORE_RegisterConfigInteger(RECT_CONFIG,
- "proxy.config.dns.search_default_domains", 1,
RECU_DYNAMIC, RECC_NULL, NULL);
-
- IOCORE_RegisterConfigInteger(RECT_CONFIG,
"proxy.config.dns.failover_number", 4, RECU_DYNAMIC, RECC_NULL, NULL);
-
- IOCORE_RegisterConfigInteger(RECT_CONFIG,
"proxy.config.dns.failover_period", 60, RECU_DYNAMIC, RECC_NULL, NULL);
-
- IOCORE_RegisterConfigInteger(RECT_CONFIG,
"proxy.config.dns.max_dns_in_flight", 60, RECU_DYNAMIC, RECC_NULL, NULL);
-
- IOCORE_RegisterConfigInteger(RECT_CONFIG,
- "proxy.config.dns.round_robin_nameservers", 0,
RECU_DYNAMIC, RECC_NULL, NULL);
-
- IOCORE_RegisterConfigString(RECT_CONFIG, "proxy.config.dns.nameservers",
NULL, RECU_DYNAMIC, RECC_NULL, NULL);
-
//
// Register statistics callbacks
//
Modified: trafficserver/traffic/trunk/proxy/config/records.config.in
URL:
http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/config/records.config.in?rev=989815&r1=989814&r2=989815&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/config/records.config.in (original)
+++ trafficserver/traffic/trunk/proxy/config/records.config.in Thu Aug 26
16:41:40 2010
@@ -349,6 +349,10 @@ CONFIG proxy.config.dns.splitdns.def_dom
CONFIG proxy.config.dns.url_expansions STRING NULL
CONFIG proxy.config.dns.round_robin_nameservers INT 0
CONFIG proxy.config.dns.nameservers STRING NULL
+ # This provides additional resilience against DNS forgery, particularly in
+ # forward or transparent proxies, but requires that the resolver populates
+ # the queries section of the response properly.
+CONFIG proxy.config.dns.validate_query_name INT 0
##############################################################################
#
# DNS Proxy
Modified: trafficserver/traffic/trunk/proxy/mgmt2/RecordsConfig.cc
URL:
http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/mgmt2/RecordsConfig.cc?rev=989815&r1=989814&r2=989815&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/mgmt2/RecordsConfig.cc (original)
+++ trafficserver/traffic/trunk/proxy/mgmt2/RecordsConfig.cc Thu Aug 26
16:41:40 2010
@@ -2267,6 +2267,8 @@ RecordElement RecordsConfig[] = {
,
{CONFIG, "proxy.config.dns.max_dns_in_flight", "", INK_INT, "60", RU_REREAD,
RR_NULL, RC_NULL, NULL, RA_NULL}
,
+ {CONFIG, "proxy.config.dns.validate_query_name", "", INK_INT, "0",
RU_REREAD, RR_NULL, RC_NULL, "[0-1]", RA_NULL}
+ ,
{CONFIG, "proxy.config.dns.splitDNS.enabled", "", INK_INT, "0", RU_REREAD,
RR_NULL, RC_INT, "[0-1]", RA_NULL}
,
{CONFIG, "proxy.config.dns.splitdns.filename", "", INK_STRING,
"splitdns.config", RU_NULL, RR_NULL, RC_NULL, NULL,
