From 358f07a0c8a3b9fd5561e662f6ac1096398b6a24 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 16 Oct 2023 20:53:11 +0800 Subject: [PATCH 1/5] pkg/util/sem: change @@global.require_secure_transport variable to invisible when SEM is on --- pkg/util/sem/sem.go | 3 ++- pkg/util/sem/sem_test.go | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/util/sem/sem.go b/pkg/util/sem/sem.go index ecc3e56bb1ece..5ff6d2daddb70 100644 --- a/pkg/util/sem/sem.go +++ b/pkg/util/sem/sem.go @@ -158,7 +158,8 @@ func IsInvisibleSysVar(varNameInLower string) bool { variable.TiDBRestrictedReadOnly, variable.TiDBTopSQLMaxTimeSeriesCount, variable.TiDBTopSQLMaxMetaCount, - tidbAuditRetractLog: + tidbAuditRetractLog, + variable.RequireSecureTransport: return true } return false diff --git a/pkg/util/sem/sem_test.go b/pkg/util/sem/sem_test.go index 2645cb79b21de..6b192e51e8517 100644 --- a/pkg/util/sem/sem_test.go +++ b/pkg/util/sem/sem_test.go @@ -104,4 +104,5 @@ func TestIsInvisibleSysVar(t *testing.T) { assert.True(IsInvisibleSysVar(variable.TiDBTopSQLMaxTimeSeriesCount)) assert.True(IsInvisibleSysVar(variable.TiDBTopSQLMaxTimeSeriesCount)) assert.True(IsInvisibleSysVar(tidbAuditRetractLog)) + assert.True(IsInvisibleSysVar(variable.RequireSecureTransport)) } From 81dd172abbf38c685f016a051b102ec91f64aef8 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 16 Oct 2023 22:56:53 +0800 Subject: [PATCH 2/5] address comment --- pkg/config/config.toml.example | 2 +- pkg/sessionctx/variable/sysvar.go | 8 ++++++++ pkg/util/sem/sem.go | 3 +-- pkg/util/sem/sem_test.go | 1 - 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/pkg/config/config.toml.example b/pkg/config/config.toml.example index ec7cd1ae748b6..b81cc3af66249 100644 --- a/pkg/config/config.toml.example +++ b/pkg/config/config.toml.example @@ -196,7 +196,7 @@ cluster-ssl-key = "" spilled-file-encryption-method = "plaintext" # Security Enhanced Mode (SEM) restricts the "SUPER" privilege and requires fine-grained privileges instead. -enable-sem = false +enable-sem = true # Automatic creation of TLS certificates. # Setting it to 'true' is recommended because it is safer and tie with the default configuration of MySQL. diff --git a/pkg/sessionctx/variable/sysvar.go b/pkg/sessionctx/variable/sysvar.go index a9cff7d91734b..ede44c4958197 100644 --- a/pkg/sessionctx/variable/sysvar.go +++ b/pkg/sessionctx/variable/sysvar.go @@ -1116,6 +1116,14 @@ var defaultSysVars = []*SysVar{ return nil }, Validation: func(vars *SessionVars, normalizedValue string, originalValue string, scope ScopeFlag) (string, error) { if vars.StmtCtx.StmtType == "Set" && TiDBOptOn(normalizedValue) { + // On tidbcloud dedicated cluster with the default configuration, if an user modify + // @@global.require_secure_transport=on, he can not login the cluster anymore! + // A workaround for this is making require_secure_transport read-only for that case. + // SEM(secure enhanced mode) is enabled by default with only that settings. + cfg := config.GetGlobalConfig() + if cfg.Security.EnableSEM { + return "", errors.New("require_secure_transport can not be set to ON with SEM(security enhanced mode) enabled") + } // Refuse to set RequireSecureTransport to ON if the connection // issuing the change is not secure. This helps reduce the chance of users being locked out. if vars.TLSConnectionState == nil { diff --git a/pkg/util/sem/sem.go b/pkg/util/sem/sem.go index 5ff6d2daddb70..ecc3e56bb1ece 100644 --- a/pkg/util/sem/sem.go +++ b/pkg/util/sem/sem.go @@ -158,8 +158,7 @@ func IsInvisibleSysVar(varNameInLower string) bool { variable.TiDBRestrictedReadOnly, variable.TiDBTopSQLMaxTimeSeriesCount, variable.TiDBTopSQLMaxMetaCount, - tidbAuditRetractLog, - variable.RequireSecureTransport: + tidbAuditRetractLog: return true } return false diff --git a/pkg/util/sem/sem_test.go b/pkg/util/sem/sem_test.go index 6b192e51e8517..2645cb79b21de 100644 --- a/pkg/util/sem/sem_test.go +++ b/pkg/util/sem/sem_test.go @@ -104,5 +104,4 @@ func TestIsInvisibleSysVar(t *testing.T) { assert.True(IsInvisibleSysVar(variable.TiDBTopSQLMaxTimeSeriesCount)) assert.True(IsInvisibleSysVar(variable.TiDBTopSQLMaxTimeSeriesCount)) assert.True(IsInvisibleSysVar(tidbAuditRetractLog)) - assert.True(IsInvisibleSysVar(variable.RequireSecureTransport)) } From d649bc2c39e85b4407e6a41363b45e7fac20efcf Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 16 Oct 2023 23:05:11 +0800 Subject: [PATCH 3/5] typo --- pkg/config/config.toml.example | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/config/config.toml.example b/pkg/config/config.toml.example index b81cc3af66249..ec7cd1ae748b6 100644 --- a/pkg/config/config.toml.example +++ b/pkg/config/config.toml.example @@ -196,7 +196,7 @@ cluster-ssl-key = "" spilled-file-encryption-method = "plaintext" # Security Enhanced Mode (SEM) restricts the "SUPER" privilege and requires fine-grained privileges instead. -enable-sem = true +enable-sem = false # Automatic creation of TLS certificates. # Setting it to 'true' is recommended because it is safer and tie with the default configuration of MySQL. From 85927291713111c5dacb6740bf4d25021e2f127d Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 16 Oct 2023 23:53:42 +0800 Subject: [PATCH 4/5] add test --- pkg/sessionctx/sessionstates/BUILD.bazel | 2 +- pkg/sessionctx/sessionstates/session_states_test.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/pkg/sessionctx/sessionstates/BUILD.bazel b/pkg/sessionctx/sessionstates/BUILD.bazel index 116e3555871d7..c24a4a626d58c 100644 --- a/pkg/sessionctx/sessionstates/BUILD.bazel +++ b/pkg/sessionctx/sessionstates/BUILD.bazel @@ -31,7 +31,7 @@ go_test( ], embed = [":sessionstates"], flaky = True, - shard_count = 15, + shard_count = 16, deps = [ "//pkg/config", "//pkg/errno", diff --git a/pkg/sessionctx/sessionstates/session_states_test.go b/pkg/sessionctx/sessionstates/session_states_test.go index 97f2597abfc55..f9b831ca29dda 100644 --- a/pkg/sessionctx/sessionstates/session_states_test.go +++ b/pkg/sessionctx/sessionstates/session_states_test.go @@ -16,6 +16,7 @@ package sessionstates_test import ( "context" + "crypto/tls" "encoding/binary" "fmt" "strconv" @@ -336,6 +337,18 @@ func TestInvisibleVars(t *testing.T) { } } +func TestIssue47665(t *testing.T) { + store := testkit.CreateMockStore(t) + tk := testkit.NewTestKit(t, store) + tk.Session().GetSessionVars().TLSConnectionState = &tls.ConnectionState{} // unrelated mock for the test. + originSEM := config.GetGlobalConfig().Security.EnableSEM + config.GetGlobalConfig().Security.EnableSEM = true + tk.MustGetErrMsg("set @@global.require_secure_transport = on", "require_secure_transport can not be set to ON with SEM(security enhanced mode) enabled") + config.GetGlobalConfig().Security.EnableSEM = originSEM + tk.MustExec("set @@global.require_secure_transport = on") + tk.MustExec("set @@global.require_secure_transport = off") // recover to default value +} + func TestSessionCtx(t *testing.T) { store := testkit.CreateMockStore(t) tk := testkit.NewTestKit(t, store) From 6e5fcffc37dee74db8cc027a88d2d80a10a90aee Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Tue, 17 Oct 2023 09:21:32 +0800 Subject: [PATCH 5/5] Update pkg/sessionctx/variable/sysvar.go Co-authored-by: bb7133 --- pkg/sessionctx/variable/sysvar.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sessionctx/variable/sysvar.go b/pkg/sessionctx/variable/sysvar.go index ede44c4958197..097d67263c8bd 100644 --- a/pkg/sessionctx/variable/sysvar.go +++ b/pkg/sessionctx/variable/sysvar.go @@ -1119,7 +1119,7 @@ var defaultSysVars = []*SysVar{ // On tidbcloud dedicated cluster with the default configuration, if an user modify // @@global.require_secure_transport=on, he can not login the cluster anymore! // A workaround for this is making require_secure_transport read-only for that case. - // SEM(secure enhanced mode) is enabled by default with only that settings. + // SEM(security enhanced mode) is enabled by default with only that settings. cfg := config.GetGlobalConfig() if cfg.Security.EnableSEM { return "", errors.New("require_secure_transport can not be set to ON with SEM(security enhanced mode) enabled")