A question from Joe Mitchell led me to investigate some access-checking
behavior that seems kinda broken. Currently, when aclinsert3() creates
a new entry in an ACL list, it effectively initializes the entry with
the current PUBLIC access rights, rather than with zero rights. Thus:
regression=# create user u1;
CREATE USER
regression=# create table t1 (f1 int);
CREATE
regression=# grant select on t1 to public;
CHANGE
regression=# grant update on t1 to u1;
CHANGE
regression=# \z t1Access permissions for database "regression"Relation | Access permissions
----------+-----------------------------------t1 | {"=r","postgres=arwdRxt","u1=rw"}
(1 row)
Notice it says "u1=rw", not just "u1=w" which is what one might expect.
The reason why it does this, apparently, is that when aclcheck() finds a
match on userid, it stops with that ACL entry and doesn't look at any
group or world entries. So, if I now do
regression=# revoke select on t1 from u1;
CHANGE
regression=# \z t1
Access permissions for database "regression"Relation | Access permissions
----------+----------------------------------t1 | {"=r","postgres=arwdRxt","u1=w"}
(1 row)
I now have a situation where u1 can't read t1, even though the rest of
the world can:
regression=> select * from t1;
ERROR: t1: Permission denied.
This is inconsistent because the same does not hold true for privileges
granted via groups. aclcheck will succeed if *any* group you are in
has the desired privilege, *or* if PUBLIC does. Thus:
regression=# create group g1 with user u1;
CREATE GROUP
regression=# create table t2 (f1 int);
CREATE
regression=# grant select on t2 to public;
CHANGE
regression=# grant update on t2 to group g1;
CHANGE
regression=# \z t2 Access permissions for database "regression"Relation | Access permissions
----------+-----------------------------------------t2 | {"=r","postgres=arwdRxt","group g1=rw"}
(1 row)
(At this point u1 is able to read t2)
regression=# revoke select on t2 from group g1;
CHANGE
regression=# \z t2 Access permissions for database "regression"Relation | Access permissions
----------+----------------------------------------t2 | {"=r","postgres=arwdRxt","group g1=w"}
(1 row)
(At this point u1 is still able to read t2)
Another problem is that if you dogrant select to public;grant update to u1;revoke select from public;
you will find that u1 still has select rights, which is undoubtedly
not what you wanted.
I believe that a more consistent approach would be to say that a user's
privileges are the union of what is granted directly to himself, to any
group he is currently a member of, and to PUBLIC. So if aclcheck
doesn't see the desired privilege granted in the user entry (if found),
it has to continue on looking at groups and then world, not just fail.
And aclinsert3 should initialize new entries to zero access rights, not
copy PUBLIC.
The only downside of this is that we'd lose the "feature" of being able
to revoke from a particular user a right that is available via PUBLIC to
everyone else. I'm not convinced that that behavior has any real use,
and certainly keeping it doesn't seem important compared to making these
other behaviors more reasonable. That "feature" doesn't work reliably
anyway, since ACL entries are dropped as soon as they go to zero rights.
Comments, objections?
regards, tom lane
