Skip to content

Commit

Permalink
Improve connection handling when talking to TCP upsteam servers.
Browse files Browse the repository at this point in the history 
Specifically, be prepared to open a new connection when we
want to make multiple queries but the upstream server accepts
fewer queries per connection.
  • Loading branch information
@simonkelley
simonkelley committed Feb 10, 2017
1 parent 68f6312 commit 361dfe5
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 79 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ version 2.77
Thanks to Kevin Darbyshire-Bryant and Eric Luehrsen
for pushing this.

Improve connection handling when talking to TCP upsteam
servers. Specifically, be prepared to open a new TCP
connection when we want to make multiple queries
but the upstream server accepts fewer queries per connection.


version 2.76
Include 0.0.0.0/8 in DNS rebind checks. This range
Expand Down
1 change: 1 addition & 0 deletions src/dnsmasq.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ union mysockaddr {
#define SERV_FROM_FILE 4096 /* read from --servers-file */
#define SERV_LOOP 8192 /* server causes forwarding loop */
#define SERV_DO_DNSSEC 16384 /* Validate DNSSEC when using this server */
#define SERV_GOT_TCP 32768 /* Got some data from the TCP connection */

struct serverfd {
int fd;
Expand Down
175 changes: 96 additions & 79 deletions src/forward.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1459,14 +1459,15 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si
unsigned char *payload = NULL;
struct dns_header *new_header = NULL;
u16 *length = NULL;

while (1)
{
int type = SERV_DO_DNSSEC;
char *domain;
size_t m;
unsigned char c1, c2;

struct server *firstsendto = NULL;

/* limit the amount of work we do, to avoid cycling forever on loops in the DNS */
if (--(*keycount) == 0)
new_status = STAT_ABANDONED;
Expand Down Expand Up @@ -1504,81 +1505,86 @@ static int tcp_key_recurse(time_t now, int status, struct dns_header *header, si
/* Find server to forward to. This will normally be the
same as for the original query, but may be another if
servers for domains are involved. */
if (search_servers(now, NULL, F_QUERY, keyname, &type, &domain, NULL) == 0)
if (search_servers(now, NULL, F_QUERY, keyname, &type, &domain, NULL) != 0)
{
struct server *start = server, *new_server = NULL;
type &= ~SERV_DO_DNSSEC;

while (1)
new_status = STAT_ABANDONED;
break;
}

type &= ~SERV_DO_DNSSEC;

while (1)
{
if (!firstsendto)
firstsendto = server;
else
{
if (type == (start->flags & SERV_TYPE) &&
(type != SERV_HAS_DOMAIN || hostname_isequal(domain, start->domain)) &&
!(start->flags & (SERV_LITERAL_ADDRESS | SERV_LOOP)))
if (!(server = server->next))
server = daemon->servers;
if (server == firstsendto)
{
new_server = start;
if (server == start)
{
new_server = NULL;
break;
}
/* can't find server to accept our query. */
new_status = STAT_ABANDONED;
break;
}

if (!(start = start->next))
start = daemon->servers;
if (start == server)
break;
}


if (new_server)

if (type != (server->flags & SERV_TYPE) ||
(type == SERV_HAS_DOMAIN && !hostname_isequal(domain, server->domain)) ||
(server->flags & (SERV_LITERAL_ADDRESS | SERV_LOOP)))
continue;

retry:
/* may need to make new connection. */
if (server->tcpfd == -1)
{
server = new_server;
/* may need to make new connection. */
if (server->tcpfd == -1)
{
if ((server->tcpfd = socket(server->addr.sa.sa_family, SOCK_STREAM, 0)) == -1)
{
new_status = STAT_ABANDONED;
break;
}

if ((server->tcpfd = socket(server->addr.sa.sa_family, SOCK_STREAM, 0)) == -1)
continue; /* No good, next server */

#ifdef HAVE_CONNTRACK
/* Copy connection mark of incoming query to outgoing connection. */
if (have_mark)
setsockopt(server->tcpfd, SOL_SOCKET, SO_MARK, &mark, sizeof(unsigned int));
/* Copy connection mark of incoming query to outgoing connection. */
if (have_mark)
setsockopt(server->tcpfd, SOL_SOCKET, SO_MARK, &mark, sizeof(unsigned int));
#endif

if (!local_bind(server->tcpfd, &server->source_addr, server->interface, 1) ||
connect(server->tcpfd, &server->addr.sa, sa_len(&server->addr)) == -1)
{
close(server->tcpfd);
server->tcpfd = -1;
new_status = STAT_ABANDONED;
break;
}


if (!local_bind(server->tcpfd, &server->source_addr, server->interface, 1) ||
connect(server->tcpfd, &server->addr.sa, sa_len(&server->addr)) == -1)
{
close(server->tcpfd);
server->tcpfd = -1;
continue; /* No good, next server */
}
}
}


if (!read_write(server->tcpfd, packet, m + sizeof(u16), 0) ||
!read_write(server->tcpfd, &c1, 1, 1) ||
!read_write(server->tcpfd, &c2, 1, 1) ||
!read_write(server->tcpfd, payload, (c1 << 8) | c2, 1))
{
new_status = STAT_ABANDONED;
server->flags &= ~SERV_GOT_TCP;
}

if (!read_write(server->tcpfd, packet, m + sizeof(u16), 0) ||
!read_write(server->tcpfd, &c1, 1, 1) ||
!read_write(server->tcpfd, &c2, 1, 1) ||
!read_write(server->tcpfd, payload, (c1 << 8) | c2, 1))
{
close(server->tcpfd);
server->tcpfd = -1;
/* We get data then EOF, reopen connection to same server,
else try next. This avoids DoS from a server which accepts
connections and then closes them. */
if (server->flags & SERV_GOT_TCP)
goto retry;
else
continue;
}

server->flags |= SERV_GOT_TCP;

m = (c1 << 8) | c2;
new_status = tcp_key_recurse(now, new_status, new_header, m, class, name, keyname, server, have_mark, mark, keycount);
break;
}

m = (c1 << 8) | c2;

new_status = tcp_key_recurse(now, new_status, new_header, m, class, name, keyname, server, have_mark, mark, keycount);

if (new_status != STAT_OK)
break;
}

if (packet)
free(packet);

Expand Down Expand Up @@ -1820,7 +1826,8 @@ unsigned char *tcp_request(int confd, time_t now,
(type == SERV_HAS_DOMAIN && !hostname_isequal(domain, last_server->domain)) ||
(last_server->flags & (SERV_LITERAL_ADDRESS | SERV_LOOP)))
continue;


retry:
if (last_server->tcpfd == -1)
{
if ((last_server->tcpfd = socket(last_server->addr.sa.sa_family, SOCK_STREAM, 0)) == -1)
Expand All @@ -1840,25 +1847,27 @@ unsigned char *tcp_request(int confd, time_t now,
continue;
}

last_server->flags &= ~SERV_GOT_TCP;
}

#ifdef HAVE_DNSSEC
if (option_bool(OPT_DNSSEC_VALID) && (last_server->flags & SERV_DO_DNSSEC))
if (option_bool(OPT_DNSSEC_VALID) && (last_server->flags & SERV_DO_DNSSEC))
{
new_size = add_do_bit(header, size, ((unsigned char *) header) + 65536);

if (size != new_size)
{
new_size = add_do_bit(header, size, ((unsigned char *) header) + 65536);

if (size != new_size)
{
added_pheader = 1;
size = new_size;
}

/* For debugging, set Checking Disabled, otherwise, have the upstream check too,
this allows it to select auth servers when one is returning bad data. */
if (option_bool(OPT_DNSSEC_DEBUG))
header->hb4 |= HB4_CD;
added_pheader = 1;
size = new_size;
}
#endif

/* For debugging, set Checking Disabled, otherwise, have the upstream check too,
this allows it to select auth servers when one is returning bad data. */
if (option_bool(OPT_DNSSEC_DEBUG))
header->hb4 |= HB4_CD;
}

#endif

*length = htons(size);

/* get query name again for logging - may have been overwritten */
Expand All @@ -1872,9 +1881,17 @@ unsigned char *tcp_request(int confd, time_t now,
{
close(last_server->tcpfd);
last_server->tcpfd = -1;
continue;
}
/* We get data then EOF, reopen connection to same server,
else try next. This avoids DoS from a server which accepts
connections and then closes them. */
if (last_server->flags & SERV_GOT_TCP)
goto retry;
else
continue;
}

last_server->flags |= SERV_GOT_TCP;

m = (c1 << 8) | c2;

if (last_server->addr.sa.sa_family == AF_INET)
Expand Down

0 comments on commit 361dfe5

Please sign in to comment.