Re: Largeobject Access Controls (r2460) - Mailing list pgsql-hackers

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>
*** base/doc/src/sgml/catalogs.sgml    (revision 2467)
--- base/doc/src/sgml/catalogs.sgml    (working copy)
***************
*** 3136,3142 ****

    <para>
     <structname>pg_largeobject</structname> should not be readable by the
!    public, since the catalog contains data in large objects of all users.
     <structname>pg_largeobject_metadata</> is a publicly readable catalog
     that only contains identifiers of large objects.
    </para>
--- 3136,3143 ----

    <para>
     <structname>pg_largeobject</structname> should not be readable by the
!    public (except for <structfield>loid</structfield>), since the catalog
!    contains data in large objects of all users.
     <structname>pg_largeobject_metadata</> is a publicly readable catalog
     that only contains identifiers of large objects.
    </para>
*** base/src/test/regress/sql/privileges.sql    (revision 2467)
--- base/src/test/regress/sql/privileges.sql    (working copy)
***************
*** 565,570 ****
--- 565,578 ----
  SELECT lo_unlink(1002);
  SELECT lo_export(1001, '/dev/null');            -- to be denied

+ -- don't allow unpriv users to access pg_largeobject contents
+ \c -
+ SELECT * FROM pg_largeobject LIMIT 0;
+
+ SET SESSION AUTHORIZATION regressuser1;
+ SELECT * FROM pg_largeobject LIMIT 0;            -- to be denied
+ SELECT loid FROM pg_largeobject LIMIT 0;
+
  -- test default ACLs
  \c -

*** base/src/test/regress/expected/privileges.out    (revision 2467)
--- base/src/test/regress/expected/privileges.out    (working copy)
***************
*** 1041,1046 ****
--- 1041,1061 ----
  SELECT lo_export(1001, '/dev/null');            -- to be denied
  ERROR:  must be superuser to use server-side lo_export()
  HINT:  Anyone can use the client-side lo_export() provided by libpq.
+ -- don't allow unpriv users to access pg_largeobject contents
+ \c -
+ SELECT * FROM pg_largeobject LIMIT 0;
+  loid | pageno | data
+ ------+--------+------
+ (0 rows)
+
+ SET SESSION AUTHORIZATION regressuser1;
+ SELECT * FROM pg_largeobject LIMIT 0;            -- to be denied
+ ERROR:  permission denied for relation pg_largeobject
+ SELECT loid FROM pg_largeobject LIMIT 0;
+  loid
+ ------
+ (0 rows)
+
  -- test default ACLs
  \c -
  CREATE SCHEMA testns;
*** base/src/bin/initdb/initdb.c    (revision 2467)
--- base/src/bin/initdb/initdb.c    (working copy)
***************
*** 1784,1789 ****
--- 1784,1790 ----
          "GRANT USAGE ON SCHEMA pg_catalog TO PUBLIC;\n",
          "GRANT CREATE, USAGE ON SCHEMA public TO PUBLIC;\n",
          "REVOKE ALL ON pg_largeobject FROM PUBLIC;\n",
+         "GRANT SELECT (loid) ON pg_largeobject TO PUBLIC;\n",
          NULL
      };

*** base/src/bin/pg_dump/pg_dump.c    (revision 2467)
--- base/src/bin/pg_dump/pg_dump.c    (working copy)
***************
*** 1945,1951 ****
      selectSourceSchema("pg_catalog");

      /* Check for BLOB OIDs */
!     if (AH->remoteVersion >= 70100)
          blobQry = "SELECT loid FROM pg_largeobject LIMIT 1";
      else
          blobQry = "SELECT oid FROM pg_class WHERE relkind = 'l' LIMIT 1";
--- 1945,1953 ----
      selectSourceSchema("pg_catalog");

      /* Check for BLOB OIDs */
!     if (AH->remoteVersion >= 80500)
!         blobQry = "SELECT oid FROM pg_largeobject_metadata LIMIT 1";
!     else if (AH->remoteVersion >= 70100)
          blobQry = "SELECT loid FROM pg_largeobject LIMIT 1";
      else
          blobQry = "SELECT oid FROM pg_class WHERE relkind = 'l' LIMIT 1";
***************
*** 1981,1987 ****
      selectSourceSchema("pg_catalog");

      /* Cursor to get all BLOB OIDs */
!     if (AH->remoteVersion >= 70100)
          blobQry = "DECLARE bloboid CURSOR FOR SELECT DISTINCT loid FROM pg_largeobject";
      else
          blobQry = "DECLARE bloboid CURSOR FOR SELECT oid FROM pg_class WHERE relkind = 'l'";
--- 1983,1991 ----
      selectSourceSchema("pg_catalog");

      /* Cursor to get all BLOB OIDs */
!     if (AH->remoteVersion >= 80500)
!         blobQry = "DECLARE bloboid CURSOR FOR SELECT oid FROM pg_largeobject_metadata";
!     else if (AH->remoteVersion >= 70100)
          blobQry = "DECLARE bloboid CURSOR FOR SELECT DISTINCT loid FROM pg_largeobject";
      else
          blobQry = "DECLARE bloboid CURSOR FOR SELECT oid FROM pg_class WHERE relkind = 'l'";