*: Add security enhanced mode part 3

docs/design/2021-03-09-dynamic-privileges.md

@@ -1,7 +1,7 @@
 # Proposal:
 
 - Author(s):     [morgo](https://github.com/morgo)
-- Last updated:  April 25, 2021
+- Last updated:  May 04, 2021
 - Discussion at: N/A
 
 ## Table of Contents
@@ -238,7 +238,7 @@ No change
 
 | Privilege Name | Description | Notes |
 | --------------- | --------------- | --------------- |
-| `RESTRICTED_SYSTEM_VARIABLES_ADMIN` | Allows changing a restricted `GLOBAL` system variable. | Currently in SEM all high risk variables are unloaded. TBD, it might be required in future that they are only visible/settable to those with this privilege and not SUPER. |
+| `RESTRICTED_VARIABLES_ADMIN` | Allows changing a restricted `GLOBAL` system variable. | Currently in SEM all high risk variables are unloaded. TBD, it might be required in future that they are only visible/settable to those with this privilege and not SUPER. |
 | `RESTRICTED_STATUS_ADMIN` | Allows observing restricted status variables. | i.e. `SHOW GLOBAL STATUS` by default hides some statistics when `SEM` is enabled. |
 | `RESTRICTED_CONNECTION_ADMIN` | A special privilege to say that their connections, etc. can’t be killed by SUPER users AND they can kill connections by all other users. Affects `KILL`, `KILL TIDB` commands. | It is intended for the CloudAdmin user in DBaaS. |
 | `RESTRICTED_USER_ADMIN` | A special privilege to say that their access can’t be changed by `SUPER` users. Statements `DROP USER`, `SET PASSWORD`, `ALTER USER`, `REVOKE` are all limited. | It is intended for the CloudAdmin user in DbaaS. |

docs/design/2021-03-09-security-enhanced-mode.md

@@ -1,7 +1,7 @@
 # Proposal:
 
 - Author(s):     [morgo](https://github.com/morgo)
-- Last updated:  April 25, 2021
+- Last updated:  May 04, 2021
 - Discussion at: N/A
 
 ## Table of Contents
@@ -49,7 +49,7 @@ A boolean option called `EnableEnhancedSecurity` (default `FALSE`) will be added
 
 ### System Variables
 
-The following system variables will be hidden unless the user has the `RESTRICTED_SYSTEM_VARIABLES_ADMIN` privilege:
+The following system variables will be hidden unless the user has the `RESTRICTED_VARIABLES_ADMIN` privilege:
 
 * variable.TiDBDDLSlowOprThreshold,
 * variable.TiDBAllowRemoveAutoInc,

executor/executor_test.go

@@ -357,6 +357,7 @@ func (s *testSuiteP1) TestShow(c *C) {
 		"CONNECTION_ADMIN Server Admin ",
 		"RESTRICTED_TABLES_ADMIN Server Admin ",
 		"RESTRICTED_STATUS_ADMIN Server Admin ",
+		"RESTRICTED_VARIABLES_ADMIN Server Admin ",
 		"RESTRICTED_USER_ADMIN Server Admin ",
 	))
 	c.Assert(len(tk.MustQuery("show table status").Rows()), Equals, 1)

executor/show.go

@@ -661,6 +661,17 @@ 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
@@ -673,7 +684,7 @@ func (e *ShowExec) fetchShowVariables() (err error) {
 		// 		otherwise, fetch the value from table `mysql.Global_Variables`.
 		for _, v := range variable.GetSysVars() {
 			if v.Scope != variable.ScopeSession {
-				if v.Hidden {
+				if v.Hidden || e.sysVarHiddenForSem(v.Name) {
 					continue
 				}
 				value, err = variable.GetGlobalSystemVar(sessionVars, v.Name)
@@ -690,7 +701,7 @@ func (e *ShowExec) fetchShowVariables() (err error) {
 	// If it is a session only variable, use the default value defined in code,
 	//   otherwise, fetch the value from table `mysql.Global_Variables`.
 	for _, v := range variable.GetSysVars() {
-		if v.Hidden {
+		if v.Hidden || e.sysVarHiddenForSem(v.Name) {
 			continue
 		}
 		value, err = variable.GetSessionOrGlobalSystemVar(sessionVars, v.Name)

planner/core/expression_rewriter.go

@@ -42,6 +42,7 @@ import (
 	"github.com/pingcap/tidb/util/codec"
 	"github.com/pingcap/tidb/util/collate"
 	"github.com/pingcap/tidb/util/hint"
+	"github.com/pingcap/tidb/util/sem"
 	"github.com/pingcap/tidb/util/stringutil"
 )
 
@@ -1220,6 +1221,10 @@ func (er *expressionRewriter) rewriteVariable(v *ast.VariableExpr) {
 		er.err = variable.ErrUnknownSystemVar.GenWithStackByArgs(name)
 		return
 	}
+	if sem.IsEnabled() && sem.IsInvisibleSysVar(sysVar.Name) {
+		err := ErrSpecificAccessDenied.GenWithStackByArgs("RESTRICTED_VARIABLES_ADMIN")
+		er.b.visitInfo = appendDynamicVisitInfo(er.b.visitInfo, "RESTRICTED_VARIABLES_ADMIN", false, err)
+	}
 	if v.ExplicitScope && !sysVar.HasNoneScope() {
 		if v.IsGlobal && !sysVar.HasGlobalScope() {
 			er.err = variable.ErrIncorrectScope.GenWithStackByArgs(name, "GLOBAL")

planner/core/planbuilder.go

@@ -722,6 +722,10 @@ func (b *PlanBuilder) buildSet(ctx context.Context, v *ast.SetStmt) (Plan, error
 			err := ErrSpecificAccessDenied.GenWithStackByArgs("SUPER or SYSTEM_VARIABLES_ADMIN")
 			b.visitInfo = appendDynamicVisitInfo(b.visitInfo, "SYSTEM_VARIABLES_ADMIN", false, err)
 		}
+		if sem.IsEnabled() && sem.IsInvisibleSysVar(strings.ToLower(vars.Name)) {
+			err := ErrSpecificAccessDenied.GenWithStackByArgs("RESTRICTED_VARIABLES_ADMIN")
+			b.visitInfo = appendDynamicVisitInfo(b.visitInfo, "RESTRICTED_VARIABLES_ADMIN", false, err)
+		}
 		assign := &expression.VarAssignment{
 			Name:     vars.Name,
 			IsGlobal: vars.IsGlobal,

privilege/privileges/privileges.go

@@ -43,9 +43,10 @@ var dynamicPrivs = []string{
 	"SYSTEM_VARIABLES_ADMIN",
 	"ROLE_ADMIN",
 	"CONNECTION_ADMIN",
-	"RESTRICTED_TABLES_ADMIN", // Can see system tables when SEM is enabled
-	"RESTRICTED_STATUS_ADMIN", // Can see all status vars when SEM is enabled.
-	"RESTRICTED_USER_ADMIN",   // User can not have their access revoked by SUPER users.
+	"RESTRICTED_TABLES_ADMIN",    // Can see system tables when SEM is enabled
+	"RESTRICTED_STATUS_ADMIN",    // Can see all status vars when SEM is enabled.
+	"RESTRICTED_VARIABLES_ADMIN", // Can see all variables when SEM is enabled
+	"RESTRICTED_USER_ADMIN",      // User can not have their access revoked by SUPER users.
 }
 var dynamicPrivLock sync.Mutex
 

privilege/privileges/privileges_test.go

@@ -1400,6 +1400,55 @@ func (s *testPrivilegeSuite) TestSecurityEnhancedModeStatusVars(c *C) {
 	}, nil, nil)
 }
 
+func (s *testPrivilegeSuite) TestSecurityEnhancedModeSysVars(c *C) {
+	tk := testkit.NewTestKit(c, s.store)
+	tk.MustExec("CREATE USER svroot1, svroot2")
+	tk.MustExec("GRANT SUPER ON *.* to svroot1 WITH GRANT OPTION")
+	tk.MustExec("SET tidb_enable_dynamic_privileges=1")
+	tk.MustExec("GRANT SUPER, RESTRICTED_VARIABLES_ADMIN ON *.* to svroot2")
+
+	sem.Enable()
+	defer sem.Disable()
+
+	// svroot1 has SUPER but in SEM will be restricted
+	tk.Se.Auth(&auth.UserIdentity{
+		Username:     "svroot1",
+		Hostname:     "localhost",
+		AuthUsername: "uroot",
+		AuthHostname: "%",
+	}, nil, nil)
+
+	tk.MustQuery(`SHOW VARIABLES LIKE 'tidb_force_priority'`).Check(testkit.Rows())
+	tk.MustQuery(`SHOW GLOBAL VARIABLES LIKE 'tidb_enable_telemetry'`).Check(testkit.Rows())
+
+	_, err := tk.Exec("SET tidb_force_priority = 'NO_PRIORITY'")
+	c.Assert(err.Error(), Equals, "[planner:1227]Access denied; you need (at least one of) the RESTRICTED_VARIABLES_ADMIN privilege(s) for this operation")
+	_, err = tk.Exec("SET GLOBAL tidb_enable_telemetry = OFF")
+	c.Assert(err.Error(), Equals, "[planner:1227]Access denied; you need (at least one of) the RESTRICTED_VARIABLES_ADMIN privilege(s) for this operation")
+
+	_, err = tk.Exec("SELECT @@session.tidb_force_priority")
+	c.Assert(err.Error(), Equals, "[planner:1227]Access denied; you need (at least one of) the RESTRICTED_VARIABLES_ADMIN privilege(s) for this operation")
+	_, err = tk.Exec("SELECT @@global.tidb_enable_telemetry")
+	c.Assert(err.Error(), Equals, "[planner:1227]Access denied; you need (at least one of) the RESTRICTED_VARIABLES_ADMIN privilege(s) for this operation")
+
+	tk.Se.Auth(&auth.UserIdentity{
+		Username:     "svroot2",
+		Hostname:     "localhost",
+		AuthUsername: "uroot",
+		AuthHostname: "%",
+	}, nil, nil)
+
+	tk.MustQuery(`SHOW VARIABLES LIKE 'tidb_force_priority'`).Check(testkit.Rows("tidb_force_priority NO_PRIORITY"))
+	tk.MustQuery(`SHOW GLOBAL VARIABLES LIKE 'tidb_enable_telemetry'`).Check(testkit.Rows("tidb_enable_telemetry ON"))
+
+	// should not actually make any change.
+	tk.MustExec("SET tidb_force_priority = 'NO_PRIORITY'")
+	tk.MustExec("SET GLOBAL tidb_enable_telemetry = ON")
+
+	tk.MustQuery(`SELECT @@session.tidb_force_priority`).Check(testkit.Rows("NO_PRIORITY"))
+	tk.MustQuery(`SELECT @@global.tidb_enable_telemetry`).Check(testkit.Rows("1"))
+}
+
 // TestViewDefiner tests that default roles are correctly applied in the algorithm definer
 // See: https://github.com/pingcap/tidb/issues/24414
 func (s *testPrivilegeSuite) TestViewDefiner(c *C) {

sessionctx/variable/sysvar.go

@@ -575,7 +575,7 @@ var defaultSysVars = []*SysVar{
 		}
 		return normalizedValue, ErrWrongValueForVar.GenWithStackByArgs(ForeignKeyChecks, originalValue)
 	}},
-	{Scope: ScopeNone, Name: Hostname, Value: ServerHostname},
+	{Scope: ScopeNone, Name: Hostname, Value: DefHostname},
 	{Scope: ScopeSession, Name: Timestamp, Value: ""},
 	{Scope: ScopeGlobal | ScopeSession, Name: CharacterSetFilesystem, Value: "binary", Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) {
 		return checkCharacterSet(normalizedValue, CharacterSetFilesystem)

sessionctx/variable/tidb_vars.go

@@ -15,7 +15,6 @@ package variable
 
 import (
 	"math"
-	"os"
 
 	"github.com/pingcap/parser/mysql"
 	"github.com/pingcap/tidb/config"
@@ -694,7 +693,6 @@ var (
 	// DDLSlowOprThreshold is the threshold for ddl slow operations, uint is millisecond.
 	DDLSlowOprThreshold            uint32 = DefTiDBDDLSlowOprThreshold
 	ForcePriority                         = int32(DefTiDBForcePriority)
-	ServerHostname, _                     = os.Hostname()
 	MaxOfMaxAllowedPacket          uint64 = 1073741824
 	ExpensiveQueryTimeThreshold    uint64 = DefTiDBExpensiveQueryTimeThreshold
 	MinExpensiveQueryTimeThreshold uint64 = 10 // 10s

tidb-server/main.go

@@ -537,6 +537,9 @@ func setGlobalVars() {
 	variable.SetSysVar(variable.TiDBSlowQueryFile, cfg.Log.SlowQueryFile)
 	variable.SetSysVar(variable.TiDBIsolationReadEngines, strings.Join(cfg.IsolationRead.Engines, ", "))
 	variable.MemoryUsageAlarmRatio.Store(cfg.Performance.MemoryUsageAlarmRatio)
+	if hostname, err := os.Hostname(); err != nil {
+		variable.SetSysVar(variable.Hostname, hostname)
+	}
 
 	if cfg.Security.EnableSEM {
 		sem.Enable()

util/sem/sem.go

@@ -14,6 +14,7 @@
 package sem
 
 import (
+	"os"
 	"strings"
 	"sync/atomic"
 
@@ -70,6 +71,7 @@ var (
 func Enable() {
 	atomic.StoreInt32(&semEnabled, 1)
 	variable.SetSysVar(variable.TiDBEnableEnhancedSecurity, variable.On)
+	variable.SetSysVar(variable.Hostname, variable.DefHostname)
 	// write to log so users understand why some operations are weird.
 	logutil.BgLogger().Info("tidb-server is operating with security enhanced mode (SEM) enabled")
 }
@@ -79,6 +81,9 @@ func Enable() {
 func Disable() {
 	atomic.StoreInt32(&semEnabled, 0)
 	variable.SetSysVar(variable.TiDBEnableEnhancedSecurity, variable.Off)
+	if hostname, err := os.Hostname(); err != nil {
+		variable.SetSysVar(variable.Hostname, hostname)
+	}
 }
 
 // IsEnabled checks if Security Enhanced Mode (SEM) is enabled
@@ -125,6 +130,33 @@ func IsInvisibleStatusVar(varName string) bool {
 	return varName == tidbGCLeaderDesc
 }
 
+// IsInvisibleSysVar returns true if the sysvar needs to be hidden
+func IsInvisibleSysVar(varNameInLower string) bool {
+	switch varNameInLower {
+	case variable.TiDBDDLSlowOprThreshold, // ddl_slow_threshold
+		variable.TiDBAllowRemoveAutoInc,
+		variable.TiDBCheckMb4ValueInUTF8,
+		variable.TiDBConfig,
+		variable.TiDBEnableSlowLog,
+		variable.TiDBExpensiveQueryTimeThreshold,
+		variable.TiDBForcePriority,
+		variable.TiDBGeneralLog,
+		variable.TiDBMetricSchemaRangeDuration,
+		variable.TiDBMetricSchemaStep,
+		variable.TiDBOptWriteRowID,
+		variable.TiDBPProfSQLCPU,
+		variable.TiDBRecordPlanInSlowLog,
+		variable.TiDBSlowQueryFile,
+		variable.TiDBSlowLogThreshold,
+		variable.TiDBEnableCollectExecutionInfo,
+		variable.TiDBMemoryUsageAlarmRatio,
+		variable.TiDBEnableTelemetry,
+		variable.TiDBRowFormatVersion:
+		return true
+	}
+	return false
+}
+
 // IsRestrictedPrivilege returns true if the privilege shuld not be satisfied by SUPER
 // As most dynamic privileges are.
 func IsRestrictedPrivilege(privNameInUpper string) bool {

util/sem/sem_test.go

@@ -17,6 +17,7 @@ import (
 	"testing"
 
 	"github.com/pingcap/parser/mysql"
+	"github.com/pingcap/tidb/sessionctx/variable"
 
 	. "github.com/pingcap/check"
 )
@@ -74,3 +75,27 @@ func (s *testSecurity) TestIsInvisibleStatusVar(c *C) {
 	c.Assert(IsInvisibleStatusVar("ddl_schema_version"), IsFalse)
 	c.Assert(IsInvisibleStatusVar("Ssl_version"), IsFalse)
 }
+
+func (s *testSecurity) TestIsInvisibleSysVar(c *C) {
+	c.Assert(IsInvisibleSysVar(variable.Hostname), IsFalse)                   // changes the value to default, but is not invisible
+	c.Assert(IsInvisibleSysVar(variable.TiDBEnableEnhancedSecurity), IsFalse) // should be able to see the mode is on.
+
+	c.Assert(IsInvisibleSysVar(variable.TiDBAllowRemoveAutoInc), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBCheckMb4ValueInUTF8), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBConfig), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBEnableSlowLog), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBExpensiveQueryTimeThreshold), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBForcePriority), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBGeneralLog), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBMetricSchemaRangeDuration), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBMetricSchemaStep), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBOptWriteRowID), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBPProfSQLCPU), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBRecordPlanInSlowLog), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBSlowQueryFile), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBSlowLogThreshold), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBEnableCollectExecutionInfo), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBMemoryUsageAlarmRatio), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBEnableTelemetry), IsTrue)
+	c.Assert(IsInvisibleSysVar(variable.TiDBRowFormatVersion), IsTrue)
+}