You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2010/08/27 01:27:43 UTC
svn commit: r989974 - in /trafficserver/traffic/branches/2.0.x: CHANGES
iocore/dns/DNS.cc iocore/dns/DNSConnection.cc iocore/dns/P_DNSConnection.h
iocore/dns/P_DNSProcessor.h proxy/config/records.config.in
proxy/mgmt2/RecordsConfig.cc
Author: zwoop
Date: Thu Aug 26 23:27:43 2010
New Revision: 989974
URL: http://svn.apache.org/viewvc?rev=989974&view=rev
Log:
TS-425 Backport of fixes for CVE-2010-2952
Modified:
trafficserver/traffic/branches/2.0.x/CHANGES
trafficserver/traffic/branches/2.0.x/iocore/dns/DNS.cc
trafficserver/traffic/branches/2.0.x/iocore/dns/DNSConnection.cc
trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSConnection.h
trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSProcessor.h
trafficserver/traffic/branches/2.0.x/proxy/config/records.config.in
trafficserver/traffic/branches/2.0.x/proxy/mgmt2/RecordsConfig.cc
Modified: trafficserver/traffic/branches/2.0.x/CHANGES
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/2.0.x/CHANGES?rev=989974&r1=989973&r2=989974&view=diff
==============================================================================
--- trafficserver/traffic/branches/2.0.x/CHANGES (original)
+++ trafficserver/traffic/branches/2.0.x/CHANGES Thu Aug 26 23:27:43 2010
@@ -2,6 +2,8 @@
Changes with Apache Traffic Server 2.0.1
+ *) Port of CVE-2010-2952 for 2.0.x [TS-TS-425].
+
*) Backport part of TS-322 that deals with indexing arrays with char
(author: Marcus Ruckert) [TS-334].
Modified: trafficserver/traffic/branches/2.0.x/iocore/dns/DNS.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/2.0.x/iocore/dns/DNS.cc?rev=989974&r1=989973&r2=989974&view=diff
==============================================================================
--- trafficserver/traffic/branches/2.0.x/iocore/dns/DNS.cc (original)
+++ trafficserver/traffic/branches/2.0.x/iocore/dns/DNS.cc Thu Aug 26 23:27:43 2010
@@ -44,7 +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;
-unsigned int dns_sequence_number = 0;
+int dns_validate_qname = 0;
unsigned int dns_handler_initialized = 0;
int dns_ns_rr = 0;
int dns_ns_rr_init_down = 1;
@@ -61,7 +61,7 @@ ClassAllocator<HostEnt> dnsBufAllocator(
// Function Prototypes
//
static bool dns_process(DNSHandler * h, HostEnt * ent, int len);
-static DNSEntry *get_dns(DNSHandler * h, u_short id);
+static DNSEntry *get_dns(DNSHandler * h, uint16 id);
// returns true when e is done
static void dns_result(DNSHandler * h, DNSEntry * e, HostEnt * ent, bool retry);
static void write_dns(DNSHandler * h);
@@ -117,6 +117,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");
@@ -152,17 +153,6 @@ DNSProcessor::open(unsigned int aip, int
void
DNSProcessor::dns_init()
{
- ink64 sval, cval = 0;
-
- DNS_READ_DYN_STAT(dns_sequence_number_stat, sval, cval);
-
- if (cval > 0) {
- dns_sequence_number = (unsigned int)
- (cval + DNS_SEQUENCE_NUMBER_RESTART_OFFSET);
- } else { // select a sequence number at random
- dns_sequence_number = (unsigned int) (ink_get_hrtime() / HRTIME_MSECOND);
- }
- Debug("dns", "initial dns_sequence_number = %d\n", (u_short) dns_sequence_number);
gethostname(try_server_names[0], 255);
Debug("dns", "localhost=%s\n", try_server_names[0]);
Debug("dns", "Round-robin nameservers = %d\n", dns_ns_rr);
@@ -385,7 +375,7 @@ DNSHandler::open_con(unsigned int aip, i
#error port me
#endif
if (r < 0) { // Add the FD to epoll fd
- Error("iocore_dns", "open_con: Failed to add %d server to epoll list\n", icon);
+ Error("[iocore_dns] open_con: Failed to add %d server to epoll list\n", icon);
} else {
con[icon].num = icon;
Debug("dns", "opening connection %d.%d.%d.%d:%d SUCCEEDED for %d", DOT_SEPARATED(aip), aport, icon);
@@ -784,7 +774,7 @@ DNSHandler::mainEvent(int event, Event *
/** Find a DNSEntry by id. */
inline static DNSEntry *
-get_dns(DNSHandler * h, u_short id)
+get_dns(DNSHandler * h, uint16 id)
{
for (DNSEntry * e = h->entries.head; e; e = (DNSEntry *) e->link.next) {
if (e->once_written_flag)
@@ -798,6 +788,7 @@ get_dns(DNSHandler * h, u_short id)
return NULL;
}
+/** Find a DNSEntry by query name and type. */
inline static DNSEntry *
get_entry(DNSHandler * h, char *qname, int qtype)
{
@@ -849,6 +840,39 @@ write_dns(DNSHandler * h)
h->in_write_dns = false;
}
+uint16
+DNSHandler::get_query_id()
+{
+ uint16 q1, q2;
+ q2 = q1 = (uint16)(generator.random() & 0xFFFF);
+ if (query_id_in_use(q2)) {
+ uint16 i = q2>>6;
+ while (qid_in_flight[i] == INTU64_MAX) {
+ if (++i == sizeof(qid_in_flight)/sizeof(uint64)) {
+ i = 0;
+ }
+ if (i == q1>>6) {
+ Error("[iocore_dns] get_query_id: Exhausted all DNS query ids");
+ return q1;
+ }
+ }
+ i <<= 6;
+ q2 &= 0x3F;
+ while (query_id_in_use(i+q2)) {
+ ++q2;
+ q2 &= 0x3F;
+ if (q2 == (q1 & 0x3F)) {
+ Error("[iocore_dns] get_query_id: Exhausted all DNS query ids");
+ return q1;
+ }
+ }
+ q2 += i;
+ }
+
+ set_query_id_in_use(q2);
+ return q2;
+}
+
/**
Construct and Write the request for a single entry (using send(3N)).
@@ -874,19 +898,15 @@ write_dns_event(DNSHandler * h, DNSEntry
}
#endif
- int id = dns_retries - e->retries;
- if (id >= MAX_DNS_RETRIES)
- id = MAX_DNS_RETRIES - 1; // limit id history
-
- ++dns_sequence_number;
-
- DNS_SET_DYN_COUNT(dns_sequence_number_stat, dns_sequence_number);
-
#ifdef DNS_PROXY
if (!e->proxy) {
#endif
- u_short i = (u_short) dns_sequence_number;
+ uint16 i = h->get_query_id();
((HEADER *) (buffer))->id = htons(i);
+ if(e->id[dns_retries - e->retries] >= 0) {
+ //clear previous id in case named was switched or domain was expanded
+ h->release_query_id(e->id[dns_retries - e->retries]);
+ }
e->id[dns_retries - e->retries] = i;
#ifdef DNS_PROXY
} else {
@@ -1186,8 +1206,20 @@ DNSEntry::post(DNSHandler * h, HostEnt *
//
if (timeout)
timeout->cancel(this);
- mutex = NULL;
+
+#ifdef DNS_PROXY
+ if (!proxy) {
+#endif
+ for (int i = 0; i < MAX_DNS_RETRIES; i++) {
+ if (id[i] < 0)
+ break;
+ h->release_query_id(id[i]);
+ }
+#ifdef DNS_PROXY
+ }
+#endif
action.mutex = NULL;
+ mutex = NULL;
dnsEntryAllocator.free(this);
return 0;
}
@@ -1202,6 +1234,20 @@ DNSEntry::postEvent(int event, Event * e
if (result_ent)
if (ink_atomic_increment(&result_ent->ref_count, -1) == 1)
dnsProcessor.free_hostent(result_ent);
+
+#ifdef DNS_PROXY
+ if (!proxy) {
+#endif
+ for (int i = 0; i < MAX_DNS_RETRIES; i++) {
+ if (id[i] < 0)
+ break;
+ dnsH->release_query_id(id[i]);
+ }
+#ifdef DNS_PROXY
+ }
+#endif
+ action.mutex = NULL;
+ mutex = NULL;
dnsEntryAllocator.free(this);
return EVENT_DONE;
}
@@ -1212,7 +1258,7 @@ dns_process(DNSHandler * handler, HostEn
{
ProxyMutex *mutex = handler->mutex;
HEADER *h = (HEADER *) (buf->buf);
- DNSEntry *e = get_dns(handler, (u_short) ntohs(h->id));
+ DNSEntry *e = get_dns(handler, (uint16) ntohs(h->id));
bool retry = false;
bool server_ok = true;
inku32 temp_ttl = 0;
@@ -1221,7 +1267,7 @@ dns_process(DNSHandler * handler, HostEn
// Do we have an entry for this id?
//
if (!e || !e->written_flag) {
- Debug("dns", "unknown DNS id = %u", (u_short) ntohs(h->id));
+ Debug("dns", "unknown DNS id = %u", (uint16) ntohs(h->id));
if (!handler->hostent_cache)
handler->hostent_cache = buf;
else
@@ -1286,15 +1332,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;
@@ -1315,6 +1388,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);
@@ -1522,24 +1597,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
//
@@ -1578,9 +1635,6 @@ ink_dns_init(ModuleVersion v)
"proxy.process.dns.in_flight",
RECD_INT, RECP_NON_PERSISTENT, (int) dns_in_flight_stat, RecRawStatSyncSum);
- RecRegisterRawStat(dns_rsb, RECT_PROCESS,
- "proxy.process.dns.sequence_number",
- RECD_INT, RECP_NULL, (int) dns_sequence_number_stat, RecRawStatSyncCount);
}
Modified: trafficserver/traffic/branches/2.0.x/iocore/dns/DNSConnection.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/2.0.x/iocore/dns/DNSConnection.cc?rev=989974&r1=989973&r2=989974&view=diff
==============================================================================
--- trafficserver/traffic/branches/2.0.x/iocore/dns/DNSConnection.cc (original)
+++ trafficserver/traffic/branches/2.0.x/iocore/dns/DNSConnection.cc Thu Aug 26 23:27:43 2010
@@ -39,8 +39,8 @@
// set in the OS
// #define RECV_BUF_SIZE (1024*64)
// #define SEND_BUF_SIZE (1024*64)
-#define FIRST_RANDOM_PORT 16000
-#define LAST_RANDOM_PORT 32000
+#define FIRST_RANDOM_PORT (16000)
+#define LAST_RANDOM_PORT (60000)
#define ROUNDUP(x, y) ((((x)+((y)-1))/(y))*(y))
@@ -49,7 +49,7 @@
//
DNSConnection::DNSConnection():
-fd(NO_FD), num(0), epoll_ptr(NULL)
+ fd(NO_FD), generator(time(NULL) ^ (long) this), num(0), epoll_ptr(NULL)
{
memset(&sa, 0, sizeof(struct sockaddr_in));
}
@@ -97,18 +97,16 @@ DNSConnection::connect(unsigned int ip,
if (bind_random_port) {
int retries = 0;
- int offset = 0;
while (retries++ < 10000) {
struct sockaddr_in bind_sa;
memset(&sa, 0, sizeof(bind_sa));
bind_sa.sin_family = AF_INET;
bind_sa.sin_addr.s_addr = INADDR_ANY;
- int p = time(NULL) + offset;
- p = (p % (LAST_RANDOM_PORT - FIRST_RANDOM_PORT)) + FIRST_RANDOM_PORT;
+ uint32 p = generator.random();
+ p = (uint16)((p % (LAST_RANDOM_PORT - FIRST_RANDOM_PORT)) + FIRST_RANDOM_PORT);
bind_sa.sin_port = htons(p);
- Debug("dns", "random port = %d\n", p);
+ Debug("dns", "random port = %u\n", p);
if ((res = socketManager.ink_bind(fd, (struct sockaddr *) &bind_sa, sizeof(bind_sa), Proto)) < 0) {
- offset += 101;
continue;
}
goto Lok;
Modified: trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSConnection.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSConnection.h?rev=989974&r1=989973&r2=989974&view=diff
==============================================================================
--- trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSConnection.h (original)
+++ trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSConnection.h Thu Aug 26 23:27:43 2010
@@ -61,7 +61,7 @@ struct DNSConnection
{
int fd;
struct sockaddr_in sa;
-
+ InkRand generator;
int num; //added by YTS Team, yamsat
Link<DNSConnection> link; //added by YTS Team, yamsat
Modified: trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSProcessor.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSProcessor.h?rev=989974&r1=989973&r2=989974&view=diff
==============================================================================
--- trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSProcessor.h (original)
+++ trafficserver/traffic/branches/2.0.x/iocore/dns/P_DNSProcessor.h Thu Aug 26 23:27:43 2010
@@ -27,13 +27,18 @@
#define DNS_PROXY 1
/*
#include "I_DNS.h"
- #include "inktomi++.h"
#include <arpa/nameser.h>
#include "I_Cache.h"
#include "P_Net.h"
*/
#include "I_EventSystem.h"
+// From new ink_port.h on trunk
+typedef unsigned short uint16;
+typedef unsigned int uint32;
+typedef unsigned long long uint64;
+#define INTU64_MAX (18446744073709551615ULL)
+
#define MAX_NAMED 32
#define DEFAULT_DNS_RETRIES 3
#define MAX_DNS_RETRIES 5
@@ -243,6 +248,11 @@ struct DNSHandler:Continuation
ink_res_state m_res;
int txn_lookup_timeout;
+ InkRand generator;
+ // bitmap of query ids in use
+ uint64 qid_in_flight[(USHRT_MAX+1)/64];
+
+
void received_one(int i)
{
failover_number[i] = failover_soon_number[i] = crossed_failover_number[i] = 0;
@@ -285,6 +295,19 @@ struct DNSHandler:Continuation
void retry_named(int ndx, ink_hrtime t, bool reopen = true);
void try_primary_named(bool reopen = true);
void switch_named(int ndx);
+ uint16 get_query_id();
+
+ void release_query_id(uint16 qid) {
+ qid_in_flight[qid >> 6] &= (uint64)~(0x1ULL << (qid & 0x3F));
+ };
+
+ void set_query_id_in_use(uint16 qid) {
+ qid_in_flight[qid >> 6] |= (uint64)(0x1ULL << (qid & 0x3F));
+ };
+
+ bool query_id_in_use(uint16 qid) {
+ return (qid_in_flight[qid >> 6] & (uint64)(0x1ULL << (qid & 0x3F))) != 0;
+ };
DNSHandler();
};
@@ -297,7 +320,7 @@ inline DNSHandler::DNSHandler()
n_con(0),
options(0),
in_flight(0), name_server(0), in_write_dns(0), hostent_cache(0), last_primary_retry(0), last_primary_reopen(0),
- m_res(0), txn_lookup_timeout(0)
+ m_res(0), txn_lookup_timeout(0), generator(time(NULL) ^ (long) this)
{
for (int i = 0; i < MAX_NAMED; i++) {
ifd[i] = -1;
@@ -306,6 +329,7 @@ inline DNSHandler::DNSHandler()
crossed_failover_number[i] = 0;
ns_down[i] = 1;
}
+ memset(&qid_in_flight, 0, sizeof(qid_in_flight));
SET_HANDLER(&DNSHandler::startEvent);
Debug("net_epoll", "inline DNSHandler::DNSHandler()");
}
Modified: trafficserver/traffic/branches/2.0.x/proxy/config/records.config.in
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/2.0.x/proxy/config/records.config.in?rev=989974&r1=989973&r2=989974&view=diff
==============================================================================
--- trafficserver/traffic/branches/2.0.x/proxy/config/records.config.in (original)
+++ trafficserver/traffic/branches/2.0.x/proxy/config/records.config.in Thu Aug 26 23:27:43 2010
@@ -338,6 +338,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/branches/2.0.x/proxy/mgmt2/RecordsConfig.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/branches/2.0.x/proxy/mgmt2/RecordsConfig.cc?rev=989974&r1=989973&r2=989974&view=diff
==============================================================================
--- trafficserver/traffic/branches/2.0.x/proxy/mgmt2/RecordsConfig.cc (original)
+++ trafficserver/traffic/branches/2.0.x/proxy/mgmt2/RecordsConfig.cc Thu Aug 26 23:27:43 2010
@@ -2587,6 +2587,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,