BUG #17157: authorizaiton of dict_int and bloom extention

On Tue, Aug 24, 2021 at 4:19 PM PG Bug reporting form <
noreply@postgresql.org> wrote:

The following bug has been logged on the website:

Bug reference: 17157
Logged by: Lily Zhang
Email address: bjzhangl@cn.ibm.com
PostgreSQL version: 13.3
Operating system: os390x
Description:

1. Since dict_int is trusted, we create extension of dict_int with normal
user. But when alter maxlen of intdict, it reports error. This is the
detail.
```
admin=> create extension dict_int;
CREATE EXTENSION
admin=> ALTER TEXT SEARCH DICTIONARY intdict (MAXLEN = 7);
ERROR: must be owner of text search dictionary intdict
```
2. Since pg13 supports trusted extension, we make bloom trusted by changing
control file. Everything runs well except drop extension with normal user
who creates this extension.
```
test=> create extension bloom;
CREATE EXTENSION
test=> drop extension bloom;
ERROR: must be superuser to drop access methods
```

Hi, here are some of my understanding, hope it can help you:

For (1), if we set an extension "trust", the database will execute the
"create" action as a superuser, so the owner of the created object is the
superuser. I think this is a "feature", not a "bug".

For (2), it was already been fixed in commit:
b1d32d3e3230f00b5baba08f75b4f665c7d6dac6.

--
There is no royal road to learning.
HighGo Software Co.

<div class="socmaildefaultfont" dir="ltr" style="font-family:Arial, Helvetica, sans-serif;font-size:10pt" ><div dir="ltr" >Got it! Thanks!</div>
<div dir="ltr" >My question:</div>
<div dir="ltr" >1. If an ordinary user create the extension, my understanding is that the object created in the extension should be the user who create the extension?</div>
<div dir="ltr" >2. Will the fix for bloom be applied on pg13?</div>
<div dir="ltr" >&nbsp;</div>
<div dir="ltr" >&nbsp;</div>
<div dir="ltr" >&nbsp;</div>
<blockquote data-history-content-modified="1" dir="ltr" style="border-left:solid #aaaaaa 2px; margin-left:5px; padding-left:5px; direction:ltr; margin-right:0px" >----- Original message -----<br>From: "Neil Chen" &lt;carpenter.nail.cz@gmail.com&gt;<br>To: bjzhangl@cn.ibm.com, pgsql-bugs@lists.postgresql.org<br>Cc:<br>Subject: [EXTERNAL] Re: BUG #17157: authorizaiton of dict_int and bloom extention<br>Date: Tue, Aug 24, 2021 4:38 PM<br>&nbsp;<br>
<div dir="ltr" ><div dir="ltr" >&nbsp;</div>&nbsp;

<div><div dir="ltr" >On Tue, Aug 24, 2021 at 4:19 PM PG Bug reporting form &lt;<a href="mailto:noreply@postgresql.org" target="_blank" >noreply@postgresql.org</a>&gt; wrote:</div>
<blockquote style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex" >The following bug has been logged on the website:<br><br>Bug reference:&nbsp; &nbsp; &nbsp; 17157<br>Logged by:&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; Lily Zhang<br>Email address:&nbsp; &nbsp; &nbsp; <a href="mailto:bjzhangl@cn.ibm.com" target="_blank" >bjzhangl@cn.ibm.com</a><br>PostgreSQL version: 13.3<br>Operating system:&nbsp; &nbsp;os390x<br>Description:&nbsp; &nbsp; &nbsp; &nbsp;<br><br>1. Since dict_int is trusted, we create extension of dict_int with normal<br>user. But when alter maxlen of intdict, it reports error. This is the<br>detail.<br>```<br>admin=&gt; create extension dict_int;<br>CREATE EXTENSION<br>admin=&gt; ALTER TEXT SEARCH DICTIONARY intdict (MAXLEN = 7);<br>ERROR:&nbsp; must be owner of text search dictionary intdict<br>```<br>2. Since pg13 supports trusted extension, we make bloom trusted by changing<br>control file. Everything runs well except drop extension with normal user<br>who creates this extension.<br>```<br>test=&gt; create extension bloom;<br>CREATE EXTENSION<br>test=&gt; drop extension bloom;<br>ERROR:&nbsp; must be superuser to drop access methods<br>```<br>&nbsp;</blockquote></div>
<div>&nbsp;</div>Hi, here are some of my understanding, hope it can help you:

<div>&nbsp;</div>
<div>For (1), if we set an extension "trust", the database will execute the "create" action as a superuser, so the owner of the created object is the superuser. I think this is a "feature", not a "bug".
<div>&nbsp;</div>
<div>For (2), it was already been fixed in commit:&nbsp;<span style="color:rgb(36,41,46);font-family:ui-monospace,SFMono-Regular,&quot;SF Mono&quot;,Menlo,Consolas,&quot;Liberation Mono&quot;,monospace;font-size:12px;text-align:right;white-space:nowrap" >b1d32d3e3230f00b5baba08f75b4f665c7d6dac6.</span>
<div>&nbsp;</div>--

<div dir="ltr" ><div dir="ltr" >There is no royal road to learning.
<div>HighGo Software Co.</div></div></div></div></div></div></blockquote>
<div dir="ltr" >&nbsp;</div></div><BR>
<BR>

Hi, IMHO, these problems are not data errors or crashes, but rules that are
inconsistent with expectations. As for what is the "correct" expectation, I
think we need more opinions from other hackers.

On Wed, Aug 25, 2021 at 11:48 AM 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 the user who create the extension?
2. Will the fix for bloom be applied on pg13?

--
There is no royal road to learning.
HighGo Software Co.

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 the user 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/

On Thu, Aug 26, 2021 at 12:22 AM Masahiko Sawada <sawada.mshk@gmail.com>
wrote:

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.

Yeah, I said much the same thing on the adjacent thread on this topic.

/messages/by-id/CAKFQuwaEiW0QbDHS0qxmGRcqZQw6O_ieV-14CWY1QG2k0zaWBw@mail.gmail.com

David J.