Re: Simplify ACL handling for large objects and removal of superuser() checks

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Vaishnavi Prabakaran <vaishnaviprabakaran(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Subject: Re: Simplify ACL handling for large objects and removal of superuser() checks
Date: 2017-09-19 07:12:59
Message-ID: CAB7nPqT2aABq9OgXp1rWFB=tdJ3SgqVRu4QpBeEQitp_eK-Shg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Tue, Sep 19, 2017 at 1:24 PM, Vaishnavi Prabakaran
<vaishnaviprabakaran(at)gmail(dot)com> wrote:
> On Mon, Aug 14, 2017, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote:
>>Attached is a set of 3 patches:
>
> I tried to review the patch and firstly patch applies cleanly without any
> noise.

Thanks for the review, Vaishnavi.

>>- 0001 removes ALLOW_DANGEROUS_LO_FUNCTIONS
>
> I think it is better to remove "ALLOW_DANGEROUS_LO_FUNCTIONS" from
> pg_config_manual.h as well.

Indeed. Fixed.

>>@@ -163,22 +150,16 @@ lo_read(int fd, char *buf, int len)
>> ....
>> + if ((lobj->flags & IFS_RDLOCK) == 0)
>>+ ereport(ERROR,
>>+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>>+ errmsg("large object descriptor %d was not opened for reading",
>>+ fd)));
>
> Do we ever reach this error? Because per my understanding

This error can be reached, and it is part of the regression tests. One
query which passed previously is now failing:
+SELECT loread(lo_open(1001, x'20000'::int), 32); -- fail, wrong mode
+ERROR: large object descriptor 0 was not opened for reading

> I think documented behavior is more appropriate and post ACL
> checks for select and update permissions in inv_open(), IFS_RDLOCK flag
> should be set regardless of mode specified.

OK, so that's one vote for keeping the existing API behavior with
write implying read. Thanks for the input. At least I can see no
complains about patches 1 and 2 :)
--
Michael

Attachment Content-Type Size
0001-Remove-ALLOW_DANGEROUS_LO_FUNCTIONS-for-LO-related-s.patch application/octet-stream 2.3 KB
0002-Replace-superuser-checks-of-large-object-import-expo.patch application/octet-stream 4.5 KB
0003-Move-ACL-checks-for-large-objects-when-opening-them.patch application/octet-stream 18.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2017-09-19 07:31:32 Re: Block level parallel vacuum WIP
Previous Message Thomas Munro 2017-09-19 06:33:34 Re: Block level parallel vacuum WIP