Skip to content

Commit b7bb275

Browse files
lance6716lance6716
authored
check: some privilege should be global-level (#407)
1 parent a482225 commit b7bb275

File tree

7 files changed

+802
-236
lines changed

7 files changed

+802
-236
lines changed

‎go.mod1

+16-12
Original file line numberDiff line numberDiff line change
@@ -2,26 +2,30 @@ module github.com/pingcap/tidb-tools
22

33
require (
44
github.com/BurntSushi/toml v0.3.1
5-
github.com/DATA-DOG/go-sqlmock v1.4.1
5+
github.com/DATA-DOG/go-sqlmock v1.5.0
66
github.com/Shopify/sarama v1.24.1
77
github.com/go-sql-driver/mysql v1.5.0
88
github.com/golang/protobuf v1.3.4
9+
github.com/onsi/ginkgo v1.11.0 // indirect
10+
github.com/onsi/gomega v1.8.1 // indirect
911
github.com/pingcap/check v0.0.0-20200212061837-5e12011dc712
1012
github.com/pingcap/dm v1.1.0-alpha.0.20200729032940-53a08d777030
11-
github.com/pingcap/errors v0.11.5-0.20200917111840-a15ef68f753d
13+
github.com/pingcap/errors v0.11.5-0.20201126102027-b0a155152ca3
1214
github.com/pingcap/failpoint v0.0.0-20200702092429-9f69995143ce
13-
github.com/pingcap/kvproto v0.0.0-20200828054126-d677e6fd224a
14-
github.com/pingcap/log v0.0.0-20200828042413-fce0951f1463
15-
github.com/pingcap/parser v0.0.0-20200924053142-5d7e8ebf605e
16-
github.com/pingcap/tidb v1.1.0-beta.0.20200927065602-486e473a86e9
17-
github.com/pingcap/tipb v0.0.0-20200618092958-4fad48b4c8c3
15+
github.com/pingcap/kvproto v0.0.0-20201215060142-f3dafca4c7fd
16+
github.com/pingcap/log v0.0.0-20201112100606-8f1e84a3abc8
17+
github.com/pingcap/parser v0.0.0-20210105063241-09b74e077464
18+
github.com/pingcap/tidb v1.1.0-beta.0.20201222032702-32d8cad845d6
19+
github.com/pingcap/tipb v0.0.0-20201209065231-aa39b1b86217
1820
github.com/siddontang/go v0.0.0-20180604090527-bdc77568d726
19-
github.com/tikv/pd v1.1.0-beta.0.20200907085700-5b04bec39b99
20-
go.etcd.io/etcd v0.5.0-alpha.5.0.20191023171146-3cf2f69b5738
21-
go.uber.org/atomic v1.6.0
21+
github.com/syndtr/goleveldb v1.0.1-0.20190625010220-02440ea7a285 // indirect
22+
github.com/tikv/pd v1.1.0-beta.0.20201125070607-d4b90eee0c70
23+
go.etcd.io/bbolt v1.3.5 // indirect
24+
go.etcd.io/etcd v0.5.0-alpha.5.0.20200824191128-ae9734ed278b
25+
go.uber.org/atomic v1.7.0
2226
go.uber.org/zap v1.16.0
23-
golang.org/x/net v0.0.0-20200813134508-3edf25e44fcc
24-
google.golang.org/grpc v1.26.0
27+
golang.org/x/net v0.0.0-20200904194848-62affa334b73
28+
google.golang.org/grpc v1.27.1
2529
)
2630

2731
replace github.com/golang/lint => golang.org/x/lint v0.0.0-20190930215403-16217165b5de

‎go.sum1

+525-148
Large diffs are not rendered by default.

‎pkg/check/privilege.go

+102-53
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,30 @@ import (
2222
"github.com/pingcap/errors"
2323
"github.com/pingcap/parser"
2424
"github.com/pingcap/parser/ast"
25+
"github.com/pingcap/parser/mysql"
2526
"github.com/pingcap/tidb-tools/pkg/dbutil"
2627
_ "github.com/pingcap/tidb/types/parser_driver" // for parser driver
2728
)
2829

30+
var (
31+
dumpPrivileges = map[mysql.PrivilegeType]struct{}{
32+
mysql.ReloadPriv: {},
33+
mysql.SelectPriv: {},
34+
}
35+
replicationPrivileges = map[mysql.PrivilegeType]struct{}{
36+
mysql.ReplicationClientPriv: {},
37+
mysql.ReplicationSlavePriv: {},
38+
}
39+
40+
// some privileges are only effective on global level. in other words, GRANT ALL ON test.* is not enough for them
41+
// https://dev.mysql.com/doc/refman/5.7/en/grant.html#grant-global-privileges
42+
privNeedGlobal = map[mysql.PrivilegeType]struct{}{
43+
mysql.ReloadPriv: {},
44+
mysql.ReplicationClientPriv: {},
45+
mysql.ReplicationSlavePriv: {},
46+
}
47+
)
48+
2949
/*****************************************************/
3050

3151
// SourceDumpPrivilegeChecker checks dump privileges of source DB.
@@ -55,7 +75,7 @@ func (pc *SourceDumpPrivilegeChecker) Check(ctx context.Context) *Result {
5575
return result
5676
}
5777

58-
verifyPrivileges(result, grants, []string{"RELOAD", "SELECT"})
78+
verifyPrivileges(result, grants, dumpPrivileges)
5979
return result
6080
}
6181

@@ -93,7 +113,7 @@ func (pc *SourceReplicatePrivilegeChecker) Check(ctx context.Context) *Result {
93113
return result
94114
}
95115

96-
verifyPrivileges(result, grants, []string{"REPLICATION SLAVE", "REPLICATION CLIENT"})
116+
verifyPrivileges(result, grants, replicationPrivileges)
97117
return result
98118
}
99119

@@ -102,74 +122,103 @@ func (pc *SourceReplicatePrivilegeChecker) Name() string {
102122
return "source db replication privilege checker"
103123
}
104124

105-
func verifyPrivileges(result *Result, grants []string, expectedGrants []string) {
125+
// TODO: if we add more privilege in future, we might add special checks (globally granted?) for that new privilege
126+
func verifyPrivileges(result *Result, grants []string, expectedGrants map[mysql.PrivilegeType]struct{}) {
127+
result.State = StateFailure
106128
if len(grants) == 0 {
107129
result.ErrorMsg = "there is no such grant defined for current user on host '%'"
108130
return
109131
}
110132

111-
// TiDB parser does not support parse `IDENTIFIED BY PASSWORD <secret>`,
112-
// but it may appear in some cases, ref: https://bugs.mysql.com/bug.php?id=78888.
113-
// We do not need the password in grant statement, so we can replace it.
114-
firstGrant := strings.Replace(grants[0], "IDENTIFIED BY PASSWORD <secret>", "IDENTIFIED BY PASSWORD 'secret'", 1)
115-
116-
// support parse `IDENTIFIED BY PASSWORD WITH {GRANT OPTION | resource_option} ...`
117-
firstGrant = strings.Replace(firstGrant, "IDENTIFIED BY PASSWORD WITH", "IDENTIFIED BY PASSWORD 'secret' WITH", 1)
118-
119-
// support parse `IDENTIFIED BY PASSWORD`
120-
if strings.HasSuffix(firstGrant, "IDENTIFIED BY PASSWORD") {
121-
firstGrant = firstGrant + " 'secret'"
133+
var (
134+
user string
135+
lackGrants = make(map[mysql.PrivilegeType]struct{}, len(expectedGrants))
136+
)
137+
for k := range expectedGrants {
138+
lackGrants[k] = struct{}{}
122139
}
123140

124-
// Aurora has some privilege failing parsing
125-
awsPrivilege := []string{"LOAD FROM S3", "SELECT INTO S3", "INVOKE LAMBDA", "INVOKE SAGEMAKER", "INVOKE COMPREHEND"}
126-
for _, p := range awsPrivilege {
127-
firstGrant = strings.Replace(firstGrant, p, "", 1)
128-
firstGrant = strings.ReplaceAll(firstGrant, ", ,", ",")
129-
}
130-
firstGrant = strings.ReplaceAll(firstGrant, "GRANT ,", "GRANT ")
131-
firstGrant = strings.ReplaceAll(firstGrant, ", ON", " ON")
141+
for i, grant := range grants {
142+
// TiDB parser does not support parse `IDENTIFIED BY PASSWORD <secret>`,
143+
// but it may appear in some cases, ref: https://bugs.mysql.com/bug.php?id=78888.
144+
// We do not need the password in grant statement, so we can replace it.
145+
grant := strings.Replace(grant, "IDENTIFIED BY PASSWORD <secret>", "IDENTIFIED BY PASSWORD 'secret'", 1)
132146

133-
// get username and hostname
134-
node, err := parser.New().ParseOneStmt(firstGrant, "", "")
135-
if err != nil {
136-
result.ErrorMsg = errors.ErrorStack(errors.Annotatef(err, "grants[0] %s, firstGrant after replace %s", grants[0], firstGrant))
137-
return
138-
}
139-
grantStmt, ok := node.(*ast.GrantStmt)
140-
if !ok {
141-
result.ErrorMsg = fmt.Sprintf("%s is not grant statment", grants[0])
142-
return
143-
}
147+
// support parse `IDENTIFIED BY PASSWORD WITH {GRANT OPTION | resource_option} ...`
148+
grant = strings.Replace(grant, "IDENTIFIED BY PASSWORD WITH", "IDENTIFIED BY PASSWORD 'secret' WITH", 1)
144149

145-
if len(grantStmt.Users) == 0 {
146-
result.ErrorMsg = fmt.Sprintf("grant has not user %s", grantStmt.Text())
147-
return
148-
}
150+
// support parse `IDENTIFIED BY PASSWORD`
151+
if strings.HasSuffix(grant, "IDENTIFIED BY PASSWORD") {
152+
grant = grant + " 'secret'"
153+
}
149154

150-
// TODO: user tidb parser(which not works very well now)
151-
lackOfPrivileges := make([]string, 0, len(expectedGrants))
152-
for _, expected := range expectedGrants {
153-
hasPrivilege := false
154-
for _, grant := range grants {
155-
if strings.Contains(grant, "ALL PRIVILEGES") {
156-
result.State = StateSuccess
155+
// get username and hostname
156+
node, err := parser.New().ParseOneStmt(grant, "", "")
157+
if err != nil {
158+
result.ErrorMsg = errors.ErrorStack(errors.Annotatef(err, "grant %s, grant after replace %s", grants[i], grant))
159+
return
160+
}
161+
grantStmt, ok := node.(*ast.GrantStmt)
162+
if !ok {
163+
switch node.(type) {
164+
case *ast.GrantProxyStmt, *ast.GrantRoleStmt:
165+
continue
166+
default:
167+
result.ErrorMsg = fmt.Sprintf("%s is not grant statment", grants[i])
157168
return
158169
}
159-
if strings.Contains(grant, expected) {
160-
hasPrivilege = true
161-
}
162170
}
163-
if !hasPrivilege {
164-
lackOfPrivileges = append(lackOfPrivileges, expected)
171+
172+
if len(grantStmt.Users) == 0 {
173+
result.ErrorMsg = fmt.Sprintf("grant has no user %s", grantStmt.Text())
174+
return
175+
} else if user == "" {
176+
// show grants will only output grants for requested user
177+
user = grantStmt.Users[0].User.Username
178+
}
179+
180+
for _, privElem := range grantStmt.Privs {
181+
if privElem.Priv == mysql.AllPriv {
182+
if grantStmt.Level.Level == ast.GrantLevelGlobal {
183+
result.State = StateSuccess
184+
return
185+
} else {
186+
// REPLICATION CLIENT, REPLICATION SLAVE, RELOAD should be global privileges,
187+
// thus a non-global GRANT ALL is not enough
188+
for expectedGrant := range lackGrants {
189+
if _, ok := privNeedGlobal[expectedGrant]; !ok {
190+
delete(lackGrants, expectedGrant)
191+
}
192+
}
193+
}
194+
} else {
195+
// check every privilege and remove it from expectedGrants
196+
if _, ok := lackGrants[privElem.Priv]; ok {
197+
if _, ok := privNeedGlobal[privElem.Priv]; ok {
198+
if grantStmt.Level.Level == ast.GrantLevelGlobal {
199+
delete(lackGrants, privElem.Priv)
200+
}
201+
} else {
202+
// currently, only SELECT privilege goes here. we didn't require SELECT to be granted globally,
203+
// dumpling could report error if an allow-list table is lack of privilege.
204+
// we only check that SELECT is granted on all columns, otherwise we can't SHOW CREATE TABLE
205+
if len(privElem.Cols) == 0 {
206+
delete(lackGrants, privElem.Priv)
207+
}
208+
}
209+
}
210+
}
165211
}
166212
}
167213

168-
user := grantStmt.Users[0]
169-
if len(lackOfPrivileges) != 0 {
170-
privileges := strings.Join(lackOfPrivileges, ",")
214+
if len(lackGrants) != 0 {
215+
lackGrantsStr := make([]string, 0, len(lackGrants))
216+
for g := range lackGrants {
217+
lackGrantsStr = append(lackGrantsStr, mysql.Priv2Str[g])
218+
}
219+
privileges := strings.Join(lackGrantsStr, ",")
171220
result.ErrorMsg = fmt.Sprintf("lack of %s privilege", privileges)
172-
result.Instruction = fmt.Sprintf("GRANT %s ON *.* TO '%s'@'%s';", privileges, user.User.Username, "%")
221+
result.Instruction = fmt.Sprintf("GRANT %s ON *.* TO '%s'@'%s';", privileges, user, "%")
173222
return
174223
}
175224

‎pkg/check/privilege_test.go

+38-10
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,6 @@ var _ = tc.Suite(&testCheckSuite{})
1515
type testCheckSuite struct{}
1616

1717
func (t *testCheckSuite) TestVerifyPrivileges(c *tc.C) {
18-
var (
19-
dumpPrivileges = []string{"RELOAD", "SELECT"}
20-
replicationPrivileges = []string{"REPLICATION SLAVE", "REPLICATION CLIENT"}
21-
)
22-
2318
cases := []struct {
2419
grants []string
2520
dumpState State
@@ -54,7 +49,7 @@ func (t *testCheckSuite) TestVerifyPrivileges(c *tc.C) {
5449
grants: []string{ // lack optional privilege
5550
"GRANT REPLICATION SLAVE ON *.* TO 'user'@'%'",
5651
"GRANT REPLICATION CLIENT ON *.* TO 'user'@'%'",
57-
"GRANT EXECUTE ON FUNCTION db1.anomaly_score TO user1@domain-or-ip-address1",
52+
"GRANT EXECUTE ON FUNCTION db1.anomaly_score TO 'user1'@'domain-or-ip-address1'",
5853
},
5954
dumpState: StateFailure,
6055
replcationState: StateSuccess,
@@ -64,7 +59,7 @@ func (t *testCheckSuite) TestVerifyPrivileges(c *tc.C) {
6459
"GRANT REPLICATION SLAVE ON *.* TO 'user'@'%'",
6560
"GRANT REPLICATION CLIENT ON *.* TO 'user'@'%'",
6661
"GRANT RELOAD ON *.* TO 'user'@'%'",
67-
"GRANT EXECUTE ON FUNCTION db1.anomaly_score TO user1@domain-or-ip-address1",
62+
"GRANT EXECUTE ON FUNCTION db1.anomaly_score TO 'user1'@'domain-or-ip-address1'",
6863
},
6964
dumpState: StateFailure,
7065
replcationState: StateSuccess,
@@ -84,11 +79,11 @@ func (t *testCheckSuite) TestVerifyPrivileges(c *tc.C) {
8479
replcationState: StateSuccess,
8580
},
8681
{
87-
grants: []string{ // lower case not supported yet
82+
grants: []string{ // lower case
8883
"GRANT all privileges ON *.* TO 'user'@'%'",
8984
},
90-
dumpState: StateFailure,
91-
replcationState: StateFailure,
85+
dumpState: StateSuccess,
86+
replcationState: StateSuccess,
9287
},
9388
{
9489
grants: []string{ // IDENTIFIED BY PASSWORD
@@ -160,6 +155,39 @@ func (t *testCheckSuite) TestVerifyPrivileges(c *tc.C) {
160155
dumpState: StateSuccess,
161156
replcationState: StateSuccess,
162157
},
158+
{
159+
grants: []string{ // privilege on db/table level is not enough to execute SHOW MASTER STATUS
160+
"GRANT ALL PRIVILEGES ON `medz`.* TO `zhangsan`@`10.8.1.9` WITH GRANT OPTION",
161+
},
162+
dumpState: StateFailure,
163+
replcationState: StateFailure,
164+
},
165+
{
166+
grants: []string{ // privilege on column level is not enough to execute SHOW CREATE TABLE
167+
"GRANT REPLICATION SLAVE, REPLICATION CLIENT, RELOAD ON *.* TO 'user'@'%'",
168+
"GRANT SELECT (c) ON `lance`.`t` TO 'user'@'%'",
169+
},
170+
dumpState: StateFailure,
171+
replcationState: StateSuccess,
172+
},
173+
{
174+
grants: []string{
175+
"GRANT RELOAD ON *.* TO `u1`@`localhost`",
176+
"GRANT SELECT, INSERT, UPDATE, DELETE ON `db1`.* TO `u1`@`localhost`",
177+
"GRANT `r1`@`%`,`r2`@`%` TO `u1`@`localhost`",
178+
},
179+
dumpState: StateSuccess,
180+
replcationState: StateFailure,
181+
},
182+
{
183+
grants: []string{
184+
"GRANT SELECT, INSERT, UPDATE, DELETE, CREATE, DROP, RELOAD, SHUTDOWN, PROCESS, FILE, REFERENCES, INDEX, ALTER, SHOW DATABASES, SUPER, CREATE TEMPORARY TABLES, LOCK TABLES, EXECUTE, REPLICATION SLAVE, REPLICATION CLIENT, CREATE VIEW, SHOW VIEW, CREATE ROUTINE, ALTER ROUTINE, CREATE USER, EVENT, TRIGGER, CREATE TABLESPACE, CREATE ROLE, DROP ROLE ON *.* TO `root`@`localhost` WITH GRANT OPTION",
185+
"GRANT APPLICATION_PASSWORD_ADMIN,AUDIT_ADMIN,BACKUP_ADMIN,BINLOG_ADMIN,BINLOG_ENCRYPTION_ADMIN,CLONE_ADMIN,CONNECTION_ADMIN,ENCRYPTION_KEY_ADMIN,GROUP_REPLICATION_ADMIN,INNODB_REDO_LOG_ARCHIVE,PERSIST_RO_VARIABLES_ADMIN,REPLICATION_APPLIER,REPLICATION_SLAVE_ADMIN,RESOURCE_GROUP_ADMIN,RESOURCE_GROUP_USER,ROLE_ADMIN,SERVICE_CONNECTION_ADMIN,SESSION_VARIABLES_ADMIN,SET_USER_ID,SYSTEM_USER,SYSTEM_VARIABLES_ADMIN,TABLE_ENCRYPTION_ADMIN,XA_RECOVER_ADMIN ON *.* TO `root`@`localhost` WITH GRANT OPTION",
186+
"GRANT PROXY ON ''@'' TO 'root'@'localhost' WITH GRANT OPTION",
187+
},
188+
dumpState: StateSuccess,
189+
replcationState: StateSuccess,
190+
},
163191
}
164192

165193
for _, cs := range cases {

0 commit comments

Comments
 (0)