Re: [PATCH] Largeobject access controls

The attached patch is an update of largeobject access controls.

It adds a new guc variable to turn on/off compatible behavior in
largeobejct access controls.

largeobject_compat_dac = [on | off] (default: off)

If the variable is turned on, all the new access control features
on largeobjects are bypassed, as if v8.4.x or prior did.
(Note that lo_import()/lo_export() checks client's superuser privilege
independent from the guc setting, because it has been checked prior to
the v8.4.x.)

--------------------------------
[kaigai(at)saba blob]$ psql postgres
psql (8.5devel)
Type "help" for help.
postgres=# SET SESSION AUTHORIZATION ymj;
SET
postgres=> SELECT loread(lo_open(1234, x'40000'::int), 100);
ERROR: permission denied for largeobject 1234

postgres=> \c -
psql (8.5devel)
You are now connected to database "postgres".
postgres=# SET largeobject_compat_dac TO on; -- set compatible largeobject
SET
postgres=# SET SESSION AUTHORIZATION ymj;
SET
postgres=> SELECT loread(lo_open(1234, x'40000'::int), 100);
loread
--------
\x
(1 row)

KaiGai Kohei wrote:
> Robert Haas wrote:
>> 2009/9/3 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>>> KaiGai Kohei wrote:
>>>> Alvaro Herrera wrote:
>>>>> Tom Lane wrote:
>>>>>> KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp> writes:
>>>>>>> BTW, currently, the default ACL of largeobject allows anything for owner
>>>>>>> and nothing for world. Do you have any comment for the default behavior?
>>>>>> Mph. I think the backlash will be too great. You have to leave the
>>>>>> default behavior the same as it is now, ie, world access.
>>>>> BTW as a default it is pretty bad. Should we have a GUC var to set the
>>>>> default LO permissions?
>>>> It seems to me a reasonable idea in direction.
>>>> However, it might be better to add a GUC variable to turn on/off LO
>>>> permission feature, not only default permissions.
>>>> It allows us to control whether the privilege mechanism should perform
>>>> in backward compatible, or not.
>>> Now we have two options:
>>>
>>> 1. A GUC variable to set the default largeobject permissions.
>>>
>>> SET largeobject_default_acl = [ ro | rw | none ]
>>> - ro : read-only
>>> - rw : read-writable
>>> - none : nothing
>>>
>>> It can control the default acl which is applied when NULL is set on
>>> the pg_largeobject_meta.lomacl. However, lo_unlink() checks ownership
>>> on the largeobject, so it is not enough compatible with v8.4.x or prior.
>>>
>>> 2. A GUC veriable to turn on/off largeobject permissions.
>>>
>>> SET largeobject_compat_dac = [ on | off ]
>>>
>>> When the variable is turned on, largeobject dac permission check is
>>> not applied as the v8.4.x or prior version did. So, the variable is
>>> named "compat" which means compatible behavior.
>>> It also does not check ownership on lo_unlink().
>>>
>>> My preference is the second approach.
>>>
>>> What's your opinion?
>> I prefer the first. There's little harm in letting users set the
>> default permissions for themselves, but a GUC that controls
>> system-wide behavior will have to be something only superusers can
>> money with, and that seems like it will reduce usability.
>
> I don't intend to allow session users to set up their default acl.
> Both operation should be always system-wide.
>
> If a normal user can change the default acl, it is also equivalent
> he can grant all permissions to public on all the largeobject with
> its acl being NULL.
> Note that PostgreSQL does not set up a certain ACLs on its creation
> time, so NULL is assigned. The default ACL means an alternarive set
> of permissions, when it is NULL.
>
>
>> Why couldn't lo_unlink() just check write privilege?
>
> Because it is inconsistent behavior.
> PostgreSQL checks its ownership on dropping a certain database objects,
> such as tabls, procedures and so on.
> It seems to me quite strange, if only largeobject checks writer permission
> to drop itself.
>
> Thanks,

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>