Re: Largeobject Access Controls (r2460)

KaiGai Kohei wrote:
> Takahiro Itagaki wrote:
>> KaiGai Kohei <kaigai@ak.jp.nec.com> wrote:
>>
>>> Tom Lane wrote:
>>>> Takahiro Itagaki <itagaki.takahiro@oss.ntt.co.jp> writes:
>>>>>    <structname>pg_largeobject</structname> should not be readable by the
>>>>>    public, since the catalog contains data in large objects of all users.
>>>> This is going to be a problem, because it will break applications that
>>>> expect to be able to read pg_largeobject.  Like, say, pg_dump.
>>> Is it a right behavior, even if we have permission checks on large objects?
>> Can we use column-level access control here?
>>
>>    REVOKE ALL ON pg_largeobject FROM PUBLIC;
>> => GRANT SELECT (loid) ON pg_largeobject TO PUBLIC;
> 
> Indeed, it seems to me reasonable.
> 
>> We use "SELECT loid FROM pg_largeobject LIMIT 1" in pg_dump. We could
>> replace pg_largeobject_metadata instead if we try to fix only pg_dump,
>> but it's no wonder that any other user applications use such queries.
>> I think to allow reading loid is a balanced solution.
> 
> Right, I also remind this query has to be fixed up by other reason right now.
> If all the large objects are empty, this query can return nothing, even if
> large object entries are in pg_largeobject_metadata.
> 
> Please wait for a while.

The attached patch fixes these matters.

* It adds "GRANT SELECT (loid) ON pg_largeobject TO PUBLIC;" during initdb
  phase to resolve the matter pointed out.

* A few queries in pg_dump were fixed to select pg_largeobject_metadata
  instead of pg_largeobject. If a dumpable large obejct is empty (it means
  no page frames are on pg_largeobject), pg_dump misunderstand no such
  large object is here.
  We have to reference pg_largeobject_metadata to check whether a certain
  large objct exists, or not.

Thanks,

$ diffstat ~/pgsql-blob-priv-fix.patch
 doc/src/sgml/catalogs.sgml               |    3 !!!
 src/bin/initdb/initdb.c                  |    1 +
 src/bin/pg_dump/pg_dump.c                |    8 !!!!!!!!
 src/test/regress/expected/privileges.out |   15 +++++++++++++++
 src/test/regress/sql/privileges.sql      |    8 ++++++++
 5 files changed, 24 insertions(+), 11 modifications(!)
-- 
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>