diff --git a/ChangeLog b/ChangeLog index 1b84709b..a1970d92 100644 --- a/ChangeLog +++ b/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 diff --git a/dnscrypt-proxy/config.go b/dnscrypt-proxy/config.go index 5179c2c3..412946c4 100644 --- a/dnscrypt-proxy/config.go +++ b/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"}, }, } } @@ -192,7 +193,9 @@ type AnonymizedDNSConfig struct { } type BrokenImplementationsConfig struct { - BrokenQueryPadding []string `toml:"broken_query_padding"` + 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 diff --git a/dnscrypt-proxy/crypto.go b/dnscrypt-proxy/crypto.go index 5e11d2e0..bbd14b57 100644 --- a/dnscrypt-proxy/crypto.go +++ b/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) - } + 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 } diff --git a/dnscrypt-proxy/dnscrypt_certs.go b/dnscrypt-proxy/dnscrypt_certs.go index d6911a69..f3c01b68 100644 --- a/dnscrypt-proxy/dnscrypt_certs.go +++ b/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) diff --git a/dnscrypt-proxy/example-dnscrypt-proxy.toml b/dnscrypt-proxy/example-dnscrypt-proxy.toml index ec7a4067..1ef37820 100644 --- a/dnscrypt-proxy/example-dnscrypt-proxy.toml +++ b/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'] diff --git a/dnscrypt-proxy/proxy.go b/dnscrypt-proxy/proxy.go index 90eaca5d..47863357 100644 --- a/dnscrypt-proxy/proxy.go +++ b/dnscrypt-proxy/proxy.go @@ -16,68 +16,69 @@ import ( ) type Proxy struct { - userName string - child bool - proxyPublicKey [32]byte - proxySecretKey [32]byte - ephemeralKeys bool - questionSizeEstimator QuestionSizeEstimator - serversInfo ServersInfo - timeout time.Duration - certRefreshDelay time.Duration - certRefreshDelayAfterFailure time.Duration - certIgnoreTimestamp bool - mainProto string - listenAddresses []string - localDoHListenAddresses []string - localDoHPath string - localDoHCertFile string - localDoHCertKeyFile string - daemonize bool - registeredServers []RegisteredServer - registeredRelays []RegisteredServer - pluginBlockIPv6 bool - pluginBlockUnqualified bool - pluginBlockUndelegated bool - cache bool - cacheSize int - cacheNegMinTTL uint32 - cacheNegMaxTTL uint32 - cacheMinTTL uint32 - cacheMaxTTL uint32 - rejectTTL uint32 - cloakTTL uint32 - queryLogFile string - queryLogFormat string - queryLogIgnoredQtypes []string - nxLogFile string - nxLogFormat string - blockNameFile string - whitelistNameFile string - blockNameLogFile string - whitelistNameLogFile string - blockNameFormat string - whitelistNameFormat string - blockIPFile string - blockIPLogFile string - blockIPFormat string - forwardFile string - cloakFile string - pluginsGlobals PluginsGlobals - sources []*Source - clientsCount uint32 - maxClients uint32 - xTransport *XTransport - allWeeklyRanges *map[string]WeeklyRanges - logMaxSize int - logMaxAge int - logMaxBackups int - blockedQueryResponse string - queryMeta []string - routes *map[string][]string - serversWithBrokenQueryPadding []string - showCerts bool - dohCreds *map[string]DOHClientCreds + userName string + child bool + proxyPublicKey [32]byte + proxySecretKey [32]byte + ephemeralKeys bool + questionSizeEstimator QuestionSizeEstimator + serversInfo ServersInfo + timeout time.Duration + certRefreshDelay time.Duration + certRefreshDelayAfterFailure time.Duration + certIgnoreTimestamp bool + mainProto string + listenAddresses []string + localDoHListenAddresses []string + localDoHPath string + localDoHCertFile string + localDoHCertKeyFile string + daemonize bool + registeredServers []RegisteredServer + registeredRelays []RegisteredServer + pluginBlockIPv6 bool + pluginBlockUnqualified bool + pluginBlockUndelegated bool + cache bool + cacheSize int + cacheNegMinTTL uint32 + cacheNegMaxTTL uint32 + cacheMinTTL uint32 + cacheMaxTTL uint32 + rejectTTL uint32 + cloakTTL uint32 + queryLogFile string + queryLogFormat string + queryLogIgnoredQtypes []string + nxLogFile string + nxLogFormat string + blockNameFile string + whitelistNameFile string + blockNameLogFile string + whitelistNameLogFile string + blockNameFormat string + whitelistNameFormat string + blockIPFile string + blockIPLogFile string + blockIPFormat string + forwardFile string + cloakFile string + pluginsGlobals PluginsGlobals + sources []*Source + clientsCount uint32 + maxClients uint32 + xTransport *XTransport + allWeeklyRanges *map[string]WeeklyRanges + logMaxSize int + logMaxAge int + logMaxBackups int + blockedQueryResponse string + queryMeta []string + routes *map[string][]string + serversBlockingFragments []string + serversDroppingLargerResponses []string + showCerts bool + dohCreds *map[string]DOHClientCreds } func (proxy *Proxy) addDNSListener(listenAddrStr string) { @@ -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) diff --git a/dnscrypt-proxy/serversInfo.go b/dnscrypt-proxy/serversInfo.go index 97ea4b6c..f78f7a20 100644 --- a/dnscrypt-proxy/serversInfo.go +++ b/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 }