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

infoschema: improve the security vunerability of TABLE_CONSTRAINTS infoschema #14037

Merged
merged 9 commits into from
Dec 13, 2019
Merged

infoschema: improve the security vunerability of TABLE_CONSTRAINTS infoschema #14037

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
71c58db
add check for infoschema.table_constrains
reafans Dec 12, 2019
f300baf
Merge branch 'master' into infoschema_constraints
reafans Dec 12, 2019
3afba83
Merge branch 'master' into infoschema_constraints
reafans Dec 12, 2019
6154ac1
add some test
reafans Dec 12, 2019
1a489ab
Merge remote-tracking branch 'origin/infoschema_constraints' into inf…
reafans Dec 12, 2019
9b54fc6
improve the test
reafans Dec 12, 2019
d2a4546
improve the test
reafans Dec 12, 2019
2435b21
Merge branch 'master' into infoschema_constraints
reafans Dec 13, 2019
a60758b
Merge branch 'master' into infoschema_constraints
reafans Dec 13, 2019
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
9 changes: 7 additions & 2 deletions infoschema/tables.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1703,10 +1703,15 @@ const (
)

// dataForTableConstraints constructs data for table information_schema.constraints.See https://dev.mysql.com/doc/refman/5.7/en/table-constraints-table.html
func dataForTableConstraints(schemas []*model.DBInfo) [][]types.Datum {
func dataForTableConstraints(ctx sessionctx.Context, schemas []*model.DBInfo) [][]types.Datum {
checker := privilege.GetPrivilegeManager(ctx)
var rows [][]types.Datum
for _, schema := range schemas {
for _, tbl := range schema.Tables {
if checker != nil && !checker.RequestVerification(ctx.GetSessionVars().ActiveRoles, schema.Name.L, tbl.Name.L, "", mysql.AllPrivMask) {
Copy link
Member

Choose a reason for hiding this comment

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

Use .O instead of .L?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it doesn't matter, both can lead to the same thing, the schema.Name.L, tbl.Name.L will be converted to capital letters in this func.

continue
}

if tbl.PKIsHandle {
record := types.MakeDatums(
catalogVal, // CONSTRAINT_CATALOG
Expand Down Expand Up @@ -2343,7 +2348,7 @@ func (it *infoschemaTable) getRows(ctx sessionctx.Context, cols []*table.Column)
case tableSessionVar:
fullRows, err = dataForSessionVar(ctx)
case tableConstraints:
fullRows = dataForTableConstraints(dbs)
fullRows = dataForTableConstraints(ctx, dbs)
case tableFiles:
case tableProfiling:
if v, ok := ctx.GetSessionVars().GetSystemVar("profiling"); ok && variable.TiDBOptOn(v) {
Expand Down
19 changes: 19 additions & 0 deletions infoschema/tables_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,25 @@ func (s *testTableSuite) TestSomeTables(c *C) {
tk.MustQuery("select * from information_schema.SESSION_VARIABLES where VARIABLE_NAME='tidb_retry_limit';").Check(testkit.Rows("tidb_retry_limit 10"))
tk.MustQuery("select * from information_schema.ENGINES;").Check(testkit.Rows("InnoDB DEFAULT Supports transactions, row-level locking, and foreign keys YES YES YES"))
tk.MustQuery("select * from information_schema.TABLE_CONSTRAINTS where TABLE_NAME='gc_delete_range';").Check(testkit.Rows("def mysql delete_range_index mysql gc_delete_range UNIQUE"))

//test the privilege of new user for information_schema.table_constraints
tk.MustExec("create user constraints_tester")
constraintsTester := testkit.NewTestKit(c, s.store)
constraintsTester.MustExec("use information_schema")
c.Assert(constraintsTester.Se.Auth(&auth.UserIdentity{
Username: "constraints_tester",
Hostname: "127.0.0.1",
}, nil, nil), IsTrue)
constraintsTester.MustQuery("select * from information_schema.TABLE_CONSTRAINTS;").Check([][]interface{}{})

//test the privilege of user with privilege of mysql.gc_delete_range for information_schema.table_constraints
tk.MustExec("CREATE ROLE r_gc_delete_range ;")
tk.MustExec("GRANT ALL PRIVILEGES ON mysql.gc_delete_range TO r_gc_delete_range;")
tk.MustExec("GRANT r_gc_delete_range TO constraints_tester;")
constraintsTester.MustExec("set role r_gc_delete_range")
c.Assert(len(constraintsTester.MustQuery("select * from information_schema.TABLE_CONSTRAINTS where TABLE_NAME='gc_delete_range';").Rows()), Greater, 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the exact count of rows? Is it unpredictable?

Copy link
Member

Choose a reason for hiding this comment

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

I think the count of rows doesn't matter here. We want to check that we can read something here but don't care what it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it can be predicted, but may be changed in the future, and what we want is whether the data can be query, not what it is.

constraintsTester.MustQuery("select * from information_schema.TABLE_CONSTRAINTS where TABLE_NAME='tables_priv';").Check([][]interface{}{})

tk.MustQuery("select * from information_schema.KEY_COLUMN_USAGE where TABLE_NAME='stats_meta' and COLUMN_NAME='table_id';").Check(
testkit.Rows("def mysql tbl def mysql stats_meta table_id 1 <nil> <nil> <nil> <nil>"))
tk.MustQuery("select * from information_schema.STATISTICS where TABLE_NAME='columns_priv' and COLUMN_NAME='Host';").Check(
Expand Down