Re: BUG #17157: authorizaiton of dict_int and bloom extention

w

On Wed, Aug 25, 2021 at 12:48 PM Li EF Zhang <bjzhangl@cn.ibm.com> wrote:
>
> Got it! Thanks!
> My question:
> 1. If an ordinary user create the extension, my understanding is that the object created in the extension should be
theuser who create the extension? 

The doc says:

If the extension is marked trusted in its control file, then it can be
installed by any user who has CREATE privilege on the current
database. In this case the extension object itself will be owned by
the calling user, but the contained objects will be owned by the
bootstrap superuser (unless the extension's script explicitly assigns
them to the calling user).

So in your case, the extension dict_int itself is owned by the user
who created it (i.g., executed CREATE EXTENSION) whereas the objects
of the extension such as intdict are owned by the superuser.

> 2. Will the fix for bloom be applied on pg13?

I think that the commit b1d32d3e3 won't be backpatched to PG13 but
this behavior seems like a bug to me. I would expect that "DROP
EXTENSION trusted-bloom" by non-superuser owner succeeds if it’s a
trusted extension. (“trusted-bloom” here means “bloom” extension you
made trusted by changing the control file)

Looking at the commit b1d32d3e3, the fact that that "DROP EXTENSION
trusted-bloom” by non-superuser can drop the access method belonging
to the extension seems like a side effect of this commit.

Previously, since we did the superuser check in
RemoveAccessMethodById() that drops an access method object from the
catalog, even if a non-superuser reaches this function (e.g., when
dropping a trusted extension), it ended up an error. By this commit,
we removed this function along with the superuser check and used
DropObjectById() instead for unifying similar purpose functions.
Therefore, "DROP EXTENSION trusted-bloom” by non-superuser succeeds.

I’m not sure whether the removal of superuser check by this commit is
intentional, but IIUC it’s okay the fact that we don’t do superuser
check at DropObjectById() or RemoveAccessMethodById(). We do the
superuser check at a higher layer (see RemoveObjects() and
check_object_ownership()). And an access method always is owned by
superuser even if it belongs to a trusted extension.  The only path
where a non-super user attempts to drop an access method without the
superuser check is when dropping the trusted extension by
non-superuser owner, which is a legitimate operation.

If my understanding is correct, I wonder if we can remove the
superuser check from RemoveAccessMethodById() in PG13 (see the
attached patch), making “DROP EXTENSION trusted-bloom” by
non-superuser owner work.

On the other hand if not correct, that is, if there is another path
where non-superuser attempts to drop an access method by calling
DropObjectById() without the prior superuser check, I think we should
fix HEAD and PG14 by bringing RemoveAccessMethodById() back to the
code.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/