From 6a1a5eec43892dee3ff6e208bceb1931c25c782e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 8 Jan 2016 02:29:02 +0100 Subject: resolved: when DNS/TCP doesn't work, try DNS/UDP again If we failed to contact a DNS server via TCP, bump of the feature level to UDP again. This way we'll switch back between UDP and TCP if we fail to contact a host. Generally, we prefer UDP over TCP, which is why UDP is a higher feature level. But some servers only support UDP but not TCP hence when reaching the lowest feature level of TCP and want to downgrade from there, pick UDP again. We this keep downgrading until we reach TCP and then we cycle through UDP and TCP. --- src/resolve/resolved-dns-server.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) (limited to 'src/resolve/resolved-dns-server.c') diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index fbd1c27c14..0de6c8cec0 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -326,11 +326,21 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { log_info("Grace period over, resuming full feature set for DNS server %s", strna(ip)); } else if (s->possible_feature_level <= s->verified_feature_level) s->possible_feature_level = s->verified_feature_level; - else if (s->n_failed_attempts >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS && - s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_WORST) { + else if (s->n_failed_attempts >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS) { _cleanup_free_ char *ip = NULL; - s->possible_feature_level --; + /* Switch one feature level down. Except when we are at TCP already, in which case we try UDP + * again. Thus, if a DNS server is not responding we'll keep toggling between UDP and TCP until it + * responds on one of them. Note that we generally prefer UDP over TCP (which is why it is at a higher + * feature level), but many DNS servers support lack TCP support. */ + + if (s->possible_feature_level == DNS_SERVER_FEATURE_LEVEL_TCP) + s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_UDP; + else { + assert(s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_WORST); + s->possible_feature_level --; + } + s->n_failed_attempts = 0; s->verified_usec = 0; -- cgit v1.2.3-54-g00ecf From 571370c1555d2aa697733479a50957aff024bbcb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 8 Jan 2016 02:46:59 +0100 Subject: resolved: when we get a packet failure from a server, don't downgrade UDP to TCP or vice versa Under the assumption that packet failures (i.e. FORMERR, SERVFAIL, NOTIMP) are caused by packet contents, not used transport, we shouldn't switch between UDP and TCP when we get them, but only downgrade the higher levels down to UDP. --- src/resolve/resolved-dns-server.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src/resolve/resolved-dns-server.c') diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 0de6c8cec0..1600e7c292 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -277,6 +277,14 @@ void dns_server_packet_failed(DnsServer *s, DnsServerFeatureLevel level) { if (s->possible_feature_level != level) return; + /* Invoked whenever we get a FORMERR, SERVFAIL or NOTIMP rcode from a server. This is an immediate trigger for + * us to go one feature level down. Except when we are already at TCP or UDP level, in which case there's no + * point in changing, under the assumption that packet failures are caused by packet contents, not by used + * transport. */ + + if (s->possible_feature_level <= DNS_SERVER_FEATURE_LEVEL_UDP) + return; + s->n_failed_attempts = (unsigned) -1; } -- cgit v1.2.3-54-g00ecf From 6bb2c08597c999c429e889cd2403b2fef5f3e1a0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 8 Jan 2016 18:50:41 +0100 Subject: resolved: rework server feature level logic This changes the DnsServer logic to count failed UDP and TCP failures separately. This is useful so that we don't end up downgrading the feature level from one UDP level to a lower UDP level just because a TCP connection we did because of a TC response failed. This also adds accounting of truncated packets. If we detect incoming truncated packets, and count too many failed TCP connections (which is the normal fall back if we get a trucnated UDP packet) we downgrade the feature level, given that the responses at the current levels don't get through, and we somehow need to make sure they become smaller, which they will do if we don't request DNSSEC or EDNS support. This makes resolved work much better with crappy DNS servers that do not implement TCP and only limited UDP packet sizes, but otherwise support DNSSEC RRs. They end up choking on the generally larger DNSSEC RRs and there's no way to retrieve the full data. --- src/resolve/resolved-dns-server.c | 149 +++++++++++++++++++++++---------- src/resolve/resolved-dns-server.h | 10 ++- src/resolve/resolved-dns-transaction.c | 13 +-- 3 files changed, 121 insertions(+), 51 deletions(-) (limited to 'src/resolve/resolved-dns-server.c') diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 1600e7c292..3baf63ffa7 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -224,31 +224,48 @@ void dns_server_move_back_and_unmark(DnsServer *s) { } } -void dns_server_packet_received(DnsServer *s, DnsServerFeatureLevel level, usec_t rtt, size_t size) { +static void dns_server_verified(DnsServer *s, DnsServerFeatureLevel level) { assert(s); - if (level == DNS_SERVER_FEATURE_LEVEL_LARGE) { - /* Even if we successfully receive a reply to a - request announcing support for large packets, that - does not mean we can necessarily receive large - packets. */ + if (s->verified_feature_level > level) + return; - if (s->verified_feature_level < DNS_SERVER_FEATURE_LEVEL_LARGE - 1) { - s->verified_feature_level = DNS_SERVER_FEATURE_LEVEL_LARGE - 1; - assert_se(sd_event_now(s->manager->event, clock_boottime_or_monotonic(), &s->verified_usec) >= 0); - } - } else if (s->verified_feature_level < level) { + if (s->verified_feature_level != level) { + log_debug("Verified feature level %s.", dns_server_feature_level_to_string(level)); s->verified_feature_level = level; - assert_se(sd_event_now(s->manager->event, clock_boottime_or_monotonic(), &s->verified_usec) >= 0); } - if (s->possible_feature_level == level) - s->n_failed_attempts = 0; + assert_se(sd_event_now(s->manager->event, clock_boottime_or_monotonic(), &s->verified_usec) >= 0); +} + +void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t rtt, size_t size) { + assert(s); + + if (protocol == IPPROTO_UDP) { + if (s->possible_feature_level == level) + s->n_failed_udp = 0; + + if (level == DNS_SERVER_FEATURE_LEVEL_LARGE) + /* Even if we successfully receive a reply to a request announcing support for large packets, + that does not mean we can necessarily receive large packets. */ + dns_server_verified(s, DNS_SERVER_FEATURE_LEVEL_LARGE - 1); + else + /* A successful UDP reply, verifies UDP, ENDS0 and DO levels */ + dns_server_verified(s, level); + + } else if (protocol == IPPROTO_TCP) { + + if (s->possible_feature_level == level) + s->n_failed_tcp = 0; + + /* Successful TCP connections are only useful to verify the TCP feature level. */ + dns_server_verified(s, DNS_SERVER_FEATURE_LEVEL_TCP); + } /* Remember the size of the largest UDP packet we received from a server, we know that we can always announce support for packets with at least this size. */ - if (s->received_udp_packet_max < size) + if (protocol == IPPROTO_UDP && s->received_udp_packet_max < size) s->received_udp_packet_max = size; if (s->max_rtt < rtt) { @@ -257,12 +274,16 @@ void dns_server_packet_received(DnsServer *s, DnsServerFeatureLevel level, usec_ } } -void dns_server_packet_lost(DnsServer *s, DnsServerFeatureLevel level, usec_t usec) { +void dns_server_packet_lost(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t usec) { assert(s); assert(s->manager); - if (s->possible_feature_level == level) - s->n_failed_attempts ++; + if (s->possible_feature_level == level) { + if (protocol == IPPROTO_UDP) + s->n_failed_udp ++; + else if (protocol == IPPROTO_TCP) + s->n_failed_tcp ++; + } if (s->resend_timeout > usec) return; @@ -274,18 +295,24 @@ void dns_server_packet_failed(DnsServer *s, DnsServerFeatureLevel level) { assert(s); assert(s->manager); + /* Invoked whenever we get a FORMERR, SERVFAIL or NOTIMP rcode from a server. */ + if (s->possible_feature_level != level) return; - /* Invoked whenever we get a FORMERR, SERVFAIL or NOTIMP rcode from a server. This is an immediate trigger for - * us to go one feature level down. Except when we are already at TCP or UDP level, in which case there's no - * point in changing, under the assumption that packet failures are caused by packet contents, not by used - * transport. */ + s->packet_failed = true; +} + +void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level) { + assert(s); + assert(s->manager); - if (s->possible_feature_level <= DNS_SERVER_FEATURE_LEVEL_UDP) + /* Invoked whenever we get a packet with TC bit set. */ + + if (s->possible_feature_level != level) return; - s->n_failed_attempts = (unsigned) -1; + s->packet_truncated = true; } void dns_server_packet_rrsig_missing(DnsServer *s) { @@ -326,35 +353,71 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { _cleanup_free_ char *ip = NULL; s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_BEST; - s->n_failed_attempts = 0; + s->n_failed_udp = 0; + s->n_failed_tcp = 0; + s->packet_failed = false; + s->packet_truncated = false; s->verified_usec = 0; s->rrsig_missing = false; in_addr_to_string(s->family, &s->address, &ip); - log_info("Grace period over, resuming full feature set for DNS server %s", strna(ip)); + log_info("Grace period over, resuming full feature set (%s) for DNS server %s", + dns_server_feature_level_to_string(s->possible_feature_level), strna(ip)); + } else if (s->possible_feature_level <= s->verified_feature_level) s->possible_feature_level = s->verified_feature_level; - else if (s->n_failed_attempts >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS) { - _cleanup_free_ char *ip = NULL; + else { + DnsServerFeatureLevel p = s->possible_feature_level; - /* Switch one feature level down. Except when we are at TCP already, in which case we try UDP - * again. Thus, if a DNS server is not responding we'll keep toggling between UDP and TCP until it - * responds on one of them. Note that we generally prefer UDP over TCP (which is why it is at a higher - * feature level), but many DNS servers support lack TCP support. */ + if (s->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS && + s->possible_feature_level == DNS_SERVER_FEATURE_LEVEL_TCP) - if (s->possible_feature_level == DNS_SERVER_FEATURE_LEVEL_TCP) + /* We are at the TCP (lowest) level, and we tried a couple of TCP connections, and it didn't + * work. Upgrade back to UDP again. */ s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_UDP; - else { - assert(s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_WORST); - s->possible_feature_level --; - } - - s->n_failed_attempts = 0; - s->verified_usec = 0; - in_addr_to_string(s->family, &s->address, &ip); - log_warning("Using degraded feature set (%s) for DNS server %s", - dns_server_feature_level_to_string(s->possible_feature_level), strna(ip)); + else if ((s->n_failed_udp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS && + s->possible_feature_level >= DNS_SERVER_FEATURE_LEVEL_UDP) || + (s->packet_failed && + s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP) || + (s->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS && + s->packet_truncated && + s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP)) + + /* Downgrade the feature one level, maybe things will work better then. We do this under any of + * three conditions: + * + * 1. We lost too many UDP packets in a row, and are on a feature level of UDP or higher. If + * the packets are lost, maybe the server cannot parse them, hence downgrading sounds like a + * good idea. We might downgrade all the way down to TCP this way. + * + * 2. We got a failure packet, and are at a feature level above UDP. Note that in this case we + * downgrade no further than UDP, under the assumption that a failure packet indicates an + * incompatible packet contents, but not a problem with the transport. + * + * 3. We got too many TCP connection failures in a row, we had at least one truncated packet, + * and are on a feature level above UDP. By downgrading things and getting rid of DNSSEC or + * EDNS0 data we hope to make the packet smaller, so that it still works via UDP given that + * TCP appears not to be a fallback. Note that if we are already at the lowest UDP level, we + * don't go further down, since that's TCP, and TCP failed too often after all. + */ + + s->possible_feature_level--; + + if (p != s->possible_feature_level) { + _cleanup_free_ char *ip = NULL; + + /* We changed the feature level, reset the counting */ + s->n_failed_udp = 0; + s->n_failed_tcp = 0; + s->packet_failed = false; + s->packet_truncated = false; + s->verified_usec = 0; + + in_addr_to_string(s->family, &s->address, &ip); + log_warning("Using degraded feature set (%s) for DNS server %s", + dns_server_feature_level_to_string(s->possible_feature_level), strna(ip)); + } } return s->possible_feature_level; diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h index da21e6571a..3ad6a0d38f 100644 --- a/src/resolve/resolved-dns-server.h +++ b/src/resolve/resolved-dns-server.h @@ -67,7 +67,10 @@ struct DnsServer { DnsServerFeatureLevel verified_feature_level; DnsServerFeatureLevel possible_feature_level; size_t received_udp_packet_max; - unsigned n_failed_attempts; + unsigned n_failed_udp; + unsigned n_failed_tcp; + bool packet_failed:1; + bool packet_truncated:1; usec_t verified_usec; usec_t features_grace_period_usec; @@ -99,9 +102,10 @@ DnsServer* dns_server_unref(DnsServer *s); void dns_server_unlink(DnsServer *s); void dns_server_move_back_and_unmark(DnsServer *s); -void dns_server_packet_received(DnsServer *s, DnsServerFeatureLevel level, usec_t rtt, size_t size); -void dns_server_packet_lost(DnsServer *s, DnsServerFeatureLevel level, usec_t usec); +void dns_server_packet_received(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t rtt, size_t size); +void dns_server_packet_lost(DnsServer *s, int protocol, DnsServerFeatureLevel level, usec_t usec); void dns_server_packet_failed(DnsServer *s, DnsServerFeatureLevel level); +void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level); void dns_server_packet_rrsig_missing(DnsServer *s); DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 1ab9550cb3..1450acb260 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -370,7 +370,7 @@ static int on_stream_complete(DnsStream *s, int error) { log_debug_errno(error, "Connection failure for DNS TCP stream, treating as lost packet: %m"); assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0); - dns_server_packet_lost(t->server, t->current_features, usec - t->start_usec); + dns_server_packet_lost(t->server, IPPROTO_TCP, t->current_features, usec - t->start_usec); dns_transaction_retry(t); return 0; @@ -670,8 +670,10 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) { dns_server_packet_failed(t->server, t->current_features); dns_transaction_retry(t); return; - } else - dns_server_packet_received(t->server, t->current_features, ts - t->start_usec, p->size); + } else if (DNS_PACKET_TC(p)) + dns_server_packet_truncated(t->server, t->current_features); + else + dns_server_packet_received(t->server, p->ipproto, t->current_features, ts - t->start_usec, p->size); break; @@ -797,7 +799,7 @@ static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *use log_debug_errno(r, "Connection failure for DNS UDP packet, treating as lost packet: %m"); assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0); - dns_server_packet_lost(t->server, t->current_features, usec - t->start_usec); + dns_server_packet_lost(t->server, IPPROTO_UDP, t->current_features, usec - t->start_usec); dns_transaction_retry(t); return 0; @@ -889,7 +891,7 @@ static int on_transaction_timeout(sd_event_source *s, usec_t usec, void *userdat case DNS_PROTOCOL_DNS: assert(t->server); - dns_server_packet_lost(t->server, t->current_features, usec - t->start_usec); + dns_server_packet_lost(t->server, t->stream ? IPPROTO_TCP : IPPROTO_UDP, t->current_features, usec - t->start_usec); break; case DNS_PROTOCOL_LLMNR: @@ -2375,6 +2377,7 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { (t->server && t->server->rrsig_missing)) { /* The server does not support DNSSEC, or doesn't augment responses with RRSIGs. */ t->answer_dnssec_result = DNSSEC_INCOMPATIBLE_SERVER; + log_debug("Cannot validate reponse, server lacks DNSSEC support."); return 0; } -- cgit v1.2.3-54-g00ecf From 6cb08a8930bdaca950b152b1e8b82466ed59511c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 8 Jan 2016 20:59:03 +0100 Subject: resolved: cache formatted server string in DnsServer structure This makes it easier to log information about a specific DnsServer object. --- src/resolve/resolved-dns-server.c | 32 +++++++++++++++++--------------- src/resolve/resolved-dns-server.h | 4 ++++ src/resolve/resolved-link.c | 8 ++------ src/resolve/resolved-resolv-conf.c | 12 +++++------- 4 files changed, 28 insertions(+), 28 deletions(-) (limited to 'src/resolve/resolved-dns-server.c') diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 3baf63ffa7..dcfef66d4c 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -135,6 +135,7 @@ DnsServer* dns_server_unref(DnsServer *s) { if (s->n_ref > 0) return NULL; + free(s->server_string); free(s); return NULL; } @@ -316,12 +317,10 @@ void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level) { } void dns_server_packet_rrsig_missing(DnsServer *s) { - _cleanup_free_ char *ip = NULL; assert(s); assert(s->manager); - in_addr_to_string(s->family, &s->address, &ip); - log_warning("DNS server %s does not augment replies with RRSIG records, DNSSEC not available.", strna(ip)); + log_warning("DNS server %s does not augment replies with RRSIG records, DNSSEC not available.", dns_server_string(s)); s->rrsig_missing = true; } @@ -350,7 +349,6 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { if (s->possible_feature_level != DNS_SERVER_FEATURE_LEVEL_BEST && dns_server_grace_period_expired(s)) { - _cleanup_free_ char *ip = NULL; s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_BEST; s->n_failed_udp = 0; @@ -360,9 +358,9 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { s->verified_usec = 0; s->rrsig_missing = false; - in_addr_to_string(s->family, &s->address, &ip); log_info("Grace period over, resuming full feature set (%s) for DNS server %s", - dns_server_feature_level_to_string(s->possible_feature_level), strna(ip)); + dns_server_feature_level_to_string(s->possible_feature_level), + dns_server_string(s)); } else if (s->possible_feature_level <= s->verified_feature_level) s->possible_feature_level = s->verified_feature_level; @@ -405,7 +403,6 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { s->possible_feature_level--; if (p != s->possible_feature_level) { - _cleanup_free_ char *ip = NULL; /* We changed the feature level, reset the counting */ s->n_failed_udp = 0; @@ -414,9 +411,9 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { s->packet_truncated = false; s->verified_usec = 0; - in_addr_to_string(s->family, &s->address, &ip); log_warning("Using degraded feature set (%s) for DNS server %s", - dns_server_feature_level_to_string(s->possible_feature_level), strna(ip)); + dns_server_feature_level_to_string(s->possible_feature_level), + dns_server_string(s)); } } @@ -451,6 +448,15 @@ int dns_server_adjust_opt(DnsServer *server, DnsPacket *packet, DnsServerFeature return dns_packet_append_opt(packet, packet_size, edns_do, NULL); } +const char *dns_server_string(DnsServer *server) { + assert(server); + + if (!server->server_string) + (void) in_addr_to_string(server->family, &server->address, &server->server_string); + + return strna(server->server_string); +} + static void dns_server_hash_func(const void *p, struct siphash *state) { const DnsServer *s = p; @@ -542,12 +548,8 @@ DnsServer *manager_set_dns_server(Manager *m, DnsServer *s) { if (m->current_dns_server == s) return s; - if (s) { - _cleanup_free_ char *ip = NULL; - - in_addr_to_string(s->family, &s->address, &ip); - log_info("Switching to system DNS server %s.", strna(ip)); - } + if (s) + log_info("Switching to system DNS server %s.", dns_server_string(s)); dns_server_unref(m->current_dns_server); m->current_dns_server = dns_server_ref(s); diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h index 3ad6a0d38f..faa22babb5 100644 --- a/src/resolve/resolved-dns-server.h +++ b/src/resolve/resolved-dns-server.h @@ -61,6 +61,8 @@ struct DnsServer { int family; union in_addr_union address; + char *server_string; + usec_t resend_timeout; usec_t max_rtt; @@ -112,6 +114,8 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s); int dns_server_adjust_opt(DnsServer *server, DnsPacket *packet, DnsServerFeatureLevel level); +const char *dns_server_string(DnsServer *server); + DnsServer *dns_server_find(DnsServer *first, int family, const union in_addr_union *in_addr); void dns_server_unlink_all(DnsServer *first); diff --git a/src/resolve/resolved-link.c b/src/resolve/resolved-link.c index 30838ef8cc..928307e004 100644 --- a/src/resolve/resolved-link.c +++ b/src/resolve/resolved-link.c @@ -463,12 +463,8 @@ DnsServer* link_set_dns_server(Link *l, DnsServer *s) { if (l->current_dns_server == s) return s; - if (s) { - _cleanup_free_ char *ip = NULL; - - in_addr_to_string(s->family, &s->address, &ip); - log_info("Switching to DNS server %s for interface %s.", strna(ip), l->name); - } + if (s) + log_info("Switching to DNS server %s for interface %s.", dns_server_string(s), l->name); dns_server_unref(l->current_dns_server); l->current_dns_server = dns_server_ref(s); diff --git a/src/resolve/resolved-resolv-conf.c b/src/resolve/resolved-resolv-conf.c index 956f380f3c..7567f4c369 100644 --- a/src/resolve/resolved-resolv-conf.c +++ b/src/resolve/resolved-resolv-conf.c @@ -147,16 +147,14 @@ clear: } static void write_resolv_conf_server(DnsServer *s, FILE *f, unsigned *count) { - _cleanup_free_ char *t = NULL; - int r; - assert(s); assert(f); assert(count); - r = in_addr_to_string(s->family, &s->address, &t); - if (r < 0) { - log_warning_errno(r, "Invalid DNS address. Ignoring: %m"); + (void) dns_server_string(s); + + if (!s->server_string) { + log_warning("Our of memory, or invalid DNS address. Ignoring server."); return; } @@ -164,7 +162,7 @@ static void write_resolv_conf_server(DnsServer *s, FILE *f, unsigned *count) { fputs("# Too many DNS servers configured, the following entries may be ignored.\n", f); (*count) ++; - fprintf(f, "nameserver %s\n", t); + fprintf(f, "nameserver %s\n", s->server_string); } static void write_resolv_conf_search( -- cgit v1.2.3-54-g00ecf From 92ec902aad1ade7acbe50efd7b8ef87fbdc63af3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 10 Jan 2016 22:58:58 +0100 Subject: resolved: rework how and when we detect whether our chosen DNS server knows DNSSEC Move detection into a set of new functions, that check whether one specific server can do DNSSEC, whether a server and a specific transaction can do DNSSEC, or whether a transaction and all its auxiliary transactions could do so. Also, do these checks both before we acquire additional RRs for the validation (so that we can skip them if the server doesn't do DNSSEC anyway), and after we acquired them all (to see if any of the lookups changed our opinion about the servers). THis also tightens the checks a bit: a server that lacks TCP support is considered incompatible with DNSSEC too. --- src/resolve/resolved-dns-server.c | 18 ++++++++++ src/resolve/resolved-dns-server.h | 2 ++ src/resolve/resolved-dns-transaction.c | 60 +++++++++++++++++++++++++++------- 3 files changed, 68 insertions(+), 12 deletions(-) (limited to 'src/resolve/resolved-dns-server.c') diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index dcfef66d4c..2a0301aa49 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -457,6 +457,24 @@ const char *dns_server_string(DnsServer *server) { return strna(server->server_string); } +bool dns_server_dnssec_supported(DnsServer *server) { + assert(server); + + /* Returns whether the server supports DNSSEC according to what we know about it */ + + if (server->possible_feature_level < DNS_SERVER_FEATURE_LEVEL_DO) + return false; + + if (server->rrsig_missing) + return false; + + /* DNSSEC servers need to support TCP properly (see RFC5966), if they don't, we assume DNSSEC is borked too */ + if (server->n_failed_tcp >= DNS_SERVER_FEATURE_RETRY_ATTEMPTS) + return false; + + return true; +} + static void dns_server_hash_func(const void *p, struct siphash *state) { const DnsServer *s = p; diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h index faa22babb5..323f702903 100644 --- a/src/resolve/resolved-dns-server.h +++ b/src/resolve/resolved-dns-server.h @@ -116,6 +116,8 @@ int dns_server_adjust_opt(DnsServer *server, DnsPacket *packet, DnsServerFeature const char *dns_server_string(DnsServer *server); +bool dns_server_dnssec_supported(DnsServer *server); + DnsServer *dns_server_find(DnsServer *first, int family, const union in_addr_union *in_addr); void dns_server_unlink_all(DnsServer *first); diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index 1450acb260..aa1970bc34 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -368,7 +368,7 @@ static int on_stream_complete(DnsStream *s, int error) { if (ERRNO_IS_DISCONNECT(error)) { usec_t usec; - log_debug_errno(error, "Connection failure for DNS TCP stream, treating as lost packet: %m"); + log_debug_errno(error, "Connection failure for DNS TCP stream: %m"); assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0); dns_server_packet_lost(t->server, IPPROTO_TCP, t->current_features, usec - t->start_usec); @@ -418,7 +418,7 @@ static int dns_transaction_open_tcp(DnsTransaction *t) { if (r < 0) return r; - if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO && dns_type_is_dnssec(t->key->type)) + if (!dns_server_dnssec_supported(t->server) && dns_type_is_dnssec(t->key->type)) return -EOPNOTSUPP; r = dns_server_adjust_opt(t->server, t->sent, t->current_features); @@ -797,7 +797,7 @@ static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *use /* UDP connection failure get reported via ICMP and then are possible delivered to us on the next * recvmsg(). Treat this like a lost packet. */ - log_debug_errno(r, "Connection failure for DNS UDP packet, treating as lost packet: %m"); + log_debug_errno(r, "Connection failure for DNS UDP packet: %m"); assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &usec) >= 0); dns_server_packet_lost(t->server, IPPROTO_UDP, t->current_features, usec - t->start_usec); @@ -842,7 +842,7 @@ static int dns_transaction_emit_udp(DnsTransaction *t) { if (t->current_features < DNS_SERVER_FEATURE_LEVEL_UDP) return -EAGAIN; - if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO && dns_type_is_dnssec(t->key->type)) + if (!dns_server_dnssec_supported(t->server) && dns_type_is_dnssec(t->key->type)) return -EOPNOTSUPP; if (r > 0 || t->dns_udp_fd < 0) { /* Server changed, or no connection yet. */ @@ -1555,6 +1555,44 @@ static int dns_transaction_is_primary_response(DnsTransaction *t, DnsResourceRec return rr->key->type == DNS_TYPE_NSEC; } +static bool dns_transaction_dnssec_supported(DnsTransaction *t) { + assert(t); + + /* Checks whether our transaction's DNS server is assumed to be compatible with DNSSEC. Returns false as soon + * as we changed our mind about a server, and now believe it is incompatible with DNSSEC. */ + + if (t->scope->protocol != DNS_PROTOCOL_DNS) + return false; + + /* If we have picked no server, then we are working from the cache or some other source, and DNSSEC might well + * be supported, hence return true. */ + if (!t->server) + return true; + + if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO) + return false; + + return dns_server_dnssec_supported(t->server); +} + +static bool dns_transaction_dnssec_supported_full(DnsTransaction *t) { + DnsTransaction *dt; + Iterator i; + + assert(t); + + /* Checks whether our transaction our any of the auxiliary transactions couldn't do DNSSEC. */ + + if (!dns_transaction_dnssec_supported(t)) + return false; + + SET_FOREACH(dt, t->dnssec_transactions, i) + if (!dns_transaction_dnssec_supported(dt)) + return false; + + return true; +} + int dns_transaction_request_dnssec_keys(DnsTransaction *t) { DnsResourceRecord *rr; @@ -1578,11 +1616,10 @@ int dns_transaction_request_dnssec_keys(DnsTransaction *t) { if (t->scope->dnssec_mode == DNSSEC_NO) return 0; - - if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO) - return 0; /* Server doesn't do DNSSEC, there's no point in requesting any RRs then. */ - if (t->server && t->server->rrsig_missing) - return 0; /* Server handles DNSSEC requests, but isn't augmenting responses with RRSIGs. No point in trying DNSSEC then. */ + if (t->answer_source != DNS_TRANSACTION_NETWORK) + return 0; /* We only need to validate stuff from the network */ + if (!dns_transaction_dnssec_supported(t)) + return 0; /* If we can't do DNSSEC anyway there's no point in geting the auxiliary RRs */ DNS_ANSWER_FOREACH(rr, t->answer) { @@ -2373,11 +2410,10 @@ int dns_transaction_validate_dnssec(DnsTransaction *t) { if (t->answer_source != DNS_TRANSACTION_NETWORK) return 0; - if (t->current_features < DNS_SERVER_FEATURE_LEVEL_DO || - (t->server && t->server->rrsig_missing)) { + if (!dns_transaction_dnssec_supported_full(t)) { /* The server does not support DNSSEC, or doesn't augment responses with RRSIGs. */ t->answer_dnssec_result = DNSSEC_INCOMPATIBLE_SERVER; - log_debug("Cannot validate reponse, server lacks DNSSEC support."); + log_debug("Cannot validate response, server lacks DNSSEC support."); return 0; } -- cgit v1.2.3-54-g00ecf From 011842775f750711833526d5bba1b818713947f5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 11 Jan 2016 18:57:59 +0100 Subject: resolved: split out resetting of DNS server counters into a function call of its own A suggested by Vito Caputo: https://github.com/systemd/systemd/pull/2289#discussion-diff-49276220 --- src/resolve/resolved-dns-server.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) (limited to 'src/resolve/resolved-dns-server.c') diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 2a0301aa49..0969e31e8a 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -344,6 +344,16 @@ static bool dns_server_grace_period_expired(DnsServer *s) { return true; } +static void dns_server_reset_counters(DnsServer *s) { + assert(s); + + s->n_failed_udp = 0; + s->n_failed_tcp = 0; + s->packet_failed = false; + s->packet_truncated = false; + s->verified_usec = 0; +} + DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { assert(s); @@ -351,13 +361,10 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { dns_server_grace_period_expired(s)) { s->possible_feature_level = DNS_SERVER_FEATURE_LEVEL_BEST; - s->n_failed_udp = 0; - s->n_failed_tcp = 0; - s->packet_failed = false; - s->packet_truncated = false; - s->verified_usec = 0; s->rrsig_missing = false; + dns_server_reset_counters(s); + log_info("Grace period over, resuming full feature set (%s) for DNS server %s", dns_server_feature_level_to_string(s->possible_feature_level), dns_server_string(s)); @@ -405,11 +412,7 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { if (p != s->possible_feature_level) { /* We changed the feature level, reset the counting */ - s->n_failed_udp = 0; - s->n_failed_tcp = 0; - s->packet_failed = false; - s->packet_truncated = false; - s->verified_usec = 0; + dns_server_reset_counters(s); log_warning("Using degraded feature set (%s) for DNS server %s", dns_server_feature_level_to_string(s->possible_feature_level), -- cgit v1.2.3-54-g00ecf