From 86f08655b8a3a99521ba6ba9eb0ed0f998012420 Mon Sep 17 00:00:00 2001 From: lysu Date: Wed, 4 Mar 2020 21:15:47 +0800 Subject: [PATCH 1/4] server: support check cert's CommanName for status-port(http/grpc) --- config/config.go | 1 + server/http_status.go | 23 ++++++++++++ server/tidb_test.go | 52 ++++++++++++++++++++++++++++ tests/cncheckcert/ca-key.pem | 9 +++++ tests/cncheckcert/ca-tidb-test-1.crt | 10 ++++++ tests/cncheckcert/client-cert-1.pem | 10 ++++++ tests/cncheckcert/client-cert-2.pem | 10 ++++++ tests/cncheckcert/client-key-1.pem | 9 +++++ tests/cncheckcert/client-key-2.pem | 9 +++++ tests/cncheckcert/server-cert.pem | 10 ++++++ tests/cncheckcert/server-key.pem | 9 +++++ 11 files changed, 152 insertions(+) create mode 100644 tests/cncheckcert/ca-key.pem create mode 100644 tests/cncheckcert/ca-tidb-test-1.crt create mode 100644 tests/cncheckcert/client-cert-1.pem create mode 100644 tests/cncheckcert/client-cert-2.pem create mode 100644 tests/cncheckcert/client-key-1.pem create mode 100644 tests/cncheckcert/client-key-2.pem create mode 100644 tests/cncheckcert/server-cert.pem create mode 100644 tests/cncheckcert/server-key.pem diff --git a/config/config.go b/config/config.go index fd85ab98de2f0..799efdb8cd430 100644 --- a/config/config.go +++ b/config/config.go @@ -249,6 +249,7 @@ type Security struct { ClusterSSLCA string `toml:"cluster-ssl-ca" json:"cluster-ssl-ca"` ClusterSSLCert string `toml:"cluster-ssl-cert" json:"cluster-ssl-cert"` ClusterSSLKey string `toml:"cluster-ssl-key" json:"cluster-ssl-key"` + ClusterAllowCN string `toml:"cluster-allow-cn" json:"cluster-allow-cn"` } // The ErrConfigValidationFailed error is used so that external callers can do a type assertion diff --git a/server/http_status.go b/server/http_status.go index 30022922251cd..6509c3d299c67 100644 --- a/server/http_status.go +++ b/server/http_status.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "crypto/tls" + "crypto/x509" "encoding/json" "fmt" "net" @@ -274,6 +275,28 @@ func (s *Server) setupStatusServerAndRPCServer(addr string, serverMux *http.Serv return } + if tlsConfig != nil && len(s.cfg.Security.ClusterAllowCN) > 0 { + cns := strings.Split(s.cfg.Security.ClusterAllowCN, ",") + if len(cns) != 0 { + checkCN := make(map[string]struct{}) + for _, cn := range cns { + cn = strings.TrimSpace(cn) + checkCN[cn] = struct{}{} + } + tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + for _, chain := range verifiedChains { + if len(chain) != 0 { + if _, match := checkCN[chain[0].Subject.CommonName]; match { + return nil + } + } + } + return errors.New("client certificate authentication failed due to CN not match") + } + tlsConfig.ClientAuth = tls.VerifyClientCertIfGiven + } + } + var l net.Listener if tlsConfig != nil { // we need to manage TLS here for cmux to distinguish between HTTP and gRPC. diff --git a/server/tidb_test.go b/server/tidb_test.go index 86a2d4c28f3f8..2d05acdd9328d 100644 --- a/server/tidb_test.go +++ b/server/tidb_test.go @@ -24,7 +24,9 @@ import ( "encoding/pem" "io/ioutil" "math/big" + "net/http" "os" + "path/filepath" "time" "github.com/go-sql-driver/mysql" @@ -215,6 +217,56 @@ func (ts *tidbTestSuite) TestStatusAPIWithTLS(c *C) { server.Close() } +func (ts *tidbTestSuite) TestStatusAPIWithTLSCNCheck(c *C) { + c.Skip("need add ca-tidb-test-1.crt to OS") + root := filepath.Join(os.Getenv("GOPATH"), "/src/github.com/pingcap/tidb") + ca := filepath.Join(root, "/tests/cncheckcert/ca-tidb-test-1.crt") + + cli := newTestServerClient() + cli.statusScheme = "https" + cfg := config.NewConfig() + cfg.Port = cli.port + cfg.Status.StatusPort = cli.statusPort + cfg.Security.ClusterSSLCA = ca + cfg.Security.ClusterSSLCert = filepath.Join(root, "/tests/cncheckcert/server-cert.pem") + cfg.Security.ClusterSSLKey = filepath.Join(root, "/tests/cncheckcert/server-key.pem") + cfg.Security.ClusterAllowCN = "tidb-client-2" + server, err := NewServer(cfg, ts.tidbdrv) + c.Assert(err, IsNil) + go server.Run() + time.Sleep(time.Millisecond * 100) + + hc := newTLSHttpClient(c, ca, + filepath.Join(root, "/tests/cncheckcert/client-cert-1.pem"), + filepath.Join(root, "/tests/cncheckcert/client-key-1.pem"), + ) + _, err = hc.Get(cli.statusURL("/status")) + c.Assert(err, NotNil) + + hc = newTLSHttpClient(c, ca, + filepath.Join(root, "/tests/cncheckcert/client-cert-2.pem"), + filepath.Join(root, "/tests/cncheckcert/client-key-2.pem"), + ) + _, err = hc.Get(cli.statusURL("/status")) + c.Assert(err, IsNil) +} + +func newTLSHttpClient(c *C, caFile, certFile, keyFile string) *http.Client { + cert, err := tls.LoadX509KeyPair(certFile, keyFile) + c.Assert(err, IsNil) + caCert, err := ioutil.ReadFile(caFile) + c.Assert(err, IsNil) + caCertPool := x509.NewCertPool() + caCertPool.AppendCertsFromPEM(caCert) + tlsConfig := &tls.Config{ + Certificates: []tls.Certificate{cert}, + RootCAs: caCertPool, + InsecureSkipVerify: true, + } + tlsConfig.BuildNameToCertificate() + return &http.Client{Transport: &http.Transport{TLSClientConfig: tlsConfig}} +} + func (ts *tidbTestSuite) TestMultiStatements(c *C) { c.Parallel() ts.runTestMultiStatements(c) diff --git a/tests/cncheckcert/ca-key.pem b/tests/cncheckcert/ca-key.pem new file mode 100644 index 0000000000000..2845bda86b81f --- /dev/null +++ b/tests/cncheckcert/ca-key.pem @@ -0,0 +1,9 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIBQwIBAAJDAL6zaGJBgNTBNhtGTfSXnbzoZa1LlfCpFlIwtXc1YBBtl9IIzRI5 +j2Cyd/DTYjO0vniHGIHv7H64336szhSGOcHV5QIDAQABAkIpDH9MnyL3KPvXlSOU +oco/bprsWZfV7N+0I238Ug3ym1QyCK3ue8m/bJveL9AXCwJWMLdvHQoiyCFnaQ6f +Ay2hGYUCIgD4YcAP4aIJL1H9vo0vmXQzFZJzklyOUcmRm00Ulvie5dsCIgDEjMGk +QZoiR9ammPSc1IKF/c6THtd1sA0rW9Vh0sCBXz8CIgCMkq4jjtyo/BoYVRcM4HmO +S+A1/pjZh1pgSRfH1mXhcE8CITRT5Rn9/TMzPQqNnlJCoZ1avSyeAW7ruBXbFSw+ +F9JZsQIhMvQjbm0ygrSwOlvRhWOzAtUKSxcs7JKfxgt9/5/XIV4C +-----END RSA PRIVATE KEY----- diff --git a/tests/cncheckcert/ca-tidb-test-1.crt b/tests/cncheckcert/ca-tidb-test-1.crt new file mode 100644 index 0000000000000..cd5e961116f5a --- /dev/null +++ b/tests/cncheckcert/ca-tidb-test-1.crt @@ -0,0 +1,10 @@ +-----BEGIN CERTIFICATE----- +MIIBXzCCAQegAwIBAgIBADANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDEwlUaURC +IENBIDUwHhcNMjAwMzA0MTI0MzIxWhcNMjAwMzA0MTM0MzIxWjAUMRIwEAYDVQQD +EwlUaURCIENBIDUwXjANBgkqhkiG9w0BAQEFAANNADBKAkMAvrNoYkGA1ME2G0ZN +9JedvOhlrUuV8KkWUjC1dzVgEG2X0gjNEjmPYLJ38NNiM7S+eIcYge/sfrjffqzO +FIY5wdXlAgMBAAGjQjBAMA4GA1UdDwEB/wQEAwICpDAdBgNVHSUEFjAUBggrBgEF +BQcDAQYIKwYBBQUHAwIwDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAAND +AKfu05xLZQ6PVMyYPjpo/RI/WchaLFRbBZKpdu+/QMaxreH7/xAqDfnslTVblWLo +uGxIrIOBL/1jR8CoAhBB1VAoJw== +-----END CERTIFICATE----- diff --git a/tests/cncheckcert/client-cert-1.pem b/tests/cncheckcert/client-cert-1.pem new file mode 100644 index 0000000000000..e6e442a649be6 --- /dev/null +++ b/tests/cncheckcert/client-cert-1.pem @@ -0,0 +1,10 @@ +-----BEGIN CERTIFICATE----- +MIIBYDCCAQigAwIBAgIBATANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDEwlUaURC +IENBIDUwHhcNMjAwMzA0MTI0MzIxWhcNMjAwMzA0MTM0MzIxWjAYMRYwFAYDVQQD +Ew10aWRiLWNsaWVudC0xMF4wDQYJKoZIhvcNAQEBBQADTQAwSgJDAMMfWMiDqdQE +xy0kE/W+V+OmYZGit7iGk+BzMe6G0w9fFmJt8ajwmHp5dMT5AmJpNF/i089Wej3K +SdEkWhWa96BBOwIDAQABoz8wPTAOBgNVHQ8BAf8EBAMCBaAwHQYDVR0lBBYwFAYI +KwYBBQUHAwEGCCsGAQUFBwMCMAwGA1UdEwEB/wQCMAAwDQYJKoZIhvcNAQELBQAD +QwAnXNy43kWtlyelP1M5s0h4KNkmYBe6McaGV5nvhD/dwFfviY1dMRQ4ChgBgc5y +vv2r0fnRd2XF+81EmNGQk79Xg9A= +-----END CERTIFICATE----- diff --git a/tests/cncheckcert/client-cert-2.pem b/tests/cncheckcert/client-cert-2.pem new file mode 100644 index 0000000000000..820f3c9b17f0a --- /dev/null +++ b/tests/cncheckcert/client-cert-2.pem @@ -0,0 +1,10 @@ +-----BEGIN CERTIFICATE----- +MIIBYDCCAQigAwIBAgIBATANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDEwlUaURC +IENBIDUwHhcNMjAwMzA0MTI0MzIxWhcNMjAwMzA0MTM0MzIxWjAYMRYwFAYDVQQD +Ew10aWRiLWNsaWVudC0yMF4wDQYJKoZIhvcNAQEBBQADTQAwSgJDANuIi1RlJKHF +KJg5D/fblj8FfRaay9CLJOhDZkabJTuMHerlpY0vf8wsY15khaJU/dRiJeAT+lvg +V4CluJLSSLuIEQIDAQABoz8wPTAOBgNVHQ8BAf8EBAMCBaAwHQYDVR0lBBYwFAYI +KwYBBQUHAwEGCCsGAQUFBwMCMAwGA1UdEwEB/wQCMAAwDQYJKoZIhvcNAQELBQAD +QwABsC2+tggurK8iUDmuaEKOqGnQdHrMMohDnSjmuS+X9xYcLlix7Haf53fBhs8B +kvUiPFk8QEiOi7xttaMHKeDLnRE= +-----END CERTIFICATE----- diff --git a/tests/cncheckcert/client-key-1.pem b/tests/cncheckcert/client-key-1.pem new file mode 100644 index 0000000000000..79a8de46f434d --- /dev/null +++ b/tests/cncheckcert/client-key-1.pem @@ -0,0 +1,9 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIBRQIBAAJDAMMfWMiDqdQExy0kE/W+V+OmYZGit7iGk+BzMe6G0w9fFmJt8ajw +mHp5dMT5AmJpNF/i089Wej3KSdEkWhWa96BBOwIDAQABAkJ4ps1zT1aX70xpsUFW +Vxhpf9wc/Yy04SJXS2O4pk2j15seJ9chhaolp26mVm1w/jyV/k6TwuKo0pM+F4pI +ePJNQQECIgDLWTCV6E1KGmprVnm/WRqUTewcNzfsXFHUlPSuIhTxUZsCIgD1pOfU +DpzwOoZb7XUogqkLKleaWWm22ffHuvuT3+ozGOECIgDDDS9Ea8pPTV1MzmsDtyV+ +oevb+L9ksf0wKx00Nq7d9wcCIgCX/++4B0bTW9OSBLCvXZKOpyfICbXhgKTTQX+0 +9CRuc+ECIgCqTejjhRnhz3rYD0wWqRRsXT/144RmiIpBslRp+HJ+5dg= +-----END RSA PRIVATE KEY----- diff --git a/tests/cncheckcert/client-key-2.pem b/tests/cncheckcert/client-key-2.pem new file mode 100644 index 0000000000000..f1555c589f1d5 --- /dev/null +++ b/tests/cncheckcert/client-key-2.pem @@ -0,0 +1,9 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIBQgIBAAJDANuIi1RlJKHFKJg5D/fblj8FfRaay9CLJOhDZkabJTuMHerlpY0v +f8wsY15khaJU/dRiJeAT+lvgV4CluJLSSLuIEQIDAQABAkJw/adQqbof9Pz+1CfO +11tOVoHaV5PdYzB8xuvmHUYdjvCG4WJcfFFkVipqE8VhJsS2Onzb7BFJ7gOqxCV6 +9+FoYgECIgD1IP2jXxqOJgI3p78YaCJnBlRUMIu/+g+MmGPcLqg8taECIgDlRPYh +m0kOwtmzTtTI5sOLDgDPew16p8jyBvBPxsk53HECIW6W/rc5DeL5tOBlFqqtOHAg +g+Ujrbjj2SYGDm9kwVP6YQIhPd/xqTo2alRt2nWA+cNFrMaXs2cbSSn1ElSLEIyu +i/4RAiFcI5yrNMt2XwyKo/A+hzFvJeuQ6xTs9I1KMYbsngXJ58k= +-----END RSA PRIVATE KEY----- diff --git a/tests/cncheckcert/server-cert.pem b/tests/cncheckcert/server-cert.pem new file mode 100644 index 0000000000000..2b66f1c10b920 --- /dev/null +++ b/tests/cncheckcert/server-cert.pem @@ -0,0 +1,10 @@ +-----BEGIN CERTIFICATE----- +MIIBYDCCAQigAwIBAgIBATANBgkqhkiG9w0BAQsFADAUMRIwEAYDVQQDEwlUaURC +IENBIDUwHhcNMjAwMzA0MTI0MzIxWhcNMjAwMzA0MTM0MzIxWjAYMRYwFAYDVQQD +Ew10aWRiLXNlcnZlci0zMF4wDQYJKoZIhvcNAQEBBQADTQAwSgJDAM+ez7yZLKp9 +spGiUqqInrH8iTZ8JPa3C8K8bYw/Fc/W028sVvf6+VLC1ZP0cJX4Lx3iyyyIfzjG +KV4+MVcBOlfn4wIDAQABoz8wPTAOBgNVHQ8BAf8EBAMCBaAwHQYDVR0lBBYwFAYI +KwYBBQUHAwEGCCsGAQUFBwMCMAwGA1UdEwEB/wQCMAAwDQYJKoZIhvcNAQELBQAD +QwCUAO+eDYvVXBIqpNWr/h513TTSzZa+ELNOrYNGr2MgQ6j8TW0+J+J4JGgj0Wm7 +1haBMbGShAHlXYK81ZShtnbPpFI= +-----END CERTIFICATE----- diff --git a/tests/cncheckcert/server-key.pem b/tests/cncheckcert/server-key.pem new file mode 100644 index 0000000000000..d225652b7061a --- /dev/null +++ b/tests/cncheckcert/server-key.pem @@ -0,0 +1,9 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIBRAIBAAJDAM+ez7yZLKp9spGiUqqInrH8iTZ8JPa3C8K8bYw/Fc/W028sVvf6 ++VLC1ZP0cJX4Lx3iyyyIfzjGKV4+MVcBOlfn4wIDAQABAkJ5hiNh6OZUBK74v2JT +nxQEaiSGV7PrFMk1esVESciilsKsRJLcyzxbpANooQQefOswHzRR/YyMmy2HiW++ +H08ENMECIgDR6EfM7CKKJGj2jz3b+2bUoBpdpuPtD9kPo4u6ubVnNz8CIgD9Nfhq +Viiwl2IentOXOWvIasGQdQXz1fWHGLP5nA2Xql0CIgCXcSGUXG2TAy/ja3cy5k/L +afN7y/O3zm5JlTIzttaFMFsCIgC7Io8Eb8bEtCzk+nbgVaStyxBhJcuPaPp7rKse +d9Gn3FUCITGo0wLxVqHmjk2Pe2n9IJK1ooupjGY1unWQ2rM8RDkqfA== +-----END RSA PRIVATE KEY----- From 7103544ee33d48573ab6cb097095a32d8096d4c8 Mon Sep 17 00:00:00 2001 From: lysu Date: Wed, 4 Mar 2020 23:04:20 +0800 Subject: [PATCH 2/4] make coverage happy --- server/http_handler_test.go | 16 +++++++++++++ server/http_status.go | 48 ++++++++++++++++++++----------------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/server/http_handler_test.go b/server/http_handler_test.go index 51858beb27f71..cccc17d752af9 100644 --- a/server/http_handler_test.go +++ b/server/http_handler_test.go @@ -15,6 +15,9 @@ package server import ( "bytes" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" "database/sql" "encoding/base64" "encoding/json" @@ -1059,6 +1062,19 @@ func (ts *HTTPHandlerTestSuite) TestDebugZip(c *C) { c.Assert(resp.Body.Close(), IsNil) } +func (ts *HTTPHandlerTestSuite) TestCheckCN(c *C) { + s := &Server{cfg: &config.Config{Security: config.Security{ClusterAllowCN: " a,b, c"}}} + tlsConfig := &tls.Config{} + s.setCNChecker(tlsConfig) + c.Assert(tlsConfig.VerifyPeerCertificate, NotNil) + err := tlsConfig.VerifyPeerCertificate(nil, [][]*x509.Certificate{{{Subject: pkix.Name{CommonName: "a"}}}}) + c.Assert(err, IsNil) + err = tlsConfig.VerifyPeerCertificate(nil, [][]*x509.Certificate{{{Subject: pkix.Name{CommonName: "b"}}}}) + c.Assert(err, IsNil) + err = tlsConfig.VerifyPeerCertificate(nil, [][]*x509.Certificate{{{Subject: pkix.Name{CommonName: "d"}}}}) + c.Assert(err, NotNil) +} + func (ts *HTTPHandlerTestSuite) TestZipInfoForSQL(c *C) { ts.startServer(c) defer ts.stopServer(c) diff --git a/server/http_status.go b/server/http_status.go index 6509c3d299c67..f98facc90dbb8 100644 --- a/server/http_status.go +++ b/server/http_status.go @@ -274,28 +274,7 @@ func (s *Server) setupStatusServerAndRPCServer(addr string, serverMux *http.Serv logutil.BgLogger().Error("invalid TLS config", zap.Error(err)) return } - - if tlsConfig != nil && len(s.cfg.Security.ClusterAllowCN) > 0 { - cns := strings.Split(s.cfg.Security.ClusterAllowCN, ",") - if len(cns) != 0 { - checkCN := make(map[string]struct{}) - for _, cn := range cns { - cn = strings.TrimSpace(cn) - checkCN[cn] = struct{}{} - } - tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { - for _, chain := range verifiedChains { - if len(chain) != 0 { - if _, match := checkCN[chain[0].Subject.CommonName]; match { - return nil - } - } - } - return errors.New("client certificate authentication failed due to CN not match") - } - tlsConfig.ClientAuth = tls.VerifyClientCertIfGiven - } - } + tlsConfig = s.setCNChecker(tlsConfig) var l net.Listener if tlsConfig != nil { @@ -333,6 +312,31 @@ func (s *Server) setupStatusServerAndRPCServer(addr string, serverMux *http.Serv } } +func (s *Server) setCNChecker(tlsConfig *tls.Config) *tls.Config { + if tlsConfig != nil && len(s.cfg.Security.ClusterAllowCN) > 0 { + cns := strings.Split(s.cfg.Security.ClusterAllowCN, ",") + if len(cns) != 0 { + checkCN := make(map[string]struct{}) + for _, cn := range cns { + cn = strings.TrimSpace(cn) + checkCN[cn] = struct{}{} + } + tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + for _, chain := range verifiedChains { + if len(chain) != 0 { + if _, match := checkCN[chain[0].Subject.CommonName]; match { + return nil + } + } + } + return errors.New("client certificate authentication failed due to CN not match") + } + tlsConfig.ClientAuth = tls.VerifyClientCertIfGiven + } + } + return tlsConfig +} + // status of TiDB. type status struct { Connections int `json:"connections"` From 5df8ec24b506b3d97060217162e2c63f6ec4721c Mon Sep 17 00:00:00 2001 From: lysu Date: Thu, 5 Mar 2020 14:16:02 +0800 Subject: [PATCH 3/4] address comments --- config/config.go | 16 ++++++++-------- server/http_handler_test.go | 2 +- server/http_status.go | 9 +++++---- server/tidb_test.go | 2 +- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/config/config.go b/config/config.go index 799efdb8cd430..048de4a71f474 100644 --- a/config/config.go +++ b/config/config.go @@ -242,14 +242,14 @@ func (l *Log) getDisableErrorStack() bool { // Security is the security section of the config. type Security struct { - SkipGrantTable bool `toml:"skip-grant-table" json:"skip-grant-table"` - SSLCA string `toml:"ssl-ca" json:"ssl-ca"` - SSLCert string `toml:"ssl-cert" json:"ssl-cert"` - SSLKey string `toml:"ssl-key" json:"ssl-key"` - ClusterSSLCA string `toml:"cluster-ssl-ca" json:"cluster-ssl-ca"` - ClusterSSLCert string `toml:"cluster-ssl-cert" json:"cluster-ssl-cert"` - ClusterSSLKey string `toml:"cluster-ssl-key" json:"cluster-ssl-key"` - ClusterAllowCN string `toml:"cluster-allow-cn" json:"cluster-allow-cn"` + SkipGrantTable bool `toml:"skip-grant-table" json:"skip-grant-table"` + SSLCA string `toml:"ssl-ca" json:"ssl-ca"` + SSLCert string `toml:"ssl-cert" json:"ssl-cert"` + SSLKey string `toml:"ssl-key" json:"ssl-key"` + ClusterSSLCA string `toml:"cluster-ssl-ca" json:"cluster-ssl-ca"` + ClusterSSLCert string `toml:"cluster-ssl-cert" json:"cluster-ssl-cert"` + ClusterSSLKey string `toml:"cluster-ssl-key" json:"cluster-ssl-key"` + ClusterVerifyCN string `toml:"cluster-verify-cn" json:"cluster-verify-cn"` } // The ErrConfigValidationFailed error is used so that external callers can do a type assertion diff --git a/server/http_handler_test.go b/server/http_handler_test.go index cccc17d752af9..59d22275d3d99 100644 --- a/server/http_handler_test.go +++ b/server/http_handler_test.go @@ -1063,7 +1063,7 @@ func (ts *HTTPHandlerTestSuite) TestDebugZip(c *C) { } func (ts *HTTPHandlerTestSuite) TestCheckCN(c *C) { - s := &Server{cfg: &config.Config{Security: config.Security{ClusterAllowCN: " a,b, c"}}} + s := &Server{cfg: &config.Config{Security: config.Security{ClusterVerifyCN: " a,b, c"}}} tlsConfig := &tls.Config{} s.setCNChecker(tlsConfig) c.Assert(tlsConfig.VerifyPeerCertificate, NotNil) diff --git a/server/http_status.go b/server/http_status.go index f98facc90dbb8..8d4fc69eaa5f1 100644 --- a/server/http_status.go +++ b/server/http_status.go @@ -313,8 +313,8 @@ func (s *Server) setupStatusServerAndRPCServer(addr string, serverMux *http.Serv } func (s *Server) setCNChecker(tlsConfig *tls.Config) *tls.Config { - if tlsConfig != nil && len(s.cfg.Security.ClusterAllowCN) > 0 { - cns := strings.Split(s.cfg.Security.ClusterAllowCN, ",") + if tlsConfig != nil && len(s.cfg.Security.ClusterVerifyCN) > 0 { + cns := strings.Split(s.cfg.Security.ClusterVerifyCN, ",") if len(cns) != 0 { checkCN := make(map[string]struct{}) for _, cn := range cns { @@ -329,9 +329,10 @@ func (s *Server) setCNChecker(tlsConfig *tls.Config) *tls.Config { } } } - return errors.New("client certificate authentication failed due to CN not match") + return errors.Errorf("client certificate authentication failed. The Common Name from "+ + "the client certificate was not found in the configuration cluster-verify-cn with value: %s", s.cfg.Security.ClusterVerifyCN) } - tlsConfig.ClientAuth = tls.VerifyClientCertIfGiven + tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert } } return tlsConfig diff --git a/server/tidb_test.go b/server/tidb_test.go index 2d05acdd9328d..30f0fa586fe7f 100644 --- a/server/tidb_test.go +++ b/server/tidb_test.go @@ -230,7 +230,7 @@ func (ts *tidbTestSuite) TestStatusAPIWithTLSCNCheck(c *C) { cfg.Security.ClusterSSLCA = ca cfg.Security.ClusterSSLCert = filepath.Join(root, "/tests/cncheckcert/server-cert.pem") cfg.Security.ClusterSSLKey = filepath.Join(root, "/tests/cncheckcert/server-key.pem") - cfg.Security.ClusterAllowCN = "tidb-client-2" + cfg.Security.ClusterVerifyCN = "tidb-client-2" server, err := NewServer(cfg, ts.tidbdrv) c.Assert(err, IsNil) go server.Run() From 8b426b76b6c42b220fc6bbe1da96b90d57819c06 Mon Sep 17 00:00:00 2001 From: lysu Date: Thu, 5 Mar 2020 14:47:28 +0800 Subject: [PATCH 4/4] change cluster-verify-cn as []string --- config/config.go | 16 ++++++++-------- server/http_handler_test.go | 2 +- server/http_status.go | 30 +++++++++++++----------------- server/tidb_test.go | 2 +- 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/config/config.go b/config/config.go index 048de4a71f474..28c89a8f6d69c 100644 --- a/config/config.go +++ b/config/config.go @@ -242,14 +242,14 @@ func (l *Log) getDisableErrorStack() bool { // Security is the security section of the config. type Security struct { - SkipGrantTable bool `toml:"skip-grant-table" json:"skip-grant-table"` - SSLCA string `toml:"ssl-ca" json:"ssl-ca"` - SSLCert string `toml:"ssl-cert" json:"ssl-cert"` - SSLKey string `toml:"ssl-key" json:"ssl-key"` - ClusterSSLCA string `toml:"cluster-ssl-ca" json:"cluster-ssl-ca"` - ClusterSSLCert string `toml:"cluster-ssl-cert" json:"cluster-ssl-cert"` - ClusterSSLKey string `toml:"cluster-ssl-key" json:"cluster-ssl-key"` - ClusterVerifyCN string `toml:"cluster-verify-cn" json:"cluster-verify-cn"` + SkipGrantTable bool `toml:"skip-grant-table" json:"skip-grant-table"` + SSLCA string `toml:"ssl-ca" json:"ssl-ca"` + SSLCert string `toml:"ssl-cert" json:"ssl-cert"` + SSLKey string `toml:"ssl-key" json:"ssl-key"` + ClusterSSLCA string `toml:"cluster-ssl-ca" json:"cluster-ssl-ca"` + ClusterSSLCert string `toml:"cluster-ssl-cert" json:"cluster-ssl-cert"` + ClusterSSLKey string `toml:"cluster-ssl-key" json:"cluster-ssl-key"` + ClusterVerifyCN []string `toml:"cluster-verify-cn" json:"cluster-verify-cn"` } // The ErrConfigValidationFailed error is used so that external callers can do a type assertion diff --git a/server/http_handler_test.go b/server/http_handler_test.go index 59d22275d3d99..33809b8582c2e 100644 --- a/server/http_handler_test.go +++ b/server/http_handler_test.go @@ -1063,7 +1063,7 @@ func (ts *HTTPHandlerTestSuite) TestDebugZip(c *C) { } func (ts *HTTPHandlerTestSuite) TestCheckCN(c *C) { - s := &Server{cfg: &config.Config{Security: config.Security{ClusterVerifyCN: " a,b, c"}}} + s := &Server{cfg: &config.Config{Security: config.Security{ClusterVerifyCN: []string{"a ", "b", "c"}}}} tlsConfig := &tls.Config{} s.setCNChecker(tlsConfig) c.Assert(tlsConfig.VerifyPeerCertificate, NotNil) diff --git a/server/http_status.go b/server/http_status.go index 8d4fc69eaa5f1..da0dee1633fc5 100644 --- a/server/http_status.go +++ b/server/http_status.go @@ -313,27 +313,23 @@ func (s *Server) setupStatusServerAndRPCServer(addr string, serverMux *http.Serv } func (s *Server) setCNChecker(tlsConfig *tls.Config) *tls.Config { - if tlsConfig != nil && len(s.cfg.Security.ClusterVerifyCN) > 0 { - cns := strings.Split(s.cfg.Security.ClusterVerifyCN, ",") - if len(cns) != 0 { - checkCN := make(map[string]struct{}) - for _, cn := range cns { - cn = strings.TrimSpace(cn) - checkCN[cn] = struct{}{} - } - tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { - for _, chain := range verifiedChains { - if len(chain) != 0 { - if _, match := checkCN[chain[0].Subject.CommonName]; match { - return nil - } + if tlsConfig != nil && len(s.cfg.Security.ClusterVerifyCN) != 0 { + checkCN := make(map[string]struct{}) + for _, cn := range s.cfg.Security.ClusterVerifyCN { + cn = strings.TrimSpace(cn) + checkCN[cn] = struct{}{} + } + tlsConfig.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error { + for _, chain := range verifiedChains { + if len(chain) != 0 { + if _, match := checkCN[chain[0].Subject.CommonName]; match { + return nil } } - return errors.Errorf("client certificate authentication failed. The Common Name from "+ - "the client certificate was not found in the configuration cluster-verify-cn with value: %s", s.cfg.Security.ClusterVerifyCN) } - tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert + return errors.Errorf("client certificate authentication failed. The Common Name from the client certificate was not found in the configuration cluster-verify-cn with value: %s", s.cfg.Security.ClusterVerifyCN) } + tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert } return tlsConfig } diff --git a/server/tidb_test.go b/server/tidb_test.go index 30f0fa586fe7f..d4239ec3ecbb6 100644 --- a/server/tidb_test.go +++ b/server/tidb_test.go @@ -230,7 +230,7 @@ func (ts *tidbTestSuite) TestStatusAPIWithTLSCNCheck(c *C) { cfg.Security.ClusterSSLCA = ca cfg.Security.ClusterSSLCert = filepath.Join(root, "/tests/cncheckcert/server-cert.pem") cfg.Security.ClusterSSLKey = filepath.Join(root, "/tests/cncheckcert/server-key.pem") - cfg.Security.ClusterVerifyCN = "tidb-client-2" + cfg.Security.ClusterVerifyCN = []string{"tidb-client-2"} server, err := NewServer(cfg, ts.tidbdrv) c.Assert(err, IsNil) go server.Run()