From bc522e3ee9723f3b0a161b5933f8d45dff57eff3 Mon Sep 17 00:00:00 2001 From: pingcap-github-bot Date: Fri, 5 Jun 2020 19:57:54 +0800 Subject: [PATCH] privileges: support `require SAN` (#17539) (#17698) * cherry pick #17539 to release-4.0 Signed-off-by: sre-bot * resolve conflict Co-authored-by: lysu --- executor/grant.go | 14 ++++-- go.mod | 2 +- go.sum | 4 +- privilege/privileges/cache.go | 22 +++++++-- privilege/privileges/cache_test.go | 8 +++- privilege/privileges/privileges.go | 59 ++++++++++++++++++++++++- privilege/privileges/privileges_test.go | 23 ++++++++-- util/misc.go | 36 +++++++++++++++ 8 files changed, 152 insertions(+), 16 deletions(-) diff --git a/executor/grant.go b/executor/grant.go index cb62291477dd2..097e2d5dcc349 100644 --- a/executor/grant.go +++ b/executor/grant.go @@ -314,8 +314,6 @@ func (e *GrantExec) grantGlobalPriv(ctx sessionctx.Context, user *ast.UserSpec) return err } -var emptyGP = privileges.GlobalPrivValue{SSLType: privileges.SslTypeNotSpecified} - func tlsOption2GlobalPriv(tlsOptions []*ast.TLSOption) (priv []byte, err error) { if len(tlsOptions) == 0 { priv = []byte("{}") @@ -332,6 +330,8 @@ func tlsOption2GlobalPriv(tlsOptions []*ast.TLSOption) (priv []byte, err error) typeName = "ISSUER" case ast.Subject: typeName = "SUBJECT" + case ast.SAN: + typeName = "SAN" } err = errors.Errorf("Duplicate require %s clause", typeName) return @@ -370,12 +370,20 @@ func tlsOption2GlobalPriv(tlsOptions []*ast.TLSOption) (priv []byte, err error) } gp.SSLType = privileges.SslTypeSpecified gp.X509Subject = tlsOpt.Value + case ast.SAN: + gp.SSLType = privileges.SslTypeSpecified + _, err = util.ParseAndCheckSAN(tlsOpt.Value) + if err != nil { + return + } + gp.SAN = tlsOpt.Value default: err = errors.Errorf("Unknown ssl type: %#v", tlsOpt.Type) return } } - if gp == emptyGP { + if gp.SSLType == privileges.SslTypeNotSpecified && len(gp.SSLCipher) == 0 && + len(gp.X509Issuer) == 0 && len(gp.X509Subject) == 0 && len(gp.SAN) == 0 { return } priv, err = json.Marshal(&gp) diff --git a/go.mod b/go.mod index fbbd975b632bd..c32c472d97778 100644 --- a/go.mod +++ b/go.mod @@ -30,7 +30,7 @@ require ( github.com/pingcap/goleveldb v0.0.0-20191226122134-f82aafb29989 github.com/pingcap/kvproto v0.0.0-20200518112156-d4aeb467de29 github.com/pingcap/log v0.0.0-20200511115504-543df19646ad - github.com/pingcap/parser v0.0.0-20200525110646-f45c2cee1dca + github.com/pingcap/parser v0.0.0-20200603032439-c4ecb4508d2f github.com/pingcap/pd/v4 v4.0.0-rc.2.0.20200520083007-2c251bd8f181 github.com/pingcap/sysutil v0.0.0-20200408114249-ed3bd6f7fdb1 github.com/pingcap/tidb-tools v4.0.0-rc.1.0.20200514040632-f76b3e428e19+incompatible diff --git a/go.sum b/go.sum index c4a5ad1edb1d5..e4a0bbf44d9e3 100644 --- a/go.sum +++ b/go.sum @@ -403,8 +403,8 @@ github.com/pingcap/log v0.0.0-20200511115504-543df19646ad h1:SveG82rmu/GFxYanffx github.com/pingcap/log v0.0.0-20200511115504-543df19646ad/go.mod h1:4rbK1p9ILyIfb6hU7OG2CiWSqMXnp3JMbiaVJ6mvoY8= github.com/pingcap/parser v0.0.0-20200424075042-8222d8b724a4/go.mod h1:9v0Edh8IbgjGYW2ArJr19E+bvL8zKahsFp+ixWeId+4= github.com/pingcap/parser v0.0.0-20200507022230-f3bf29096657/go.mod h1:9v0Edh8IbgjGYW2ArJr19E+bvL8zKahsFp+ixWeId+4= -github.com/pingcap/parser v0.0.0-20200525110646-f45c2cee1dca h1:eB+uC09C+T768jQUKFG80Qx/DAfnP/9TgKPQkalnEnw= -github.com/pingcap/parser v0.0.0-20200525110646-f45c2cee1dca/go.mod h1:9v0Edh8IbgjGYW2ArJr19E+bvL8zKahsFp+ixWeId+4= +github.com/pingcap/parser v0.0.0-20200603032439-c4ecb4508d2f h1:tdjuYfon3J2Wae1g6ya72No+x+329SiWoENoVIS/HJY= +github.com/pingcap/parser v0.0.0-20200603032439-c4ecb4508d2f/go.mod h1:9v0Edh8IbgjGYW2ArJr19E+bvL8zKahsFp+ixWeId+4= github.com/pingcap/pd/v4 v4.0.0-rc.1.0.20200422143320-428acd53eba2/go.mod h1:s+utZtXDznOiL24VK0qGmtoHjjXNsscJx3m1n8cC56s= github.com/pingcap/pd/v4 v4.0.0-rc.2.0.20200520083007-2c251bd8f181 h1:FM+PzdoR3fmWAJx3ug+p5aOgs5aZYwFkoDL7Potdsz0= github.com/pingcap/pd/v4 v4.0.0-rc.2.0.20200520083007-2c251bd8f181/go.mod h1:q4HTx/bA8aKBa4S7L+SQKHvjRPXCRV0tA0yRw0qkZSA= diff --git a/privilege/privileges/cache.go b/privilege/privileges/cache.go index 0d3af938f9e4a..eef8bda55d56d 100644 --- a/privilege/privileges/cache.go +++ b/privilege/privileges/cache.go @@ -30,6 +30,7 @@ import ( "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/util" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/hack" "github.com/pingcap/tidb/util/logutil" @@ -113,10 +114,12 @@ const ( // GlobalPrivValue is store json format for priv column in mysql.global_priv. type GlobalPrivValue struct { - SSLType SSLType `json:"ssl_type,omitempty"` - SSLCipher string `json:"ssl_cipher,omitempty"` - X509Issuer string `json:"x509_issuer,omitempty"` - X509Subject string `json:"x509_subject,omitempty"` + SSLType SSLType `json:"ssl_type,omitempty"` + SSLCipher string `json:"ssl_cipher,omitempty"` + X509Issuer string `json:"x509_issuer,omitempty"` + X509Subject string `json:"x509_subject,omitempty"` + SAN string `json:"san,omitempty"` + SANs map[util.SANType][]string `json:"-"` } // RequireStr returns describe string after `REQUIRE` clause. @@ -141,6 +144,10 @@ func (g *GlobalPrivValue) RequireStr() string { s = append(s, "SUBJECT") s = append(s, "'"+g.X509Subject+"'") } + if len(g.SAN) > 0 { + s = append(s, "SAN") + s = append(s, "'"+g.SAN+"'") + } if len(s) > 0 { require = strings.Join(s, " ") } @@ -629,6 +636,13 @@ func (p *MySQLPrivilege) decodeGlobalPrivTableRow(row chunk.Row, fs []*ast.Resul value.Priv.SSLCipher = privValue.SSLCipher value.Priv.X509Issuer = privValue.X509Issuer value.Priv.X509Subject = privValue.X509Subject + value.Priv.SAN = privValue.SAN + if len(value.Priv.SAN) > 0 { + value.Priv.SANs, err = util.ParseAndCheckSAN(value.Priv.SAN) + if err != nil { + value.Broken = true + } + } } } default: diff --git a/privilege/privileges/cache_test.go b/privilege/privileges/cache_test.go index aa3979f78ff02..2d19d3a4aa47d 100644 --- a/privilege/privileges/cache_test.go +++ b/privilege/privileges/cache_test.go @@ -23,6 +23,7 @@ import ( "github.com/pingcap/tidb/privilege/privileges" "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/store/mockstore" + "github.com/pingcap/tidb/util" ) var _ = Suite(&testCacheSuite{}) @@ -85,7 +86,8 @@ func (s *testCacheSuite) TestLoadGlobalPrivTable(c *C) { mustExec(c, se, "use mysql;") mustExec(c, se, "truncate table global_priv") - mustExec(c, se, `INSERT INTO mysql.global_priv VALUES ("%", "tu", "{\"access\":0,\"plugin\":\"mysql_native_password\",\"ssl_type\":3, \"ssl_cipher\":\"cipher\",\"x509_subject\":\"\C=ZH1\", \"x509_issuer\":\"\C=ZH2\", \"password_last_changed\":1}")`) + mustExec(c, se, `INSERT INTO mysql.global_priv VALUES ("%", "tu", "{\"access\":0,\"plugin\":\"mysql_native_password\",\"ssl_type\":3, + \"ssl_cipher\":\"cipher\",\"x509_subject\":\"\C=ZH1\", \"x509_issuer\":\"\C=ZH2\", \"san\":\"\IP:127.0.0.1, IP:1.1.1.1, DNS:pingcap.com, URI:spiffe://mesh.pingcap.com/ns/timesh/sa/me1\", \"password_last_changed\":1}")`) var p privileges.MySQLPrivilege err = p.LoadGlobalPrivTable(se) @@ -95,6 +97,10 @@ func (s *testCacheSuite) TestLoadGlobalPrivTable(c *C) { c.Assert(p.Global["tu"][0].Priv.SSLType, Equals, privileges.SslTypeSpecified) c.Assert(p.Global["tu"][0].Priv.X509Issuer, Equals, "C=ZH2") c.Assert(p.Global["tu"][0].Priv.X509Subject, Equals, "C=ZH1") + c.Assert(p.Global["tu"][0].Priv.SAN, Equals, "IP:127.0.0.1, IP:1.1.1.1, DNS:pingcap.com, URI:spiffe://mesh.pingcap.com/ns/timesh/sa/me1") + c.Assert(len(p.Global["tu"][0].Priv.SANs[util.IP]), Equals, 2) + c.Assert(p.Global["tu"][0].Priv.SANs[util.DNS][0], Equals, "pingcap.com") + c.Assert(p.Global["tu"][0].Priv.SANs[util.URI][0], Equals, "spiffe://mesh.pingcap.com/ns/timesh/sa/me1") } func (s *testCacheSuite) TestLoadDBTable(c *C) { diff --git a/privilege/privileges/privileges.go b/privilege/privileges/privileges.go index f712060e79de3..98126c2e8deff 100644 --- a/privilege/privileges/privileges.go +++ b/privilege/privileges/privileges.go @@ -15,6 +15,7 @@ package privileges import ( "crypto/tls" + "crypto/x509" "fmt" "strings" @@ -275,6 +276,7 @@ func (p *UserPrivileges) checkSSL(priv *globalPrivRecord, tlsState *tls.Connecti hasCert = false matchIssuer checkResult matchSubject checkResult + matchSAN checkResult ) for _, chain := range tlsState.VerifiedChains { if len(chain) == 0 { @@ -301,11 +303,19 @@ func (p *UserPrivileges) checkSSL(priv *globalPrivRecord, tlsState *tls.Connecti zap.String("require", priv.Priv.X509Subject), zap.String("given", given)) } } + if len(priv.Priv.SANs) > 0 { + matchOne := checkCertSAN(priv, cert, priv.Priv.SANs) + if matchOne { + matchSAN = pass + } else if matchSAN == notCheck { + matchSAN = fail + } + } hasCert = true } - checkResult := hasCert && matchIssuer != fail && matchSubject != fail + checkResult := hasCert && matchIssuer != fail && matchSubject != fail && matchSAN != fail if !checkResult && !hasCert { - logutil.BgLogger().Info("ssl check failure, require issuer/subject but no verified cert", + logutil.BgLogger().Info("ssl check failure, require issuer/subject/SAN but no verified cert", zap.String("user", priv.User), zap.String("host", priv.Host)) } return checkResult @@ -314,6 +324,51 @@ func (p *UserPrivileges) checkSSL(priv *globalPrivRecord, tlsState *tls.Connecti } } +func checkCertSAN(priv *globalPrivRecord, cert *x509.Certificate, sans map[util.SANType][]string) (r bool) { + r = true + for typ, requireOr := range sans { + var ( + unsupported bool + given []string + ) + switch typ { + case util.URI: + for _, uri := range cert.URIs { + given = append(given, uri.String()) + } + case util.DNS: + given = cert.DNSNames + case util.IP: + for _, ip := range cert.IPAddresses { + given = append(given, ip.String()) + } + default: + unsupported = true + } + if unsupported { + logutil.BgLogger().Warn("skip unsupported SAN type", zap.String("type", string(typ)), + zap.String("user", priv.User), zap.String("host", priv.Host)) + continue + } + var givenMatchOne bool + for _, req := range requireOr { + for _, give := range given { + if req == give { + givenMatchOne = true + break + } + } + } + if !givenMatchOne { + logutil.BgLogger().Info("ssl check failure for subject", zap.String("user", priv.User), zap.String("host", priv.Host), + zap.String("require", priv.Priv.SAN), zap.Strings("given", given), zap.String("type", string(typ))) + r = false + return + } + } + return +} + // DBIsVisible implements the Manager interface. func (p *UserPrivileges) DBIsVisible(activeRoles []*auth.RoleIdentity, db string) bool { if SkipWithGrant { diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index 0117db286141d..5c3eeea952a97 100644 --- a/privilege/privileges/privileges_test.go +++ b/privilege/privileges/privileges_test.go @@ -20,6 +20,7 @@ import ( "crypto/x509" "crypto/x509/pkix" "fmt" + "net/url" "os" "strings" "testing" @@ -489,6 +490,8 @@ func (s *testPrivilegeSuite) TestCheckCertBasedAuth(c *C) { mustExec(c, se, `CREATE USER 'r13_broken_user'@'localhost'require issuer '/C=US/ST=California/L=San Francisco/O=PingCAP/OU=TiDB/CN=TiDB admin' subject '/C=ZH/ST=Beijing/L=Haidian/O=PingCAP.Inc/OU=TiDB/CN=tester1'`) mustExec(c, se, "UPDATE mysql.global_priv set priv = 'abc' where `user` = 'r13_broken_user' and `host` = 'localhost'") + mustExec(c, se, `CREATE USER 'r14_san_only_pass'@'localhost' require san 'URI:spiffe://mesh.pingcap.com/ns/timesh/sa/me1'`) + mustExec(c, se, `CREATE USER 'r15_san_only_fail'@'localhost' require san 'URI:spiffe://mesh.pingcap.com/ns/timesh/sa/me2'`) mustExec(c, se, "flush privileges") defer func() { @@ -506,6 +509,8 @@ func (s *testPrivilegeSuite) TestCheckCertBasedAuth(c *C) { mustExec(c, se, "drop user 'r11_cipher_only'@'localhost'") mustExec(c, se, "drop user 'r12_old_tidb_user'@'localhost'") mustExec(c, se, "drop user 'r13_broken_user'@'localhost'") + mustExec(c, se, "drop user 'r14_san_only_pass'@'localhost'") + mustExec(c, se, "drop user 'r15_san_only_fail'@'localhost'") }() // test without ssl or ca @@ -553,12 +558,17 @@ func (s *testPrivilegeSuite) TestCheckCertBasedAuth(c *C) { util.MockPkixAttribute(util.CommonName, "tester1"), }, }, - tls.TLS_AES_128_GCM_SHA256) + tls.TLS_AES_128_GCM_SHA256, func(c *x509.Certificate) { + var url url.URL + url.UnmarshalBinary([]byte("spiffe://mesh.pingcap.com/ns/timesh/sa/me1")) + c.URIs = append(c.URIs, &url) + }) c.Assert(se.Auth(&auth.UserIdentity{Username: "r1", Hostname: "localhost"}, nil, nil), IsTrue) c.Assert(se.Auth(&auth.UserIdentity{Username: "r2", Hostname: "localhost"}, nil, nil), IsTrue) c.Assert(se.Auth(&auth.UserIdentity{Username: "r3", Hostname: "localhost"}, nil, nil), IsTrue) c.Assert(se.Auth(&auth.UserIdentity{Username: "r4", Hostname: "localhost"}, nil, nil), IsTrue) c.Assert(se.Auth(&auth.UserIdentity{Username: "r5", Hostname: "localhost"}, nil, nil), IsTrue) + c.Assert(se.Auth(&auth.UserIdentity{Username: "r14_san_only_pass", Hostname: "localhost"}, nil, nil), IsTrue) // test require but give nothing se.GetSessionVars().TLSConnectionState = nil @@ -673,15 +683,22 @@ func (s *testPrivilegeSuite) TestCheckCertBasedAuth(c *C) { tls.TLS_AES_128_GCM_SHA256) c.Assert(se.Auth(&auth.UserIdentity{Username: "r10_issuer_disorder", Hostname: "localhost"}, nil, nil), IsFalse) + // test mismatch san + c.Assert(se.Auth(&auth.UserIdentity{Username: "r15_san_only_fail", Hostname: "localhost"}, nil, nil), IsFalse) + // test old data and broken data c.Assert(se.Auth(&auth.UserIdentity{Username: "r12_old_tidb_user", Hostname: "localhost"}, nil, nil), IsTrue) c.Assert(se.Auth(&auth.UserIdentity{Username: "r13_broken_user", Hostname: "localhost"}, nil, nil), IsFalse) } -func connectionState(issuer, subject pkix.Name, cipher uint16) *tls.ConnectionState { +func connectionState(issuer, subject pkix.Name, cipher uint16, opt ...func(c *x509.Certificate)) *tls.ConnectionState { + cert := &x509.Certificate{Issuer: issuer, Subject: subject} + for _, o := range opt { + o(cert) + } return &tls.ConnectionState{ - VerifiedChains: [][]*x509.Certificate{{{Issuer: issuer, Subject: subject}}}, + VerifiedChains: [][]*x509.Certificate{{cert}}, CipherSuite: cipher, } } diff --git a/util/misc.go b/util/misc.go index ad52368969944..707b07a180c27 100644 --- a/util/misc.go +++ b/util/misc.go @@ -266,6 +266,42 @@ func MockPkixAttribute(name, value string) pkix.AttributeTypeAndValue { } } +// SANType is enum value for GlobalPrivValue.SANs keys. +type SANType string + +const ( + // URI indicates uri info in SAN. + URI = SANType("URI") + // DNS indicates dns info in SAN. + DNS = SANType("DNS") + // IP indicates ip info in SAN. + IP = SANType("IP") +) + +var supportSAN = map[SANType]struct{}{ + URI: {}, + DNS: {}, + IP: {}, +} + +// ParseAndCheckSAN parses and check SAN str. +func ParseAndCheckSAN(san string) (map[SANType][]string, error) { + sanMap := make(map[SANType][]string) + sans := strings.Split(san, ",") + for _, san := range sans { + kv := strings.SplitN(san, ":", 2) + if len(kv) != 2 { + return nil, errors.Errorf("invalid SAN value %s", san) + } + k, v := SANType(strings.ToUpper(strings.TrimSpace(kv[0]))), strings.TrimSpace(kv[1]) + if _, s := supportSAN[k]; !s { + return nil, errors.Errorf("unsupported SAN key %s, current only support %v", k, supportSAN) + } + sanMap[k] = append(sanMap[k], v) + } + return sanMap, nil +} + // CheckSupportX509NameOneline parses and validate input str is X509_NAME_oneline format // and precheck check-item is supported by TiDB // https://www.openssl.org/docs/manmaster/man3/X509_NAME_oneline.html