TLS ECH: Update ECH configuration parsing logic to be more robust (#6441)

#6441 (comment)

---------

Co-authored-by: 风扇滑翔翼 <Fangliding.fshxy@outlook.com>

Author: jamesliu96

transport/internet/tls/config.go

@@ -450,7 +450,7 @@ func (c *Config) GetTLSConfig(opts ...Option) *tls.Config {
 		for _, s := range tls.CipherSuites() {
 			id[s.Name] = s.ID
 		}
-		for _, n := range strings.Split(c.CipherSuites, ":") {
+		for n := range strings.SplitSeq(c.CipherSuites, ":") {
 			if id[n] != 0 {
 				config.CipherSuites = append(config.CipherSuites, id[n])
 			}

transport/internet/tls/ech.go

@@ -55,19 +55,16 @@ func ApplyECH(c *Config, config *tls.Config) error {
 			}
 			config.EncryptedClientHelloConfigList = ECHConfig
 		}()
-		// direct base64 config
+		// query config from dns
 		if strings.Contains(c.EchConfigList, "://") {
-			// query config from dns
-			parts := strings.Split(c.EchConfigList, "+")
+			// parse ECH DNS server in format of "example.com+https://1.1.1.1/dns-query"
+			parts := strings.SplitN(c.EchConfigList, "+", 2)
 			if len(parts) == 2 {
-				// parse ECH DNS server in format of "example.com+https://1.1.1.1/dns-query"
 				nameToQuery = parts[0]
 				DNSServer = parts[1]
-			} else if len(parts) == 1 {
+			} else {
 				// normal format
 				DNSServer = parts[0]
-			} else {
-				return errors.New("Invalid ECH DNS server format: ", c.EchConfigList)
 			}
 			if nameToQuery == "" {
 				return errors.New("Using DNS for ECH Config needs serverName or use Server format example.com+https://1.1.1.1/dns-query")
@@ -77,6 +74,7 @@ func ApplyECH(c *Config, config *tls.Config) error {
 				return errors.New("Failed to query ECH DNS record for domain: ", nameToQuery, " at server: ", DNSServer).Base(err)
 			}
 		} else {
+			// direct base64 config
 			ECHConfig, err = base64.StdEncoding.DecodeString(c.EchConfigList)
 			if err != nil {
 				return errors.New("Failed to unmarshal ECHConfigList: ", err)
@@ -157,7 +155,7 @@ func QueryRecord(domain string, server string, sockopt *internet.SocketConfig) (
 	// If expire is zero value, it means we are in initial state, wait for the query to finish
 	// otherwise return old value immediately and update in a goroutine
 	// but if the cache is too old, wait for update
-	if configRecord.expire == (time.Time{}) || configRecord.expire.Add(time.Hour*4).Before(time.Now()) {
+	if configRecord.expire.IsZero() || configRecord.expire.Add(time.Hour*4).Before(time.Now()) {
 		return echConfigCache.Update(domain, server, false, sockopt)
 	} else {
 		// If someone already acquired the lock, it means it is updating, do not start another update goroutine
@@ -256,14 +254,17 @@ func dnsQuery(server string, domain string, sockopt *internet.SocketConfig) ([]b
 		}
 		dnsResolve = respBody
 	} else if strings.HasPrefix(server, "udp://") { // for classic udp dns server
-		udpServerAddr := server[len("udp://"):]
+		udpServerURL, err := url.Parse(server)
+		if err != nil {
+			return nil, 0, errors.New("failed to parse udp dns server ", server, " for ECH: ", err)
+		}
 		// default port 53 if not specified
-		if !strings.Contains(udpServerAddr, ":") {
-			udpServerAddr = udpServerAddr + ":53"
+		if udpServerURL.Port() == "" {
+			udpServerURL.Host = udpServerURL.Host + ":53"
 		}
-		dest, err := net.ParseDestination("udp" + ":" + udpServerAddr)
+		dest, err := net.ParseDestination("udp" + ":" + udpServerURL.Host)
 		if err != nil {
-			return nil, 0, errors.New("failed to parse udp dns server ", udpServerAddr, " for ECH: ", err)
+			return nil, 0, errors.New("failed to parse udp dns server ", udpServerURL.Host, " for ECH: ", err)
 		}
 		dnsTimeoutCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 		defer cancel()

transport/internet/tls/ech_test.go

@@ -19,8 +19,7 @@ func TestECHDial(t *testing.T) {
 	// test concurrent Dial(to test cache problem)
 	wg := sync.WaitGroup{}
 	for range 10 {
-		wg.Add(1)
-		go func() {
+		wg.Go(func() {
 			TLSConfig := config.GetTLSConfig()
 			TLSConfig.NextProtos = []string{"http/1.1"}
 			client := &http.Client{
@@ -36,8 +35,7 @@ func TestECHDial(t *testing.T) {
 			if !strings.Contains(string(body), "sni=encrypted") {
 				t.Error("ECH Dial success but SNI is not encrypted")
 			}
-			wg.Done()
-		}()
+		})
 	}
 	wg.Wait()
 	// check cache