From f2cb145af2b893295797b36ec8e8603f784e69e6 Mon Sep 17 00:00:00 2001 From: Morgan Tocker Date: Sun, 16 May 2021 21:25:39 -0600 Subject: [PATCH] planner, privilege: Add security enhanced mode part 4 (#24416) --- executor/executor_test.go | 1 + planner/core/planbuilder.go | 40 ++++++++++++++++++++- privilege/privilege.go | 3 ++ privilege/privileges/privileges.go | 16 +++++++++ privilege/privileges/privileges_test.go | 47 +++++++++++++++++++++++++ 5 files changed, 106 insertions(+), 1 deletion(-) diff --git a/executor/executor_test.go b/executor/executor_test.go index d67bb6b48b8f4..e359837d8fb92 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_USER_ADMIN Server Admin ", )) c.Assert(len(tk.MustQuery("show table status").Rows()), Equals, 1) } diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 6fc98bc522508..43997217da54b 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -24,6 +24,7 @@ import ( "github.com/pingcap/errors" "github.com/pingcap/parser" "github.com/pingcap/parser/ast" + "github.com/pingcap/parser/auth" "github.com/pingcap/parser/charset" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" @@ -36,6 +37,7 @@ import ( "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/planner/property" "github.com/pingcap/tidb/planner/util" + "github.com/pingcap/tidb/privilege" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/stmtctx" "github.com/pingcap/tidb/sessionctx/variable" @@ -2276,9 +2278,16 @@ func (b *PlanBuilder) buildSimple(node ast.StmtNode) (Plan, error) { p.setSchemaAndNames(buildBRIESchema()) err := ErrSpecificAccessDenied.GenWithStackByArgs("SUPER or BACKUP_ADMIN") b.visitInfo = appendDynamicVisitInfo(b.visitInfo, "BACKUP_ADMIN", false, err) - case *ast.GrantRoleStmt, *ast.RevokeRoleStmt: + case *ast.GrantRoleStmt: err := ErrSpecificAccessDenied.GenWithStackByArgs("SUPER or ROLE_ADMIN") b.visitInfo = appendDynamicVisitInfo(b.visitInfo, "ROLE_ADMIN", false, err) + case *ast.RevokeRoleStmt: + err := ErrSpecificAccessDenied.GenWithStackByArgs("SUPER or ROLE_ADMIN") + b.visitInfo = appendDynamicVisitInfo(b.visitInfo, "ROLE_ADMIN", false, err) + // Check if any of the users are RESTRICTED + for _, user := range raw.Users { + b.visitInfo = appendVisitInfoIsRestrictedUser(b.visitInfo, b.ctx, user, "RESTRICTED_USER_ADMIN") + } case *ast.RevokeStmt: b.visitInfo = collectVisitInfoFromRevokeStmt(b.ctx, b.visitInfo, raw) case *ast.KillStmt: @@ -2292,12 +2301,23 @@ func (b *PlanBuilder) buildSimple(node ast.StmtNode) (Plan, error) { err := ErrSpecificAccessDenied.GenWithStackByArgs("SUPER or CONNECTION_ADMIN") b.visitInfo = appendDynamicVisitInfo(b.visitInfo, "CONNECTION_ADMIN", false, err) } + b.visitInfo = appendVisitInfoIsRestrictedUser(b.visitInfo, b.ctx, &auth.UserIdentity{Username: pi.User, Hostname: pi.Host}, "RESTRICTED_CONNECTION_ADMIN") } } case *ast.UseStmt: if raw.DBName == "" { return nil, ErrNoDB } + case *ast.DropUserStmt: + // The main privilege checks for DROP USER are currently performed in executor/simple.go + // because they use complex OR conditions (not supported by visitInfo). + for _, user := range raw.UserList { + b.visitInfo = appendVisitInfoIsRestrictedUser(b.visitInfo, b.ctx, user, "RESTRICTED_USER_ADMIN") + } + case *ast.SetPwdStmt: + if raw.User != nil { + b.visitInfo = appendVisitInfoIsRestrictedUser(b.visitInfo, b.ctx, raw.User, "RESTRICTED_USER_ADMIN") + } case *ast.ShutdownStmt: b.visitInfo = appendVisitInfo(b.visitInfo, mysql.ShutdownPriv, "", "", "", nil) } @@ -2339,6 +2359,10 @@ func collectVisitInfoFromRevokeStmt(sctx sessionctx.Context, vi []visitInfo, stm for _, priv := range allPrivs { vi = appendVisitInfo(vi, priv, dbName, tableName, "", nil) } + for _, u := range stmt.Users { + // For SEM, make sure the users are not restricted + vi = appendVisitInfoIsRestrictedUser(vi, sctx, u.User, "RESTRICTED_USER_ADMIN") + } if nonDynamicPrivilege { // Dynamic privileges use their own GRANT OPTION. If there were any non-dynamic privilege requests, // we need to attach the "GLOBAL" version of the GRANT OPTION. @@ -2347,6 +2371,20 @@ func collectVisitInfoFromRevokeStmt(sctx sessionctx.Context, vi []visitInfo, stm return vi } +// appendVisitInfoIsRestrictedUser appends additional visitInfo if the user has a +// special privilege called "RESTRICTED_USER_ADMIN". It only applies when SEM is enabled. +func appendVisitInfoIsRestrictedUser(visitInfo []visitInfo, sctx sessionctx.Context, user *auth.UserIdentity, priv string) []visitInfo { + if !sem.IsEnabled() { + return visitInfo + } + checker := privilege.GetPrivilegeManager(sctx) + if checker != nil && checker.RequestDynamicVerificationWithUser("RESTRICTED_USER_ADMIN", false, user) { + err := ErrSpecificAccessDenied.GenWithStackByArgs(priv) + visitInfo = appendDynamicVisitInfo(visitInfo, priv, false, err) + } + return visitInfo +} + func collectVisitInfoFromGrantStmt(sctx sessionctx.Context, vi []visitInfo, stmt *ast.GrantStmt) []visitInfo { // To use GRANT, you must have the GRANT OPTION privilege, // and you must have the privileges that you are granting. diff --git a/privilege/privilege.go b/privilege/privilege.go index cf59ce9b0314e..f732d9da1199b 100644 --- a/privilege/privilege.go +++ b/privilege/privilege.go @@ -50,6 +50,9 @@ type Manager interface { // Dynamic privileges are only assignable globally, and have their own grantable attribute. RequestDynamicVerification(activeRoles []*auth.RoleIdentity, privName string, grantable bool) bool + // RequestDynamicVerification verifies a DYNAMIC privilege for a specific user. + RequestDynamicVerificationWithUser(privName string, grantable bool, user *auth.UserIdentity) bool + // ConnectionVerification verifies user privilege for connection. ConnectionVerification(user, host string, auth, salt []byte, tlsState *tls.ConnectionState) (string, string, bool) diff --git a/privilege/privileges/privileges.go b/privilege/privileges/privileges.go index 6ac58e04e44e4..e0c63cfb14bfc 100644 --- a/privilege/privileges/privileges.go +++ b/privilege/privileges/privileges.go @@ -45,6 +45,7 @@ var dynamicPrivs = []string{ "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. } var dynamicPrivLock sync.Mutex @@ -56,6 +57,21 @@ type UserPrivileges struct { *Handle } +// RequestDynamicVerificationWithUser implements the Manager interface. +func (p *UserPrivileges) RequestDynamicVerificationWithUser(privName string, grantable bool, user *auth.UserIdentity) bool { + if SkipWithGrant { + return true + } + + if user == nil { + return false + } + + mysqlPriv := p.Handle.Get() + roles := mysqlPriv.getDefaultRoles(user.Username, user.Hostname) + return mysqlPriv.RequestDynamicVerification(roles, user.Username, user.Hostname, privName, grantable) +} + // RequestDynamicVerification implements the Manager interface. func (p *UserPrivileges) RequestDynamicVerification(activeRoles []*auth.RoleIdentity, privName string, grantable bool) bool { if SkipWithGrant { diff --git a/privilege/privileges/privileges_test.go b/privilege/privileges/privileges_test.go index c7a825a4d894d..2af31f3699f7d 100644 --- a/privilege/privileges/privileges_test.go +++ b/privilege/privileges/privileges_test.go @@ -1428,6 +1428,53 @@ func (s *testPrivilegeSuite) TestViewDefiner(c *C) { tk.MustExec("select * from test_view2") } +func (s *testPrivilegeSuite) TestSecurityEnhancedModeRestrictedUsers(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("CREATE USER ruroot1, ruroot2, ruroot3") + tk.MustExec("CREATE ROLE notimportant") + tk.MustExec("GRANT SUPER, CREATE USER ON *.* to ruroot1 WITH GRANT OPTION") + tk.MustExec("SET tidb_enable_dynamic_privileges=1") + tk.MustExec("GRANT SUPER, RESTRICTED_USER_ADMIN, CREATE USER ON *.* to ruroot2 WITH GRANT OPTION") + tk.MustExec("GRANT RESTRICTED_USER_ADMIN ON *.* to ruroot3") + tk.MustExec("GRANT notimportant TO ruroot2, ruroot3") + + sem.Enable() + defer sem.Disable() + + stmts := []string{ + "SET PASSWORD for ruroot3 = 'newpassword'", + "REVOKE notimportant FROM ruroot3", + "REVOKE SUPER ON *.* FROM ruroot3", + "DROP USER ruroot3", + } + + // ruroot1 has SUPER but in SEM will be restricted + tk.Se.Auth(&auth.UserIdentity{ + Username: "ruroot1", + Hostname: "localhost", + AuthUsername: "uroot", + AuthHostname: "%", + }, nil, nil) + + for _, stmt := range stmts { + err := tk.ExecToErr(stmt) + c.Assert(err.Error(), Equals, "[planner:1227]Access denied; you need (at least one of) the RESTRICTED_USER_ADMIN privilege(s) for this operation") + } + + // Switch to ruroot2, it should be permitted + tk.Se.Auth(&auth.UserIdentity{ + Username: "ruroot2", + Hostname: "localhost", + AuthUsername: "uroot", + AuthHostname: "%", + }, nil, nil) + + for _, stmt := range stmts { + err := tk.ExecToErr(stmt) + c.Assert(err, IsNil) + } +} + func (s *testPrivilegeSuite) TestDynamicPrivsRegistration(c *C) { se := newSession(c, s.store, s.dbName) pm := privilege.GetPrivilegeManager(se)