Skip to content

Commit

Permalink
sql: fix SHOW ALL ZONE CONFIGURATION displaying unprivileged entries
Browse files Browse the repository at this point in the history
* Made `crdb_internal.zones` not display entries which the executing
user does not have access to.
* Add a delegate for ShowZoneConfig, which triggers on SHOW ALL
ZONE CONFIGURATIONS, to use crdb_internal.zones table and run it as
the executing user so rows are hidden if the user does not have
permission.

Release note (bug fix): Previously, SHOW ALL ZONE CONFIGURATION ZONES
and crdb_internal.zones shows results for resources the user does not
have access to. This will instead filter out those entries from
displaying.
  • Loading branch information
otan committed Nov 5, 2019
1 parent 537cf9c commit d07c378
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 31 deletions.
64 changes: 62 additions & 2 deletions pkg/ccl/logictestccl/testdata/logic_test/crdb_internal
Expand Up @@ -56,13 +56,59 @@ statement ok
CREATE DATABASE db2; ALTER DATABASE db2 CONFIGURE ZONE USING num_replicas = 3;

statement ok
CREATE TABLE t3 (a INT PRIMARY KEY, b INT); CREATE INDEX myindex ON t3 (b); ALTER INDEX myindex CONFIGURE ZONE USING num_replicas = 5; ALTER TABLE t3 CONFIGURE ZONE USING num_replicas = 8
CREATE TABLE t3 (a INT PRIMARY KEY, b INT); CREATE INDEX myt3index ON t3 (b); ALTER INDEX myt3index CONFIGURE ZONE USING num_replicas = 5; ALTER TABLE t3 CONFIGURE ZONE USING num_replicas = 8

statement ok
CREATE TABLE t4 (a INT PRIMARY KEY, b INT); ALTER TABLE t4 CONFIGURE ZONE USING num_replicas = 7; GRANT ALL ON t4 TO testuser
CREATE TABLE t4 (a INT PRIMARY KEY, b INT); CREATE INDEX myt4index ON t4 (b); ALTER TABLE t4 CONFIGURE ZONE USING num_replicas = 7; ALTER INDEX myt4index CONFIGURE ZONE USING num_replicas = 5; GRANT ALL ON t4 TO testuser

user testuser

query IT
SELECT zone_id, target FROM crdb_internal.zones ORDER BY 1
----
0 RANGE default
16 RANGE meta
17 RANGE system
22 RANGE liveness
57 TABLE test.public.t4
57 INDEX test.public.t4@myt4index

query TT
SELECT * FROM [SHOW ALL ZONE CONFIGURATIONS] ORDER BY 1
----
INDEX test.public.t4@myt4index ALTER INDEX test.public.t4@myt4index CONFIGURE ZONE USING
num_replicas = 5
RANGE default ALTER RANGE default CONFIGURE ZONE USING
range_min_bytes = 16777216,
range_max_bytes = 67108864,
gc.ttlseconds = 90000,
num_replicas = 3,
constraints = '[]',
lease_preferences = '[]'
RANGE liveness ALTER RANGE liveness CONFIGURE ZONE USING
range_min_bytes = 16777216,
range_max_bytes = 67108864,
gc.ttlseconds = 600,
num_replicas = 5,
constraints = '[]',
lease_preferences = '[]'
RANGE meta ALTER RANGE meta CONFIGURE ZONE USING
range_min_bytes = 16777216,
range_max_bytes = 67108864,
gc.ttlseconds = 3600,
num_replicas = 5,
constraints = '[]',
lease_preferences = '[]'
RANGE system ALTER RANGE system CONFIGURE ZONE USING
range_min_bytes = 16777216,
range_max_bytes = 67108864,
gc.ttlseconds = 90000,
num_replicas = 5,
constraints = '[]',
lease_preferences = '[]'
TABLE test.public.t4 ALTER TABLE test.public.t4 CONFIGURE ZONE USING
num_replicas = 7

query error pq: user testuser has no privileges on database db2
SHOW ZONE CONFIGURATION FOR DATABASE db2

Expand All @@ -72,6 +118,20 @@ SHOW ZONE CONFIGURATION FOR TABLE t2
query error pq: user testuser has no privileges on relation t3
SHOW ZONE CONFIGURATION FOR TABLE t3

query error pq: user testuser has no privileges on relation t3
SHOW ZONE CONFIGURATION FOR INDEX myt3index

query TT
SHOW ZONE CONFIGURATION FOR INDEX myt4index
----
INDEX test.public.t4@myt4index ALTER INDEX test.public.t4@myt4index CONFIGURE ZONE USING
range_min_bytes = 16777216,
range_max_bytes = 67108864,
gc.ttlseconds = 90000,
num_replicas = 5,
constraints = '[]',
lease_preferences = '[]'

query TT
SHOW ZONE CONFIGURATION FOR TABLE t4
----
Expand Down
29 changes: 26 additions & 3 deletions pkg/sql/crdb_internal.go
Expand Up @@ -2163,6 +2163,25 @@ CREATE TABLE crdb_internal.zones (
return err
}

var table *TableDescriptor
if zs.Database != "" {
database, err := sqlbase.GetDatabaseDescFromID(ctx, p.txn, sqlbase.ID(id))
if err != nil {
return err
}
if p.CheckAnyPrivilege(ctx, database) != nil {
continue
}
} else if zoneSpecifier.TableOrIndex.Table.TableName != "" {
table, err = sqlbase.GetTableDescFromID(ctx, p.txn, sqlbase.ID(id))
if err != nil {
return err
}
if p.CheckAnyPrivilege(ctx, table) != nil {
continue
}
}

// Write down information about the zone in the table.
// TODO (rohany): We would like to just display information about these
// subzone placeholders, but there are a few tests that depend on this
Expand All @@ -2189,10 +2208,14 @@ CREATE TABLE crdb_internal.zones (
}

if len(subzones) > 0 {
table, err := sqlbase.GetTableDescFromID(ctx, p.txn, sqlbase.ID(id))
if err != nil {
return err
if table == nil {
return errors.AssertionFailedf(
"object id %d with #subzones %d is not a table",
id,
len(subzones),
)
}

for i, s := range subzones {
index, err := table.FindIndexByID(sqlbase.IndexID(s.IndexID))
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/delegate/delegate.go
Expand Up @@ -102,6 +102,9 @@ func TryDelegate(
case *tree.ShowVar:
return d.delegateShowVar(t)

case *tree.ShowZoneConfig:
return d.delegateShowZoneConfig(t)

case *tree.ShowTransactionStatus:
return d.delegateShowVar(&tree.ShowVar{Name: "transaction_status"})

Expand Down
22 changes: 22 additions & 0 deletions pkg/sql/delegate/show_zone_config.go
@@ -0,0 +1,22 @@
// Copyright 2019 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package delegate

import "github.com/cockroachdb/cockroach/pkg/sql/sem/tree"

// ShowZoneConfig only delegates if it selecting ALL configurations.
func (d *delegator) delegateShowZoneConfig(n *tree.ShowZoneConfig) (tree.Statement, error) {
// Specifying a specific zone; fallback to non-delegation logic.
if n.ZoneSpecifier != (tree.ZoneSpecifier{}) {
return nil, nil
}
return parse(`SELECT target, raw_config_sql FROM crdb_internal.zones`)
}
40 changes: 14 additions & 26 deletions pkg/sql/show_zone_config.go
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/protoutil"
"github.com/cockroachdb/errors"
"gopkg.in/yaml.v2"
)

Expand Down Expand Up @@ -66,33 +67,20 @@ func (p *planner) ShowZoneConfig(ctx context.Context, n *tree.ShowZoneConfig) (p
constructor: func(ctx context.Context, p *planner) (planNode, error) {
v := p.newContainerValuesNode(showZoneConfigColumns, 0)

// This signifies SHOW ALL.
// However, SHOW ALL should be handled by the delegate.
if n.ZoneSpecifier == (tree.ZoneSpecifier{}) {
// SHOW ALL ZONE CONFIGURATIONS case.
rows, err := p.ExtendedEvalContext().ExecCfg.InternalExecutor.Query(
ctx,
"show-all-zone-configurations",
p.txn,
`SELECT * FROM crdb_internal.zones`,
)
if err != nil {
return nil, err
}
for i := range rows {
if _, err := v.rows.AddRow(ctx, rows[i]); err != nil {
v.Close(ctx)
return nil, err
}
}
} else {
row, err := getShowZoneConfigRow(ctx, p, n.ZoneSpecifier)
if err != nil {
v.Close(ctx)
return nil, err
}
if _, err := v.rows.AddRow(ctx, row); err != nil {
v.Close(ctx)
return nil, err
}
return nil, errors.AssertionFailedf("zone must be specified")
}

row, err := getShowZoneConfigRow(ctx, p, n.ZoneSpecifier)
if err != nil {
v.Close(ctx)
return nil, err
}
if _, err := v.rows.AddRow(ctx, row); err != nil {
v.Close(ctx)
return nil, err
}
return v, nil
},
Expand Down

0 comments on commit d07c378

Please sign in to comment.