util/security: support client TLS verifies CommonName of server (#53358)

close #53357
pingcap · May 20, 2024 · 397a460 · 397a460
1 parent f5ac93e
commit 397a460
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 11 deletions.
diff --git a/pkg/util/security.go b/pkg/util/security.go
@@ -289,7 +289,10 @@ func NewTLSConfig(opts ...TLSConfigOption) (*tls.Config, error) {
 	// 3. handle verify Common Name
 
 	if len(builder.verifyCN) > 0 {
+		// set RequireAndVerifyClientCert so server can verify the Common Name of client
 		tlsCfg.ClientAuth = tls.RequireAndVerifyClientCert
+		// set InsecureSkipVerify to false so client can verify the Common Name of server
+		tlsCfg.InsecureSkipVerify = false
 		verifyFuncs = append(verifyFuncs, verifyCommonName(builder.verifyCN))
 	}
 

diff --git a/pkg/util/security_test.go b/pkg/util/security_test.go
@@ -16,7 +16,6 @@ package util_test
 
 import (
 	"bytes"
-	"context"
 	"crypto/rand"
 	"crypto/rsa"
 	"crypto/tls"
@@ -75,10 +74,8 @@ func TestVerifyCommonNameAndRotate(t *testing.T) {
 		util.WithVerifyCommonName([]string{"client1"}),
 	)
 	require.NoError(t, err)
-	ctx, cancel := context.WithCancel(context.Background())
-	server, port := runServer(ctx, serverTLS, t)
+	server, port := runServer(serverTLS, t)
 	defer func() {
-		cancel()
 		server.Close()
 	}()
 	url := fmt.Sprintf("https://127.0.0.1:%d", port)
@@ -95,6 +92,20 @@ func TestVerifyCommonNameAndRotate(t *testing.T) {
 	require.Equal(t, "This an example server", string(body))
 	require.NoError(t, resp.Body.Close())
 
+	// client1 also check server's Common Name
+	clientTLS1Verify, err := util.NewTLSConfig(
+		util.WithCAContent(caData),
+		util.WithCertAndKeyContent(client1Cert, client1Key),
+		util.WithVerifyCommonName([]string{"server"}),
+	)
+	require.NoError(t, err)
+	resp, err = util.ClientWithTLS(clientTLS1Verify).Get(url)
+	require.NoError(t, err)
+	body, err = io.ReadAll(resp.Body)
+	require.NoError(t, err)
+	require.Equal(t, "This an example server", string(body))
+	require.NoError(t, resp.Body.Close())
+
 	// client2 can't visit server
 	dir := t.TempDir()
 	certPath := filepath.Join(dir, "client.pem")
@@ -140,10 +151,8 @@ func TestTLSVersion(t *testing.T) {
 		util.WithCertAndKeyContent(serverCert, serverKey),
 	)
 	require.NoError(t, err)
-	ctx, cancel := context.WithCancel(context.Background())
-	server, port := runServer(ctx, serverTLS, t)
+	server, port := runServer(serverTLS, t)
 	defer func() {
-		cancel()
 		server.Close()
 	}()
 	url := fmt.Sprintf("https://127.0.0.1:%d", port)
@@ -198,10 +207,8 @@ func TestCA(t *testing.T) {
 		util.WithCertAndKeyContent(serverCert, serverKey),
 	)
 	require.NoError(t, err)
-	ctx, cancel := context.WithCancel(context.Background())
-	server, port := runServer(ctx, serverTLS, t)
+	server, port := runServer(serverTLS, t)
 	defer func() {
-		cancel()
 		server.Close()
 	}()
 	url := fmt.Sprintf("https://127.0.0.1:%d", port)
@@ -256,7 +263,7 @@ func handler(w http.ResponseWriter, req *http.Request) {
 	w.Write([]byte("This an example server"))
 }
 
-func runServer(ctx context.Context, tlsCfg *tls.Config, t *testing.T) (*http.Server, int) {
+func runServer(tlsCfg *tls.Config, t *testing.T) (*http.Server, int) {
 	http.HandleFunc("/", handler)
 	server := &http.Server{Addr: ":0", Handler: nil}