Skip to content

Commit

Permalink
privileges: support require SAN (#17539) (#17698)
Browse files Browse the repository at this point in the history 
* cherry pick #17539 to release-4.0

Signed-off-by: sre-bot <sre-bot@pingcap.com>

* resolve conflict

Co-authored-by: lysu <sulifx@gmail.com>
  • Loading branch information
@sre-bot @lysu
sre-bot and lysu authored Jun 5, 2020
1 parent 32ff824 commit bc522e3
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 16 deletions.
14 changes: 11 additions & 3 deletions executor/grant.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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("{}")
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
22 changes: 18 additions & 4 deletions privilege/privileges/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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.
Expand All @@ -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, " ")
}
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 7 additions & 1 deletion privilege/privileges/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
59 changes: 57 additions & 2 deletions privilege/privileges/privileges.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ package privileges

import (
"crypto/tls"
"crypto/x509"
"fmt"
"strings"

Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand Down
23 changes: 20 additions & 3 deletions privilege/privileges/privileges_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"crypto/x509"
"crypto/x509/pkix"
"fmt"
"net/url"
"os"
"strings"
"testing"
Expand Down Expand Up @@ -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() {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
}
}
Expand Down
36 changes: 36 additions & 0 deletions util/misc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit bc522e3

Please sign in to comment.