planner: make some show stmt more fine-grained privilege check (pin…

…gcap#35493) close pingcap#35393
morgo · Jun 21, 2022 · ff1b6ff · ff1b6ff
1 parent 3b34234
commit ff1b6ff
Show file tree

Hide file tree

Showing 5 changed files with 35 additions and 16 deletions.
diff --git a/executor/showtest/show_test.go b/executor/showtest/show_test.go
@@ -795,20 +795,32 @@ 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("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")
+
 }
 
 func TestIssue18878(t *testing.T) {

diff --git a/parser/parser.go b/parser/parser.go
@@ -19010,19 +19010,19 @@ yynewstate:
 		}
 	case 1896:
 		{
-			parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsMeta}
+			parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsMeta, Table: &ast.TableName{Name: model.NewCIStr("STATS_META"), Schema: model.NewCIStr(mysql.SystemDB)}}
 		}
 	case 1897:
 		{
-			parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsHistograms}
+			parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr("STATS_HISTOGRAMS"), Schema: model.NewCIStr(mysql.SystemDB)}}
 		}
 	case 1898:
 		{
 			parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsTopN}
 		}
 	case 1899:
 		{
-			parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsBuckets}
+			parser.yyVAL.item = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr("STATS_BUCKETS"), Schema: model.NewCIStr(mysql.SystemDB)}}
 		}
 	case 1900:
 		{

diff --git a/parser/parser.y b/parser/parser.y
@@ -10821,19 +10821,19 @@ ShowTargetFilterable:
 	}
 |	"STATS_META"
 	{
-		$$ = &ast.ShowStmt{Tp: ast.ShowStatsMeta}
+		$$ = &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}
+		$$ = &ast.ShowStmt{Tp: ast.ShowStatsHistograms, Table: &ast.TableName{Name: model.NewCIStr("STATS_HISTOGRAMS"), Schema: model.NewCIStr(mysql.SystemDB)}}
 	}
 |	"STATS_TOPN"
 	{
 		$$ = &ast.ShowStmt{Tp: ast.ShowStatsTopN}
 	}
 |	"STATS_BUCKETS"
 	{
-		$$ = &ast.ShowStmt{Tp: ast.ShowStatsBuckets}
+		$$ = &ast.ShowStmt{Tp: ast.ShowStatsBuckets, Table: &ast.TableName{Name: model.NewCIStr("STATS_BUCKETS"), Schema: model.NewCIStr(mysql.SystemDB)}}
 	}
 |	"STATS_HEALTHY"
 	{

diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go
@@ -2986,13 +2986,18 @@ 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:
-		user := b.ctx.GetSessionVars().User
+	case ast.ShowStatsExtended, ast.ShowStatsHealthy, ast.ShowStatsTopN, ast.ShowHistogramsInFlight, ast.ShowColumnStatsUsage:
 		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:
+		var err error
+		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)
 	case ast.ShowRegions:
 		tableInfo, err := b.is.TableByName(show.Table.Schema, show.Table.Name)
 		if err != nil {

diff --git 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",