Re: [PATCH] Largeobject access controls

The attached patch is revised from the previous revision at the following points:

- The "largeobject_compat_acl" is renamed to "largeobject_check_acl".
Its default is on, and turning it off means the largeobject stuff
performs in compatible mode for the v8.4.x or prior releases.
- Notification messages were eliminated at the compatible mode.
It always allows to bypass ACL checks without any warnings.

Thanks,

$ diffstat sepgsql-02-blob-8.5devel-r2328.patch.gz
doc/src/sgml/config.sgml | 28 ++
doc/src/sgml/ref/allfiles.sgml | 1
doc/src/sgml/ref/alter_large_object.sgml | 75 +++++
doc/src/sgml/ref/grant.sgml | 8
doc/src/sgml/ref/revoke.sgml | 6
doc/src/sgml/reference.sgml | 1
src/backend/catalog/Makefile | 6
src/backend/catalog/aclchk.c | 249 ++++++++++++++++++
src/backend/catalog/dependency.c | 14 +
src/backend/catalog/pg_largeobject.c | 357 +++++++!!!!!!!!!!!!!!!!!!
src/backend/catalog/pg_shdepend.c | 8
src/backend/commands/alter.c | 5
src/backend/commands/comment.c | 11
src/backend/commands/tablecmds.c | 1
src/backend/libpq/be-fsstubs.c | 49 +--
src/backend/parser/gram.y | 20 +
src/backend/storage/large_object/inv_api.c | 115 ++---!!!
src/backend/tcop/utility.c | 3
src/backend/utils/adt/acl.c | 5
src/backend/utils/cache/syscache.c | 13
src/backend/utils/misc/guc.c | 10
src/backend/utils/misc/postgresql.conf.sample | 1
src/bin/psql/large_obj.c | 10
src/include/catalog/dependency.h | 1
src/include/catalog/indexing.h | 3
src/include/catalog/pg_largeobject_metadata.h | 67 ++++
src/include/nodes/parsenodes.h | 1
src/include/utils/acl.h | 6
src/include/utils/syscache.h | 1
src/test/regress/expected/privileges.out | 206 +++++++++++++++
src/test/regress/expected/sanity_check.out | 3
src/test/regress/sql/privileges.sql | 85 ++++++
32 files changed, 976 insertions(+), 77 deletions(-), 316 modifications(!)

KaiGai Kohei wrote:
> Jaime Casanova wrote:
>> 2009/9/23 KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>:
>>> Jaime,
>>>
>>> KaiGai Kohei wrote:
>>> | > ALTER LARGE OBJECT is working, but now that we can change the owner of
>>> | > a LO we should be able to see who the actual owner is... i mean we
>>> | > should add an owner column in \dl for psql (maybe \dl+) and maybe an
>>> | > lo_owner() function.
>>> |
>>> | I would like to buy your idea at the revised patch.
>>>
>>> Now we don't have xxx_owner() function for other database objects,
>>> such as tables, procedures and so on.
>> good point, but we have has_xxxxxx_privileges() family of functions
>> but i think we can add them later if needed...
>
> Yes, the has_xxxxxx_privileges() family should be added later or soon.
> Anyway, what I wanted to say is we have no special functions to show
> owner of the database objects.
>
>>> Jaime Casanova wrote:
>>>>> Do you think the "largeobject_compat_acl" is a meaningful name, instead?
>>>> maybe something like "largeobject_security_controls"?
>>> It is important to contain a term of "compat" which means compatible,
>>> because this GUC does not disables all the security checks.
>>> The v8.4.x checks superuser privilege on using lo_import()/lo_export().
>>> It is also checked in this patch, even if the GUC is turned on.
>>>
>>> The purpose of the GUC is to provide compatible behavior, not to provide
>>> a stuff to turn on/off all the security features in largeobjects.
>>>
>> that's why the section in the postgresql.conf is called
>> "VERSION/PLATFORM COMPATIBILITY" and the subsection "Previous
>> PostgreSQL Versions" we have other compatibilty GUC and no ones of
>> those has the term "compat" in it...
>
> Indeed, I put the "largeobject_compat_acl" in the compatibility section,
> but no other GUCs have "compat" prefix/suffix. It seems to me fair enough.
>
>>> So, I still prefer the "largeobject_compat_acl".
>> maybe "enhanced_largeobjects_checks" or "enhanced_lo_checks"
>> or make the GUC an enum and name it "largeobject_control_checks" with
>> posible values "basic" and "enhanced"
>
> But, isn't the "enhanced" tumid expression? It just applies native database
> privilege mechanism on largeobjects, as if it does on other objects.
>
> An other alternative is "largeobject_check_acl". What's your feeling?
>
> Thanks,

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