Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

privilege: limit the privileges in memory schemas #35260

Merged
merged 9 commits into from
Jun 13, 2022
Merged

privilege: limit the privileges in memory schemas #35260

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e6acef2
privilege: disallow creating tables in metrics_schema
tangenta Jun 9, 2022
97ac8f0
Merge branch 'master' into fix-metrics-schema-priv
hawkingrei Jun 9, 2022
34a7cb2
skip checking SelectPriv for information_schema
tangenta Jun 9, 2022
ce6f0aa
Merge branch 'master' into fix-metrics-schema-priv
hawkingrei Jun 9, 2022
8541168
refactor performance_schema related privilege
tangenta Jun 13, 2022
3a600fa
Merge branch 'master' into fix-metrics-schema-priv
ti-chi-bot Jun 13, 2022
a616111
fix typo
tangenta Jun 13, 2022
c88ce69
Merge branch 'master' into fix-metrics-schema-priv
ti-chi-bot Jun 13, 2022
dae3ffd
fix integration test
tangenta Jun 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 0 additions & 29 deletions executor/grant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,13 +511,6 @@ func (e *GrantExec) grantDBLevel(priv *ast.PrivElem, user *ast.UserSpec, interna
dbName = e.ctx.GetSessionVars().CurrentDB
}

// Some privilege can not be granted to performance_schema.* in MySQL.
// As TiDB ignores the privilege management part for this system database,
// check is performed here
if strings.EqualFold(dbName, "performance_schema") && e.checkPerformanceSchemaPriv(priv.Priv) {
return e.dbAccessDenied(dbName)
}

sql := new(strings.Builder)
sqlexec.MustFormatSQL(sql, "UPDATE %n.%n SET ", mysql.SystemDB, mysql.DBTable)
err := composeDBPrivUpdate(sql, priv.Priv, "Y")
Expand Down Expand Up @@ -582,28 +575,6 @@ func (e *GrantExec) grantColumnLevel(priv *ast.PrivElem, user *ast.UserSpec, int
return nil
}

func (e *GrantExec) dbAccessDenied(dbName string) error {
user := e.ctx.GetSessionVars().User
u := user.Username
h := user.Hostname
if len(user.AuthUsername) > 0 && len(user.AuthHostname) > 0 {
u = user.AuthUsername
h = user.AuthHostname
}
return ErrDBaccessDenied.GenWithStackByArgs(u, h, dbName)
}

// If the privilege can not be granted, return true
func (e *GrantExec) checkPerformanceSchemaPriv(privType mysql.PrivilegeType) bool {
// Attempts to use GRANT ALL as shorthand for granting privileges
// at the database leval fail with an error
// See https://dev.mysql.com/doc/refman/8.0/en/performance-schema-table-characteristics.html for more detail
// Others are rejected in MySQL 8.0
return privType == mysql.AllPriv || privType == mysql.CreatePriv ||
privType == mysql.ReferencesPriv || privType == mysql.AlterPriv || privType == mysql.ExecutePriv ||
privType == mysql.IndexPriv || privType == mysql.CreateViewPriv || privType == mysql.ShowViewPriv
}

// composeGlobalPrivUpdate composes update stmt assignment list string for global scope privilege update.
func composeGlobalPrivUpdate(sql *strings.Builder, priv mysql.PrivilegeType, value string) error {
if priv != mysql.AllPriv {
Expand Down
47 changes: 15 additions & 32 deletions executor/grant_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"strings"
"testing"

"github.com/pingcap/tidb/errno"
"github.com/pingcap/tidb/executor"
"github.com/pingcap/tidb/infoschema"
"github.com/pingcap/tidb/parser/auth"
Expand Down Expand Up @@ -522,41 +523,23 @@ func TestPerformanceSchemaPrivGrant(t *testing.T) {
tk.MustExec("drop user issue27867;")
}()
require.True(t, tk.Session().Auth(&auth.UserIdentity{Username: "root", Hostname: "localhost"}, nil, nil))
err := tk.ExecToErr("grant all on performance_schema.* to issue27867;")
require.Error(t, err)
require.EqualError(t, err, "[executor:1044]Access denied for user 'root'@'%' to database 'performance_schema'")
tk.MustGetErrCode("grant all on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
// Check case insensitivity
err = tk.ExecToErr("grant all on PERFormanCE_scHemA.* to issue27867;")
require.Error(t, err)
require.EqualError(t, err, "[executor:1044]Access denied for user 'root'@'%' to database 'PERFormanCE_scHemA'")
tk.MustGetErrCode("grant all on PERFormanCE_scHemA.* to issue27867;", errno.ErrDBaccessDenied)
// Check other database privileges
tk.MustExec("grant select on performance_schema.* to issue27867;")
tk.MustExec("grant insert on performance_schema.* to issue27867;")
tk.MustExec("grant update on performance_schema.* to issue27867;")
tk.MustExec("grant delete on performance_schema.* to issue27867;")
tk.MustExec("grant drop on performance_schema.* to issue27867;")
tk.MustExec("grant lock tables on performance_schema.* to issue27867;")
err = tk.ExecToErr("grant create on performance_schema.* to issue27867;")
require.Error(t, err)
require.EqualError(t, err, "[executor:1044]Access denied for user 'root'@'%' to database 'performance_schema'")
err = tk.ExecToErr("grant references on performance_schema.* to issue27867;")
require.Error(t, err)
require.EqualError(t, err, "[executor:1044]Access denied for user 'root'@'%' to database 'performance_schema'")
err = tk.ExecToErr("grant alter on PERFormAnCE_scHemA.* to issue27867;")
require.Error(t, err)
require.EqualError(t, err, "[executor:1044]Access denied for user 'root'@'%' to database 'PERFormAnCE_scHemA'")
err = tk.ExecToErr("grant execute on performance_schema.* to issue27867;")
require.Error(t, err)
require.EqualError(t, err, "[executor:1044]Access denied for user 'root'@'%' to database 'performance_schema'")
err = tk.ExecToErr("grant index on PERFormanCE_scHemA.* to issue27867;")
require.Error(t, err)
require.EqualError(t, err, "[executor:1044]Access denied for user 'root'@'%' to database 'PERFormanCE_scHemA'")
err = tk.ExecToErr("grant create view on performance_schema.* to issue27867;")
require.Error(t, err)
require.EqualError(t, err, "[executor:1044]Access denied for user 'root'@'%' to database 'performance_schema'")
err = tk.ExecToErr("grant show view on performance_schema.* to issue27867;")
require.Error(t, err)
require.EqualError(t, err, "[executor:1044]Access denied for user 'root'@'%' to database 'performance_schema'")
tk.MustGetErrCode("grant insert on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant update on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant delete on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant drop on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant lock tables on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant create on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant references on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant alter on PERFormAnCE_scHemA.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant execute on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant index on PERFormanCE_scHemA.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant create view on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
tk.MustGetErrCode("grant show view on performance_schema.* to issue27867;", errno.ErrDBaccessDenied)
}

func TestGrantDynamicPrivs(t *testing.T) {
Expand Down
21 changes: 18 additions & 3 deletions planner/core/planbuilder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3233,6 +3233,7 @@ func collectVisitInfoFromGrantStmt(sctx sessionctx.Context, vi []visitInfo, stmt
}
var nonDynamicPrivilege bool
var allPrivs []mysql.PrivilegeType
authErr := genAuthErrForGrantStmt(sctx, dbName)
for _, item := range stmt.Privs {
if item.Priv == mysql.ExtendedPriv {
// The observed MySQL behavior is that the error is:
Expand Down Expand Up @@ -3262,20 +3263,34 @@ func collectVisitInfoFromGrantStmt(sctx sessionctx.Context, vi []visitInfo, stmt
}
break
}
vi = appendVisitInfo(vi, item.Priv, dbName, tableName, "", nil)
vi = appendVisitInfo(vi, item.Priv, dbName, tableName, "", authErr)
}

for _, priv := range allPrivs {
vi = appendVisitInfo(vi, priv, dbName, tableName, "", nil)
vi = appendVisitInfo(vi, priv, dbName, tableName, "", authErr)
}
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.
vi = appendVisitInfo(vi, mysql.GrantPriv, dbName, tableName, "", nil)
vi = appendVisitInfo(vi, mysql.GrantPriv, dbName, tableName, "", authErr)
}
return vi, nil
}

func genAuthErrForGrantStmt(sctx sessionctx.Context, dbName string) error {
if !strings.EqualFold(dbName, variable.PerformanceSchema) {
return nil
}
user := sctx.GetSessionVars().User
u := user.Username
h := user.Hostname
if len(user.AuthUsername) > 0 && len(user.AuthHostname) > 0 {
u = user.AuthUsername
h = user.AuthHostname
}
return ErrDBaccessDenied.FastGenByArgs(u, h, dbName)
}

func (b *PlanBuilder) getDefaultValue(col *table.Column) (*expression.Constant, error) {
var (
value types.Datum
Expand Down
26 changes: 7 additions & 19 deletions privilege/privileges/privileges.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"sync"

"github.com/pingcap/tidb/infoschema"
"github.com/pingcap/tidb/infoschema/perfschema"
"github.com/pingcap/tidb/parser/auth"
"github.com/pingcap/tidb/parser/mysql"
"github.com/pingcap/tidb/privilege"
Expand Down Expand Up @@ -136,29 +135,18 @@ func (p *UserPrivileges) RequestVerification(activeRoles []*auth.RoleIdentity, d
}
}

switch dbLowerName {
case util.InformationSchemaName.L:
if util.IsMemDB(dbLowerName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR also disallows creating views on performance_schema and metrics_schema.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MySQL cannot create views in performance_schema.

mysql> create view performance_schema.v as select * from test.t;
ERROR 1044 (42000): Access denied for user 'root'@'%' to database 'performance_schema'

switch priv {
case mysql.CreatePriv, mysql.AlterPriv, mysql.DropPriv, mysql.IndexPriv, mysql.CreateViewPriv,
mysql.InsertPriv, mysql.UpdatePriv, mysql.DeletePriv:
mysql.InsertPriv, mysql.UpdatePriv, mysql.DeletePriv, mysql.ReferencesPriv, mysql.ExecutePriv,
mysql.ShowViewPriv, mysql.LockTablesPriv:
return false
}
return true
// We should be very careful of limiting privileges, so ignore `mysql` for now.
case util.PerformanceSchemaName.L:
if perfschema.IsPredefinedTable(table) {
switch priv {
case mysql.CreatePriv, mysql.AlterPriv, mysql.DropPriv, mysql.IndexPriv, mysql.InsertPriv, mysql.UpdatePriv, mysql.DeletePriv:
return false
}
}
case util.MetricSchemaName.L:
if infoschema.IsMetricTable(table) {
switch priv {
case mysql.CreatePriv, mysql.AlterPriv, mysql.DropPriv, mysql.IndexPriv, mysql.InsertPriv, mysql.UpdatePriv, mysql.DeletePriv:
return false
if dbLowerName == util.InformationSchemaName.L {
return true
} else if dbLowerName == util.MetricSchemaName.L {
// PROCESS is the same with SELECT for metrics_schema.
case mysql.SelectPriv:
if priv == mysql.SelectPriv && infoschema.IsMetricTable(table) {
priv |= mysql.ProcessPriv
}
}
Expand Down
3 changes: 3 additions & 0 deletions privilege/privileges/privileges_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,9 @@ func TestSystemSchema(t *testing.T) {
err = tk.ExecToErr("create table metric_schema.t(a int)")
require.Error(t, err)
require.True(t, terror.ErrorEqual(err, core.ErrTableaccessDenied))

tk.MustGetErrCode("create table metrics_schema.t (id int);", errno.ErrTableaccessDenied)
tk.MustGetErrCode("create table performance_schema.t (id int);", errno.ErrTableaccessDenied)
}

func TestPerformanceSchema(t *testing.T) {
Expand Down