Partial backport of DNS address/name checking (r16621), and backport of 0x20 hack (r17171).

svn:r17636
This commit is contained in:
Nick Mathewson 2008-12-17 12:51:36 +00:00
parent 2548454bc5
commit a750683d2f
6 changed files with 109 additions and 8 deletions

View File

@ -32,7 +32,6 @@ Changes in version 0.2.0.33 - 200?-??-??
- Use 64 bits instead of 32 bits for connection identifiers used with
the controller protocol, to greatly reduce risk of identifier reuse.
o Minor features:
- Report the case where all signatures in a detached set are rejected
differently than the case where there is an error handling the detached
@ -40,6 +39,15 @@ Changes in version 0.2.0.33 - 200?-??-??
- When we realize that another process has modified our cached
descriptors, print out a more useful error message rather than
triggering an assertion. Fixes bug 885. Patch from Karsten.
- Implement the 0x20 hack to better resist DNS poisoning: set the
case on outgoing DNS requests randomly, and reject responses that do
not match the case correctly. This logic can be disabled with the
ServerDNSRamdomizeCase setting, if you are using one of the 0.3%
of servers that do not reliably preserve case in replies. See
"Increased DNS Forgery Resistance through 0x20-Bit Encoding"
for more info.
- Check DNS replies for more matching fields to better resist DNS
poisoning.
Changes in version 0.2.0.32 - 2008-11-20

View File

@ -8,8 +8,10 @@ Backport for 0.2.0:
Backport for 0.2.0 once better tested:
o r16136: prevent circid collision. [Also backport to 0.1.2.x??]
o r16558: Avoid mis-routing CREATED cells.
- r16621: Make some DNS code more robust (partial; see also libevent
Xo r16621: Make some DNS code more robust (partial; see also libevent
approach). (Also maybe r16674)
[Partially backported. Instead of the basic name checking, I backported
r17171 instead, to be even more resistant to poisoning.]
- r17091: distinguish "no routers support pending circuits" from
"no circuits are pending." See also r17181 and r17184.
- r17137: send END cell in response to connect to nonexistent hidserv port.

View File

@ -274,6 +274,7 @@ static config_var_t _option_vars[] = {
V(ServerDNSAllowBrokenResolvConf, BOOL, "1"),
V(ServerDNSAllowNonRFC953Hostnames, BOOL,"0"),
V(ServerDNSDetectHijacking, BOOL, "1"),
V(ServerDNSRandomizeCase, BOOL, "1"),
V(ServerDNSResolvConfFile, STRING, NULL),
V(ServerDNSSearchDomains, BOOL, "0"),
V(ServerDNSTestAddresses, CSV,

View File

@ -198,6 +198,10 @@ dns_init(void)
{
init_cache_map();
evdns_set_transaction_id_fn(dns_get_transaction_id);
if (get_options()->ServerDNSRandomizeCase)
evdns_set_option("randomize-case", "1", DNS_OPTIONS_ALL);
else
evdns_set_option("randomize-case", "0", DNS_OPTIONS_ALL);
if (server_mode(get_options()))
return configure_nameservers(1);
return 0;

View File

@ -307,6 +307,9 @@ static int global_max_retransmits = 3; /* number of times we'll retransmit a req
/* number of timeouts in a row before we consider this server to be down */
static int global_max_nameserver_timeout = 3;
/* DOCDOC */
static int global_randomize_case = 1;
/* These are the timeout values for nameservers. If we find a nameserver is down */
/* we try to probe it at intervals as given below. Values are in seconds. */
static const struct timeval global_nameserver_timeouts[] = {{10, 0}, {60, 0}, {300, 0}, {900, 0}, {3600, 0}};
@ -377,6 +380,9 @@ inet_aton(const char *c, struct in_addr *addr)
#define ISSPACE(c) isspace((int)(unsigned char)(c))
#define ISDIGIT(c) isdigit((int)(unsigned char)(c))
#define ISALPHA(c) isalpha((int)(unsigned char)(c))
#define TOLOWER(c) (char)tolower((int)(unsigned char)(c))
#define TOUPPER(c) (char)toupper((int)(unsigned char)(c))
#ifndef NDEBUG
static const char *
@ -813,9 +819,10 @@ name_parse(u8 *packet, int length, int *idx, char *name_out, size_t name_out_len
static int
reply_parse(u8 *packet, int length) {
int j = 0; /* index into packet */
int k;
u16 _t; /* used by the macros */
u32 _t32; /* used by the macros */
char tmp_name[256]; /* used by the macros */
char tmp_name[256], cmp_name[256]; /* used by the macros */
u16 trans_id, questions, answers, authority, additional, datalength;
u16 flags = 0;
@ -823,6 +830,7 @@ reply_parse(u8 *packet, int length) {
struct reply reply;
struct request *req = NULL;
unsigned int i;
int name_matches = 0;
GET16(trans_id);
GET16(flags);
@ -848,11 +856,28 @@ reply_parse(u8 *packet, int length) {
/* if (!answers) return; */ /* must have an answer of some form */
/* This macro skips a name in the DNS reply. */
#define SKIP_NAME \
#define GET_NAME \
do { tmp_name[0] = '\0'; \
if (name_parse(packet, length, &j, tmp_name, sizeof(tmp_name))<0) \
goto err; \
} while(0);
#define TEST_NAME \
do { tmp_name[0] = '\0'; \
cmp_name[0] = '\0'; \
k = j; \
if (name_parse(packet, length, &j, tmp_name, sizeof(tmp_name))<0) \
goto err; \
if (name_parse(req->request, req->request_len, &k, cmp_name, sizeof(cmp_name))<0) \
goto err; \
if (global_randomize_case) { \
if (strcmp(tmp_name, cmp_name) == 0) \
name_matches = 1; /* we ignore mismatching names */ \
} else { \
if (strcasecmp(tmp_name, cmp_name) == 0) \
name_matches = 1; \
} \
} while(0)
reply.type = req->request_type;
@ -861,11 +886,14 @@ reply_parse(u8 *packet, int length) {
/* the question looks like
* <label:name><u16:type><u16:class>
*/
SKIP_NAME;
TEST_NAME;
j += 4;
if (j >= length) goto err;
}
if (!name_matches)
goto err;
/* now we have the answer section which looks like
* <label:name><u16:type><u16:class><u32:ttl><u16:len><data...>
*/
@ -875,7 +903,7 @@ reply_parse(u8 *packet, int length) {
/* XXX I'd be more comfortable if we actually checked the name */
/* here. -NM */
SKIP_NAME;
GET_NAME;
GET16(type);
GET16(class);
GET32(ttl);
@ -1082,6 +1110,22 @@ evdns_set_transaction_id_fn(uint16_t (*fn)(void))
trans_id_function = default_transaction_id_fn;
}
static void
get_random_bytes(char *buf, size_t n)
{
unsigned i;
for (i = 0; i < n-1; i += 2) {
u16 tid = trans_id_function();
buf[i] = (tid >> 8) & 0xff;
buf[i+1] = tid & 0xff;
}
if (i < n) {
u16 tid = trans_id_function();
buf[i] = tid & 0xff;
}
}
/* Try to choose a strong transaction id which isn't already in flight */
static u16
transaction_id_pick(void) {
@ -1143,17 +1187,34 @@ nameserver_pick(void) {
/* this is called when a namesever socket is ready for reading */
static void
nameserver_read(struct nameserver *ns) {
struct sockaddr_storage ss;
struct sockaddr *sa = (struct sockaddr *)&ss;
struct sockaddr_in *sin;
socklen_t addrlen = sizeof(ss);
u8 packet[1500];
for (;;) {
const int r =
(int)recv(ns->socket, packet,(socklen_t)sizeof(packet), 0);
(int)recvfrom(ns->socket, packet,(socklen_t)sizeof(packet), 0,
sa, &addrlen);
if (r < 0) {
int err = last_error(ns->socket);
if (error_is_eagain(err)) return;
nameserver_failed(ns, strerror(err));
return;
}
if (sa->sa_family != AF_INET) {
log(EVDNS_LOG_WARN,
"Address family mismatch on received DNS packet.");
return;
}
sin = (struct sockaddr_in *)sa;
if (sin->sin_addr.s_addr != ns->address) {
log(EVDNS_LOG_WARN,
"Address mismatch on received DNS packet. Address was %s.",
debug_ntoa(sin->sin_addr.s_addr));
return;
}
ns->timedout = 0;
reply_parse(packet, r);
}
@ -2243,12 +2304,29 @@ request_new(int type, const char *name, int flags,
/* the request data is alloced in a single block with the header */
struct request *const req =
(struct request *) malloc(sizeof(struct request) + request_max_len);
char namebuf[256];
int rlen;
(void) flags;
if (!req) return NULL;
memset(req, 0, sizeof(struct request));
if (global_randomize_case) {
unsigned i;
char randbits[32];
strlcpy(namebuf, name, sizeof(namebuf));
get_random_bytes(randbits, (name_len+7)/8);
for (i = 0; i < name_len; ++i) {
if (ISALPHA(namebuf[i])) {
if ((randbits[i >> 3] & (1<<(i%7))))
namebuf[i] = TOLOWER(namebuf[i]);
else
namebuf[i] = TOUPPER(namebuf[i]);
}
}
name = namebuf;
}
/* request data lives just after the header */
req->request = ((u8 *) req) + sizeof(struct request);
/* denotes that the request data shouldn't be free()ed */
@ -2690,7 +2768,13 @@ evdns_set_option(const char *option, const char *val, int flags)
if (!(flags & DNS_OPTION_MISC)) return 0;
log(EVDNS_LOG_DEBUG, "Setting retries to %d", retries);
global_max_retransmits = retries;
} else if (!strncmp(option, "randomize-case:", 15)) {
int randcase = strtoint(val);
if (!(flags & DNS_OPTION_MISC)) return 0;
log(EVDNS_LOG_DEBUG, "Setting randomize_case to %d", randcase);
global_randomize_case = randcase;
}
return 0;
}
@ -3127,7 +3211,7 @@ evdns_server_callback(struct evdns_server_request *req, void *data)
}
}
r = evdns_request_respond(req, 0);
r = evdns_server_request_respond(req, 0);
if (r<0)
printf("eeek, couldn't send reply.\n");
}

View File

@ -2297,6 +2297,8 @@ typedef struct {
* the local domains. */
int ServerDNSDetectHijacking; /**< Boolean: If true, check for DNS failure
* hijacking. */
int ServerDNSRandomizeCase; /**< Boolean: Use the 0x20-hack to prevent
* DNS poisoning attacks. */
char *ServerDNSResolvConfFile; /**< If provided, we configure our internal
* resolver from the file here rather than from
* /etc/resolv.conf (Unix) or the registry (Windows). */