Try harder to work around Cisco and Quad9 bugs

ChangeLog

@@ -1,9 +1,7 @@
 * Version 2.0.42
  - Quad9 was put back into the list of broken implementations. They
 drop responses larger than questions instead of truncating them.
- - Queries for servers that don't properly handle padding are now
-padded to 1472 bytes. This mitigates the issue with Quad9 while
-still working around the limitations of Cisco resolvers.
+ - More workarounds were implemented for servers dropping UDP fragments.
 
 * Version 2.0.41
  - Precompiled ARM binaries are compatible with ARMv5 CPUs. The

dnscrypt-proxy/config.go

@@ -134,7 +134,8 @@ func newConfig() Config {
 		LBEstimator:              true,
 		BlockedQueryResponse:     "hinfo",
 		BrokenImplementations: BrokenImplementationsConfig{
-			BrokenQueryPadding: []string{"cisco", "cisco-ipv6", "cisco-familyshield", "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"},
+			FragmentsBlocked:       []string{"cisco", "cisco-ipv6", "cisco-familyshield", "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"},
+			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"},
 		},
 	}
 }
@@ -193,6 +194,8 @@ type AnonymizedDNSConfig struct {
 
 type BrokenImplementationsConfig struct {
 	BrokenQueryPadding     []string `toml:"broken_query_padding"`
+	FragmentsBlocked       []string `toml:"fragments_blocked"`
+	LargerResponsesDropped []string `toml:"larger_responses_dropped"`
 }
 
 type LocalDoHConfig struct {
@@ -502,7 +505,12 @@ func ConfigLoad(proxy *Proxy, flags *ConfigFlags) error {
 	}
 	proxy.dohCreds = &creds
 
-	proxy.serversWithBrokenQueryPadding = config.BrokenImplementations.BrokenQueryPadding
+	// 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
 
 	if *flags.ListAll {
 		config.ServerNames = nil

dnscrypt-proxy/crypto.go

@@ -80,18 +80,24 @@ func (proxy *Proxy) Encrypt(serverInfo *ServerInfo, packet []byte, proto string)
 	}
 	minQuestionSize := QueryOverhead + len(packet)
 	if proto == "udp" {
-		if serverInfo.knownBugs.incorrectPadding {
-			// XXX - Note: Cisco's broken implementation doesn't accept more than 1472 bytes
-			minQuestionSize = Max(1472, minQuestionSize)
-		} else {
 		minQuestionSize = Max(proxy.questionSizeEstimator.MinQuestionSize(), minQuestionSize)
-		}
 	} else {
 		var xpad [1]byte
 		rand.Read(xpad[:])
 		minQuestionSize += int(xpad[0])
 	}
 	paddedLength := Min(MaxDNSUDPPacketSize, (Max(minQuestionSize, QueryOverhead)+1+63) & ^63)
+	if proto == "udp" {
+		if serverInfo.knownBugs.fragmentsBlocked {
+			if serverInfo.knownBugs.largerQueriesDropped {
+				paddedLength = MaxDNSUDPSafePacketSize
+			} else {
+				paddedLength = Min(MaxDNSUDPSafePacketSize, paddedLength)
+			}
+		} else if serverInfo.knownBugs.largerQueriesDropped {
+			paddedLength = MaxDNSUDPPacketSize
+		}
+	}
 	if serverInfo.RelayUDPAddr != nil && proto == "tcp" {
 		paddedLength = MaxDNSPacketSize
 	}

dnscrypt-proxy/dnscrypt_certs.go

@@ -38,7 +38,7 @@ func FetchCurrentDNSCryptCert(proxy *Proxy, serverName *string, proto string, pk
 		relayUDPAddr, relayTCPAddr = nil, nil
 	}
 	tryFragmentsSupport := true
-	if knownBugs.incorrectPadding {
+	if knownBugs.fragmentsBlocked {
 		tryFragmentsSupport = false
 	}
 	in, rtt, fragmentsBlocked, err := dnsExchange(proxy, proto, &query, serverAddress, relayUDPAddr, relayTCPAddr, serverName, tryFragmentsSupport)

dnscrypt-proxy/example-dnscrypt-proxy.toml

@@ -624,14 +624,19 @@ cache_neg_max_ttl = 600
 
 # Cisco servers currently cannot handle queries larger than 1472 bytes, and don't
 # truncate reponses larger than questions as expected by the DNSCrypt protocol.
-# Quad9 ignores the query instead of sending a truncated response when the
-# response is larger than the question.
+
 # This prevents large responses from being received over UDP, and breaks relaying.
 # A workaround for the first issue will be applied to servers in list below.
 # Relaying cannot be reliable until the servers are fixed.
 # Do not change that list until the bugs are fixed server-side.
 
-broken_query_padding = ['cisco', 'cisco-ipv6', 'cisco-familyshield', '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']
+fragments_blocked = ['cisco', 'cisco-ipv6', 'cisco-familyshield', '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']
+
+# 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']
 
 
 

dnscrypt-proxy/proxy.go

@@ -75,7 +75,8 @@ type Proxy struct {
 	blockedQueryResponse           string
 	queryMeta                      []string
 	routes                         *map[string][]string
-	serversWithBrokenQueryPadding []string
+	serversBlockingFragments       []string
+	serversDroppingLargerResponses []string
 	showCerts                      bool
 	dohCreds                       *map[string]DOHClientCreds
 }
@@ -473,6 +474,11 @@ func (proxy *Proxy) processIncomingQuery(clientProto string, serverProto string,
 		pluginsState.serverName = serverName
 		if serverInfo.Proto == stamps.StampProtoTypeDNSCrypt {
 			sharedKey, encryptedQuery, clientNonce, err := proxy.Encrypt(serverInfo, query, serverProto)
+			if err != nil && serverProto == "udp" {
+				dlog.Debug("Unable to pad for UDP, re-encrypting query for TCP")
+				serverProto = "tcp"
+				sharedKey, encryptedQuery, clientNonce, err = proxy.Encrypt(serverInfo, query, serverProto)
+			}
 			if err != nil {
 				pluginsState.returnCode = PluginsReturnCodeParseError
 				pluginsState.ApplyLoggingPlugins(&proxy.pluginsGlobals)

dnscrypt-proxy/serversInfo.go

@@ -32,7 +32,8 @@ type RegisteredServer struct {
 }
 
 type ServerBugs struct {
-	incorrectPadding bool
+	fragmentsBlocked     bool
+	largerQueriesDropped bool
 }
 
 type DOHClientCreds struct {
@@ -319,10 +320,17 @@ func fetchDNSCryptServerInfo(proxy *Proxy, name string, stamp stamps.ServerStamp
 		stamp.ServerPk = serverPk
 	}
 	knownBugs := ServerBugs{}
-	for _, buggyServerName := range proxy.serversWithBrokenQueryPadding {
+	for _, buggyServerName := range proxy.serversBlockingFragments {
 		if buggyServerName == name {
-			knownBugs.incorrectPadding = true
-			dlog.Infof("Known bug in [%v]: padding is not correctly handled", name)
+			knownBugs.fragmentsBlocked = true
+			dlog.Infof("Known bug in [%v]: fragmented questions over UDP are blocked", name)
+			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
 		}
 	}
@@ -331,11 +339,11 @@ func fetchDNSCryptServerInfo(proxy *Proxy, name string, stamp stamps.ServerStamp
 		return ServerInfo{}, err
 	}
 	certInfo, rtt, fragmentsBlocked, err := FetchCurrentDNSCryptCert(proxy, &name, proxy.mainProto, stamp.ServerPk, stamp.ServerAddrStr, stamp.ProviderName, isNew, relayUDPAddr, relayTCPAddr, knownBugs)
-	if !knownBugs.incorrectPadding && fragmentsBlocked {
+	if !knownBugs.fragmentsBlocked && fragmentsBlocked {
 		dlog.Debugf("[%v] drops fragmented queries", name)
-		knownBugs.incorrectPadding = true
+		knownBugs.fragmentsBlocked = true
 	}
-	if knownBugs.incorrectPadding && (relayUDPAddr != nil || relayTCPAddr != nil) {
+	if knownBugs.fragmentsBlocked && (relayUDPAddr != nil || relayTCPAddr != nil) {
 		dlog.Warnf("[%v] is incompatible with anonymization", name)
 		relayTCPAddr, relayUDPAddr = nil, nil
 	}