Thread: ACLs versus ALTER OWNER
I've noticed yet another hole in our handling of object permissions,
which is that ALTER OWNER doesn't modify the object ACL list at all.
This leads to unpleasant results. For example, in CVS tip:
regression=# \c - alice
You are now connected as new user "alice".
regression=> create table atable (f1 int);
CREATE TABLE
regression=> grant select on atable to public;
GRANT
regression=> \z atable Access privileges for database "regression"Schema | Name | Type | Access
privileges
--------+--------+-------+--------------------------------public | atable | table | {alice=arwdRxt/alice,=r/alice}
(1 row)
regression=> \c - postgres
You are now connected as new user "postgres".
regression=# alter table atable owner to bob;
ALTER TABLE
regression=# \c - bob
You are now connected as new user "bob".
regression=> insert into atable values(1);
ERROR: permission denied for relation atable
Bob hasn't got insert permissions on his own table ... the ACL says so.
Well, since Bob is now the owner he can fix that:
regression=> grant all on atable to bob;
GRANT
regression=> insert into atable values(1);
INSERT 154991 1
but he's not out of the woods yet. The ACL now looks like this:
regression=> \z atable Access privileges for database "regression"Schema | Name | Type |
Accessprivileges
--------+--------+-------+------------------------------------------------public | atable | table |
{alice=arwdRxt/alice,=r/alice,bob=arwdRxt/bob}
(1 row)
Alice still has all permissions, and PUBLIC still has select
permissions, and there isn't a darn thing Bob can do about it
because he didn't grant those permissions:
regression=> revoke all on atable from alice;
REVOKE
regression=> revoke all on atable from public;
REVOKE
regression=> \z atable Access privileges for database "regression"Schema | Name | Type |
Accessprivileges
--------+--------+-------+------------------------------------------------public | atable | table |
{alice=arwdRxt/alice,=r/alice,bob=arwdRxt/bob}
(1 row)
Even more interesting, the superuser can't fix it either, at least not
without manual hacking of the ACL entry, because any GRANT/REVOKE the
superuser issues on the object will be treated as issued by Bob.
The *only* way to get rid of those rights is to persuade Alice to revoke
them. (Or for the superuser to revert the ownership change, revoke the
rights as-if-Alice, and then give the table back to Bob. Blech.)
ISTM that reasonable behavior for ALTER OWNER would include doing
surgery on the object's ACL to replace references to the old owner by
references to the new owner. A simplistic approach would just be to do
that everywhere in both the grantor and grantee fields. If there are
existing entries mentioning the new owner then this could produce
duplicate ACL entries, which would need to be merged together.
I think there are corner cases where the merging might produce
unintuitive results, but it couldn't be as spectacularly bad as
doing nothing is.
Comments?
regards, tom lane
Dear Tom, > [...] > Even more interesting, the superuser can't fix it either, Due to how ACL are defined in SQL, I restate my suggestion that the super user should be able to change ANY right, including the GRANTOR field, with an appropriate syntax, something like: REVOKE ALL ON TABLE foo FROM GRANTOR [USER] alice; The super user must really be a *super* user. > ISTM that reasonable behavior for ALTER OWNER would include doing > surgery on the object's ACL to replace references to the old owner by > references to the new owner. [...] I'm about so submit a fix for "create database" so that ownership and acl would be fixed wrt to the owner of the database. This patch will include a function to switch grantor rights that might be of interest for the above purpose, as it may save you little time. I'll try to send the patch submission this week-end. > I think there are corner cases where the merging might produce > unintuitive results, but it couldn't be as spectacularly bad as > doing nothing is. I agree that these is work to do in the ACL area... As an additionnal suggestion, I noticed in my tests that nothing is really tested in the regression tests. It would be useful to have tests cases of acl with accesses allowed or forbidden, maybe with a systematic and exhaustive approach... It takes time to do that, but I think it would be useful so as to measure what is needed. Have a nice day, -- Fabien Coelho - coelho@cri.ensmp.fr
> REVOKE ALL ON TABLE foo FROM GRANTOR [USER] alice; > > The super user must really be a *super* user. I think we need a proper 'effective user' facility. At the moment, there's breakage if a super user creates a language, then drops their superuser privs, then the dump cannot be restored. All other failure cases also exist. eg if a gumby user creates a table in a schema, then has his permission to create tables in that schema revoked. The dump will be broken. The solution seems to me that we need to have an 'effective_user' SET option so that the superuser doing the restore can still create tables owned by the gumby, even though the gumby does not have privileges to do so. Chris
On Wed, 2004-06-02 at 18:44, Christopher Kings-Lynne wrote: > > REVOKE ALL ON TABLE foo FROM GRANTOR [USER] alice; > > > > The super user must really be a *super* user. > > I think we need a proper 'effective user' facility. > > At the moment, there's breakage if a super user creates a language, then > drops their superuser privs, then the dump cannot be restored. > > All other failure cases also exist. eg if a gumby user creates a table > in a schema, then has his permission to create tables in that schema > revoked. The dump will be broken. > > The solution seems to me that we need to have an 'effective_user' SET > option so that the superuser doing the restore can still create tables > owned by the gumby, even though the gumby does not have privileges to do > so. If I remember correctly, we already have this option. ALTER table OWNER to newowner; Perhaps pg_dump should just include; ALTER relation OWNER to originalowner; at the end of the dump, instead of connecting as the owner to restore it. > > Chris > > > ---------------------------(end of broadcast)--------------------------- > TIP 3: if posting/reading through Usenet, please send an appropriate > subscribe-nomail command to majordomo@postgresql.org so that your > message can get through to the mailing list cleanly Regards, John
Fabien COELHO <coelho@cri.ensmp.fr> writes:
> Due to how ACL are defined in SQL, I restate my suggestion that the super
> user should be able to change ANY right, including the GRANTOR field,
I'm unconvinced of this: that philosophy soon leads you into allowing
the superuser to create self-inconsistent sets of rights, such as rights
that flow from "nowhere" (i.e., are not traceable through an unbroken
chain to the original owner's grant options). The changes we have been
making recently are specifically designed to prevent such situations,
and I don't really wish to backtrack.
It's worth pointing out also that the superuser can always brute-force
things:
UPDATE pg_class SET relacl = '{ ... anything ...}' WHERE ...
and so we don't really need to provide escape hatches in GRANT/REVOKE
that are only useful to superusers. I think our concern with
GRANT/REVOKE should be to provide a self-consistent set of operations.
We're about there AFAICT with respect to GRANT/REVOKE themselves, but
ALTER OWNER as currently defined breaks it.
regards, tom lane
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> I think we need a proper 'effective user' facility.
> At the moment, there's breakage if a super user creates a language, then
> drops their superuser privs, then the dump cannot be restored.
The problem here is not with pg_dump; the problem is that dropping
privileges doesn't cascade to dropping objects that are dependent on
those privileges. AFAICS the SQL spec requires us to be able to do
the latter. If we're gonna invest work on fixing this, we ought to do
what the spec tells us to, not invent warts on the security model.
Tossing in expedient concepts like "effective user" is a great recipe
for creating unfixable security holes.
regards, tom lane
> The problem here is not with pg_dump; the problem is that dropping > privileges doesn't cascade to dropping objects that are dependent on > those privileges. AFAICS the SQL spec requires us to be able to do > the latter. The spec really requires that?? So basically we have RESTRICT and CASCADE on REVOKE? That seems pretty odd to me. What's so wrong about allowing someone to create tables for a while and then revoking their permission to do it from now on?? That's exactly what we do for databases at the moment, we have an 'OWNER' clause. And that's how I coded tablespaces to be dumped as well. Either way, our concept of a superuser surely isn't in the spec, so can we at least fix that problem? ie. we dump lanugages as default session_authorization and then ALTER LANGUAGE it to change it to the correct user? Same for CREATE OPERATOR CLASS and ALTER OP CLASS, and CREATE CAST commands for binary-compatible casts. (I do note that neither of those ALTER forms allows changing owner and there is no ALTER CAST at all - we'd need to add them). > If we're gonna invest work on fixing this, we ought to do > what the spec tells us to, not invent warts on the security model. Sure. Let's be honest though and admit that there are a lot of broken dumps out there at the moment. For me, I have to change all my users to superusers before dumping, then change them all back after a restore. This is because we did a security crackdown and tightened up on everyone's privileges... Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> The problem here is not with pg_dump; the problem is that dropping
>> privileges doesn't cascade to dropping objects that are dependent on
>> those privileges. AFAICS the SQL spec requires us to be able to do
>> the latter.
> The spec really requires that?? So basically we have RESTRICT and
> CASCADE on REVOKE?
Well, the spec doesn't have create permissions per se, but they do have
a "usage" right on domains, and they specify that revoking that results
in dropping objects:
7) For every abandoned domain descriptor DO, let S1.DN be the <domain name> of DO. The following
<dropdomain statement> is effectively executed without further Access Rule checking:
DROP DOMAIN S1.DN CASCADE
Similarly, revoking access to tables etc. results in physical changes to
views that reference those tables. So I think the idea is pretty clear.
regards, tom lane
> Well, the spec doesn't have create permissions per se, but they do have > a "usage" right on domains, and they specify that revoking that results > in dropping objects: > > 7) For every abandoned domain descriptor DO, let S1.DN be the > <domain name> of DO. The following <drop domain statement> is > effectively executed without further Access Rule checking: > > DROP DOMAIN S1.DN CASCADE Hmmm. Seems pretty harsh. But barring us implementing that (I don't see it happening for 7.5), just had an idea :) How about pg_dumpall dumps all users as superusers, and then changes them back to what they're supposed to be at the bottom of the script :) Easy :) Chris
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> How about pg_dumpall dumps all users as superusers, and then changes
> them back to what they're supposed to be at the bottom of the script :)
Leaves you in kind of a dangerous state if the script doesn't complete,
doesn't it?
Someone else suggested having pg_dump dump all objects without ownership
(so, on restore, they'd all initially be owned by the user running the
script, hopefully a superuser) and then doing ALTER OWNERs and GRANTs at
the bottom. This seems a little cleaner to me, though it's got the
problem that somebody would have to go off and implement the remaining
ALTER OWNER commands.
regards, tom lane
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> Someone else suggested having pg_dump dump all objects without ownership
>> (so, on restore, they'd all initially be owned by the user running the
>> script, hopefully a superuser) and then doing ALTER OWNERs and GRANTs at
>> the bottom.
> Actually, this would probably only be reasonable if you fixed the ACLs
> after an ALTER OWNER, like you proposed earlier.
I was envisioning pg_dump not issuing any GRANTs until after the
ALTER OWNER steps, so it really wouldn't matter whether ALTER OWNER did
anything to the ACL list; it'd still be NULL at that point anyway.
(I do, however, have every intention of fixing ALTER OWNER that way
before 7.5 freeze.)
BTW, is pg_dump careful about the order in which it issues GRANTs?
Specifically, what about being sure that chains of GRANT OPTIONs
are re-granted in a legal sequence? I don't recall any smarts in
the code about that...
regards, tom lane
> Someone else suggested having pg_dump dump all objects without ownership > (so, on restore, they'd all initially be owned by the user running the > script, hopefully a superuser) and then doing ALTER OWNERs and GRANTs at > the bottom. Actually, this would probably only be reasonable if you fixed the ACLs after an ALTER OWNER, like you proposed earlier. Chris
>>How about pg_dumpall dumps all users as superusers, and then changes >>them back to what they're supposed to be at the bottom of the script :) > > > Leaves you in kind of a dangerous state if the script doesn't complete, > doesn't it? If your script doesn't complete, it can leave you in all sorts of bad states, but I guess this is a reasonably bad one. > Someone else suggested having pg_dump dump all objects without ownership > (so, on restore, they'd all initially be owned by the user running the > script, hopefully a superuser) and then doing ALTER OWNERs and GRANTs at > the bottom. This seems a little cleaner to me, though it's got the > problem that somebody would have to go off and implement the remaining > ALTER OWNER commands. I guess that's me... I'll have a crack at it, but don't let that stop anyone from piping up and helping me :) Chris
On Wed, Jun 02, 2004 at 10:54:36PM +0800, Christopher Kings-Lynne wrote: > >Well, the spec doesn't have create permissions per se, but they do have > >a "usage" right on domains, and they specify that revoking that results > >in dropping objects: > > > > 7) For every abandoned domain descriptor DO, let S1.DN be the > > <domain name> of DO. The following <drop domain statement> is > > effectively executed without further Access Rule checking: > > > > DROP DOMAIN S1.DN CASCADE > > Hmmm. Seems pretty harsh. But barring us implementing that (I don't > see it happening for 7.5), just had an idea :) > > How about pg_dumpall dumps all users as superusers, and then changes > them back to what they're supposed to be at the bottom of the script :) Huh, how about a GUC var, say "creating_user", which would make objects created by the superuser as created by whoever is mentioned there? The dump connects only as superuser and changes creating_user as needed. Not a pretty idea, but weren't you looking for kludges? :-) -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "La persona que no quería pecar / estaba obligada a sentarse en duras y empinadas sillas / desprovistas, por cierto de blandos atenuantes" (Patricio Vogel)
Dear Tom,
> > Due to how ACL are defined in SQL, I restate my suggestion that the super
> > user should be able to change ANY right, including the GRANTOR field,
>
> I'm unconvinced of this: that philosophy soon leads you into allowing
> the superuser to create self-inconsistent sets of rights, such as rights
> [...]
I understand that you want to keep the acl state always consistent, so as
to simplify access checks that can rely on the acl to be consistent, and
to avoid some transitive closure or whatever while checking some access.
That is reasonnable enough, for performance.
However there is no necessary contradiction, as a "with grantor" can be
interpreted in two ways:
(1) execute the grant/revoke with the specified id.
(2) include this acl whatever the consistency.
Under interpretation (1), this is just a short hand for a set session
authorization xxx ; revoke/grant ; back to initial;
Now you might argue that the convinience is not necessarily needed, as the
su can set the session authorization as it wishes (I guess). From my
little practice, it would not look that bad to have such a simple and
quick syntax for such tasks.
Moreover, ROLES have such a limited "WITH GRANTOR" syntax, so the
parser may need it anyway someday.
> It's worth pointing out also that the superuser can always brute-force
> things: UPDATE pg_class SET relacl = '{ ... anything ...}' WHERE ...
Sure, but the syntax is not an easy one. As a super user, I wish I could
really fix rights with something better than such a syntax, as it is
really ugly and error prone.
> I think our concern with GRANT/REVOKE should be to provide a
> self-consistent set of operations. We're about there AFAICT with respect
> to GRANT/REVOKE themselves, but ALTER OWNER as currently defined breaks
> it.
I agree that the GRANT/REVOKE syntax as basically self consistent wrt
basic users, but it is not necessarily the case wrt a super-user.
Have a nice day,
--
Fabien Coelho - coelho@cri.ensmp.fr