[PATCH] Largeobject Access Controls (r2460) - Mailing list pgsql-hackers

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@ak.jp.nec.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@ak.jp.nec.com>