From cf99e5a45ec9ace84a7947b730f4eb3bf0d63edf Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Sat, 18 Jun 2022 22:32:05 +0800 Subject: [PATCH 1/5] make some show stmt more fine-grained privilege check --- executor/showtest/show_test.go | 22 +++++++++++++++------- parser/parser.go | 6 +++--- parser/parser.y | 6 +++--- planner/core/planbuilder.go | 9 ++++++++- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/executor/showtest/show_test.go b/executor/showtest/show_test.go index 0993505d9ad8a..7bfcb4b203094 100644 --- a/executor/showtest/show_test.go +++ b/executor/showtest/show_test.go @@ -794,20 +794,28 @@ func TestShowStatsPrivilege(t *testing.T) { tk1 := testkit.NewTestKit(t, store) require.True(t, tk1.Session().Auth(&auth.UserIdentity{Username: "show_stats", Hostname: "%"}, nil, nil)) + e := "[planner:1142]SHOW command denied to user 'show_stats'@'%' for table" + err := tk1.ExecToErr("show stats_meta") + require.ErrorContains(t, err, e) + err = tk1.ExecToErr("SHOW STATS_BUCKETS") + require.ErrorContains(t, err, e) + err = tk1.ExecToErr("SHOW STATS_HISTOGRAMS") + require.ErrorContains(t, err, e) + eqErr := plannercore.ErrDBaccessDenied.GenWithStackByArgs("show_stats", "%", mysql.SystemDB) - _, err := tk1.Exec("show stats_meta") - require.EqualError(t, err, eqErr.Error()) - _, err = tk1.Exec("SHOW STATS_BUCKETS") - require.EqualError(t, err, eqErr.Error()) - _, err = tk1.Exec("SHOW STATS_HEALTHY") - require.EqualError(t, err, eqErr.Error()) - _, err = tk1.Exec("SHOW STATS_HISTOGRAMS") + err = tk1.ExecToErr("SHOW STATS_HEALTHY") require.EqualError(t, err, eqErr.Error()) tk.MustExec("grant select on mysql.* to show_stats") tk1.MustExec("show stats_meta") tk1.MustExec("SHOW STATS_BUCKETS") tk1.MustExec("SHOW STATS_HEALTHY") tk1.MustExec("SHOW STATS_HISTOGRAMS") + + tk.MustExec("create user a@'%' identified by '';") + require.True(t, tk1.Session().Auth(&auth.UserIdentity{Username: "a", Hostname: "%"}, nil, nil)) + tk.MustExec("grant select on mysql.stats_meta to a@'%';") + tk.MustExec("show stats_meta;") + } func TestIssue18878(t *testing.T) { diff --git a/parser/parser.go b/parser/parser.go index 85555ab305611..ea094d0642286 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -18992,11 +18992,11 @@ yynewstate: } case 1895: { - parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsMeta} + parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsMeta, Table: &ast.TableName{Name: model.NewCIStr(yyS[yypt-0].ident), Schema: model.NewCIStr(mysql.SystemDB)}} } case 1896: { - parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsHistograms} + parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr(yyS[yypt-0].ident), Schema: model.NewCIStr(mysql.SystemDB)}} } case 1897: { @@ -19004,7 +19004,7 @@ yynewstate: } case 1898: { - parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsBuckets} + parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr(yyS[yypt-0].ident), Schema: model.NewCIStr(mysql.SystemDB)}} } case 1899: { diff --git a/parser/parser.y b/parser/parser.y index d88806b27db88..aac23fe256ce0 100644 --- a/parser/parser.y +++ b/parser/parser.y @@ -10810,11 +10810,11 @@ ShowTargetFilterable: } | "STATS_META" { - $$ = &ast.ShowStmt{Tp: ast.ShowStatsMeta} + $$ = &ast.ShowStmt{Tp: ast.ShowStatsMeta, Table: &ast.TableName{Name: model.NewCIStr($1), Schema: model.NewCIStr(mysql.SystemDB)}} } | "STATS_HISTOGRAMS" { - $$ = &ast.ShowStmt{Tp: ast.ShowStatsHistograms} + $$ = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr($1), Schema: model.NewCIStr(mysql.SystemDB)}} } | "STATS_TOPN" { @@ -10822,7 +10822,7 @@ ShowTargetFilterable: } | "STATS_BUCKETS" { - $$ = &ast.ShowStmt{Tp: ast.ShowStatsBuckets} + $$ = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr($1), Schema: model.NewCIStr(mysql.SystemDB)}} } | "STATS_HEALTHY" { diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 040573420a076..cd0bddcf95951 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -2986,13 +2986,20 @@ func (b *PlanBuilder) buildShow(ctx context.Context, show *ast.ShowStmt) (Plan, p.setSchemaAndNames(buildShowNextRowID()) b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, show.Table.Schema.L, show.Table.Name.L, "", ErrPrivilegeCheckFail) return p, nil - case ast.ShowStatsBuckets, ast.ShowStatsHistograms, ast.ShowStatsMeta, ast.ShowStatsExtended, ast.ShowStatsHealthy, ast.ShowStatsTopN, ast.ShowHistogramsInFlight, ast.ShowColumnStatsUsage: + case ast.ShowStatsExtended, ast.ShowStatsHealthy, ast.ShowStatsTopN, ast.ShowHistogramsInFlight, ast.ShowColumnStatsUsage: user := b.ctx.GetSessionVars().User var err error if user != nil { err = ErrDBaccessDenied.GenWithStackByArgs(user.AuthUsername, user.AuthHostname, mysql.SystemDB) } b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, mysql.SystemDB, "", "", err) + case ast.ShowStatsBuckets, ast.ShowStatsHistograms, ast.ShowStatsMeta: + user := b.ctx.GetSessionVars().User + var err error + if user != nil { + err = ErrTableaccessDenied.GenWithStackByArgs("SHOW", user.AuthUsername, user.AuthHostname, show.Table.Name.L) + } + b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, show.Table.Schema.L, show.Table.Name.L, "", err) case ast.ShowRegions: tableInfo, err := b.is.TableByName(show.Table.Schema, show.Table.Name) if err != nil { From 827dd82689023521cbc2419816b6110da870dd4f Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Sat, 18 Jun 2022 22:37:20 +0800 Subject: [PATCH 2/5] unit update --- executor/showtest/show_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/executor/showtest/show_test.go b/executor/showtest/show_test.go index 7bfcb4b203094..8487d670c3042 100644 --- a/executor/showtest/show_test.go +++ b/executor/showtest/show_test.go @@ -814,7 +814,11 @@ func TestShowStatsPrivilege(t *testing.T) { tk.MustExec("create user a@'%' identified by '';") require.True(t, tk1.Session().Auth(&auth.UserIdentity{Username: "a", Hostname: "%"}, nil, nil)) tk.MustExec("grant select on mysql.stats_meta to a@'%';") - tk.MustExec("show stats_meta;") + tk.MustExec("grant select on mysql.stats_buckets to a@'%';") + tk.MustExec("grant select on mysql.stats_histograms to a@'%';") + tk1.MustExec("show stats_meta") + tk1.MustExec("SHOW STATS_BUCKETS") + tk1.MustExec("SHOW STATS_HISTOGRAMS") } From 7a1b2bc306bc64a2eac9438ec844d34cfe55fa8d Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Sat, 18 Jun 2022 23:19:15 +0800 Subject: [PATCH 3/5] fix --- parser/parser.go | 6 +++--- parser/parser.y | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/parser/parser.go b/parser/parser.go index ea094d0642286..9f40ca0c29a40 100644 --- a/parser/parser.go +++ b/parser/parser.go @@ -18992,11 +18992,11 @@ yynewstate: } case 1895: { - parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsMeta, Table: &ast.TableName{Name: model.NewCIStr(yyS[yypt-0].ident), Schema: model.NewCIStr(mysql.SystemDB)}} + parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsMeta, Table: &ast.TableName{Name: model.NewCIStr("STATS_META"), Schema: model.NewCIStr(mysql.SystemDB)}} } case 1896: { - parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr(yyS[yypt-0].ident), Schema: model.NewCIStr(mysql.SystemDB)}} + parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr("STATS_HISTOGRAMS"), Schema: model.NewCIStr(mysql.SystemDB)}} } case 1897: { @@ -19004,7 +19004,7 @@ yynewstate: } case 1898: { - parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr(yyS[yypt-0].ident), Schema: model.NewCIStr(mysql.SystemDB)}} + parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr("STATS_BUCKETS"), Schema: model.NewCIStr(mysql.SystemDB)}} } case 1899: { diff --git a/parser/parser.y b/parser/parser.y index aac23fe256ce0..b6b8a526a2dda 100644 --- a/parser/parser.y +++ b/parser/parser.y @@ -10810,11 +10810,11 @@ ShowTargetFilterable: } | "STATS_META" { - $$ = &ast.ShowStmt{Tp: ast.ShowStatsMeta, Table: &ast.TableName{Name: model.NewCIStr($1), Schema: model.NewCIStr(mysql.SystemDB)}} + $$ = &ast.ShowStmt{Tp: ast.ShowStatsMeta, Table: &ast.TableName{Name: model.NewCIStr("STATS_META"), Schema: model.NewCIStr(mysql.SystemDB)}} } | "STATS_HISTOGRAMS" { - $$ = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr($1), Schema: model.NewCIStr(mysql.SystemDB)}} + $$ = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr("STATS_HISTOGRAMS"), Schema: model.NewCIStr(mysql.SystemDB)}} } | "STATS_TOPN" { @@ -10822,7 +10822,7 @@ ShowTargetFilterable: } | "STATS_BUCKETS" { - $$ = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr($1), Schema: model.NewCIStr(mysql.SystemDB)}} + $$ = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr("STATS_BUCKETS"), Schema: model.NewCIStr(mysql.SystemDB)}} } | "STATS_HEALTHY" { From c03507525cf69628ad0c7f9a59b5f4e776fc5111 Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Sun, 19 Jun 2022 10:15:58 +0800 Subject: [PATCH 4/5] refactor code --- planner/core/planbuilder.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index cd0bddcf95951..e4f60984a75b3 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -2987,16 +2987,14 @@ func (b *PlanBuilder) buildShow(ctx context.Context, show *ast.ShowStmt) (Plan, b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, show.Table.Schema.L, show.Table.Name.L, "", ErrPrivilegeCheckFail) return p, nil case ast.ShowStatsExtended, ast.ShowStatsHealthy, ast.ShowStatsTopN, ast.ShowHistogramsInFlight, ast.ShowColumnStatsUsage: - user := b.ctx.GetSessionVars().User var err error - if user != nil { + if user := b.ctx.GetSessionVars().User; user != nil { err = ErrDBaccessDenied.GenWithStackByArgs(user.AuthUsername, user.AuthHostname, mysql.SystemDB) } b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, mysql.SystemDB, "", "", err) case ast.ShowStatsBuckets, ast.ShowStatsHistograms, ast.ShowStatsMeta: - user := b.ctx.GetSessionVars().User var err error - if user != nil { + if user := b.ctx.GetSessionVars().User; user != nil { err = ErrTableaccessDenied.GenWithStackByArgs("SHOW", user.AuthUsername, user.AuthHostname, show.Table.Name.L) } b.visitInfo = appendVisitInfo(b.visitInfo, mysql.SelectPriv, show.Table.Schema.L, show.Table.Name.L, "", err) From 5c9495d8e5bd01e605f446424c27659d85a4b9f9 Mon Sep 17 00:00:00 2001 From: likzn <1020193211@qq.com> Date: Tue, 21 Jun 2022 18:08:27 +0800 Subject: [PATCH 5/5] fix unit --- plugin/integration_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/plugin/integration_test.go b/plugin/integration_test.go index 96bb5c3295a5f..1c62fa440da6d 100644 --- a/plugin/integration_test.go +++ b/plugin/integration_test.go @@ -485,11 +485,13 @@ func TestAuditLogNormal(t *testing.T) { sql: "show stats_histograms", stmtType: "Show", dbs: "mysql", + tables: "stats_histograms", }, { sql: "show stats_meta", stmtType: "Show", dbs: "mysql", + tables: "stats_meta", }, { sql: "show status",