diff --git a/docs/design/2021-03-09-dynamic-privileges.md b/docs/design/2021-03-09-dynamic-privileges.md index 7ad0d59d2c54e..c85c0dc0c5305 100644 --- a/docs/design/2021-03-09-dynamic-privileges.md +++ b/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. | diff --git a/docs/design/2021-03-09-security-enhanced-mode.md b/docs/design/2021-03-09-security-enhanced-mode.md index e939fec67c154..efc5b79f499e4 100644 --- a/docs/design/2021-03-09-security-enhanced-mode.md +++ b/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, diff --git a/executor/executor_test.go b/executor/executor_test.go index e359837d8fb92..5e6b4490f5eb6 100644 --- a/executor/executor_test.go +++ b/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) diff --git a/executor/show.go b/executor/show.go index ab08e5ba4cbf5..09e3d0c71e7b4 100644 --- a/executor/show.go +++ b/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) diff --git a/planner/core/expression_rewriter.go b/planner/core/expression_rewriter.go index 8b52318a260a1..eb154d0201ecb 100644 --- a/planner/core/expression_rewriter.go +++ b/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") diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 43997217da54b..7dc2459dace33 100644 --- a/planner/core/planbuilder.go +++ b/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, diff --git a/privilege/privileges/privileges.go b/privilege/privileges/privileges.go index e0c63cfb14bfc..0e8d88a90c5a1 100644 --- a/privilege/privileges/privileges.go +++ b/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 diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index 2af31f3699f7d..2f6cbef8af2cf 100644 --- a/privilege/privileges/privileges_test.go +++ b/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) { diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 73a8ca0066450..574e649656205 100644 --- a/sessionctx/variable/sysvar.go +++ b/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) diff --git a/sessionctx/variable/tidb_vars.go b/sessionctx/variable/tidb_vars.go index e416f9a695fc3..7fdccecb5a97e 100644 --- a/sessionctx/variable/tidb_vars.go +++ b/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 diff --git a/tidb-server/main.go b/tidb-server/main.go index 6429ba960a0cb..05f4ecc59c25a 100644 --- a/tidb-server/main.go +++ b/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() diff --git a/util/sem/sem.go b/util/sem/sem.go index 8c3d2b456d991..d29d29b601559 100644 --- a/util/sem/sem.go +++ b/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 { diff --git a/util/sem/sem_test.go b/util/sem/sem_test.go index c303d2195c7f4..073a195139c37 100644 --- a/util/sem/sem_test.go +++ b/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) +}