Stop recording pg_shdepend rows lolor cannot describe.

Creating a large object recorded a shared dependency whose classId was the
OID of lolor.pg_largeobject, an ordinary table rather than a catalog the
dependency machinery can describe.  DROP ROLE on any role that had created
one failed with "unrecognized object class", and the rows were never
removed because inv_drop() deleted with PERFORM_DELETION_SKIP_ORIGINAL.
pg_shdepend is shared across the cluster, so the stored classId was a
per-database relation OID meaningless in any other database.

Objects in lolor storage are rows in ordinary tables and cannot take part
in pg_shdepend at all, so drop the recording and the matching deletion, and
remove the rows earlier versions left behind.  Ownership is consequently
untracked, which is inherent to storing large objects outside the catalogs;
add lolor.check_orphans() to report objects whose owner is gone.

Author: ibrarahmad

README.md

@@ -124,4 +124,6 @@ database user. Upgrading to 1.3.0 revokes the privilege; see the release notes.
 
 - Native large object functionality cannot be used while you are using the lolor extension.
 - lolor does not support the following statements: `ALTER LARGE OBJECT`, `GRANT ON LARGE OBJECT`, `COMMENT ON LARGE OBJECT`, and `REVOKE ON LARGE OBJECT`.
+- Objects in lolor storage are rows in ordinary tables and so cannot participate in `pg_shdepend`. `DROP ROLE`, `DROP OWNED BY` and `REASSIGN OWNED BY` do not see them: a role that owns them or appears in their ACL can be dropped without a warning, and `REASSIGN OWNED` / `DROP OWNED` leave them untouched. Before dropping a role, run `REASSIGN OWNED BY` or `DROP OWNED BY` in each database that has lolor, then `DROP ROLE`. Afterwards run `lolor.check_orphans()` in each such database and repair anything it reports with `lolor.fix_orphans(new_owner)`.
+- Role OIDs come from a cluster-wide counter that wraps around, so a new role can receive a dropped role's OID and silently become the owner or grantee of that role's orphaned objects. This cannot be detected after the fact, which is another reason to run `lolor.check_orphans()` promptly after dropping roles.
 - Large object migration is node-local. Native large objects live in `pg_catalog.pg_largeobject`, which is never replicated, so each node holds an independent set and `migrate_from_native()` migrates only the local node's objects; with spock installed, the migration DML runs in repair mode and is not replicated. Run the migration on every node that holds native large objects — for example with `spock.replicate_ddl('SELECT lolor.migrate_from_native()')`, which queues the command so that each node executes it locally. Migrated objects keep their original native OIDs, which are not node-encoded: if different nodes hold different objects under the same OID, the nodes' lolor contents will diverge and later replicated changes to those objects can conflict. Newly created large objects are collision-free, since new OIDs are node-encoded via `lolor.node` and checked against existing rows.

docs/lolor_release_notes.md

@@ -11,6 +11,10 @@
 * **Security fix: `lo_import()` and `lo_export()` were executable by any database user.** These read and write files on the server as the account PostgreSQL runs under, so core revokes `EXECUTE` on them from `PUBLIC`. lolor replaces them by renaming the originals to `*_orig`; an ACL belongs to a function rather than a name, so the restriction stayed on the parked original while each replacement got the default `EXECUTE TO PUBLIC`. Any user could read an arbitrary server file with `lo_import()` or overwrite one with `lo_export()`. The replacements are now locked down at install time, and upgrading revokes the privilege on existing installations in either state. Versions 1.0 through 1.2.2 are affected.
 * The extension is no longer marked `trusted`. Installing lolor renames functions in `pg_catalog` for the whole database, which a non-superuser should not be able to do.
 * Fixed the `lolor.node` upper bound. The GUC accepted 0..16 while a generated OID reserves four bits for the node id, so node 16 did not fit: the node field of every OID it generated read back as 0. The bound is now derived from the encoding (`LOLOR_MAX_NODE_ID`), giving 0..15. **If you have `lolor.node = 16` configured**, the server still starts but logs `16 is outside the valid range for parameter "lolor.node" (0 .. 15)` and falls back to 0, which is the node id it was effectively using already. Set it to a value in 0..15, and check for OID collisions against whichever node is genuinely 0.
+* **Fixed `DROP ROLE` failing with "unrecognized object class".** Creating a large object recorded a `pg_shdepend` row whose `classId` was the OID of `lolor.pg_largeobject`, an ordinary table rather than a catalog. Any role that had created a large object became undroppable, and the rows were never cleaned up because `inv_drop()` deleted with `PERFORM_DELETION_SKIP_ORIGINAL`. lolor no longer records these rows, and the upgrade removes the ones already present.
+  * The cleanup is per database, so `ALTER EXTENSION lolor UPDATE` must be run in every database that has lolor. Until it is, `DROP ROLE` anywhere in the cluster keeps reporting "owner of objects in database X".
+  * A database that ran `DROP EXTENSION lolor` on an earlier version still has the rows, now pointing at a table that no longer exists, and no upgrade script will run there. Clean it by hand as superuser in that database: find the stale class OIDs with `SELECT DISTINCT classid FROM pg_shdepend WHERE dbid = (SELECT oid FROM pg_database WHERE datname = current_database()) AND classid NOT IN (SELECT oid FROM pg_class)`, then `DELETE FROM pg_shdepend WHERE dbid = <that dbid> AND classid IN (<those OIDs>)`.
+* New helpers `lolor.check_orphans()` and `lolor.fix_orphans(new_owner)` for objects whose owner, grantee or grantor has been dropped. Objects in lolor storage cannot participate in `pg_shdepend`, so `DROP ROLE` does not notice them; `check_orphans()` lists them and which reference is dangling, and `fix_orphans()` reassigns dead owners, replacing them in the ACL as `REASSIGN OWNED` would so that grants they made survive, and drops entries that still name a missing role.
 * Expanded test coverage: TAP tests for dump/restore, streaming and logical replication, and standby promotion; regression tests for `lo_lseek`, `lo_tell`, and `lo_truncate`.
 * Security hardening: addressed Codacy/Flawfinder warnings.
 

expected/lolor.out

@@ -730,6 +730,247 @@ ERROR:  16 is outside the valid range for parameter "lolor.node" (0 .. 15)
 SET lolor.node = 15;
 SET lolor.node = 1;
 --
+-- Permission enforcement.
+--
+-- Objects in lolor storage have no catalog entry, so lolor cannot use the
+-- syscache-backed owner and ACL checks and reimplements them against its own
+-- tables.  Exercise that path rather than assuming it matches core.
+--
+CREATE EXTENSION lolor;
+CREATE ROLE lolor_alice;
+CREATE ROLE lolor_bob;
+-- Large object error messages quote the OID, which is generated and so
+-- differs between runs.  Report the message with digits masked instead.
+CREATE FUNCTION lolor_expect_error(cmd text) RETURNS text AS $$
+BEGIN
+  EXECUTE cmd;
+  RETURN 'unexpectedly succeeded';
+EXCEPTION WHEN OTHERS THEN
+  RETURN regexp_replace(SQLERRM, '[0-9]+', 'NNN', 'g');
+END
+$$ LANGUAGE plpgsql;
+SET ROLE lolor_alice;
+SELECT lo_from_bytea(0, 'alice private data') AS alice_oid \gset
+SELECT convert_from(lo_get(:alice_oid), 'UTF8') AS owner_can_read;
+   owner_can_read   
+--------------------
+ alice private data
+(1 row)
+
+RESET ROLE;
+-- A different role gets nothing without a grant.
+SET ROLE lolor_bob;
+SELECT lolor_expect_error(format('SELECT lo_get(%s)', :alice_oid)) AS read_denied;
+              read_denied               
+----------------------------------------
+ permission denied for large object NNN
+(1 row)
+
+SELECT lolor_expect_error(format('SELECT lo_open(%s, 262144)', :alice_oid)) AS open_denied;
+              open_denied               
+----------------------------------------
+ permission denied for large object NNN
+(1 row)
+
+SELECT lolor_expect_error(format('SELECT lo_put(%s, 0, ''x'')', :alice_oid)) AS write_denied;
+              write_denied              
+----------------------------------------
+ permission denied for large object NNN
+(1 row)
+
+SELECT lolor_expect_error(format('SELECT lo_unlink(%s)', :alice_oid)) AS unlink_denied;
+           unlink_denied           
+-----------------------------------
+ must be owner of large object NNN
+(1 row)
+
+RESET ROLE;
+-- The superuser bypasses the check, as in core.
+SELECT convert_from(lo_get(:alice_oid), 'UTF8') AS superuser_can_read;
+ superuser_can_read 
+--------------------
+ alice private data
+(1 row)
+
+-- GRANT/ALTER on a large object act on pg_largeobject_metadata, where an
+-- object in lolor storage has no row.  This limitation is documented; assert
+-- it so that a change in behaviour is noticed.
+SELECT lolor_expect_error(
+  format('GRANT SELECT ON LARGE OBJECT %s TO lolor_bob', :alice_oid)) AS grant_unsupported;
+        grant_unsupported        
+---------------------------------
+ large object NNN does not exist
+(1 row)
+
+SELECT lolor_expect_error(
+  format('ALTER LARGE OBJECT %s OWNER TO lolor_bob', :alice_oid)) AS alter_unsupported;
+        alter_unsupported        
+---------------------------------
+ large object NNN does not exist
+(1 row)
+
+SELECT lo_unlink(:alice_oid);
+ lo_unlink 
+-----------
+         1
+(1 row)
+
+--
+-- Objects in lolor storage are rows in ordinary tables and cannot participate
+-- in pg_shdepend, so DROP ROLE does not notice that a role still owns one.
+-- lolor.check_orphans() exists to make the consequence findable.
+--
+SET ROLE lolor_alice;
+SELECT lo_from_bytea(0, 'owned by a role about to vanish') AS orphan_oid \gset
+RESET ROLE;
+SELECT count(*) AS orphans_before FROM lolor.check_orphans();
+ orphans_before 
+----------------
+              0
+(1 row)
+
+DROP ROLE lolor_alice;
+SELECT count(*) AS orphans_after FROM lolor.check_orphans();
+ orphans_after 
+---------------
+             1
+(1 row)
+
+SELECT lo_unlink(:orphan_oid);
+ lo_unlink 
+-----------
+         1
+(1 row)
+
+SELECT count(*) AS orphans_cleared FROM lolor.check_orphans();
+ orphans_cleared 
+-----------------
+               0
+(1 row)
+
+--
+-- fix_orphans() must behave like REASSIGN OWNED: grants the dead owner made
+-- survive with the new owner as grantor, an entry the new owner already held
+-- merges into its owner entry, and only entries that still name a missing
+-- role are dropped.  ACLs can only be set in native storage, so build them
+-- there and migrate in.
+--
+CREATE ROLE lolor_dead_owner;
+CREATE ROLE lolor_carol;
+CREATE ROLE lolor_dave;
+SELECT lolor.disable();
+ disable 
+---------
+ t
+(1 row)
+
+SET ROLE lolor_dead_owner;
+SELECT lo_from_bytea(0, 'granted to bob') AS granted_oid \gset
+GRANT SELECT ON LARGE OBJECT :granted_oid TO lolor_bob;
+SELECT lo_from_bytea(0, 'grant chain') AS chain_oid \gset
+GRANT SELECT ON LARGE OBJECT :chain_oid TO lolor_dave WITH GRANT OPTION;
+SELECT lo_from_bytea(0, 'new owner already a grantee') AS merge_oid \gset
+GRANT SELECT ON LARGE OBJECT :merge_oid TO lolor_carol;
+RESET ROLE;
+SET ROLE lolor_dave;
+GRANT SELECT ON LARGE OBJECT :chain_oid TO lolor_bob;
+RESET ROLE;
+SELECT lolor.enable();
+ enable 
+--------
+ t
+(1 row)
+
+SELECT lolor.migrate_from_native();
+NOTICE:  migrated 12 large object(s) (12 data page(s)) from native to lolor storage
+ migrate_from_native 
+---------------------
+                  12
+(1 row)
+
+DROP ROLE lolor_dead_owner;
+SELECT role_kind, count(*) FROM lolor.check_orphans() GROUP BY 1 ORDER BY 1;
+ role_kind | count 
+-----------+-------
+ grantee   |     3
+ grantor   |     3
+ owner     |     3
+(3 rows)
+
+SELECT lolor.fix_orphans('lolor_carol') AS objects_repaired;
+ objects_repaired 
+------------------
+                3
+(1 row)
+
+SELECT count(*) AS orphans_left FROM lolor.check_orphans();
+ orphans_left 
+--------------
+            0
+(1 row)
+
+-- Role OIDs vary per run, so compare the rebuilt ACLs by name
+SELECT CASE m.oid WHEN :granted_oid THEN 'granted' WHEN :chain_oid THEN 'chain' ELSE 'merge' END AS obj,
+       pg_get_userbyid(m.lomowner) AS owner,
+       a.grantee::regrole::text AS grantee, a.grantor::regrole::text AS grantor, a.privilege_type
+  FROM lolor.pg_largeobject_metadata m, aclexplode(m.lomacl) a
+ WHERE m.oid IN (:granted_oid, :chain_oid, :merge_oid)
+ ORDER BY 1, 3, 4, 5;
+   obj   |    owner    |   grantee   |   grantor   | privilege_type 
+---------+-------------+-------------+-------------+----------------
+ chain   | lolor_carol | lolor_bob   | lolor_dave  | SELECT
+ chain   | lolor_carol | lolor_carol | lolor_carol | SELECT
+ chain   | lolor_carol | lolor_carol | lolor_carol | UPDATE
+ chain   | lolor_carol | lolor_dave  | lolor_carol | SELECT
+ granted | lolor_carol | lolor_bob   | lolor_carol | SELECT
+ granted | lolor_carol | lolor_carol | lolor_carol | SELECT
+ granted | lolor_carol | lolor_carol | lolor_carol | UPDATE
+ merge   | lolor_carol | lolor_carol | lolor_carol | SELECT
+ merge   | lolor_carol | lolor_carol | lolor_carol | UPDATE
+(9 rows)
+
+-- The grantee kept access and the new owner has it
+SET ROLE lolor_bob;
+SELECT convert_from(lo_get(:granted_oid), 'UTF8') AS bob_still_reads;
+ bob_still_reads 
+-----------------
+ granted to bob
+(1 row)
+
+RESET ROLE;
+SET ROLE lolor_carol;
+SELECT convert_from(lo_get(:chain_oid), 'UTF8') AS new_owner_reads;
+ new_owner_reads 
+-----------------
+ grant chain
+(1 row)
+
+RESET ROLE;
+SELECT lo_unlink(:granted_oid);
+ lo_unlink 
+-----------
+         1
+(1 row)
+
+SELECT lo_unlink(:chain_oid);
+ lo_unlink 
+-----------
+         1
+(1 row)
+
+SELECT lo_unlink(:merge_oid);
+ lo_unlink 
+-----------
+         1
+(1 row)
+
+DROP ROLE lolor_carol;
+DROP ROLE lolor_dave;
+DROP ROLE lolor_bob;
+DROP FUNCTION lolor_expect_error(text);
+DROP EXTENSION lolor;
+NOTICE:  migrated 9 large object(s) from lolor to native storage
+--
 -- 64-bit interface and page-boundary I/O.  lo_put(), lo_tell64() and
 -- lo_truncate64() had no coverage.
 --
@@ -1050,6 +1291,8 @@ SELECT lo_unlink(:multi_oid);
 --
 SELECT lo_get(0);
 ERROR:  large object 0 does not exist
+SELECT lo_unlink(0);
+ERROR:  large object 0 does not exist
 BEGIN;
 SELECT lo_open(0, 262144);
 ERROR:  large object 0 does not exist

lolor--1.2.2--1.3.0.sql

@@ -47,6 +47,114 @@ BEGIN
 END;
 $$;
 
+/*
+ * Large objects in lolor storage that refer to a role which no longer exists,
+ * as owner, grantee or grantor.
+ *
+ * Objects in lolor storage are rows in ordinary tables, so they cannot
+ * participate in pg_shdepend: DROP ROLE will not notice them the way it
+ * notices native large objects.  That is inherent to storing them outside the
+ * catalogs; this function makes the consequence findable, and fix_orphans()
+ * repairs it.  Joins pg_roles rather than pg_authid so that a grantee of
+ * EXECUTE can actually run it.
+ */
+CREATE FUNCTION lolor.check_orphans()
+RETURNS TABLE (loid oid, role_oid oid, role_kind text) AS $$
+  SELECT m.oid, m.lomowner, 'owner'
+  FROM lolor.pg_largeobject_metadata m
+  WHERE NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles r WHERE r.oid = m.lomowner)
+  UNION
+  SELECT m.oid, a.grantee, 'grantee'
+  FROM lolor.pg_largeobject_metadata m, pg_catalog.aclexplode(m.lomacl) a
+  WHERE a.grantee <> 0
+    AND NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles r WHERE r.oid = a.grantee)
+  UNION
+  SELECT m.oid, a.grantor, 'grantor'
+  FROM lolor.pg_largeobject_metadata m, pg_catalog.aclexplode(m.lomacl) a
+  WHERE NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles r WHERE r.oid = a.grantor)
+  ORDER BY 1, 3, 2
+$$ LANGUAGE sql STABLE;
+
+REVOKE ALL ON FUNCTION lolor.check_orphans() FROM PUBLIC;
+
+/*
+ * Repair what check_orphans() reports.  Objects whose owner is gone go to
+ * new_owner, and the ACL is rebuilt the way core's aclnewowner() does it: the
+ * old owner is replaced by new_owner wherever it appears as grantee or
+ * grantor, entries that then coincide are merged, and only entries that still
+ * name a missing role are dropped.  Grants the old owner made therefore
+ * survive, as they do under REASSIGN OWNED.  Returns the number of objects
+ * changed.
+ *
+ * One UPDATE is essential: the ACL rewrite has to see the old owner, and a
+ * separate UPDATE of lomowner would already have lost it.
+ */
+CREATE FUNCTION lolor.fix_orphans(new_owner regrole)
+RETURNS bigint AS $$
+DECLARE
+  fixed bigint;
+BEGIN
+  IF NOT EXISTS (SELECT 1 FROM pg_roles WHERE rolname = current_user AND rolsuper) THEN
+    RAISE EXCEPTION 'must be superuser to repair orphaned large objects';
+  END IF;
+
+  WITH o AS (
+    SELECT m.oid, m.lomowner AS old_owner, m.lomacl,
+           NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles r WHERE r.oid = m.lomowner) AS owner_dead
+    FROM lolor.pg_largeobject_metadata m)
+  UPDATE lolor.pg_largeobject_metadata m
+     SET lomowner = CASE WHEN o.owner_dead THEN new_owner::oid ELSE m.lomowner END,
+         -- aclexplode() yields one row per privilege.  Substitute the owner,
+         -- regroup so coinciding entries merge, then rebuild with
+         -- makeaclitem().  An empty result is NULL, meaning default
+         -- privileges.
+         lomacl = (
+           SELECT array_agg(pg_catalog.makeaclitem(s.grantee, s.grantor, s.privs, s.is_grantable)
+                            ORDER BY s.grantee, s.grantor, s.is_grantable)
+           FROM (SELECT CASE WHEN o.owner_dead AND a.grantee = o.old_owner
+                             THEN new_owner::oid ELSE a.grantee END AS grantee,
+                        CASE WHEN o.owner_dead AND a.grantor = o.old_owner
+                             THEN new_owner::oid ELSE a.grantor END AS grantor,
+                        a.is_grantable,
+                        string_agg(DISTINCT a.privilege_type, ',') AS privs
+                 FROM pg_catalog.aclexplode(o.lomacl) a
+                 GROUP BY 1, 2, 3) s
+           WHERE (s.grantee = 0
+                  OR EXISTS (SELECT 1 FROM pg_catalog.pg_roles r WHERE r.oid = s.grantee))
+             AND EXISTS (SELECT 1 FROM pg_catalog.pg_roles r WHERE r.oid = s.grantor))
+    FROM o
+   WHERE m.oid = o.oid
+     AND (o.owner_dead
+          OR EXISTS (SELECT 1 FROM pg_catalog.aclexplode(o.lomacl) a
+                     WHERE (a.grantee <> 0
+                            AND NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles r WHERE r.oid = a.grantee))
+                        OR NOT EXISTS (SELECT 1 FROM pg_catalog.pg_roles r WHERE r.oid = a.grantor)));
+  GET DIAGNOSTICS fixed = ROW_COUNT;
+
+  RETURN fixed;
+END;
+$$ LANGUAGE plpgsql VOLATILE;
+
+REVOKE ALL ON FUNCTION lolor.fix_orphans(regrole) FROM PUBLIC;
+
+/*
+ * Remove the bogus pg_shdepend rows left by earlier versions.
+ *
+ * Through 1.2.2, creating a large object recorded a pg_shdepend row whose
+ * classId was the OID of lolor.pg_largeobject -- an ordinary table, not a
+ * catalog the dependency machinery can describe.  DROP ROLE on any role that
+ * had created one failed with "unrecognized object class", and the rows were
+ * never removed.
+ *
+ * pg_shdepend is shared across the cluster, so restrict the delete to this
+ * database: the same classId in another database is an unrelated relation.
+ */
+DELETE FROM pg_catalog.pg_shdepend
+WHERE dbid = (SELECT oid FROM pg_catalog.pg_database
+               WHERE datname = current_database())
+  AND classid IN ('lolor.pg_largeobject'::regclass,
+                  'lolor.pg_largeobject_metadata'::regclass);
+
 /*
  * lolor.migrate_from_native()
  *