Skip to content

Commit ec37f20

Browse files
AmoebaProtozoaAmoebaProtozoa
authored
[ cherry-pick 7.1 ] Improve SEM read only & Mask show plugins (pingcap#598)
* improve sem read only Signed-off-by: AmoebaProtozoa <[email protected]> * mask show plugins (pingcap#585) Signed-off-by: AmoebaProtozoa <[email protected]> * fix TestSetGlobalVars Signed-off-by: AmoebaProtozoa <[email protected]> * make check Signed-off-by: AmoebaProtozoa <[email protected]> --------- Signed-off-by: AmoebaProtozoa <[email protected]>
1 parent 426210c commit ec37f20

File tree

10 files changed

+101
-14
lines changed

10 files changed

+101
-14
lines changed

‎executor/set.go

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,14 @@ func (e *SetExecutor) setSysVariable(ctx context.Context, name string, v *expres
142142
sessionVars.StmtCtx.AppendWarning(exeerrors.ErrInstanceScope.GenWithStackByArgs(sysVar.Name))
143143
}
144144

145+
// Check if the variable is read only under SEM.
146+
if err := e.checkSysVarReadOnlyForSEM(ctx, v, sysVar); err != nil {
147+
// If the assignment violates sem ReadOnly, we simply append warning and ignore it, instead of
148+
// returning error.
149+
sessionVars.StmtCtx.AppendWarning(err)
150+
return nil
151+
}
152+
145153
if v.IsGlobal {
146154
valStr, err := e.getVarValue(ctx, v, sysVar)
147155
if err != nil {
@@ -271,6 +279,50 @@ func (e *SetExecutor) setCharset(cs, co string, isSetName bool) error {
271279
return errors.Trace(sessionVars.SetSystemVar(variable.CollationConnection, coDb))
272280
}
273281

282+
// checkReadOnly checks if the variable assignment violates sem read-only rules.
283+
func (e *SetExecutor) checkSysVarReadOnlyForSEM(
284+
ctx context.Context,
285+
v *expression.VarAssignment,
286+
sysVar *variable.SysVar) error {
287+
if !sem.IsEnabled() || !sem.IsReadOnlySysVar(strings.ToLower(sysVar.Name)) {
288+
return nil
289+
}
290+
// If the user has SUPER or RESTRICTED_VARIABLES_ADMIN, skip check.
291+
pm := privilege.GetPrivilegeManager(e.ctx)
292+
if pm == nil || pm.RequestDynamicVerification(e.ctx.GetSessionVars().ActiveRoles, "RESTRICTED_VARIABLES_ADMIN", false) {
293+
return nil
294+
}
295+
var (
296+
targetValue string // The value to be set.
297+
currentValue string // The current value of the variable.
298+
err error
299+
)
300+
// Obtain the target and current value of the set statement, must use getVarValue to handle the default value.
301+
if v.IsGlobal {
302+
targetValue, err = e.getVarValue(ctx, v, sysVar)
303+
if err != nil {
304+
return err
305+
}
306+
currentValue = sysVar.Value
307+
} else {
308+
targetValue, err = e.getVarValue(ctx, v, nil)
309+
if err != nil {
310+
return err
311+
}
312+
currentValue, err = e.ctx.GetSessionVars().GetGlobalSystemVar(ctx, v.Name)
313+
if err != nil {
314+
return err
315+
}
316+
}
317+
// Normalize the target value without appending warnings.
318+
targetValue = sysVar.ValidateWithRelaxedValidation(e.ctx.GetSessionVars(), targetValue, sysVar.Scope)
319+
// Skip check sem read-only if the target value is the same as the current value.
320+
if targetValue == currentValue {
321+
return nil
322+
}
323+
return core.ErrSpecificAccessDenied.GenWithStackByArgs("SUPER or RESTRICTED_VARIABLES_ADMIN")
324+
}
325+
274326
func (e *SetExecutor) getVarValue(ctx context.Context, v *expression.VarAssignment, sysVar *variable.SysVar) (value string, err error) {
275327
if v.IsDefault {
276328
// To set a SESSION variable to the GLOBAL value or a GLOBAL value

‎executor/show.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1796,6 +1796,10 @@ func (e *ShowExec) fetchShowProcedureStatus() error {
17961796
}
17971797

17981798
func (e *ShowExec) fetchShowPlugins() error {
1799+
// When SEM strict mode is enabled, return an empty list for plugins.
1800+
if sem.IsStrictMode() {
1801+
return nil
1802+
}
17991803
tiPlugins := plugin.GetAll()
18001804
for _, ps := range tiPlugins {
18011805
for _, p := range ps {

‎planner/core/planbuilder.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,7 @@ func (b *PlanBuilder) buildSet(ctx context.Context, v *ast.SetStmt) (Plan, error
967967
err := ErrSpecificAccessDenied.GenWithStackByArgs("SUPER or SYSTEM_VARIABLES_ADMIN")
968968
b.visitInfo = appendDynamicVisitInfo(b.visitInfo, "SYSTEM_VARIABLES_ADMIN", false, err)
969969
}
970-
if sem.IsEnabled() && (sem.IsInvisibleSysVar(strings.ToLower(vars.Name)) || sem.IsReadOnlySysVar(strings.ToLower(vars.Name))) {
970+
if sem.IsEnabled() && sem.IsInvisibleSysVar(strings.ToLower(vars.Name)) {
971971
err := errmsg.WithInvisibleSysVarErrTag(ErrSpecificAccessDenied.GenWithStackByArgs("RESTRICTED_VARIABLES_ADMIN"))
972972
b.visitInfo = appendDynamicVisitInfo(b.visitInfo, "RESTRICTED_VARIABLES_ADMIN", false, err)
973973
}

‎sessionctx/variable/sysvar.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ var defaultSysVars = []*SysVar{
6868
{Scope: ScopeNone, Name: LogBin, Value: Off, Type: TypeBool},
6969
{Scope: ScopeNone, Name: VersionComment, Value: "TiDB Server (Apache License 2.0) " + versioninfo.TiDBEdition + " Edition, MySQL 5.7 compatible"},
7070
{Scope: ScopeNone, Name: Version, Value: mysql.ServerVersion},
71-
{Scope: ScopeNone, Name: DataDir, Value: "/usr/local/mysql/data/"},
71+
{Scope: ScopeNone, Name: DataDir, Value: DefDataDir},
7272
{Scope: ScopeNone, Name: Socket, Value: ""},
7373
{Scope: ScopeNone, Name: "license", Value: "Apache License 2.0"},
7474
{Scope: ScopeNone, Name: "have_ssl", Value: "DISABLED", Type: TypeBool},

‎sessionctx/variable/tidb_vars.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1061,6 +1061,7 @@ const (
10611061
// Default TiDB system variable values.
10621062
const (
10631063
DefHostname = "localhost"
1064+
DefDataDir = "/usr/local/mysql/data/"
10641065
DefIndexLookupConcurrency = ConcurrencyUnset
10651066
DefIndexLookupJoinConcurrency = ConcurrencyUnset
10661067
DefIndexSerialScanConcurrency = 1

‎tidb-server/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ go_test(
110110
"//parser/mysql",
111111
"//sessionctx/variable",
112112
"//testkit/testsetup",
113+
"@com_github_pingcap_failpoint//:failpoint",
113114
"@com_github_stretchr_testify//require",
114115
"@io_opencensus_go//stats/view",
115116
"@org_uber_go_goleak//:goleak",

‎tidb-server/main_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"os"
1919
"testing"
2020

21+
"github.com/pingcap/failpoint"
2122
"github.com/pingcap/tidb/config"
2223
"github.com/pingcap/tidb/parser/mysql"
2324
"github.com/pingcap/tidb/sessionctx/variable"
@@ -50,6 +51,12 @@ func TestRunMain(t *testing.T) {
5051

5152
func TestSetGlobalVars(t *testing.T) {
5253
defer view.Stop()
54+
55+
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/util/sem/skipSEM", "return"))
56+
defer func() {
57+
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/util/sem/skipSEM"))
58+
}()
59+
5360
require.Equal(t, "tikv,tiflash,tidb", variable.GetSysVar(variable.TiDBIsolationReadEngines).Value)
5461
require.Equal(t, "1073741824", variable.GetSysVar(variable.TiDBMemQuotaQuery).Value)
5562
require.NotEqual(t, "test", variable.GetSysVar(variable.Version).Value)

‎util/sem/restricted_statement.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,14 +256,15 @@ func verifyShow(stmt *ast.ShowStmt) error {
256256
ast.ShowSessionStates,
257257
// "SHOW CONFIG" command is necessary for lightning to function,
258258
// therefore, it's access is restricted via privileges.
259-
ast.ShowConfig:
259+
ast.ShowConfig,
260+
// "SHOW PLUGINS" command is necessary for mysql workbench,
261+
// hence we return an empty result in fetchShowPlugins instead of returning an error here.
262+
ast.ShowPlugins:
260263
return nil
261264
case ast.ShowCreateResourceGroup:
262265
return dbterror.ErrNotSupportedOnServerless.GenWithStackByCause("SHOW CREATE RESOURCE GROUP")
263266
case ast.ShowCreatePlacementPolicy:
264267
return dbterror.ErrNotSupportedOnServerless.GenWithStackByCause("SHOW CREATE PLACEMENT POLICY")
265-
case ast.ShowPlugins:
266-
return dbterror.ErrNotSupportedOnServerless.GenWithStackByCause("SHOW PLUGINS")
267268
case ast.ShowDrainerStatus:
268269
return dbterror.ErrNotSupportedOnServerless.GenWithStackByCause("SHOW DRAINER STATUS")
269270
case ast.ShowPumpStatus:

‎util/sem/sem.go

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,14 +137,16 @@ func Enable(level string) error {
137137
// Disable disables SEM. This is intended to be used by the test-suite.
138138
// Dynamic configuration by users may be a security risk.
139139
func Disable() {
140-
if IsStrictMode() {
140+
switch atomic.LoadInt32(&semEnabled) {
141+
case levelBasicVal:
142+
variable.SetSysVar(variable.TiDBEnableEnhancedSecurity, variable.Off)
143+
if hostname, err := os.Hostname(); err == nil {
144+
variable.SetSysVar(variable.Hostname, hostname)
145+
}
146+
case levelStrictVal:
141147
disableStrictMode()
142148
}
143149
atomic.StoreInt32(&semEnabled, 0)
144-
variable.SetSysVar(variable.TiDBEnableEnhancedSecurity, variable.Off)
145-
if hostname, err := os.Hostname(); err == nil {
146-
variable.SetSysVar(variable.Hostname, hostname)
147-
}
148150
}
149151

150152
// IsEnabled checks if Security Enhanced Mode (SEM) is enabled.
@@ -235,8 +237,7 @@ func IsInvisibleSysVar(varNameInLower string) bool {
235237
variable.TiDBStmtSummaryFilename,
236238
tidbAuditRetractLog,
237239
variable.TiDBEnableAsyncCommit,
238-
variable.TiDBEnableResourceControl,
239-
variable.DataDir:
240+
variable.TiDBEnableResourceControl:
240241
return true
241242
}
242243
return IsStrictMode() && strictModeInvisibleSysVar(varNameInLower)

‎util/sem/strict_sem.go

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,23 @@
1515
package sem
1616

1717
import (
18+
"os"
1819
"sync/atomic"
1920

21+
"github.com/pingcap/tidb/config"
2022
"github.com/pingcap/tidb/parser/mysql"
2123
"github.com/pingcap/tidb/sessionctx/variable"
2224
)
2325

2426
// enableStrictMode changes some variable's default value and restrictions.
2527
func enableStrictMode() {
28+
// Mark sem as enabled.
29+
variable.SetSysVar(variable.TiDBEnableEnhancedSecurity, variable.On)
30+
// Mask @@hostname to localhost.
31+
variable.SetSysVar(variable.Hostname, variable.DefHostname)
32+
// Mask @@datadir to it's default value.
33+
variable.SetSysVar(variable.DataDir, variable.DefDataDir)
34+
// Set password validation rules.
2635
variable.SetSysVarMin(variable.ValidatePasswordLength, 8)
2736
variable.SetSysVarMin(variable.ValidatePasswordMixedCaseCount, 1)
2837
variable.SetSysVarMin(variable.ValidatePasswordNumberCount, 1)
@@ -31,6 +40,15 @@ func enableStrictMode() {
3140

3241
// disableStrictMode changes variable's default value and restrictions back to normal.
3342
func disableStrictMode() {
43+
// Mark sem as disabled.
44+
variable.SetSysVar(variable.TiDBEnableEnhancedSecurity, variable.Off)
45+
// Restore @@hostname.
46+
if hostname, err := os.Hostname(); err == nil {
47+
variable.SetSysVar(variable.Hostname, hostname)
48+
}
49+
// Restore @@datadir.
50+
variable.SetSysVar(variable.DataDir, config.GetGlobalConfig().Path)
51+
// Set password validation rules.
3452
variable.SetSysVarMin(variable.ValidatePasswordLength, 0)
3553
variable.SetSysVarMin(variable.ValidatePasswordMixedCaseCount, 0)
3654
variable.SetSysVarMin(variable.ValidatePasswordNumberCount, 0)
@@ -138,7 +156,6 @@ func strictModeInvisibleTable(dbLowerName, tblLowerName string) bool {
138156
func strictModeInvisibleSysVar(varNameInLower string) bool {
139157
switch varNameInLower {
140158
case
141-
variable.DataDir,
142159
variable.PluginDir,
143160
variable.PluginLoad,
144161
variable.TiDBCheckMb4ValueInUTF8,
@@ -236,7 +253,10 @@ func strictModeReadOnlySysVar(varNameInLower string) bool {
236253
variable.TiDBTxnScope,
237254
variable.ValidatePasswordEnable,
238255
// TODO: remove this after the next cse upgrade to 7.1
239-
variable.TiDBPessimisticTransactionFairLocking:
256+
variable.TiDBPessimisticTransactionFairLocking,
257+
// The following variables contain sensitive information, so we mask them when enabling sem and
258+
// mark them as read-only.
259+
variable.DataDir:
240260
return true
241261

242262
}

0 commit comments

Comments
 (0)