[PATCH] Largeobject Access Controls (r2460)

The attached patch is an updated revision of Largeobject Access Controls.

List of updates:
* rebased to the latest CVS HEAD

* SGML documentation fixes:
- The future version number was replaced as:
"In the 8.4.x series and earlier release, ..."
- Other strange English representations and typoes were fixed.

* Fixed OID conflicts in system catalog definition.
The new TOAST relation for pg_trigger used same OID number with
pg_largeobject_metadata.

* Fixed incorrect error code in pg_largeobject_ownercheck().
It raised _UNDEFINED_FUNCTION, but should be _UNDEFINED_OBJECT.

* Renamed GUC parameter to "lo_compat_privileges" from
"large_object_privilege_checks".

* pg_largeobject_aclmask() and pg_largeobject_aclcheck(), not
take an argument of snapshot, were removed.
Currently, the caller provide an appropriate snapshot them.

Thanks,

Jaime Casanova wrote:
> 2009/11/12 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>> The attached patch is a revised version of large object privileges
>> based on the feedbacks at the last commit fest.
>>
>
> please update the patch, it's giving an error when 'make check' is
> trying to "create template1" in initdb:
>
> creating template1 database in
> /home/postgres/pg_releases/pgsql/src/test/regress/./tmp_check/data/base/1
> ... TRAP: FailedAssertion("!(reln->md_fd[forkNum] == ((void *)0))",
> File: "md.c", Line: 254)
> child process was terminated by signal 6: Aborted
>
>
> Meanwhile, i will make some comments:
>
> This manual will be specific for 8.5 so i think all mentions to the
> version should be removed
> for example;
>
> + In this version, a large object has OID of its owner, access permissions
> + and OID of the largeobject itself.
>
> + Prior to the version 8.5.x release does not have any
> privilege checks on
> + large objects.
>
> the parameter name (large_object_privilege_checks) is confusing enough
> that we have to make this statements to clarify... let's think in a
> better less confuse name
> + Please note that it is not equivalent to disable all the security
> + checks corresponding to large objects.
> + For example, the <literal>lo_import()</literal> and
> + <literal>lo_export</literal> need superuser privileges independent
> + from this setting as prior versions were doing.
>
> this will not be off by default? it should be for compatibility
> reasons... i remember there was a discussion about that but can't
> remember the conclusion
>
> Mmm... One of them? the first?
> + The one is <literal>SELECT</literal>.
>
> + Even if a transaction modified access rights and commit it, it is
> + not invisible from other transaction which already opened the large
> + object.
>
> The other one, the second
> + The other is <literal>UPDATE</literal>.
>
>
> it seems there is an "are" that should not be there :)
> +
> + These functions are originally requires database superuser privilege,
> + and it allows to bypass the default database privilege checks, so
> + we don't need to check an obvious test twice.
>
> a typo, obviously
> + For largeo bjects, this privilege also allows to read from
> + the target large object.
>
>
> We have two versions of these functions one that a recieve an SnapShot
> parameter and other that don't...
> what is the rationale of this? AFAIU, the one that doesn't receive
> SnapShot is calling the other one with SnapShotNow, can't we simply
> call it that way and drop the version of the functions that doesn't
> have that parameter?
> + pg_largeobject_aclmask(Oid lobj_oid, Oid roleid,
> + AclMode mask, AclMaskHow how)
>
> + pg_largeobject_aclcheck(Oid lobj_oid, Oid roleid, AclMode mode)
>

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