Skip to content

Commit

Permalink
sql: disallow GRANT/REVOKE on system tables
Browse files Browse the repository at this point in the history
Disallow GRANT/REVOKE operations on system objects to avoid potential
deadlocks related to version bumps.

Release justification: bug fix
Release note (sql change): Disallow `GRANT/REVOKE` operations on system
tables.
Release note (bug fix): Fix a bug where `GRANT/REVOKE` on the
`system.lease` table would result in a deadlock.
  • Loading branch information
RichardJCai committed Mar 3, 2021
1 parent 432d6dd commit dd84b74
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 37 deletions.
12 changes: 12 additions & 0 deletions pkg/sql/grant_revoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ import (
"context"
"fmt"

"github.com/cockroachdb/cockroach/pkg/keys"
"github.com/cockroachdb/cockroach/pkg/security"
"github.com/cockroachdb/cockroach/pkg/sql/catalog"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/dbdesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/schemadesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/typedesc"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
"github.com/cockroachdb/cockroach/pkg/sql/privilege"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry"
Expand Down Expand Up @@ -185,6 +188,15 @@ func (n *changePrivilegesNode) startExec(params runParams) error {
// we update them in KV below.
b := p.txn.NewBatch()
for _, descriptor := range descriptors {
// Disallow privilege changes on system objects. For more context, see #43842.
op := "REVOKE"
if n.isGrant {
op = "GRANT"
}
if descriptor.GetID() < keys.MinUserDescID {
return pgerror.Newf(pgcode.InsufficientPrivilege, "cannot %s on system object", op)
}

if err := p.CheckPrivilege(ctx, descriptor, privilege.GRANT); err != nil {
return err
}
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/grant_table
Original file line number Diff line number Diff line change
Expand Up @@ -1922,3 +1922,11 @@ b public t2 root ALL

statement error pq: invalid privilege type USAGE for table
GRANT USAGE ON t TO testuser

# Grant / Revoke should not work on system tables.

statement error pq: cannot GRANT on system object
GRANT SELECT ON system.lease TO testuser

statement error pq: cannot REVOKE on system object
REVOKE SELECT ON system.lease FROM testuser
68 changes: 34 additions & 34 deletions pkg/sql/logictest/testdata/logic_test/system
Original file line number Diff line number Diff line change
Expand Up @@ -468,110 +468,110 @@ DROP DATABASE system
statement error user root does not have DROP privilege on relation users
DROP TABLE system.users

statement error user root does not have ALL privilege on database system
statement error pq: cannot GRANT on system object
GRANT ALL ON DATABASE system TO testuser

statement error user root does not have INSERT privilege on database system
statement error pq: cannot GRANT on system object
GRANT GRANT, SELECT, INSERT ON DATABASE system TO testuser

statement ok
statement error pq: cannot GRANT on system object
GRANT GRANT, SELECT ON DATABASE system TO testuser

statement error user root does not have ALL privilege on relation namespace
statement error pq: cannot GRANT on system object
GRANT ALL ON system.namespace TO testuser

statement error user root does not have INSERT privilege on relation namespace
statement error pq: cannot GRANT on system object
GRANT GRANT, SELECT, INSERT ON system.namespace TO testuser

statement ok
statement error pq: cannot GRANT on system object
GRANT GRANT, SELECT ON system.namespace TO testuser

statement ok
statement error pq: cannot GRANT on system object
GRANT SELECT ON system.descriptor TO testuser

# Superusers must have exactly the allowed privileges.
statement error user root does not have ALL privilege on database system
statement error pq: cannot GRANT on system object
GRANT ALL ON DATABASE system TO root

statement error user root does not have DELETE privilege on database system
statement error pq: cannot GRANT on system object
GRANT DELETE, INSERT ON DATABASE system TO root

statement error user root does not have ALL privilege on relation namespace
statement error pq: cannot GRANT on system object
GRANT ALL ON system.namespace TO root

statement error user root does not have DELETE privilege on relation descriptor
statement error pq: cannot GRANT on system object
GRANT DELETE, INSERT ON system.descriptor TO root

statement error user root does not have ALL privilege on relation descriptor
statement error pq: cannot GRANT on system object
GRANT ALL ON system.descriptor TO root

statement error user root must have exactly GRANT, SELECT privileges on system database with ID=.*
statement error pq: cannot REVOKE on system object
REVOKE GRANT ON DATABASE system FROM root

statement error user root must have exactly GRANT, SELECT privileges on system table with ID=.*
statement error pq: cannot REVOKE on system object
REVOKE GRANT ON system.namespace FROM root

statement error user root does not have ALL privilege on relation namespace
statement error pq: cannot REVOKE on system object
REVOKE ALL ON system.namespace FROM root

statement error user root does not have privileges over system table with ID=.*
statement error pq: cannot REVOKE on system object
REVOKE GRANT,SELECT ON system.namespace FROM root

statement error user root does not have ALL privilege on database system
statement error pq: cannot GRANT on system object
GRANT ALL ON DATABASE system TO admin

statement error user root does not have DELETE privilege on database system
statement error pq: cannot GRANT on system object
GRANT DELETE, INSERT ON DATABASE system TO admin

statement error user admin must have exactly GRANT, SELECT privileges on system database with ID=.*
statement error pq: cannot REVOKE on system object
REVOKE GRANT ON DATABASE system FROM admin

statement error user root does not have ALL privilege on relation namespace
statement error pq: cannot GRANT on system object
GRANT ALL ON system.namespace TO admin

statement error user root does not have DELETE privilege on relation descriptor
statement error pq: cannot GRANT on system object
GRANT DELETE, INSERT ON system.descriptor TO admin

statement error user root does not have ALL privilege on relation descriptor
statement error pq: cannot GRANT on system object
GRANT ALL ON system.descriptor TO admin

statement error user admin must have exactly GRANT, SELECT privileges on system table with ID=.*
statement error pq: cannot REVOKE on system object
REVOKE GRANT ON system.descriptor FROM admin

statement error user admin must have exactly GRANT, SELECT privileges on system database with ID=.*
statement error pq: cannot REVOKE on system object
REVOKE GRANT ON DATABASE system FROM admin

statement error user admin must have exactly GRANT, SELECT privileges on system table with ID=.*
statement error pq: cannot REVOKE on system object
REVOKE GRANT ON system.namespace FROM admin

statement error user root does not have ALL privilege on relation namespace
statement error pq: cannot REVOKE on system object
REVOKE ALL ON system.namespace FROM admin

statement error user admin does not have privileges over system table with ID=.*
statement error pq: cannot REVOKE on system object
REVOKE GRANT,SELECT ON system.namespace FROM admin

# Some tables (we test system.lease here) used to allow multiple privilege sets for
# backwards compatibility, and superusers were allowed very wide privileges.
# We make sure this is no longer the case.
statement error user root does not have ALL privilege on relation lease
statement error pq: cannot GRANT on system object
GRANT ALL ON system.lease TO testuser

statement error user root does not have CREATE privilege on relation lease
statement error pq: cannot GRANT on system object
GRANT CREATE on system.lease to root

statement error user root does not have CREATE privilege on relation lease
statement error pq: cannot GRANT on system object
GRANT CREATE on system.lease to admin

statement error user root does not have CREATE privilege on relation lease
statement error pq: cannot GRANT on system object
GRANT CREATE on system.lease to testuser

statement error user root does not have ALL privilege on relation lease
statement error pq: cannot GRANT on system object
GRANT ALL ON system.lease TO root

statement error user root does not have ALL privilege on relation lease
statement error pq: cannot GRANT on system object
GRANT ALL ON system.lease TO admin

statement error user root does not have ALL privilege on relation lease
statement error pq: cannot GRANT on system object
GRANT ALL ON system.lease TO testuser

# NB: the "order by" is necessary or this test is flaky under DistSQL.
Expand Down
7 changes: 4 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/user
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,6 @@ root
statement ok
ALTER USER testuser CREATEROLE

statement ok
GRANT SELECT ON system.role_options to testuser

user testuser

statement ok
Expand All @@ -148,6 +145,8 @@ CREATE ROLE user4 CREATEROLE
statement ok
CREATE USER user5 NOLOGIN

user root

query TTT
SELECT * FROM system.role_options
----
Expand All @@ -156,6 +155,8 @@ user4 CREATEROLE NULL
user4 NOLOGIN NULL
user5 NOLOGIN NULL

user testuser

statement ok
DROP ROLE user4

Expand Down

0 comments on commit dd84b74

Please sign in to comment.