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.
DNSCrypt · Mar 26, 2020 · 74095d3 · 74095d3
1 parent fb04a62
commit 74095d3
Show file tree

Hide file tree

Showing 6 changed files with 12 additions and 40 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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