From 06cef5ff1b26a18d1c66f8340c36f23cb93f27b4 Mon Sep 17 00:00:00 2001 From: Lingyu Song Date: Mon, 19 Aug 2019 19:08:17 +0800 Subject: [PATCH] executor, privileges: fix privilege check fail for `CREATE USER` and `DROP USER` (#11589) --- executor/simple.go | 75 ++++++++++++++++++++++++------- executor/simple_test.go | 36 +++++++++++++++ planner/core/logical_plan_test.go | 12 ----- planner/core/planbuilder.go | 16 ------- 4 files changed, 94 insertions(+), 45 deletions(-) diff --git a/executor/simple.go b/executor/simple.go index a31bf39148434..1408aa8339ea6 100644 --- a/executor/simple.go +++ b/executor/simple.go @@ -609,6 +609,23 @@ func (e *SimpleExec) executeRollback(s *ast.RollbackStmt) error { } func (e *SimpleExec) executeCreateUser(ctx context.Context, s *ast.CreateUserStmt) error { + // Check `CREATE USER` privilege. + if !config.GetGlobalConfig().Security.SkipGrantTable { + checker := privilege.GetPrivilegeManager(e.ctx) + if checker == nil { + return errors.New("miss privilege checker") + } + activeRoles := e.ctx.GetSessionVars().ActiveRoles + if !checker.RequestVerification(activeRoles, mysql.SystemDB, mysql.UserTable, "", mysql.InsertPriv) { + if s.IsCreateRole && !checker.RequestVerification(activeRoles, "", "", "", mysql.CreateRolePriv) { + return core.ErrSpecificAccessDenied.GenWithStackByArgs("CREATE ROLE") + } + if !s.IsCreateRole && !checker.RequestVerification(activeRoles, "", "", "", mysql.CreateUserPriv) { + return core.ErrSpecificAccessDenied.GenWithStackByArgs("CREATE User") + } + } + } + users := make([]string, 0, len(s.Specs)) for _, spec := range s.Specs { exists, err1 := userExists(e.ctx, spec.User.Username, spec.User.Hostname) @@ -639,7 +656,7 @@ func (e *SimpleExec) executeCreateUser(ctx context.Context, s *ast.CreateUserStm if s.IsCreateRole { sql = fmt.Sprintf(`INSERT INTO %s.%s (Host, User, Password, Account_locked) VALUES %s;`, mysql.SystemDB, mysql.UserTable, strings.Join(users, ", ")) } - _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), sql) + _, _, err := e.ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(e.ctx, sql) if err != nil { return err } @@ -759,8 +776,32 @@ func (e *SimpleExec) executeGrantRole(s *ast.GrantRoleStmt) error { } func (e *SimpleExec) executeDropUser(s *ast.DropUserStmt) error { + // Check privileges. + // Check `CREATE USER` privilege. + if !config.GetGlobalConfig().Security.SkipGrantTable { + checker := privilege.GetPrivilegeManager(e.ctx) + if checker == nil { + return errors.New("miss privilege checker") + } + activeRoles := e.ctx.GetSessionVars().ActiveRoles + if !checker.RequestVerification(activeRoles, mysql.SystemDB, mysql.UserTable, "", mysql.DeletePriv) { + if s.IsDropRole && !checker.RequestVerification(activeRoles, "", "", "", mysql.DropRolePriv) { + return core.ErrSpecificAccessDenied.GenWithStackByArgs("DROP ROLE") + } + if !s.IsDropRole && !checker.RequestVerification(activeRoles, "", "", "", mysql.CreateUserPriv) { + return core.ErrSpecificAccessDenied.GenWithStackByArgs("CREATE USER") + } + } + } + failedUsers := make([]string, 0, len(s.UserList)) notExistUsers := make([]string, 0, len(s.UserList)) + sysSession, err := e.getSysSession() + defer e.releaseSysSession(sysSession) + if err != nil { + return err + } + sqlExecutor := sysSession.(sqlexec.SQLExecutor) for _, user := range s.UserList { exists, err := userExists(e.ctx, user.Username, user.Hostname) @@ -773,13 +814,13 @@ func (e *SimpleExec) executeDropUser(s *ast.DropUserStmt) error { } // begin a transaction to delete a user. - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), "begin"); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), "begin"); err != nil { return err } sql := fmt.Sprintf(`DELETE FROM %s.%s WHERE Host = '%s' and User = '%s';`, mysql.SystemDB, mysql.UserTable, user.Hostname, user.Username) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), sql); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil { failedUsers = append(failedUsers, user.String()) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), "rollback"); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil { return err } continue @@ -787,9 +828,9 @@ func (e *SimpleExec) executeDropUser(s *ast.DropUserStmt) error { // delete privileges from mysql.db sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE Host = '%s' and User = '%s';`, mysql.SystemDB, mysql.DBTable, user.Hostname, user.Username) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), sql); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil { failedUsers = append(failedUsers, user.String()) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), "rollback"); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil { return err } continue @@ -797,9 +838,9 @@ func (e *SimpleExec) executeDropUser(s *ast.DropUserStmt) error { // delete privileges from mysql.tables_priv sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE Host = '%s' and User = '%s';`, mysql.SystemDB, mysql.TablePrivTable, user.Hostname, user.Username) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), sql); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil { failedUsers = append(failedUsers, user.String()) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), "rollback"); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil { return err } continue @@ -807,18 +848,18 @@ func (e *SimpleExec) executeDropUser(s *ast.DropUserStmt) error { // delete relationship from mysql.role_edges sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE TO_HOST = '%s' and TO_USER = '%s';`, mysql.SystemDB, mysql.RoleEdgeTable, user.Hostname, user.Username) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), sql); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil { failedUsers = append(failedUsers, user.String()) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), "rollback"); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil { return err } continue } sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE FROM_HOST = '%s' and FROM_USER = '%s';`, mysql.SystemDB, mysql.RoleEdgeTable, user.Hostname, user.Username) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), sql); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil { failedUsers = append(failedUsers, user.String()) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), "rollback"); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil { return err } continue @@ -826,25 +867,25 @@ func (e *SimpleExec) executeDropUser(s *ast.DropUserStmt) error { // delete relationship from mysql.default_roles sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE DEFAULT_ROLE_HOST = '%s' and DEFAULT_ROLE_USER = '%s';`, mysql.SystemDB, mysql.DefaultRoleTable, user.Hostname, user.Username) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), sql); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil { failedUsers = append(failedUsers, user.String()) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), "rollback"); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil { return err } continue } sql = fmt.Sprintf(`DELETE FROM %s.%s WHERE HOST = '%s' and USER = '%s';`, mysql.SystemDB, mysql.DefaultRoleTable, user.Hostname, user.Username) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), sql); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), sql); err != nil { failedUsers = append(failedUsers, user.String()) - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), "rollback"); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), "rollback"); err != nil { return err } continue } //TODO: need delete columns_priv once we implement columns_priv functionality. - if _, err := e.ctx.(sqlexec.SQLExecutor).Execute(context.Background(), "commit"); err != nil { + if _, err := sqlExecutor.Execute(context.Background(), "commit"); err != nil { failedUsers = append(failedUsers, user.String()) } } diff --git a/executor/simple_test.go b/executor/simple_test.go index 4f0cef6838eea..23542f1b6b951 100644 --- a/executor/simple_test.go +++ b/executor/simple_test.go @@ -516,3 +516,39 @@ func (s *testSuite3) TestStmtAutoNewTxn(c *C) { tk.MustExec("rollback") tk.MustQuery("select * from auto_new").Check(testkit.Rows("1", "2")) } + +func (s *testSuite3) TestIssue9111(c *C) { + // CREATE USER / DROP USER fails if admin doesn't have insert privilege on `mysql.user` table. + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create user 'user_admin'@'localhost';") + tk.MustExec("grant create user on *.* to 'user_admin'@'localhost';") + + // Create a new session. + se, err := session.CreateSession4Test(s.store) + c.Check(err, IsNil) + defer se.Close() + c.Assert(se.Auth(&auth.UserIdentity{Username: "user_admin", Hostname: "localhost"}, nil, nil), IsTrue) + + ctx := context.Background() + _, err = se.Execute(ctx, `create user test_create_user`) + c.Check(err, IsNil) + _, err = se.Execute(ctx, `drop user test_create_user`) + c.Check(err, IsNil) + + tk.MustExec("revoke create user on *.* from 'user_admin'@'localhost';") + tk.MustExec("grant insert, delete on mysql.User to 'user_admin'@'localhost';") + + _, err = se.Execute(ctx, `flush privileges`) + c.Check(err, IsNil) + _, err = se.Execute(ctx, `create user test_create_user`) + c.Check(err, IsNil) + _, err = se.Execute(ctx, `drop user test_create_user`) + c.Check(err, IsNil) + + _, err = se.Execute(ctx, `create role test_create_user`) + c.Check(err, IsNil) + _, err = se.Execute(ctx, `drop role test_create_user`) + c.Check(err, IsNil) + + tk.MustExec("drop user 'user_admin'@'localhost';") +} diff --git a/planner/core/logical_plan_test.go b/planner/core/logical_plan_test.go index 2582df5137ab2..264cf7745b8e2 100644 --- a/planner/core/logical_plan_test.go +++ b/planner/core/logical_plan_test.go @@ -1712,18 +1712,6 @@ func (s *testPlanSuite) TestVisitInfo(c *C) { {mysql.IndexPriv, "test", "t", "", nil}, }, }, - { - sql: `create user 'test'@'%' identified by '123456'`, - ans: []visitInfo{ - {mysql.CreateUserPriv, "", "", "", ErrSpecificAccessDenied}, - }, - }, - { - sql: `drop user 'test'@'%'`, - ans: []visitInfo{ - {mysql.CreateUserPriv, "", "", "", ErrSpecificAccessDenied}, - }, - }, { sql: `grant all privileges on test.* to 'test'@'%'`, ans: []visitInfo{ diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 4243b95b58fcd..44f4a38aa2c0a 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -1309,22 +1309,6 @@ func (b *PlanBuilder) buildSimple(node ast.StmtNode) (Plan, error) { p := &Simple{Statement: node} switch raw := node.(type) { - case *ast.CreateUserStmt: - if raw.IsCreateRole { - err := ErrSpecificAccessDenied.GenWithStackByArgs("CREATE ROLE") - b.visitInfo = appendVisitInfo(b.visitInfo, mysql.CreateRolePriv, "", "", "", err) - } else { - err := ErrSpecificAccessDenied.GenWithStackByArgs("CREATE USER") - b.visitInfo = appendVisitInfo(b.visitInfo, mysql.CreateUserPriv, "", "", "", err) - } - case *ast.DropUserStmt: - if raw.IsDropRole { - err := ErrSpecificAccessDenied.GenWithStackByArgs("DROP ROLE") - b.visitInfo = appendVisitInfo(b.visitInfo, mysql.DropRolePriv, "", "", "", err) - } else { - err := ErrSpecificAccessDenied.GenWithStackByArgs("CREATE USER") - b.visitInfo = appendVisitInfo(b.visitInfo, mysql.CreateUserPriv, "", "", "", err) - } case *ast.AlterUserStmt: err := ErrSpecificAccessDenied.GenWithStackByArgs("CREATE USER") b.visitInfo = appendVisitInfo(b.visitInfo, mysql.CreateUserPriv, "", "", "", err)