Fix PQ certificate fallback on fragmented UDP paths

Try the rollover-sized certificate query over UDP first and return as
soon as the small query succeeds so that a truncated response can
trigger TCP fallback instead of waiting for large UDP probes to time
out.

We keep relayed UDP packets at the intended size after accounting for
the Anonymized DNSCrypt header.

When retrying through a relay over TCP, retain a rollover-sized
inner query because the relay still forwards it to the resolver over
UDP and applies the anti-amplification thing.

Preserve the UDP RTT when TCP is used only to complete certificate
retrieval, so resolver latency does not incorrectly appear to be the
TCP fallback latency.

Author: jedisct1

dnscrypt-proxy/dnscrypt_certs.go

@@ -74,7 +74,7 @@ func FetchCurrentDNSCryptCert(
 	}
 	if in.Truncated && proto != "tcp" {
 		dlog.Debugf("[%v] certificate response was truncated, retrying over TCP", *serverName)
-		if inTCP, rttTCP, _, errTCP := DNSExchange(
+		if inTCP, _, _, errTCP := DNSExchange(
 			proxy,
 			"tcp",
 			query,
@@ -83,7 +83,9 @@ func FetchCurrentDNSCryptCert(
 			serverName,
 			false,
 		); errTCP == nil {
-			in, rtt = inTCP, rttTCP
+			// TCP completed the certificate set; the UDP RTT still represents
+			// normal query latency when force_tcp is disabled.
+			in = inTCP
 		}
 	}
 	now := uint32(time.Now().Unix())

dnscrypt-proxy/dnsutils.go

@@ -372,15 +372,10 @@ type DNSExchangeResponse struct {
 	err              error
 }
 
-// A resolver and an Anonymized DNSCrypt relay both refuse to return a UDP
-// response larger than the request that triggered it, so a post-quantum
-// certificate only comes back over UDP when the certificate probe is padded past
-// the response. A PQ certificate is ~1.3 KB, ~1.5 KB once paired with the classical
-// certificate; during a key rotation the set doubles to two classical plus two PQ
-// certificates, roughly 3 KB. The large probe is padded past that rollover size so
-// PQ discovery keeps working through relays even mid-rotation. The fragments-blocked
-// probe stays small on purpose, so a PQ resolver truncates it and we still learn of
-// PQ support through the TC bit and fall back to TCP.
+// The large certificate probe covers the PQ rollover set. It is tried first;
+// the small probe follows later to detect paths that block fragmented UDP.
+// Relayed TCP retries retain the large inner query because the relay forwards
+// the lookup over UDP and enforces amplification limits.
 const (
 	certProbePaddedLen           = 3200
 	certProbeFragmentsBlockedLen = 480
@@ -422,6 +417,10 @@ func DNSExchange(
 			}
 			queryCopy := cloneMsg(query)
 			queryCopy.ID += uint16(options)
+			delay := time.Duration(250*tries) * time.Millisecond
+			if tryFragmentsSupport {
+				delay += 250 * time.Millisecond
+			}
 			go func(query *dns.Msg, delay time.Duration) {
 				time.Sleep(delay)
 				option := DNSExchangeResponse{err: errors.New("Canceled")}
@@ -431,9 +430,9 @@ func DNSExchange(
 					option = _dnsExchange(proxy, proto, query, serverAddress, relay, certProbeFragmentsBlockedLen)
 				}
 				option.fragmentsBlocked = true
-				option.priority = 1
+				option.priority = 0
 				channel <- option
-			}(queryCopy, time.Duration(250*tries)*time.Millisecond)
+			}(queryCopy, delay)
 			options++
 		}
 		var bestOption *DNSExchangeResponse
@@ -487,21 +486,10 @@ func _dnsExchange(
 	var rtt time.Duration
 
 	if proto == "udp" {
-		qNameLen, padding := len(query.Question[0].Header().Name), 0
-		if qNameLen < paddedLen {
-			padding = paddedLen - qNameLen
-		}
-		if padding > 0 {
-			paddingRR := &dns.PADDING{Padding: strings.Repeat("00", padding)}
-			query.Pseudo = append(query.Pseudo, paddingRR)
-			if query.UDPSize == 0 {
-				query.UDPSize = uint16(MaxDNSPacketSize)
-			}
-		}
-		if err := query.Pack(); err != nil {
+		binQuery, err := packDNSExchangeQuery(query, paddedLen, relay != nil)
+		if err != nil {
 			return DNSExchangeResponse{err: err}
 		}
-		binQuery := query.Data
 		udpAddr, err := net.ResolveUDPAddr("udp", serverAddress)
 		if err != nil {
 			return DNSExchangeResponse{err: err}
@@ -531,10 +519,19 @@ func _dnsExchange(
 		rtt = time.Since(now)
 		packet = packet[:length]
 	} else {
-		if err := query.Pack(); err != nil {
+		var binQuery []byte
+		var err error
+		if relay == nil {
+			err = query.Pack()
+			binQuery = query.Data
+		} else {
+			// The TCP stream itself does not fragment. Padding gives the relay
+			// enough request bytes for the full PQ response on its UDP hop.
+			binQuery, err = packDNSExchangeQuery(query, certProbePaddedLen, false)
+		}
+		if err != nil {
 			return DNSExchangeResponse{err: err}
 		}
-		binQuery := query.Data
 		tcpAddr, err := net.ResolveTCPAddr("tcp", serverAddress)
 		if err != nil {
 			return DNSExchangeResponse{err: err}
@@ -581,3 +578,34 @@ func _dnsExchange(
 	}
 	return DNSExchangeResponse{response: &msg, rtt: rtt, err: nil}
 }
+
+func packDNSExchangeQuery(query *dns.Msg, paddedLen int, viaRelay bool) ([]byte, error) {
+	if paddedLen <= 0 {
+		if err := query.Pack(); err != nil {
+			return nil, err
+		}
+		return query.Data, nil
+	}
+	if viaRelay {
+		paddedLen -= anonymizedDNSHeaderSize
+	}
+	if paddedLen <= 0 {
+		return nil, errors.New("Padded length is too small for an anonymized DNS header")
+	}
+
+	paddingRR := &dns.PADDING{}
+	query.Pseudo = append(query.Pseudo, paddingRR)
+	if query.UDPSize == 0 {
+		query.UDPSize = uint16(MaxDNSPacketSize)
+	}
+	if err := query.Pack(); err != nil {
+		return nil, err
+	}
+	if padding := paddedLen - len(query.Data); padding > 0 {
+		paddingRR.Padding = strings.Repeat("00", padding)
+		if err := query.Pack(); err != nil {
+			return nil, err
+		}
+	}
+	return query.Data, nil
+}

dnscrypt-proxy/proxy.go

@@ -587,6 +587,8 @@ func (proxy *Proxy) startAcceptingClients() {
 	proxy.localDoHListeners = nil
 }
 
+const anonymizedDNSHeaderSize = 8 + 2 + net.IPv6len + 2
+
 func (proxy *Proxy) prepareForRelay(ip net.IP, port int, encryptedQuery *[]byte) {
 	anonymizedDNSHeader := []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00}
 	relayedQuery := append(anonymizedDNSHeader, ip.To16()...)