Re: New Model For Role Attributes and Fine Grained Permssions

Hi All,

This is a "proof-of-concept" patch for a new model around role attributes
and fine grained permissions meant to alleviate the current over dependence
on superuser.

This is not yet complete and only serves as a proof-of-concept at this
point, but I wanted to share it in the hopes of receiving comments,
suggestions and general feedback.

The general gist of this patch is as follows:

* New system catalog "pg_permission" that relates role id's to permissions.

* New syntax.
- GRANT <permission> TO <role>
- REVOKE <permission> FROM <role>
where, <permission> is one of an enumerated value, such as "CREATE ROLE" or
"CREATE DATABASE".

* Refactor CREATEDB and NOCREATEDB role attribute to "CREATE DATABASE"
permission set by GRANT or REVOKE.

* Refactor CREATEROLE and NOCREATEROLE role attribute to "CREATE ROLE"
permission set by GRANT or REVOKE.

Again, this is meant to serve as a proof-of-concept. It is not
comprehensive and only demonstrates how this might work with a few already
defined permissions.

I have attached the current patch based on master.

Any comments or feedback would be greatly appreciated.

Thanks,
Adam

--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com

On 08/19/2014 04:27 AM, Brightwell, Adam wrote:
> Hi All,
>
> This is a "proof-of-concept" patch for a new model around role attributes
> and fine grained permissions meant to alleviate the current over dependence
> on superuser.

Hmm. How does this get us any closer to fine-grained permissions? I
guess we need this, so that you can grant/revoke the permissions, but I
thought the hard part is defining what the fine-grained permissions are,
in a way that you can't escalate yourself to full superuser through any
of them.

The new syntax could be useful even if we never get around to do
anything about current superuser checks, so I'm going to give this a
quick review just on its own merits.

Please add documentation. That's certainly required before this can be
committed, but it'll make reviewing the syntax much easier too.

> This is not yet complete and only serves as a proof-of-concept at this
> point, but I wanted to share it in the hopes of receiving comments,
> suggestions and general feedback.
>
> The general gist of this patch is as follows:
>
> * New system catalog "pg_permission" that relates role id's to permissions.
>
> * New syntax.
> - GRANT <permission> TO <role>
> - REVOKE <permission> FROM <role>
> where, <permission> is one of an enumerated value, such as "CREATE ROLE" or
> "CREATE DATABASE".

The syntax for GRANT/REVOKE is quite complicated already. Do we want to
overload it even more? Also keep in mind that the SQL standard specifies
GRANT/REVOKE, so we have to be careful to not clash with the SQL
standard syntax, or any conceivable future syntax the SQL committee
might add to it (although we have plenty of PostgreSQL extensions in it
already). For example, this is currently legal:

GRANT CREATE ALL ON TABLESPACE <typename> TO <role>

Is that too close to the new syntax, to cause confusion? Does the new
syntax work for all the more fine-grained permissions you have in mind?
I'm particularly worried of conflicts with this existing syntax:

GRANT role_name [, ...] TO role_name [, ...] [ WITH ADMIN OPTION ]

- Heikki

Heikki,

* Heikki Linnakangas (hlinnakangas(at)vmware(dot)com) wrote:
> On 08/19/2014 04:27 AM, Brightwell, Adam wrote:
> >This is a "proof-of-concept" patch for a new model around role attributes
> >and fine grained permissions meant to alleviate the current over dependence
> >on superuser.
>
> Hmm. How does this get us any closer to fine-grained permissions?

This patch, by itself, does not- but it adds the structure to allow us
to add more permissions without having to add more fields to pg_authid,
and we could build into pg_permission the "WITH ADMIN OPTION" and
"grantor" bits that the normal GRANT syntax already supports- but
without having to chew up additional bits for these granted permissions
which are applied to roles instead of objects in the system.

As for specific examples where this could be used beyond those
presented, consider things like:

CREATE EXTENSION
CREATE TABLESPACE
COPY (server-side)

The question which I've been kicking around is the possible additional
constraints which we may want/need for these- a list of extensions which
the role is allowed to create, a set of directories which the role is
allowed to create tablespaces within, similairly a set of directories
the role is allowed to use server-side COPY with (and perhaps a
distinction between COPY FROM and COPY TO).

> I
> guess we need this, so that you can grant/revoke the permissions,
> but I thought the hard part is defining what the fine-grained
> permissions are, in a way that you can't escalate yourself to full
> superuser through any of them.

This is definitely a concern- which is why I mention the specific items
above as needing to be constrained in some fashion. CREATE EXTENSION
and CREATE TABLESPACE are, in a way, "naturally" constrained if you
imagine an environment where the user with those permissions doesn't
have direct access to modify the filesystem. COPY, on the other hand,
would allow the user to gain access to pg_authid through indirect means
and therefore gain access to an actual superuser role on the system,
potentially.

(Ok- it might be possible to do that with CREATE TABLESPACE too, but
it'd be a bit more interesting to work through that than being able to
just COPY to a bytea and download the result)

> The syntax for GRANT/REVOKE is quite complicated already. Do we want
> to overload it even more? Also keep in mind that the SQL standard
> specifies GRANT/REVOKE, so we have to be careful to not clash with
> the SQL standard syntax, or any conceivable future syntax the SQL
> committee might add to it (although we have plenty of PostgreSQL
> extensions in it already). For example, this is currently legal:

I agree that there are concerns in this area and I've not got any great
solutions. There are certainly pros and cons to using GRANT. It's
familiar and natural to DBAs, but it runs the risk of conflicting with
future SQL syntax, or even our own GRANT role. We can avoid the latter
by keeping to reserved keywords only, but that may lead to some rather
odd syntax (how would the "GRANT COPY ON (dir1, dir2, dir3)" work?).

Is there a good alternative to consider though..?

Thanks!

Stephen

On Thu, Aug 21, 2014 at 10:49 PM, Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> * Heikki Linnakangas (hlinnakangas(at)vmware(dot)com) wrote:
>> On 08/19/2014 04:27 AM, Brightwell, Adam wrote:
>> >This is a "proof-of-concept" patch for a new model around role attributes
>> >and fine grained permissions meant to alleviate the current over dependence
>> >on superuser.
>>
>> Hmm. How does this get us any closer to fine-grained permissions?
>
> This patch, by itself, does not- but it adds the structure to allow us
> to add more permissions without having to add more fields to pg_authid,
> and we could build into pg_permission the "WITH ADMIN OPTION" and
> "grantor" bits that the normal GRANT syntax already supports- but
> without having to chew up additional bits for these granted permissions
> which are applied to roles instead of objects in the system.
>
> As for specific examples where this could be used beyond those
> presented, consider things like:
>
> CREATE EXTENSION
> CREATE TABLESPACE
> COPY (server-side)

I'm not opposed to this in theory, but I agree with Heikki that the
syntax you've picked (as well as the internal teminology of the patch)
is not sufficiently unambiguous. "Permission" could refer to a lot
of things, and the GRANT syntax does a lot of things already. Since
the patch appears to aim to supplant what we current do with ALTER
ROLE .. [NO] {CREATEDB | CREATEROLE }, how about something like:

ALTER ROLE role_name { GRANT | REVOKE } CAPABILITY capability_name;

In terms of catalog structure, I doubt that a
row-per-capability-per-user makes sense. Why not just add a new
rolcapability column to pg_authid? A single int64 interpreted as a
bitmask would give us room for 64 capabilities at a fraction of the
storage and lookup cost of a separate catalog. If that's not enough,
the column could be stored as an integer array or vector or something,
but a bigger problem is that Tom's head will likely explode if you
tell him you're going to have more than 64 of these things.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company