Re: DROP OWNED BY fails to clean out pg_init_privs grants

On Fri, May 24, 2024 at 4:00 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Oh! That does seem like it would make what I said wrong, but how would
> > it even know who the original owner was? Shouldn't we be recreating
> > the object with the owner it had at dump time?
>
> Keep in mind that the whole point here is for the pg_dump script to
> just say "CREATE EXTENSION foo", not to mess with the individual
> objects therein.  So the objects are (probably) going to be owned by
> the user that issued CREATE EXTENSION.
>
> In the original conception, that was the end of it: what you got for
> the member objects was whatever state CREATE EXTENSION left behind.
> The idea of pg_init_privs is to support dump/reload of subsequent
> manual alterations of privileges for extension-created objects.
> I'm not, at this point, 100% certain that that's a fully realizable
> goal.  But I definitely think it's insane to expect that to work
> without also tracking changes in the ownership of said objects.
>
> Maybe forbidding ALTER OWNER on extension-owned objects isn't
> such a bad idea?

I think the root of my confusion, or at least one of the roots of my
confusion, was whether we were talking about altering the extension or
the objects within the extension. If somebody creates an extension as
user nitin and afterwards changes the ownership to user lucia, then I
would hope for the same pg_init_privs contents as if user lucia had
created the extension in the first place. I think that might be hard
to achieve in complex cases, if the extension changes the ownership of
some objects during the creation process, or if some ownership has
been changed afterward. But in the simple case where nothing like that
happens, it sounds possible. If, on the other hand, somebody creates
the extension and modifies the ownership of an object in the
extension, then I agree that pg_init_privs shouldn't be updated.
However, it also seems pretty clear that pg_init_privs is not
recording enough state for pg_dump to mimic the ownership change,
because it only records the original privileges, not the original
ownership.

So, can we recreate the original privileges without recreating the
original ownership? It doesn't really seem like this is going to work
in general. If nitin creates the extension and grants privileges to
lucia, and then the ownership of the extension is changed to swara,
then nitin is no longer a valid grantor. Even if we could fix that by
modifying the grants to substitute swara for nitin, that could create
impermissible circularities in the permissions graph. Maybe there are
some scenarios where we can fix things up, but it doesn't seem
possible in general.

In a perfect world, the fix here is probably to have pg_init_privs or
something similar record the ownership as well as the permissions, but
that is not back-patchable and nobody's on the hook to fix up somebody
else's feature. So what do we do? Imposing a constraint that you can't
change the ownership of an extension-owned object, as you propose,
seems fairly likely to break a few existing extension scripts, and
also won't fix existing instances that are already broken, but maybe
it's still worth considering if we don't have a better idea. Another
line of thinking might be to somehow nerf pg_init_privs, or the use of
it, so that we don't even try to cover this case e.g. if the ownership
is changed, we nuke the pg_init_privs entry or resnap it to the
current state, and the dump fails to recreate the original state but
we get out from under that by declaring the case unsupported (with
appropriate documentation changes, hopefully). pg_init_privs seems
adequate for normal system catalog entries where the ownership
shouldn't ever change from the bootstrap superuser, but extensions
seem like they require more infrastructure.

I think the only thing we absolutely have to fix here is the dangling
ACL references. Those are a hazard, because the OID could be reused by
an unrelated role, which seems like it could even give rise to
security concerns. Making the whole pg_init_privs system actually work
for this case case would be even better, but that seems to require
filling in at least one major hole the original design, which is a lot
to ask for (1) a back-patched fix or (2) a patch that someone who is
not the original author has to try to write and be responsible for
despite not being the one who created the problem.

--
Robert Haas
EDB: http://www.enterprisedb.com