Thread: Sorting out acl fixes

Sorting out acl fixes

From
Christopher Kings-Lynne
Date:
I'm playing around trying to fix the problem where ACLs enter an illegal 
state when you change the owner of a table, say.

This is the current situation:

test=# create user gumby;
CREATE USER
test=# create user other;
CREATE USER
test=# create table test (a int4);
CREATE TABLE
test=# grant select on test to other;
GRANT
test=# alter table test owner to gumby;
ALTER TABLE
test=# \dp               Access privileges for database "test" Schema | Name | Type  |             Access privileges
--------+------+-------+------------------------------------------- public | test | table |
{chriskl=arwdRxt/chriskl,other=r/chriskl}
(1 row)

test=# \dt       List of relations Schema | Name | Type  | Owner
--------+------+-------+------- public | test | table | gumby
(1 row)

Now, the chriskl user's old owner privs are still there, but are granted 
by chriskl still.  The initial fix would be to modify the acl to be like 
this after owner change:

{chriskl=arwdRxt/gumby,other=r/chriskl}

Perhaps even:

{gumby=arwdRxt/chriskl,other=r/gumby}

But there's a few other options:

1. Should we make the owner aclitem NEVER appear in the acl list?  ie. 
when we do the first grant on an object, we don't put in a default acl 
for the owner.  Instead we special case the aclcheck to always allow the 
owner full privilieges?  Also, if the first grant was 'select' for the 
'other' user, and then we changed the owner to the 'other' user, should 
we erase the 'other' user's aclitem?

2. Should we just whenever the owner is changed, change all grantors 
that are the old owner to the new owner?

3. Should we do (2) but only when the grantor is the old owner and the 
grantee is the old owner?

Is there a logical way of determining which of these is correct?

Chris



Re: Sorting out acl fixes

From
Christopher Kings-Lynne
Date:
> 1. Should we make the owner aclitem NEVER appear in the acl list?  ie. 
> when we do the first grant on an object, we don't put in a default acl 
> for the owner.  Instead we special case the aclcheck to always allow the 
> owner full privilieges?  Also, if the first grant was 'select' for the 
> 'other' user, and then we changed the owner to the 'other' user, should 
> we erase the 'other' user's aclitem?

I forgot to mention - under this schema grants/revokes to the owner 
become no-ops.

Chris



Re: Sorting out acl fixes

From
Tom Lane
Date:
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
> 1. Should we make the owner aclitem NEVER appear in the acl list?  ie. 
> when we do the first grant on an object, we don't put in a default acl 
> for the owner.  Instead we special case the aclcheck to always allow the 
> owner full privilieges?

That would be *entirely* unacceptable, since it would for example
prevent the owner from making the table read-only to himself.
I think that's an important feature to preserve.

The solution I had in mind was for ALTER OWNER to run through the ACL
and replace the old owner ID with the new one wherever the old one
appears, in both grantor and grantee positions.  So in your example{chriskl=arwdRxt/chriskl,other=r/chriskl}
becomes{gumby=arwdRxt/gumby,other=r/gumby}

You could skip doing this when the ACL is null of course, since the
default assumption about its contents will change in just the same way.

The minimum you could safely do is make this replacement in every
place where the old owner appears as a grantor, but leave grantees
alone.  This rule produces
{chriskl=arwdRxt/gumby,other=r/gumby}

Now IMHO this would be an utterly bizarre behavior ... but it would
at least produce a legal, consistent state of the ACL, in which every
granted right is traceable back to the new owner's implicit grant
options.  If the new owner gumby didn't want chriskl to have those
permissions, he'd at least be able to revoke 'em.  One would think
though that the first alternative is much more likely to be what
people would expect.

>     {chriskl=arwdRxt/gumby,other=r/chriskl}

When gumby is the owner, this is an illegal ACL: chriskl is granting
rights he doesn't have grant option for.
        regards, tom lane


Re: Sorting out acl fixes

From
Christopher Kings-Lynne
Date:
> The solution I had in mind was for ALTER OWNER to run through the ACL
> and replace the old owner ID with the new one wherever the old one
> appears, in both grantor and grantee positions.  So in your example
>     {chriskl=arwdRxt/chriskl,other=r/chriskl}
> becomes
>     {gumby=arwdRxt/gumby,other=r/gumby}
> 
> You could skip doing this when the ACL is null of course, since the
> default assumption about its contents will change in just the same way.

What about fixing existing bad acls?  I can't figure out a grant or 
revoke statement to do it?  Do I have to update to set the relacl to 
null and then re-run the fixed set of grants?

Chris



Re: Sorting out acl fixes

From
Tom Lane
Date:
Christopher Kings-Lynne <chriskl@familyhealth.com.au> writes:
>> The solution I had in mind was for ALTER OWNER to run through the ACL
>> and replace the old owner ID with the new one wherever the old one
>> appears, in both grantor and grantee positions.

> What about fixing existing bad acls?

When did that get to be part of the requirements?  I don't even know
who you expect to do this (backend? pg_dump? user?) or at what level
you think the fixing should happen (GRANT/REVOKE?  UPDATE pg_class
SET relacl = fixme(relacl)?  direct hacking of the ACL array?).  To
say nothing of the semantic problems of deciding what an invalid
ACL is really supposed to mean.

I'll be satisfied if ALTER OWNER does not transform a valid
configuration into an invalid one.  Right now it fails to meet that
minimal requirement.  Considering we are weeks past feature freeze,
I don't want to get into inventing a magic wand that can fix existing
breakage automatically.
        regards, tom lane


Re: Sorting out acl fixes

From
Christopher Kings-Lynne
Date:
> When did that get to be part of the requirements?  I don't even know
> who you expect to do this (backend? pg_dump? user?) or at what level
> you think the fixing should happen (GRANT/REVOKE?  UPDATE pg_class
> SET relacl = fixme(relacl)?  direct hacking of the ACL array?).  To
> say nothing of the semantic problems of deciding what an invalid
> ACL is really supposed to mean.

I was referring to fixing my own database that is full of these acls 
that dump incorrectly - perhaps you don't give me enough credit.  I'm 
thinking that if I can find a watertight way of fixing it at pg_dump 
time I should, for pre 7.5 databases.  Should I?

Chris