Re: [PATCH] DefaultACLs

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>, Petr Jelinek <pjmodos(at)pjmodos(dot)net>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Joshua Tolley <eggyknap(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH] DefaultACLs
Date: 2009-08-05 00:53:16
Message-ID: 25451.1249433596@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I looked at this patch a bit. I haven't actually read any of the code
yet, just reviewed the on-list discussions and the docs, but I think I can
make a few comments at the definitional level.

First: there's already been some discussion about the VIEW problem.
I understand that the patch adds a "GRANT ON VIEW" syntax that works
like "GRANT ON SEQUENCE" in that if you say VIEW it insists the object
really is a view, but without taking away the existing laxness that
allows you to still say GRANT ON TABLE for a view. This seems reasonable
in isolation, but it creates a big problem for both this patch and the
GRANT ON ALL patch, in that it's unclear how to treat views if they
could be considered either tables or views. Unfortunately I think
we have no choice but to live with the "existing laxness", because
(a) to do otherwise will break pretty nearly every pg_dump script, and
(b) the SQL standard says we have to accept "GRANT ON TABLE view".
In fact "GRANT ON VIEW" is not in the SQL standard and there is no good
reason to expect applications will adopt it at all.

So my feeling is that adding GRANT ON VIEW is a bad idea. The main
argument for doing it seemed to be that the author wanted to be able
to grant different default privileges for tables and views, but I'm
unconvinced that there's a strong use-case for that. You could very
easily have one set of default privileges for all of tables, views,
and sequences, simply ignoring any bits that weren't relevant for
particular objects. It seems to me that that would handle common
cases just fine. For complicated cases not so much, but it's not
clear to me that filtering by the object subtype is all that useful
for complicated cases anyway. People will want more functionality
than that. Which leads into my next item.

Second: both this patch and GRANT ON ALL are built on the assumption
that the only way to filter/classify objects is by schema membership.
Now I don't object to that as an initial implementation restriction,
but I don't like hard-wiring it into the syntax. It is very clear to me
that we'll want other filter rules in the future --- an immediate example
is being able to say that new children of an inheritance parent table
should inherit its GRANTs. So to my mind, designing the syntax around
ALTER SCHEMA is right out. Maybe we could do something like
ALTER DEFAULT PRIVILEGES ON TABLES IN SCHEMA foo GRANT ...
where the "IN SCHEMA foo" part would be subject to generalization later.
This also matches up a bit better with the proposed syntax for GRANT
ON ALL (which also uses "IN SCHEMA foo").

Third: speaking of syntax, I don't like the way that this is gratituously
different from GRANT/REVOKE. I don't like using ADD/DROP instead of
GRANT/REVOKE, nor the unnecessary AND business. I think we should
minimize confusion by using something that is spelled as nearly like
GRANT/REVOKE as possible.

Fourth: the system's existing treatment of default permissions is
owner-dependent, that is the implied set of permissions is typically
GRANT ALL TO <owner> (from himself, with grant option). I do not
understand how schema-level default ACLs will play nicely with that,
except in the special case where the schema owner also owns every
contained object. If you copy the schema-level ACL directly to a
contained object with a different owner it will definitely be the wrong
thing, but if you try to translate the ownership it will confuse people
too. And confusion in a security-related behavior is a Bad Thing.
Furthermore, having the schema owner able to control the grants made
for objects not owned by him is a huge security hole.

What I suggest as a way to resolve this last point is that a default ACL
should apply only to objects owned by the user who creates/modifies the
default ACL. In this view, the question of which schema the objects are
in is just an additional filter condition, not the primary determinant
of which objects a default ACL applies to. Every user has his own set
of default ACLs. (This is another reason not to design the syntax
around ALTER SCHEMA.)

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dan Colish 2009-08-05 01:16:13 Convert stmt back into queryString
Previous Message Joshua D. Drake 2009-08-05 00:36:09 Re: the case for machine-readable error fields