Changes based on feedback

psiphon/common/protocol/packed.go

@@ -808,12 +808,15 @@ func init() {
 
 		{151, "dns_qname_random_casing", intConverter},
 		{152, "dns_qname_must_match", intConverter},
-		{153, "inproxy_broker_dns_qname_random_casing", intConverter},
-		{154, "inproxy_broker_dns_qname_must_match", intConverter},
-		{155, "inproxy_webrtc_dns_qname_random_casing", intConverter},
-		{156, "inproxy_webrtc_dns_qname_must_match", intConverter},
-
-		// Next key value = 157
+		{153, "dns_qname_mismatches", intConverter},
+		{154, "inproxy_broker_dns_qname_random_casing", intConverter},
+		{155, "inproxy_broker_dns_qname_must_match", intConverter},
+		{156, "inproxy_broker_dns_qname_mismatches", intConverter},
+		{157, "inproxy_webrtc_dns_qname_random_casing", intConverter},
+		{158, "inproxy_webrtc_dns_qname_must_match", intConverter},
+		{159, "inproxy_webrtc_dns_qname_mismatches", intConverter},
+
+		// Next key value = 160
 	}
 
 	for _, spec := range packedAPIParameterSpecs {

psiphon/common/resolver/resolver.go

@@ -41,6 +41,7 @@ import (
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/transforms"
 	lrucache "github.com/cognusion/go-cache-lru"
 	"github.com/miekg/dns"
+	"golang.org/x/net/idna"
 )
 
 const (
@@ -220,7 +221,9 @@ type ResolveParameters struct {
 	//
 	// RFC 1035 does not specify that the question section in the response must
 	// exactly match the question section in the request, but this behavior is
-	// common.
+	// expected [1].
+	//
+	// [1]: https://datatracker.ietf.org/doc/html/draft-vixie-dnsext-dns0x20-00#section-2.2.
 	ResponseQNameMustMatch bool
 
 	// IncludeEDNS0 indicates whether to include the EDNS(0) UDP maximum
@@ -231,6 +234,7 @@ type ResolveParameters struct {
 	IncludeEDNS0 bool
 
 	firstAttemptWithAnswer int32
+	qnameMismatches        int32
 }
 
 // GetFirstAttemptWithAnswer returns the index of the first request attempt
@@ -250,6 +254,26 @@ func (r *ResolveParameters) setFirstAttemptWithAnswer(attempt int) {
 	atomic.StoreInt32(&r.firstAttemptWithAnswer, int32(attempt))
 }
 
+// GetQNameMismatches returns, for the most recent ResolveIP call using this
+// ResolveParameters, the number of DNS requests where the response's question
+// section either:
+//   - Did not contain exactly one entry; or
+//   - Contained one entry that had a QName (hostname) that did not match the
+//     QName sent in the DNS request.
+//
+// This information is used for logging metrics.
+//
+// The caller is responsible for synchronizing use of a ResolveParameters
+// instance (e.g, use a distinct ResolveParameters per ResolveIP to ensure
+// GetQNameMismatches refers to a specific ResolveIP).
+func (r *ResolveParameters) GetQNameMismatches() int {
+	return int(atomic.LoadInt32(&r.qnameMismatches))
+}
+
+func (r *ResolveParameters) setQNameMismatches(mismatches int) {
+	atomic.StoreInt32(&r.qnameMismatches, int32(mismatches))
+}
+
 // Implementation note: Go's standard net.Resolver supports specifying a
 // custom Dial function. This could be used to implement at least a large
 // subset of the Resolver functionality on top of Go's standard library
@@ -1525,8 +1549,14 @@ func performDNSQuery(
 		}
 	}
 
+	// Convert to punycode.
+	hostname, err := idna.ToASCII(hostname)
+	if err != nil {
+		return nil, nil, -1, errors.Trace(err)
+	}
+
 	if useRandomQNameCasing {
-		hostname = common.ToRandomCasing(hostname, params.RandomQNameCasingSeed)
+		hostname = common.ToRandomASCIICasing(hostname, params.RandomQNameCasingSeed)
 	}
 
 	// UDPSize sets the receive buffer to > 512, even when we don't include
@@ -1558,7 +1588,7 @@ func performDNSQuery(
 	startTime := time.Now()
 
 	// Send the DNS request
-	err := dnsConn.WriteMsg(request)
+	err = dnsConn.WriteMsg(request)
 	if err != nil {
 		return nil, nil, -1, errors.Trace(err)
 	}
@@ -1566,6 +1596,10 @@ func performDNSQuery(
 	// Read and process the DNS response
 	var IPs []net.IP
 	var TTLs []time.Duration
+	var qnameMismatches int
+	defer func() {
+		params.setQNameMismatches(qnameMismatches)
+	}()
 	var lastErr error
 	RTT := time.Duration(-1)
 	for {
@@ -1606,13 +1640,9 @@ func performDNSQuery(
 			continue
 		}
 
-		if responseQNameMustMatch {
-			if len(response.Question) != 1 {
-				lastErr = errors.Tracef("unexpected QDCount")
-				logWarning(lastErr)
-				continue
-			}
-			if response.Question[0].Name != dns.Fqdn(hostname) {
+		if len(response.Question) != 1 || response.Question[0].Name != dns.Fqdn(hostname) {
+			qnameMismatches++
+			if responseQNameMustMatch {
 				lastErr = errors.Tracef("unexpected QName")
 				logWarning(lastErr)
 				continue

psiphon/common/resolver/resolver_test.go

@@ -577,7 +577,14 @@ func runTestResolver() error {
 	params.ProtocolTransformSpec = nil
 	params.ProtocolTransformSeed = nil
 
-	// Test: random QName casing
+	// Test: random QName (hostname) casing
+	//
+	// Note: there's a (1/2)^N chance that the QName (hostname) with randomized
+	// casing has the same casing as the input QName, where N is the number of
+	// Unicode letters in the QName. Currently, with the domain "example.com"
+	// there is a (1/2)^10=~0.00098 chance, which means we expect this to happen
+	// every 1/(1/2)^10=1024 runs. In such an event these tests will either
+	// give a false positive or false negative depending on the subtest.
 
 	if randomQNameCasingOkServer.getRequestCount() != 0 {
 		return errors.TraceNew("unexpected random QName casing server request count")
@@ -586,20 +593,30 @@ func runTestResolver() error {
 	resolver.cache.Flush()
 
 	params.AttemptsPerServer = 0
+	params.AttemptsPerPreferredServer = 1
 	params.AlternateDNSServer = randomQNameCasingOkServer.getAddr()
 	params.PreferAlternateDNSServer = true
 	params.RandomQNameCasingSeed = seed
 
+	_, err = resolver.ResolveIP(ctx, networkID, params, exampleDomain)
+	if err != nil {
+		return errors.Trace(err)
+	}
+
+	resolver.cache.Flush()
 	params.ResponseQNameMustMatch = true
+
 	_, err = resolver.ResolveIP(ctx, networkID, params, exampleDomain)
 	if err == nil {
-		errors.TraceNew("unexpected success")
+		return errors.TraceNew("expected QName mismatch")
 	}
 
-	params.ResponseQNameMustMatch = false
-	IPs, err = resolver.ResolveIP(ctx, networkID, params, exampleDomain)
+	resolver.cache.Flush()
+	params.AlternateDNSServer = okServer.getAddr()
+
+	_, err = resolver.ResolveIP(ctx, networkID, params, exampleDomain)
 	if err == nil {
-		errors.TraceNew("unexpected success")
+		return errors.TraceNew("expected server to not support random QName casing")
 	}
 
 	err = checkResult(IPs)
@@ -849,6 +866,8 @@ func (s *testDNSServer) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
 		return
 	}
 
+	fmt.Println(s.expectRandomQNameCasing, r.Question[0].Name, dns.Fqdn(exampleDomain))
+
 	if len(r.Question) != 1 ||
 		(!s.expectRandomQNameCasing &&
 			r.Question[0].Name != dns.Fqdn(exampleDomain)) {
@@ -886,6 +905,11 @@ func (s *testDNSServer) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
 		}
 	}
 
+	if s.expectRandomQNameCasing {
+		// Simulate a server that does not preserve the casing of the QName.
+		m.Question[0].Name = dns.Fqdn(exampleDomain)
+	}
+
 	w.WriteMsg(m)
 }
 

psiphon/common/utils.go

@@ -33,7 +33,6 @@ import (
 	"os"
 	"strings"
 	"time"
-	"unicode"
 
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/errors"
 	"github.com/Psiphon-Labs/psiphon-tunnel-core/psiphon/common/prng"
@@ -295,26 +294,24 @@ func MaxDuration(durations ...time.Duration) time.Duration {
 	return max
 }
 
-// ToRandomCasing returns s with each Unicode letter randomly mapped to either
-// its upper or lower case.
-func ToRandomCasing(s string, seed *prng.Seed) string {
+// ToRandomASCIICasing returns s with each ASCII letter randomly mapped to
+// either its upper or lower case.
+func ToRandomASCIICasing(s string, seed *prng.Seed) string {
 
 	PRNG := prng.NewPRNGWithSeed(seed)
 
-	var result strings.Builder
-	result.Grow(len(s))
+	var b strings.Builder
+	b.Grow(len(s))
 
 	for _, r := range s {
-		if unicode.IsLetter(r) {
-			if PRNG.FlipCoin() {
-				result.WriteRune(unicode.ToLower(r))
-			} else {
-				result.WriteRune(unicode.ToUpper(r))
-			}
+		isLower := ('a' <= r && r <= 'z')
+		isUpper := ('A' <= r && r <= 'Z')
+		if (isLower || isUpper) && PRNG.FlipCoin() {
+			b.WriteRune(r ^ 0x20)
 		} else {
-			result.WriteRune(r)
+			b.WriteRune(r)
 		}
 	}
 
-	return result.String()
+	return b.String()
 }

psiphon/common/utils_test.go

@@ -168,16 +168,16 @@ func TestSleepWithContext(t *testing.T) {
 }
 
 func TestToRandomCasing(t *testing.T) {
-	s := "test.to.random.casing.aaaa.bbbb.cccc.dd" // 32 Unicode letters
+	s := "test.to.random.ascii.casing.aaaa.bbbb.c" // 32 Unicode letters
 
 	seed, err := prng.NewSeed()
 	if err != nil {
 		t.Errorf("NewPRNG failed: %s", err)
 	}
 
-	randomized := ToRandomCasing(s, seed)
+	randomized := ToRandomASCIICasing(s, seed)
 
-	// Note: There's a (1/2)^32 chance that the randomized string has the same
+	// Note: there's a (1/2)^32 chance that the randomized string has the same
 	// casing as the input string.
 	if strings.Compare(s, randomized) == 0 {
 		t.Errorf("expected random casing")
@@ -187,7 +187,7 @@ func TestToRandomCasing(t *testing.T) {
 		t.Errorf("expected strings to be identical minus casing")
 	}
 
-	replaySameSeed := ToRandomCasing(s, seed)
+	replaySameSeed := ToRandomASCIICasing(s, seed)
 
 	if strings.Compare(randomized, replaySameSeed) != 0 {
 		t.Errorf("expected randomized string with same seed to be identical")

psiphon/frontingDialParameters.go

@@ -526,6 +526,9 @@ func (meekDialParameters *FrontedMeekDialParameters) GetMetrics(overridePrefix s
 			logFields[prefix+"dns_qname_must_match"] = "1"
 		}
 
+		logFields[prefix+"dns_qname_mismatches"] = strconv.Itoa(
+			meekDialParameters.ResolveParameters.GetQNameMismatches())
+
 		logFields[prefix+"dns_attempt"] = strconv.Itoa(
 			meekDialParameters.ResolveParameters.GetFirstAttemptWithAnswer())
 	}

psiphon/inproxy.go

@@ -2093,6 +2093,9 @@ func (dialParams *InproxySTUNDialParameters) GetMetrics() common.LogFields {
 			logFields["inproxy_webrtc_dns_qname_must_match"] = "1"
 		}
 
+		logFields["inproxy_webrtc_dns_qname_mismatches"] = strconv.Itoa(
+			dialParams.ResolveParameters.GetQNameMismatches())
+
 		logFields["inproxy_webrtc_dns_attempt"] = strconv.Itoa(
 			dialParams.ResolveParameters.GetFirstAttemptWithAnswer())
 	}

psiphon/notice.go

@@ -616,6 +616,8 @@ func noticeWithDialParameters(noticeType string, dialParams *DialParameters, pos
 			}
 
 			if postDial {
+				args = append(args, "DNSQNameMismatches", dialParams.ResolveParameters.GetQNameMismatches())
+
 				args = append(args, "DNSAttempt", dialParams.ResolveParameters.GetFirstAttemptWithAnswer())
 			}
 		}

psiphon/server/api.go

@@ -1122,6 +1122,7 @@ var baseDialParams = []requestParamSpec{
 	{"dns_transform", isAnyString, requestParamOptional},
 	{"dns_qname_random_casing", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 	{"dns_qname_must_match", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
+	{"dns_qname_mismatches", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"dns_attempt", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"http_transform", isAnyString, requestParamOptional},
 	{"seed_transform", isAnyString, requestParamOptional},
@@ -1166,12 +1167,14 @@ var inproxyDialParams = []requestParamSpec{
 	{"inproxy_broker_dns_transform", isAnyString, requestParamOptional},
 	{"inproxy_broker_dns_qname_random_casing", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 	{"inproxy_broker_dns_qname_must_match", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
+	{"inproxy_broker_dns_qname_mismatches", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"inproxy_broker_dns_attempt", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"inproxy_webrtc_dns_preresolved", isAnyString, requestParamOptional},
 	{"inproxy_webrtc_dns_preferred", isAnyString, requestParamOptional},
 	{"inproxy_webrtc_dns_transform", isAnyString, requestParamOptional},
 	{"inproxy_broker_dns_qname_random_casing", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
 	{"inproxy_webrtc_dns_qname_must_match", isBooleanFlag, requestParamOptional | requestParamLogFlagAsBool},
+	{"inproxy_webrtc_dns_qname_mismatches", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"inproxy_webrtc_dns_attempt", isIntString, requestParamOptional | requestParamLogStringAsInt},
 	{"inproxy_webrtc_stun_server", isAnyString, requestParamOptional},
 	{"inproxy_webrtc_stun_server_resolved_ip_address", isAnyString, requestParamOptional},

psiphon/serverApi.go

@@ -1286,6 +1286,9 @@ func getBaseAPIParameters(
 				params["dns_qname_must_match"] = "1"
 			}
 
+			params["dns_qname_mismatches"] = strconv.Itoa(
+				dialParams.ResolveParameters.GetQNameMismatches())
+
 			params["dns_attempt"] = strconv.Itoa(
 				dialParams.ResolveParameters.GetFirstAttemptWithAnswer())
 		}