Skip to content

Commit 4a5cff8

Browse files
committed
gossip: Try to detect broken ISP resolvers and discard broken replies
This is a best effort attempt to skip connection attempts if we detect a broken ISP resolver. A broken ISP resolver is a resolver that will replace NXDOMAIN replies with a dummy response. This is best effort in that it'll only detect a single fixed dummy reply, it'll check only on startup, and will not detect if we switched networks. It should be good enough for most cases, and in the worst case it will result in a connection attempt that does not complete. Signed-off-by: Christian Decker <[email protected]> Reported-by: Glenn Willen <@gwillen>
1 parent e556bf4 commit 4a5cff8

File tree

3 files changed

+56
-4
lines changed

3 files changed

+56
-4
lines changed

common/wireaddr.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ static bool separate_address_and_port(const tal_t *ctx, const char *arg,
290290

291291
bool wireaddr_from_hostname(struct wireaddr *addr, const char *hostname,
292292
const u16 port, bool *no_dns,
293+
struct sockaddr *broken_reply,
293294
const char **err_msg)
294295
{
295296
struct sockaddr_in6 *sa6;
@@ -342,6 +343,12 @@ bool wireaddr_from_hostname(struct wireaddr *addr, const char *hostname,
342343
*err_msg = gai_strerror(gai_err);
343344
return false;
344345
}
346+
347+
if (broken_reply != NULL && memeq(addrinfo->ai_addr, addrinfo->ai_addrlen, broken_reply, tal_len(broken_reply))) {
348+
res = false;
349+
goto cleanup;
350+
}
351+
345352
/* Use only the first found address */
346353
if (addrinfo->ai_family == AF_INET) {
347354
sa4 = (struct sockaddr_in *) addrinfo->ai_addr;
@@ -353,6 +360,7 @@ bool wireaddr_from_hostname(struct wireaddr *addr, const char *hostname,
353360
res = true;
354361
}
355362

363+
cleanup:
356364
/* Clean up */
357365
freeaddrinfo(addrinfo);
358366
return res;
@@ -392,7 +400,7 @@ bool parse_wireaddr(const char *arg, struct wireaddr *addr, u16 defport,
392400

393401
/* Resolve with getaddrinfo */
394402
if (!res)
395-
res = wireaddr_from_hostname(addr, ip, port, no_dns, err_msg);
403+
res = wireaddr_from_hostname(addr, ip, port, no_dns, NULL, err_msg);
396404

397405
finish:
398406
if (!res && err_msg && !*err_msg)

common/wireaddr.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@ char *fmt_wireaddr_without_port(const tal_t *ctx, const struct wireaddr *a);
8989
* we wanted to do a DNS lookup. */
9090
bool wireaddr_from_hostname(struct wireaddr *addr, const char *hostname,
9191
const u16 port, bool *no_dns,
92+
struct sockaddr *broken_reply,
9293
const char **err_msg);
9394

9495
void wireaddr_from_ipv4(struct wireaddr *addr,

gossipd/gossip.c

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ struct daemon {
172172

173173
/* @see lightningd.config.use_dns */
174174
bool use_dns;
175+
176+
/* The address that the broken response returns instead of
177+
* NXDOMAIN. NULL if we have not detected a broken resolver. */
178+
struct sockaddr *broken_resolver_response;
175179
};
176180

177181
/* Peers we're trying to reach. */
@@ -360,6 +364,37 @@ new_local_peer_state(struct peer *peer, const struct crypto_state *cs)
360364
return lps;
361365
}
362366

367+
/**
368+
* Some ISP resolvers will reply with a dummy IP to queries that would otherwise
369+
* result in an NXDOMAIN reply. This just checks whether we have one such
370+
* resolver upstream and remembers its reply so we can try to filter future
371+
* dummies out.
372+
*/
373+
static bool broken_resolver(struct daemon *daemon)
374+
{
375+
struct addrinfo *addrinfo;
376+
struct addrinfo hints;
377+
char *hostname = "nxdomain-test.doesntexist";
378+
int err;
379+
memset(&hints, 0, sizeof(hints));
380+
hints.ai_family = AF_UNSPEC;
381+
hints.ai_socktype = SOCK_STREAM;
382+
hints.ai_protocol = 0;
383+
hints.ai_flags = AI_ADDRCONFIG;
384+
err = getaddrinfo(hostname, tal_fmt(tmpctx, "%d", 42),
385+
&hints, &addrinfo);
386+
387+
daemon->broken_resolver_response =
388+
tal_free(daemon->broken_resolver_response);
389+
390+
if (err == 0) {
391+
daemon->broken_resolver_response = tal_dup(daemon, struct sockaddr, addrinfo->ai_addr);
392+
freeaddrinfo(addrinfo);
393+
}
394+
395+
return daemon->broken_resolver_response != NULL;
396+
}
397+
363398
static struct peer *new_peer(const tal_t *ctx,
364399
struct daemon *daemon,
365400
const struct pubkey *their_id,
@@ -2855,6 +2890,11 @@ static struct io_plan *gossip_init(struct daemon_conn *master,
28552890
} else
28562891
daemon->proxyaddr = NULL;
28572892

2893+
if (broken_resolver(daemon)) {
2894+
status_trace("Broken DNS resolver detected, will check for "
2895+
"dummy replies");
2896+
}
2897+
28582898
/* Load stored gossip messages */
28592899
gossip_store_load(daemon->rstate, daemon->rstate->store);
28602900

@@ -3061,7 +3101,8 @@ static const char *seedname(const tal_t *ctx, const struct pubkey *id)
30613101
}
30623102

30633103
static struct wireaddr_internal *
3064-
seed_resolve_addr(const tal_t *ctx, const struct pubkey *id)
3104+
seed_resolve_addr(const tal_t *ctx, const struct pubkey *id,
3105+
struct sockaddr *broken_reply)
30653106
{
30663107
struct wireaddr_internal *a;
30673108
const char *addr;
@@ -3072,7 +3113,7 @@ seed_resolve_addr(const tal_t *ctx, const struct pubkey *id)
30723113
a = tal(ctx, struct wireaddr_internal);
30733114
a->itype = ADDR_INTERNAL_WIREADDR;
30743115
if (!wireaddr_from_hostname(&a->u.wireaddr, addr, DEFAULT_PORT, NULL,
3075-
NULL)) {
3116+
broken_reply, NULL)) {
30763117
status_trace("Could not resolve %s", addr);
30773118
return tal_free(a);
30783119
} else {
@@ -3170,7 +3211,8 @@ static void try_reach_peer(struct daemon *daemon, const struct pubkey *id,
31703211
wireaddr_from_unresolved(a, seedname(tmpctx, id),
31713212
DEFAULT_PORT);
31723213
} else if (daemon->use_dns) {
3173-
a = seed_resolve_addr(tmpctx, id);
3214+
a = seed_resolve_addr(tmpctx, id,
3215+
daemon->broken_resolver_response);
31743216
}
31753217
}
31763218

@@ -3685,6 +3727,7 @@ int main(int argc, char *argv[])
36853727
timers_init(&daemon->timers, time_mono());
36863728
daemon->broadcast_interval = 30000;
36873729
daemon->last_announce_timestamp = 0;
3730+
daemon->broken_resolver_response = NULL;
36883731
/* stdin == control */
36893732
daemon_conn_init(daemon, &daemon->master, STDIN_FILENO, recv_req,
36903733
master_gone);

0 commit comments

Comments
 (0)