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.

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

sql: users with select permissions can alter tables and partitions with zone configurations #40693

Closed
awoods187 opened this issue Sep 11, 2019 · 2 comments · Fixed by #40879
Closed
Assignees
Labels
A-sql-privileges SQL privilege handling and permission checks. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-2 Medium-high impact: many users impacted, risks of availability and difficult-to-fix data errors

Comments

@awoods187
Copy link
Contributor

In our docs, we specify that Currently, only members of the admin role can configure replication zones. By default, the root user belongs to the admin role.

We don't appear to follow that. Here is an example of creating users and databases:

create user andy;
create database foo;
use foo;
create table t (a int);
ALTER TABLE t PARTITION BY LIST (rowid) ( PARTITION new_york VALUES IN ('1'), PARTITION chicago VALUES IN ('2'), PARTITION seattle VALUES IN ('3') );
grant select on table t to andy;

Andy is able to alter tables and partitions:

./cockroach sql --insecure --user=andy
use foo;
ALTER TABLE t CONFIGURE ZONE USING gc.ttlseconds = 600;
ALTER PARTITION seattle OF TABLE t CONFIGURE ZONE USING constraints='[]';

Proof of no create table privileges:

create table t2 (a int);
pq: user andy does not have CREATE privilege on database foo
@awoods187 awoods187 added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-2 Medium-high impact: many users impacted, risks of availability and difficult-to-fix data errors labels Sep 11, 2019
@awoods187 awoods187 added the A-sql-privileges SQL privilege handling and permission checks. label Sep 11, 2019
@solongordon
Copy link
Contributor

Here are the privileges I'm planning to require:

  • ALTER TABLE ... CONFIGURE ZONE: CREATE privilege on table
  • ALTER DATABASE ... CONFIGURE ZONE: CREATE privilege on database
  • ALTER RANGE ... CONFIGURE ZONE: Only root user or users with the admin role

@solongordon
Copy link
Contributor

solongordon commented Sep 17, 2019

Turns out this is complicated by the fact that we want to allow CONFIGURE ZONE on system tables, but no one has CREATE privileges on them, including the root user. So probably we always want to allow admins to run CONFIGURE ZONE on system tables, even if they don't have CREATE permission.

craig bot pushed a commit that referenced this issue Sep 23, 2019
40879: sql: check privileges in CONFIGURE ZONE commands r=solongordon a=solongordon

Previously, any user could apply zone configurations to any object. This
commit enforces that users must have appropriate privileges to run a
CONFIGURE ZONE command. For system ranges, the system database, and
tables in that database, the user must be an admin. For other databases
and tables, the user must have CREATE privileges on the object.

Release note (backward-incompatible change): CONFIGURE ZONE commands
now fail if the user does not have sufficient privileges. If the target
is a system range, the "system" database, or a table in that database,
the user must be an admin. For all other databases and tables, the user
must have the CREATE privilege on the target database or table.

Note that this change may be backward-incompatible for users who run
scripted CONFIGURE ZONE commands via a user with restricted permissions.
To add the necessary permissions, use the GRANT command via an admin
user. To grant the admin role to a user, run `GRANT admin TO <user>`. To
grant the CREATE privilege on a database or table, run `GRANT CREATE ON
[DATABASE | TABLE] <name> TO <user>`.

Release justification: Fix for high-priority bug in existing
functionality.

Fixes #40693

Co-authored-by: Solon Gordon <solon@cockroachlabs.com>
@craig craig bot closed this as completed in 9e04472 Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-privileges SQL privilege handling and permission checks. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. S-2 Medium-high impact: many users impacted, risks of availability and difficult-to-fix data errors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants