Re: [PATCH] Largeobject access controls - Mailing list pgsql-hackers

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