From: Lennart Poettering Date: Wed, 11 Nov 2020 20:03:04 +0000 (+0100) Subject: resolved: when we can't parse a packet, downgrade feature level X-Git-Tag: v248-rc1~102^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=2c42a217a2ef4a62933290f5ef204bcf89ea52fd;p=thirdparty%2Fsystemd.git resolved: when we can't parse a packet, downgrade feature level So far we didn't really handle the case where we can't parse a reply packet. Since this apparently happens in real-life though, let's add some minimal logic, to downgrade/restart if we see this. --- diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c index 40da80b39ae..70bb7178dd8 100644 --- a/src/resolve/resolved-dns-server.c +++ b/src/resolve/resolved-dns-server.c @@ -240,6 +240,7 @@ static void dns_server_reset_counters(DnsServer *s) { s->n_failed_tcp = 0; s->n_failed_tls = 0; s->packet_truncated = false; + s->packet_invalid = false; s->verified_usec = 0; /* Note that we do not reset s->packet_bad_opt and s->packet_rrsig_missing here. We reset them only when the @@ -366,6 +367,17 @@ void dns_server_packet_rcode_downgrade(DnsServer *s, DnsServerFeatureLevel level log_debug("Downgrading transaction feature level fixed an RCODE error, downgrading server %s too.", strna(dns_server_string_full(s))); } +void dns_server_packet_invalid(DnsServer *s, DnsServerFeatureLevel level) { + assert(s); + + /* Invoked whenever we got a packet we couldn't parse at all */ + + if (s->possible_feature_level != level) + return; + + s->packet_invalid = true; +} + static bool dns_server_grace_period_expired(DnsServer *s) { usec_t ts; @@ -442,6 +454,22 @@ DnsServerFeatureLevel dns_server_possible_feature_level(DnsServer *s) { log_debug("Server doesn't support DNS-over-TLS, downgrading protocol..."); s->possible_feature_level--; + + } else if (s->packet_invalid && + s->possible_feature_level > DNS_SERVER_FEATURE_LEVEL_UDP && + s->possible_feature_level != DNS_SERVER_FEATURE_LEVEL_TLS_PLAIN) { + + /* Downgrade from DO to EDNS0 + from EDNS0 to UDP, from TLS+DO to plain TLS. Or in + * other words, if we receive a packet we cannot parse jump to the next lower feature + * level that actually has an influence on the packet layout (and not just the + * transport). */ + + log_debug("Got invalid packet from server, downgrading protocol..."); + s->possible_feature_level = + s->possible_feature_level == DNS_SERVER_FEATURE_LEVEL_TLS_DO ? DNS_SERVER_FEATURE_LEVEL_TLS_PLAIN : + DNS_SERVER_FEATURE_LEVEL_IS_DNSSEC(s->possible_feature_level) ? DNS_SERVER_FEATURE_LEVEL_EDNS0 : + DNS_SERVER_FEATURE_LEVEL_UDP; + } else if (s->packet_bad_opt && s->possible_feature_level >= DNS_SERVER_FEATURE_LEVEL_EDNS0) { @@ -903,13 +931,15 @@ void dns_server_dump(DnsServer *s, FILE *f) { "\tFailed TCP attempts: %u\n" "\tSeen truncated packet: %s\n" "\tSeen OPT RR getting lost: %s\n" - "\tSeen RRSIG RR missing: %s\n", + "\tSeen RRSIG RR missing: %s\n" + "\tSeen invalid packet: %s\n", s->received_udp_packet_max, s->n_failed_udp, s->n_failed_tcp, yes_no(s->packet_truncated), yes_no(s->packet_bad_opt), - yes_no(s->packet_rrsig_missing)); + yes_no(s->packet_rrsig_missing), + yes_no(s->packet_invalid)); } void dns_server_unref_stream(DnsServer *s) { diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h index 96e46226eed..b6f96607482 100644 --- a/src/resolve/resolved-dns-server.h +++ b/src/resolve/resolved-dns-server.h @@ -41,6 +41,7 @@ typedef enum DnsServerFeatureLevel { #define DNS_SERVER_FEATURE_LEVEL_WORST 0 #define DNS_SERVER_FEATURE_LEVEL_BEST (_DNS_SERVER_FEATURE_LEVEL_MAX - 1) #define DNS_SERVER_FEATURE_LEVEL_IS_TLS(x) IN_SET(x, DNS_SERVER_FEATURE_LEVEL_TLS_PLAIN, DNS_SERVER_FEATURE_LEVEL_TLS_DO) +#define DNS_SERVER_FEATURE_LEVEL_IS_DNSSEC(x) ((x) >= DNS_SERVER_FEATURE_LEVEL_DO) const char* dns_server_feature_level_to_string(int i) _const_; int dns_server_feature_level_from_string(const char *s) _pure_; @@ -78,9 +79,10 @@ struct DnsServer { unsigned n_failed_tcp; unsigned n_failed_tls; - bool packet_truncated:1; - bool packet_bad_opt:1; - bool packet_rrsig_missing:1; + bool packet_truncated:1; /* Set when TC bit was set on reply */ + bool packet_bad_opt:1; /* Set when OPT was missing or otherwise bad on reply */ + bool packet_rrsig_missing:1; /* Set when RRSIG was missing */ + bool packet_invalid:1; /* Set when we failed to parse a reply */ usec_t verified_usec; usec_t features_grace_period_usec; @@ -119,6 +121,7 @@ void dns_server_packet_truncated(DnsServer *s, DnsServerFeatureLevel level); void dns_server_packet_rrsig_missing(DnsServer *s, DnsServerFeatureLevel level); void dns_server_packet_bad_opt(DnsServer *s, DnsServerFeatureLevel level); void dns_server_packet_rcode_downgrade(DnsServer *s, DnsServerFeatureLevel level); +void dns_server_packet_invalid(DnsServer *s, DnsServerFeatureLevel level); 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 bd2ae5e4fb7..66226c06818 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1189,6 +1189,16 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p, bool encrypt /* After the superficial checks, actually parse the message. */ r = dns_packet_extract(p); if (r < 0) { + if (t->server) { + dns_server_packet_invalid(t->server, t->current_feature_level); + + r = dns_transaction_maybe_restart(t); + if (r < 0) + goto fail; + if (r > 0) /* Transaction got restarted... */ + return; + } + dns_transaction_complete(t, DNS_TRANSACTION_INVALID_REPLY); return; }