TLS client: Skip TLS' built-in verification when using pinnedPeerCertSha256; Fixes

https://github.com/XTLS/Xray-core/pull/5532#issuecomment-3745598515
https://github.com/XTLS/Xray-core/pull/5532#issuecomment-3759930283
https://github.com/XTLS/Xray-core/pull/5532#issuecomment-3760057266
https://github.com/XTLS/Xray-core/pull/5532#issuecomment-3760540231

transport/internet/tls/config.go

@@ -281,35 +281,41 @@ func (c *Config) parseServerName() string {
 }
 
 func (r *RandCarrier) verifyPeerCert(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) (err error) {
-	// extract x509 certificates from rawCerts(verifiedChains will be nil if InsecureSkipVerify is true)
+	// extract x509 certificates from rawCerts (verifiedChains will be nil if InsecureSkipVerify is true)
 	certs := make([]*x509.Certificate, len(rawCerts))
 	for i, asn1Data := range rawCerts {
 		certs[i], _ = x509.ParseCertificate(asn1Data)
 	}
+	if len(certs) == 0 {
+		return errors.New("unexpected certs")
+	}
+	if certs[0].IsCA {
+		slices.Reverse(certs)
+	}
 
 	// directly return success if pinned cert is leaf
-	// or add the CA to RootCAs if pinned cert is CA(and can be used in VerifyPeerCertInNames for Self signed CA)
+	// or replace RootCAs if pinned cert is CA (and can be used in VerifyPeerCertInNames)
-	RootCAs := r.RootCAs
+	CAs := r.RootCAs
 	var verifyResult verifyResult
 	var verifiedCert *x509.Certificate
 	if r.PinnedPeerCertSha256 != nil {
 		verifyResult, verifiedCert = verifyChain(certs, r.PinnedPeerCertSha256)
 		switch verifyResult {
 		case certNotFound:
-			return errors.New("peer cert is unrecognized")
+			return errors.New("peer cert is unrecognized (againsts pinnedPeerCertSha256)")
 		case foundLeaf:
 			return nil
 		case foundCA:
-			RootCAs = x509.NewCertPool()
+			CAs = x509.NewCertPool()
-			RootCAs.AddCert(verifiedCert)
+			CAs.AddCert(verifiedCert)
 		default:
-			panic("impossible PinnedPeerCertificateSha256 verify result")
+			panic("impossible pinnedPeerCertSha256 verify result")
 		}
 	}
 
-	if len(r.VerifyPeerCertInNames) > 0 {
+	if r.VerifyPeerCertInNames != nil { // RAW's Dial() may make it empty but not nil
 		opts := x509.VerifyOptions{
-			Roots:         RootCAs,
+			Roots:         CAs,
 			CurrentTime:   time.Now(),
 			Intermediates: x509.NewCertPool(),
 		}
@@ -321,9 +327,15 @@ func (r *RandCarrier) verifyPeerCert(rawCerts [][]byte, verifiedChains [][]*x509
 				return nil
 			}
 		}
-	} else if len(verifiedChains) == 0 && verifyResult == foundCA { // if found ca and verifiedChains is empty, we need to verify here
+		if verifyResult == foundCA {
+			errors.New("peer cert is invalid (againsts pinned CA and verifyPeerCertInNames)")
+		}
+		return errors.New("peer cert is invalid (againsts root CAs and verifyPeerCertInNames)")
+	}
+
+	if verifyResult == foundCA { // if found CA, we need to verify here
 		opts := x509.VerifyOptions{
-			Roots:         RootCAs,
+			Roots:         CAs,
 			CurrentTime:   time.Now(),
 			Intermediates: x509.NewCertPool(),
 			DNSName:       r.Config.ServerName,
@@ -334,8 +346,10 @@ func (r *RandCarrier) verifyPeerCert(rawCerts [][]byte, verifiedChains [][]*x509
 		if _, err := certs[0].Verify(opts); err == nil {
 			return nil
 		}
+		return errors.New("peer cert is invalid (againsts pinned CA and serverName)")
 	}
-	return nil
+
+	return nil // len(r.PinnedPeerCertSha256)==nil && len(r.VerifyPeerCertInNames)==nil
 }
 
 type RandCarrier struct {
@@ -386,6 +400,11 @@ func (c *Config) GetTLSConfig(opts ...Option) *tls.Config {
 	} else {
 		randCarrier.VerifyPeerCertInNames = nil
 	}
+	if len(c.PinnedPeerCertSha256) > 0 {
+		config.InsecureSkipVerify = true
+	} else {
+		randCarrier.PinnedPeerCertSha256 = nil
+	}
 
 	for _, opt := range opts {
 		opt(config)
@@ -540,17 +559,16 @@ const (
 	foundCA
 )
 
-func verifyChain(certs []*x509.Certificate, PinnedPeerCertificateSha256 [][]byte) (verifyResult, *x509.Certificate) {
+func verifyChain(certs []*x509.Certificate, pinnedPeerCertSha256 [][]byte) (verifyResult, *x509.Certificate) {
 	for _, cert := range certs {
 		certHash := GenerateCertHash(cert)
-		for _, c := range PinnedPeerCertificateSha256 {
+		for _, c := range pinnedPeerCertSha256 {
 			if hmac.Equal(certHash, c) {
 				if cert.IsCA {
 					return foundCA, cert
 				} else {
 					return foundLeaf, cert
 				}
-
 			}
 		}
 	}