Re: [PATCH] Largeobject Access Controls (r2432)

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)

--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157