sql: Database-level permissions are confusing

[bdarnell]

Our database-level permissions don't work in quite the same way as in either postgres or mysql, and this can be confusing. (This applies to the DDL permissions SELECT/INSERT/UPDATE/DELETE; the CREATE and DROP permissions work as expected)

In CockroachDB, database-level permissions are copied to new tables when they are created, but subsequent changes to the database permissions are not reflected on existing tables. To change permissions on all current and future tables, two commands are required:

GRANT SELECT ON DATABASE d TO u;
GRANT SELECT ON TABLE d.* TO u;

In MySQL, database-level permissions are not copied to newly-created tables. Instead, permission checks use the union of database-level and table-level permissions at runtime, so changes to the database permissions affect all tables. Admins can choose to use database-level permissions exclusively and never touch table-level permissions.

In PostgreSQL, the DDL permissions are not allowed at the database level (i.e. you can't say anything equivalent to GRANT SELECT ON DATABASE d TO u). Instead, there is a separate ALTER DEFAULT PRIVILEGES command (which works the same as in CockroachDB, by copying the grants to new tables but changes do not affect existing ones):

ALTER DEFAULT PRIVILEGES IN SCHEMA d GRANT SELECT ON TABLES TO u

CockroachDB uses the same syntax as mysql, but the semantics of postgres. I think we should change to match one or the other: either introduce the postgres-style ALTER DEFAULT PRIVILEGES syntax and deprecate GRANT ON DATABASE, or keep the current syntax and change to mysql-style semantics.

CC @cockroachdb/sql-language @mberhault

Epic CRDB-2586

[knz]

👍 on choosing either the mysql or pg semantics

unless we had our own separate syntax, the inconsiderate reuse here is indeed confusing

[ajwerner]

I've done some research on this topic and am going to use this issue to both
express my findings and a proposal for a pathway forward. The tl;dr is, as
noted on this issue, a bunch of stuff is different but we should just keep
our quirky model for this release as we think towards a major overhaul later.

Concepts in the Postgres Privilege World

In postgres, catalog entities contain privileges which can be explicitly
GRANTed or REVOKEd. For example, databases carry things like CREATE
and other operations that modify the database or its contents (note that role
creation is a separate thing and lives above the database). In addition to
the relevant permissions to interact with the schema, schema entities also
carry default privileges for the things that they contain. There is an
explicit separation of concepts between the set of permissions on an entity
like a database and the default privileges it carries for new children are
distinct concepts.

Cockroach

In cockroach, databases and tables carry a PrivilegeDescriptor which is just
a slice of []UserPrivilege. We conflate the privileges on a descriptor with
the privileges for new children.

In upcoming work we'll be making user-defined schemas carry privileges and we're
currently in the process of adding privileges to user-defined types
#51622.

We need to decide how we want to model these new things in terms of
privileges and how we want that to interact with postgres compatibility.

Differences

Below find a non-exhaustive list of distinct differences between postgres
and cockroach with regards to privileges.

Separated notions of privileges and default privileges

Because of this you can run:

    crdb> CREATE DATABASE foo_db;
    crdb> GRANT SELECT ON foo_db TO bar_user;

Which would be an error in postgres. There is no postgres direct equivalent.
In postgres every object like a type or relation is a member of a schema.
You can however set the default privileges for a schema.

    -- posgres
    postgres=> ALTER DEFAULT PRIVILEGES IN SCHEMA baz_schema GRANT SELECT TO bar_user;

You can similarly set the default privileges on the database for new schemas
with something like:

    postgres=> ALTER DEFAULT PRIVILEGES GRANT SELECT TO bar_user;

Glob patterns vs explicit statements (#55049)

This creates ambiguity problems because of the lack of specification for the
type to which GRANT or REVOKE statements apply.

In postgres, to grant SELECT to all tables you would type:

    GRANT SELECT ON ALL TABLES IN SCHEMA baz_schema TO bar_user;

In cockroach however, for a database you'd write:

    GRANT SELECT ON foo_db.* TO bar_user;

Of course, that's nonsense in postgres on several levels. With user-defined
schemas, if we keep the basic structure, that would look like:

    GRANT SELECT ON baz_schema.* TO bar_user;

Furthermore, postgres provides support for granting all privileges with GRANT ALL and revoking all privileges with REVOKE ALL.

Check out this nasty logic:

    // GetObjectNames returns the list of all objects in the given
    // database and schema.
    // TODO(solon): when separate schemas are supported, this
    // API should be extended to use schema descriptors.
    //
    // TODO(ajwerner,rohany): This API is utilized to support glob patterns that
    // are fundamentally sometimes ambiguous (see GRANT and the ambiguity between
    // tables and types). Furthermore, the fact that this buffers everything
    // in ram in unfortunate.
    GetObjectNames(
            ctx context.Context, txn *kv.Txn, codec keys.SQLCodec,
            db sqlbase.DatabaseDescriptorInterface,
            scName string, flags tree.DatabaseListFlags,
    ) (tree.TableNames, error)

cockroach/pkg/sql/resolver.go

Lines 285 to 289 in 4912bee

	// We set required to false here, because there could be type names in
	// the returned set of names, so we don't want to error out if we
	// couldn't resolve a name into a table.
	descriptor, err := resolver.ResolveMutableExistingTableObject(ctx, p,
	&objectNames[i], false /* required */, tree.ResolveAnyTableKind)

Public schemas contain default CREATE and USAGE rights to all users (PUBLIC).

Tables created by one user are not automatically visble to another unless
there's a default privilege in the schema indicating that the user should
have access to that new table.

Given the tendency and need to grant permissions on the database and the
fact that tables are (as of 20.1) members of a database, by default all
users in a database tend to have permissions on newly created tables.
The opposite tends to be true in postgres unless you intentionally
set up default privileges.

    ALTER DEFAULT PRIVILEGES FOR TABLES GRANT SELECT TO PUBLIC;

Tracking who performed grants

In postgres, each grant carries with it the role which created the grant.
We don't do this in cockroach.

Postgres uses this to implement a restriction that only users who granted
a privilege are allowed to revoke it.

Tracked in #71999

Ownership

• Update (10/6/2020): Fixed by sql: Add ownership concept for objects #51856.

Database objects in postgres have owners. Owners have all privileges on an
object. This ends up relating to default privileges pretty closely.
Postgres is able to get by providing very weak default privileges because
owners which create database objects have full privileges. It's worth
noting that the public schema in a database offers CREATE privileges to
all users by default, thus enabling general object creation.

#30875

pg_default_acl

The catalog pg_default_acl stores initial privileges to be assigned to newly
created objects. I'm sure there are other incompatibilities with the system catalog.

https://www.postgresql.org/docs/12/catalog-pg-default-acl.html

Fixed in #67872

Superuser

Cockroach pretty much has this concept in "admin". The difference here is just
terminology.

Tracked in #67087

New users

In postgres, it is clear what default privileges a new user should get. In cockroach
it's not clear and our decision is perhaps unfortunate.

#20663

What shall we do about it.

In cockroachdb, we have a pretty primitive setup today. However, to move
ourselves towards the postgres model and retain any sort of compatibiltiy
in this release seems quite hard. Instead our goals in the 20.2 timeframe
should be modest an calculated.

In particular, let's not strive towards any major increase in postgres
compatibility in the short term. Instead, let's wait until we have a clearer
vision of goals w.r.t. IAM in general and compatibility.

There are other things we know we want to change:

• String-based permissions
  • TODO(ajwerner): Fill in bug about permissions and historical queries
  • Ideally role's would have numerical IDs.
• Special principles (root, node) that are used in the code.

Given that, I propose that we do the minimal thing in terms of changes for
this release.

Short term plan

Retain the PrivilegeDescriptor as-is. This will enable us to build
tools like re-parenting a database as a schema. We'll just keep our
quirks and extend it to schema and types. We'll defer a major overhaul
until a later release.

This leaves open some questions regarding policy for members of user-defined
schemas. My current thinking is that we should copy permissions from the
parent schema if it is a real user-defined schema and from the parent database
if the parent schema is public.

Longer term

Postgres compatibility and the model changes it will entail.

References

https://www.postgresql.org/docs/12/sql-grant.html
https://www.postgresql.org/docs/12/sql-alterdefaultprivileges.html

[ajwerner]

cc @thtruo

[ajwerner]

Ownership was fixed in Ownership portion of this was tracked in #30875 and closed by #51856.

[rafiss]

With the new ALTER DEFAULT PRIVILEGES work (#66785) that is arriving in v21.2, I think we are in a better place.

A few remaining items remain that we will be finishing up in the near-term:

• sql: support ALTER DEFAULT PRIVILEGES IN SCHEMA #67376
• sql: disallow granting postgres incompatible database privileges #68731
• sql: remove check for CREATE privilege on database substituting for USAGE privilege on schema #68917

And there are a few smaller items that we aren't getting to in the near-term, but are tracked:

• sql: support SUPERUSER role option #67087
• sql: support tracking which users grant privileges for revoke compatibility #71999

Given the state of all the above, I'll go ahead and close this issue in favor of the smaller remaining open issues. If I have missed something that warrants keeping this open, please don't hesitate to re-open it and comment.