From 9e0447242d6678e71b1ea69048dd3167f43abdd7 Mon Sep 17 00:00:00 2001 From: Solon Gordon Date: Wed, 18 Sep 2019 13:15:16 -0400 Subject: [PATCH] sql: check privileges in CONFIGURE ZONE commands 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 `. To grant the CREATE privilege on a database or table, run `GRANT CREATE ON [DATABASE | TABLE] TO `. Release justification: Fix for high-priority bug in existing functionality. Fixes #40693 --- pkg/ccl/logictestccl/testdata/logic_test/zone | 65 +++++++++++++++++++ pkg/sql/set_zone_config.go | 38 ++++++++++- 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/pkg/ccl/logictestccl/testdata/logic_test/zone b/pkg/ccl/logictestccl/testdata/logic_test/zone index 463480ce75da..f0aef991f7bc 100644 --- a/pkg/ccl/logictestccl/testdata/logic_test/zone +++ b/pkg/ccl/logictestccl/testdata/logic_test/zone @@ -692,3 +692,68 @@ t40417 CREATE TABLE t40417 ( ); ALTER PARTITION p1 OF INDEX "my database".public.t40417@primary CONFIGURE ZONE USING num_replicas = 1 + +subtest authorization + +statement ok +CREATE DATABASE auth; +CREATE TABLE auth.t (x INT PRIMARY KEY) PARTITION BY LIST (x) ( + PARTITION p VALUES IN (1) +); +CREATE INDEX x ON auth.t (x) PARTITION BY LIST (x) ( + PARTITION p VALUES IN (1) +) + +user testuser + +# User should have no CONFIGURE ZONE abilities by default. +statement error only users with the admin role are allowed to alter system ranges +ALTER RANGE default CONFIGURE ZONE USING num_replicas = 3 + +statement error pq: user testuser does not have CREATE privilege on database auth +ALTER DATABASE auth CONFIGURE ZONE USING num_replicas = 3 + +statement error pq: only users with the admin role are allowed to alter system tables +ALTER TABLE system.jobs CONFIGURE ZONE USING num_replicas = 3 + +statement error user testuser does not have CREATE privilege on relation t +ALTER TABLE auth.t CONFIGURE ZONE USING num_replicas = 3 + +statement error user testuser does not have CREATE privilege on relation t +ALTER PARTITION p OF TABLE auth.t CONFIGURE ZONE USING num_replicas = 3 + +statement error user testuser does not have CREATE privilege on relation t +ALTER PARTITION p OF INDEX auth.t@x CONFIGURE ZONE USING num_replicas = 3 + +# Granting CREATE on databases and tables should allow CONFIGURE ZONE on those +# objects. +user root + +statement ok +GRANT CREATE ON DATABASE auth TO testuser + +statement ok +GRANT CREATE ON TABLE auth.t TO testuser + +user testuser + +statement ok +ALTER DATABASE auth CONFIGURE ZONE USING num_replicas = 3 + +statement ok +ALTER TABLE auth.t CONFIGURE ZONE USING num_replicas = 3 + +# Granting the admin role should allow configuring zones on system tables and +# ranges. +user root + +statement ok +GRANT admin TO testuser + +user testuser + +statement ok +ALTER RANGE default CONFIGURE ZONE USING num_replicas = 3 + +statement ok +ALTER TABLE system.jobs CONFIGURE ZONE USING num_replicas = 3 diff --git a/pkg/sql/set_zone_config.go b/pkg/sql/set_zone_config.go index 67431c8d60a8..21018418ab7d 100644 --- a/pkg/sql/set_zone_config.go +++ b/pkg/sql/set_zone_config.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/server/serverpb" "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/sqlbase" "github.com/cockroachdb/cockroach/pkg/sql/types" @@ -93,6 +94,10 @@ func loadYAML(dst interface{}, yamlString string) { } func (p *planner) SetZoneConfig(ctx context.Context, n *tree.SetZoneConfig) (planNode, error) { + if err := checkPrivilegeForSetZoneConfig(ctx, p, n.ZoneSpecifier); err != nil { + return nil, err + } + var yamlConfig tree.TypedExpr if n.YAMLConfig != nil { @@ -158,6 +163,36 @@ func (p *planner) SetZoneConfig(ctx context.Context, n *tree.SetZoneConfig) (pla }, nil } +func checkPrivilegeForSetZoneConfig(ctx context.Context, p *planner, zs tree.ZoneSpecifier) error { + // For system ranges, the system database, or system tables, the user must be + // an admin. Otherwise we require CREATE privileges on the database or table + // in question. + if zs.NamedZone != "" { + return p.RequireAdminRole(ctx, "alter system ranges") + } + if zs.Database != "" { + if zs.Database == "system" { + return p.RequireAdminRole(ctx, "alter the system database") + } + dbDesc, err := p.ResolveUncachedDatabaseByName(ctx, string(zs.Database), true) + if err != nil { + return err + } + return p.CheckPrivilege(ctx, dbDesc, privilege.CREATE) + } + tableDesc, err := p.resolveTableForZone(ctx, &zs) + if err != nil { + if zs.TargetsIndex() && zs.TableOrIndex.Table.TableName == "" { + err = errors.WithHint(err, "try specifying the index as @") + } + return err + } + if tableDesc.ParentID == keys.SystemDatabaseID { + return p.RequireAdminRole(ctx, "alter system tables") + } + return p.CheckPrivilege(ctx, tableDesc, privilege.CREATE) +} + // setZoneConfigRun contains the run-time state of setZoneConfigNode during local execution. type setZoneConfigRun struct { numAffected int @@ -240,9 +275,6 @@ func (n *setZoneConfigNode) startExec(params runParams) error { // descriptor. table, err := params.p.resolveTableForZone(params.ctx, &n.zoneSpecifier) if err != nil { - if n.zoneSpecifier.TargetsIndex() && n.zoneSpecifier.TableOrIndex.Table.TableName == "" { - return errors.WithHint(err, "try specifying the index as @") - } return err }