From 74095d38ede60d2b65f0b8d218640f589de72840 Mon Sep 17 00:00:00 2001 From: Frank Denis Date: Thu, 26 Mar 2020 17:20:34 +0100 Subject: [PATCH] Remove LargerResponsesDropped dnsdist drops DNSCrypt queries shorter than 256 bytes, interpreting them as not being encrypted instead. This is surprising when doing ad-hoc testing, but absolutely fine, and we will never send shorter encrypted queries on normal circumstances. So, remove a useless knob. --- ChangeLog | 9 ++++----- dnscrypt-proxy/config.go | 13 +++---------- dnscrypt-proxy/crypto.go | 8 ++------ dnscrypt-proxy/example-dnscrypt-proxy.toml | 11 ++--------- dnscrypt-proxy/proxy.go | 1 - dnscrypt-proxy/serversInfo.go | 10 +--------- 6 files changed, 12 insertions(+), 40 deletions(-) diff --git a/ChangeLog b/ChangeLog index 3d6b1f76..ec23dec0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -2,11 +2,10 @@ - Version 1.4.0 of the dnsdist load balancer (presumably used by quad9, cleanbrowsing, qualityology, freetsa.org, ffmuc.net, opennic-bongobow, sth-dnscrypt-se, ams-dnscrypt-nl and more) -unintentionally introduced a regression preventing large queries -from being received over UDP. Temporary workarounds have been -introduced to improve reliability with these resolvers for regular -DNSCrypt. Unfortunately, anonymized DNS cannot be reliable until -dnsdist is updated on these servers. +is preventing queries over 1500 bytes from being received over UDP. +Temporary workarounds have been introduced to improve reliability +with these resolvers for regular DNSCrypt. Unfortunately, anonymized +DNS cannot be reliable until dnsdist is updated on these servers. - New option in the `[anonymized_dns]` section: `skip_incompatible`, to ignore resolvers incompatible with Anonymized DNS instead of using them without a relay. diff --git a/dnscrypt-proxy/config.go b/dnscrypt-proxy/config.go index 2a835f6c..11b18840 100644 --- a/dnscrypt-proxy/config.go +++ b/dnscrypt-proxy/config.go @@ -139,10 +139,6 @@ func newConfig() Config { "quad9-dnscrypt-ip4-filter-alt", "quad9-dnscrypt-ip4-filter-pri", "quad9-dnscrypt-ip4-nofilter-alt", "quad9-dnscrypt-ip4-nofilter-pri", "quad9-dnscrypt-ip6-filter-alt", "quad9-dnscrypt-ip6-filter-pri", "quad9-dnscrypt-ip6-nofilter-alt", "quad9-dnscrypt-ip6-nofilter-pri", "cleanbrowsing-adult", "cleanbrowsing-family-ipv6", "cleanbrowsing-family", "cleanbrowsing-security", }, - LargerResponsesDropped: []string{ - "quad9-dnscrypt-ip4-filter-alt", "quad9-dnscrypt-ip4-filter-pri", "quad9-dnscrypt-ip4-nofilter-alt", "quad9-dnscrypt-ip4-nofilter-pri", "quad9-dnscrypt-ip6-filter-alt", "quad9-dnscrypt-ip6-filter-pri", "quad9-dnscrypt-ip6-nofilter-alt", "quad9-dnscrypt-ip6-nofilter-pri", - "cleanbrowsing-adult", "cleanbrowsing-family-ipv6", "cleanbrowsing-family", "cleanbrowsing-security", - }, }, } } @@ -201,9 +197,8 @@ type AnonymizedDNSConfig struct { } type BrokenImplementationsConfig struct { - BrokenQueryPadding []string `toml:"broken_query_padding"` - FragmentsBlocked []string `toml:"fragments_blocked"` - LargerResponsesDropped []string `toml:"larger_responses_dropped"` + BrokenQueryPadding []string `toml:"broken_query_padding"` + FragmentsBlocked []string `toml:"fragments_blocked"` } type LocalDoHConfig struct { @@ -517,10 +512,8 @@ func ConfigLoad(proxy *Proxy, flags *ConfigFlags) error { // Backwards compatibility config.BrokenImplementations.FragmentsBlocked = append(config.BrokenImplementations.FragmentsBlocked, config.BrokenImplementations.BrokenQueryPadding...) - config.BrokenImplementations.LargerResponsesDropped = append(config.BrokenImplementations.LargerResponsesDropped, config.BrokenImplementations.BrokenQueryPadding...) - proxy.serversBlockingFragments = config.BrokenImplementations.FragmentsBlocked - proxy.serversDroppingLargerResponses = config.BrokenImplementations.LargerResponsesDropped + proxy.serversBlockingFragments = config.BrokenImplementations.BrokenQueryPadding if *flags.ListAll { config.ServerNames = nil diff --git a/dnscrypt-proxy/crypto.go b/dnscrypt-proxy/crypto.go index 8590311e..a8e1fc56 100644 --- a/dnscrypt-proxy/crypto.go +++ b/dnscrypt-proxy/crypto.go @@ -87,12 +87,8 @@ func (proxy *Proxy) Encrypt(serverInfo *ServerInfo, packet []byte, proto string) minQuestionSize += int(xpad[0]) } paddedLength := Min(MaxDNSUDPPacketSize, (Max(minQuestionSize, QueryOverhead)+1+63) & ^63) - if proto == "udp" { - if serverInfo.knownBugs.fragmentsBlocked { - paddedLength = MaxDNSUDPSafePacketSize - } else if serverInfo.knownBugs.largerQueriesDropped { - paddedLength = MaxDNSUDPPacketSize - } + if proto == "udp" && serverInfo.knownBugs.fragmentsBlocked { + paddedLength = MaxDNSUDPSafePacketSize } if serverInfo.RelayUDPAddr != nil && proto == "tcp" { paddedLength = MaxDNSPacketSize diff --git a/dnscrypt-proxy/example-dnscrypt-proxy.toml b/dnscrypt-proxy/example-dnscrypt-proxy.toml index 8548cbf9..508bb3ce 100644 --- a/dnscrypt-proxy/example-dnscrypt-proxy.toml +++ b/dnscrypt-proxy/example-dnscrypt-proxy.toml @@ -626,21 +626,14 @@ cache_neg_max_ttl = 600 # truncate reponses larger than questions as expected by the DNSCrypt protocol. # This prevents large responses from being received over UDP and over relays. # -# The `dnsdist` server software properly truncates DNSCrypt responses, but -# introduced a change in version 1.4.0 that inadvertently broke relaying for the -# same reason. They are aware of it and are working on a fix. +# The `dnsdist` server software drops incoming packets larger than 1500 bytes. +# They are aware of it and are working on a fix. # # The list below enables workarounds to make non-relayed usage more reliable # until the servers are fixed. fragments_blocked = ['cisco', 'cisco-ipv6', 'cisco-familyshield', 'cisco-familyshield-ipv6', 'quad9-dnscrypt-ip4-filter-alt', 'quad9-dnscrypt-ip4-filter-pri', 'quad9-dnscrypt-ip4-nofilter-alt', 'quad9-dnscrypt-ip4-nofilter-pri', 'quad9-dnscrypt-ip6-filter-alt', 'quad9-dnscrypt-ip6-filter-pri', 'quad9-dnscrypt-ip6-nofilter-alt', 'quad9-dnscrypt-ip6-nofilter-pri', 'cleanbrowsing-adult', 'cleanbrowsing-family-ipv6', 'cleanbrowsing-family', 'cleanbrowsing-security'] -# Quad9 ignores the query instead of sending a truncated response when the -# response is larger than the question. -# Do not change that list until the bugs are fixed server-side. - -larger_responses_dropped = ['quad9-dnscrypt-ip4-filter-alt', 'quad9-dnscrypt-ip4-filter-pri', 'quad9-dnscrypt-ip4-nofilter-alt', 'quad9-dnscrypt-ip4-nofilter-pri', 'quad9-dnscrypt-ip6-filter-alt', 'quad9-dnscrypt-ip6-filter-pri', 'quad9-dnscrypt-ip6-nofilter-alt', 'quad9-dnscrypt-ip6-nofilter-pri', 'cleanbrowsing-adult', 'cleanbrowsing-family-ipv6', 'cleanbrowsing-family', 'cleanbrowsing-security'] - diff --git a/dnscrypt-proxy/proxy.go b/dnscrypt-proxy/proxy.go index fdee0fcf..58e66915 100644 --- a/dnscrypt-proxy/proxy.go +++ b/dnscrypt-proxy/proxy.go @@ -76,7 +76,6 @@ type Proxy struct { queryMeta []string routes *map[string][]string serversBlockingFragments []string - serversDroppingLargerResponses []string showCerts bool dohCreds *map[string]DOHClientCreds skipAnonIncompatbibleResolvers bool diff --git a/dnscrypt-proxy/serversInfo.go b/dnscrypt-proxy/serversInfo.go index 49b01e9f..528b106c 100644 --- a/dnscrypt-proxy/serversInfo.go +++ b/dnscrypt-proxy/serversInfo.go @@ -32,8 +32,7 @@ type RegisteredServer struct { } type ServerBugs struct { - fragmentsBlocked bool - largerQueriesDropped bool + fragmentsBlocked bool } type DOHClientCreds struct { @@ -327,13 +326,6 @@ func fetchDNSCryptServerInfo(proxy *Proxy, name string, stamp stamps.ServerStamp break } } - for _, buggyServerName := range proxy.serversDroppingLargerResponses { - if buggyServerName == name { - knownBugs.largerQueriesDropped = true - dlog.Infof("Known bug in [%v]: truncated responses are not sent when a response is larger than the query", name) - break - } - } relayUDPAddr, relayTCPAddr, err := route(proxy, name) if err != nil { return ServerInfo{}, err