Skip to content

Commit

Permalink
infoschema: check hidden sysvars for SEM (#37587)
Browse files Browse the repository at this point in the history
close #37586
  • Loading branch information
CbcWestwolf committed Sep 4, 2022
1 parent aa5645a commit 331707c
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 13 deletions.
3 changes: 3 additions & 0 deletions executor/infoschema_reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,9 @@ func (e *memtableRetriever) setDataForVariablesInfo(ctx sessionctx.Context) erro
sysVars := variable.GetSysVars()
rows := make([][]types.Datum, 0, len(sysVars))
for _, sv := range sysVars {
if infoschema.SysVarHiddenForSem(ctx, sv.Name) {
continue
}
currentVal, err := ctx.GetSessionVars().GetSessionOrGlobalSystemVar(sv.Name)
if err != nil {
currentVal = ""
Expand Down
15 changes: 2 additions & 13 deletions executor/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -799,17 +799,6 @@ func (e *ShowExec) fetchShowMasterStatus() error {
return nil
}

func (e *ShowExec) sysVarHiddenForSem(sysVarNameInLower string) bool {
if !sem.IsEnabled() || !sem.IsInvisibleSysVar(sysVarNameInLower) {
return false
}
checker := privilege.GetPrivilegeManager(e.ctx)
if checker == nil || checker.RequestDynamicVerification(e.ctx.GetSessionVars().ActiveRoles, "RESTRICTED_VARIABLES_ADMIN", false) {
return false
}
return true
}

func (e *ShowExec) fetchShowVariables() (err error) {
var (
value string
Expand Down Expand Up @@ -839,7 +828,7 @@ func (e *ShowExec) fetchShowVariables() (err error) {
} else if fieldPatternsLike != nil && !fieldPatternsLike.DoMatch(v.Name) {
continue
}
if e.sysVarHiddenForSem(v.Name) {
if infoschema.SysVarHiddenForSem(e.ctx, v.Name) {
continue
}
value, err = sessionVars.GetGlobalSystemVar(v.Name)
Expand All @@ -864,7 +853,7 @@ func (e *ShowExec) fetchShowVariables() (err error) {
} else if fieldPatternsLike != nil && !fieldPatternsLike.DoMatch(v.Name) {
continue
}
if e.sysVarHiddenForSem(v.Name) {
if infoschema.SysVarHiddenForSem(e.ctx, v.Name) {
continue
}
value, err = sessionVars.GetSessionOrGlobalSystemVar(v.Name)
Expand Down
17 changes: 17 additions & 0 deletions infoschema/tables.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/pingcap/tidb/parser/model"
"github.com/pingcap/tidb/parser/mysql"
"github.com/pingcap/tidb/parser/terror"
"github.com/pingcap/tidb/privilege"
"github.com/pingcap/tidb/session/txninfo"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/sessionctx/variable"
Expand All @@ -45,6 +46,7 @@ import (
"github.com/pingcap/tidb/util/execdetails"
"github.com/pingcap/tidb/util/logutil"
"github.com/pingcap/tidb/util/pdapi"
"github.com/pingcap/tidb/util/sem"
"github.com/pingcap/tidb/util/stmtsummary"
"github.com/tikv/client-go/v2/tikv"
"go.uber.org/zap"
Expand Down Expand Up @@ -1853,12 +1855,27 @@ func GetTiFlashStoreCount(ctx sessionctx.Context) (cnt uint64, err error) {
return cnt, nil
}

// SysVarHiddenForSem checks if a given sysvar is hidden according to SEM and privileges.
func SysVarHiddenForSem(ctx sessionctx.Context, sysVarNameInLower string) bool {
if !sem.IsEnabled() || !sem.IsInvisibleSysVar(sysVarNameInLower) {
return false
}
checker := privilege.GetPrivilegeManager(ctx)
if checker == nil || checker.RequestDynamicVerification(ctx.GetSessionVars().ActiveRoles, "RESTRICTED_VARIABLES_ADMIN", false) {
return false
}
return true
}

// GetDataFromSessionVariables return the [name, value] of all session variables
func GetDataFromSessionVariables(ctx sessionctx.Context) ([][]types.Datum, error) {
sessionVars := ctx.GetSessionVars()
sysVars := variable.GetSysVars()
rows := make([][]types.Datum, 0, len(sysVars))
for _, v := range sysVars {
if SysVarHiddenForSem(ctx, v.Name) {
continue
}
var value string
value, err := sessionVars.GetSessionOrGlobalSystemVar(v.Name)
if err != nil {
Expand Down
14 changes: 14 additions & 0 deletions privilege/privileges/privileges_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1971,7 +1971,9 @@ func TestSecurityEnhancedModeSysVars(t *testing.T) {
tk := testkit.NewTestKit(t, store)
tk.MustExec("CREATE USER svroot1, svroot2")
tk.MustExec("GRANT SUPER ON *.* to svroot1 WITH GRANT OPTION")
tk.MustExec("GRANT SELECT ON performance_schema.* to svroot1")
tk.MustExec("GRANT SUPER, RESTRICTED_VARIABLES_ADMIN ON *.* to svroot2")
tk.MustExec("GRANT SELECT ON performance_schema.* to svroot2")

sem.Enable()
defer sem.Disable()
Expand All @@ -1985,9 +1987,17 @@ func TestSecurityEnhancedModeSysVars(t *testing.T) {
}, nil, nil)

tk.MustQuery(`SHOW VARIABLES LIKE 'tidb_force_priority'`).Check(testkit.Rows())
tk.MustQuery(`SELECT * FROM information_schema.variables_info WHERE variable_name = 'tidb_force_priority'`).Check(testkit.Rows())
tk.MustQuery(`SELECT * FROM performance_schema.session_variables WHERE variable_name = 'tidb_force_priority'`).Check(testkit.Rows())
tk.MustQuery(`SHOW GLOBAL VARIABLES LIKE 'tidb_enable_telemetry'`).Check(testkit.Rows())
tk.MustQuery(`SELECT * FROM information_schema.variables_info WHERE variable_name = 'tidb_enable_telemetry'`).Check(testkit.Rows())
tk.MustQuery(`SELECT * FROM performance_schema.session_variables WHERE variable_name = 'tidb_enable_telemetry'`).Check(testkit.Rows())
tk.MustQuery(`SHOW GLOBAL VARIABLES LIKE 'tidb_top_sql_max_time_series_count'`).Check(testkit.Rows())
tk.MustQuery(`SELECT * FROM information_schema.variables_info WHERE variable_name = 'tidb_top_sql_max_time_series_count'`).Check(testkit.Rows())
tk.MustQuery(`SELECT * FROM performance_schema.session_variables WHERE variable_name = 'tidb_top_sql_max_time_series_count'`).Check(testkit.Rows())
tk.MustQuery(`SHOW GLOBAL VARIABLES LIKE 'tidb_top_sql_max_meta_count'`).Check(testkit.Rows())
tk.MustQuery(`SELECT * FROM information_schema.variables_info WHERE variable_name = 'tidb_top_sql_max_meta_count'`).Check(testkit.Rows())
tk.MustQuery(`SELECT * FROM performance_schema.session_variables WHERE variable_name = 'tidb_top_sql_max_meta_count'`).Check(testkit.Rows())

_, err := tk.Exec("SET @@global.tidb_force_priority = 'NO_PRIORITY'")
require.EqualError(t, err, "[planner:1227]Access denied; you need (at least one of) the RESTRICTED_VARIABLES_ADMIN privilege(s) for this operation")
Expand All @@ -2011,7 +2021,11 @@ func TestSecurityEnhancedModeSysVars(t *testing.T) {
}, nil, nil)

tk.MustQuery(`SHOW VARIABLES LIKE 'tidb_force_priority'`).Check(testkit.Rows("tidb_force_priority NO_PRIORITY"))
tk.MustQuery(`SELECT COUNT(*) FROM information_schema.variables_info WHERE variable_name = 'tidb_top_sql_max_meta_count'`).Check(testkit.Rows("1"))
tk.MustQuery(`SELECT COUNT(*) FROM performance_schema.session_variables WHERE variable_name = 'tidb_top_sql_max_meta_count'`).Check(testkit.Rows("1"))
tk.MustQuery(`SHOW GLOBAL VARIABLES LIKE 'tidb_enable_telemetry'`).Check(testkit.Rows("tidb_enable_telemetry ON"))
tk.MustQuery(`SELECT COUNT(*) FROM information_schema.variables_info WHERE variable_name = 'tidb_enable_telemetry'`).Check(testkit.Rows("1"))
tk.MustQuery(`SELECT COUNT(*) FROM performance_schema.session_variables WHERE variable_name = 'tidb_enable_telemetry'`).Check(testkit.Rows("1"))

// should not actually make any change.
tk.MustExec("SET @@global.tidb_force_priority = 'NO_PRIORITY'")
Expand Down

0 comments on commit 331707c

Please sign in to comment.