diff --git a/README.md b/README.md index 07861a9..8477b6c 100644 --- a/README.md +++ b/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. diff --git a/docs/lolor_release_notes.md b/docs/lolor_release_notes.md index 0e9d82b..2e65c5d 100644 --- a/docs/lolor_release_notes.md +++ b/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 = AND classid IN ()`. +* 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. diff --git a/expected/lolor.out b/expected/lolor.out index 674ec34..26f78a2 100644 --- a/expected/lolor.out +++ b/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 diff --git a/lolor--1.2.2--1.3.0.sql b/lolor--1.2.2--1.3.0.sql index 53fc468..c1e445d 100644 --- a/lolor--1.2.2--1.3.0.sql +++ b/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() * diff --git a/sql/lolor.sql b/sql/lolor.sql index cf25c11..d6d6b0d 100644 --- a/sql/lolor.sql +++ b/sql/lolor.sql @@ -317,6 +317,119 @@ SET lolor.node = 16; 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; +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; +SELECT lolor_expect_error(format('SELECT lo_open(%s, 262144)', :alice_oid)) AS open_denied; +SELECT lolor_expect_error(format('SELECT lo_put(%s, 0, ''x'')', :alice_oid)) AS write_denied; +SELECT lolor_expect_error(format('SELECT lo_unlink(%s)', :alice_oid)) AS unlink_denied; +RESET ROLE; + +-- The superuser bypasses the check, as in core. +SELECT convert_from(lo_get(:alice_oid), 'UTF8') AS superuser_can_read; + +-- 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; +SELECT lolor_expect_error( + format('ALTER LARGE OBJECT %s OWNER TO lolor_bob', :alice_oid)) AS alter_unsupported; + +SELECT lo_unlink(:alice_oid); + +-- +-- 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(); +DROP ROLE lolor_alice; +SELECT count(*) AS orphans_after FROM lolor.check_orphans(); +SELECT lo_unlink(:orphan_oid); +SELECT count(*) AS orphans_cleared FROM lolor.check_orphans(); + +-- +-- 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(); +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(); +SELECT lolor.migrate_from_native(); +DROP ROLE lolor_dead_owner; +SELECT role_kind, count(*) FROM lolor.check_orphans() GROUP BY 1 ORDER BY 1; +SELECT lolor.fix_orphans('lolor_carol') AS objects_repaired; +SELECT count(*) AS orphans_left FROM lolor.check_orphans(); +-- 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; +-- 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; +RESET ROLE; +SET ROLE lolor_carol; +SELECT convert_from(lo_get(:chain_oid), 'UTF8') AS new_owner_reads; +RESET ROLE; +SELECT lo_unlink(:granted_oid); +SELECT lo_unlink(:chain_oid); +SELECT lo_unlink(:merge_oid); +DROP ROLE lolor_carol; +DROP ROLE lolor_dave; +DROP ROLE lolor_bob; +DROP FUNCTION lolor_expect_error(text); +DROP EXTENSION lolor; + -- -- 64-bit interface and page-boundary I/O. lo_put(), lo_tell64() and -- lo_truncate64() had no coverage. @@ -429,6 +542,7 @@ SELECT lo_unlink(:multi_oid); -- Error paths. -- SELECT lo_get(0); +SELECT lo_unlink(0); BEGIN; SELECT lo_open(0, 262144); ROLLBACK; diff --git a/src/lolor_inv_api.c b/src/lolor_inv_api.c index 6307186..659bd99 100644 --- a/src/lolor_inv_api.c +++ b/src/lolor_inv_api.c @@ -39,9 +39,7 @@ #include "access/sysattr.h" #include "access/table.h" #include "access/xact.h" -#include "catalog/dependency.h" #include "catalog/indexing.h" -#include "catalog/objectaccess.h" #include "catalog/pg_largeobject.h" #include "catalog/pg_largeobject_metadata.h" #include "libpq/libpq-fs.h" @@ -217,18 +215,23 @@ lolor_inv_create(Oid lobjId) lobjId_new = LOLOR_LargeObjectCreate(lobjId); /* - * dependency on the owner of largeobject + * No shared dependency is recorded for the owner, and no object access + * hook is invoked. Both take a classId, and core passes + * LargeObjectRelationId: a genuine catalog that the dependency machinery + * and security modules know how to describe. An object in lolor storage + * is a row in an ordinary table, and there is no classId that describes + * it. Passing the OID of lolor.pg_largeobject produced pg_shdepend rows + * that DROP ROLE could not interpret, failing with "unrecognized object + * class" and leaving the role undroppable; they were never removed + * either, because inv_drop() deleted with PERFORM_DELETION_SKIP_ORIGINAL. + * pg_shdepend is a shared catalog, so the stored classId was a + * per-database relation OID meaningless elsewhere. Handing that same OID + * to an object access hook would mislead a security module the same way. * - * Note that LO dependencies are recorded using classId - * LOLOR_LargeObjectRelationId for backwards-compatibility reasons. Using - * LOLOR_LargeObjectMetadataRelationId instead would simplify matters for the - * backend, but it'd complicate pg_dump and possibly break other clients. + * Not tracking ownership is a real limitation, and inherent in storing + * large objects outside the catalogs. lolor.check_orphans() reports + * objects whose owner no longer exists. */ - recordDependencyOnOwner(get_LOLOR_LargeObjectRelationId(), - lobjId_new, GetUserId()); - - /* Post creation hook for new large object */ - InvokeObjectPostCreateHook(get_LOLOR_LargeObjectRelationId(), lobjId_new, 0); /* * Advance command counter to make new tuple visible to later operations. @@ -347,16 +350,14 @@ lolor_inv_close(LargeObjectDesc *obj_desc) int lolor_inv_drop(Oid lobjId) { - ObjectAddress object; - /* - * Delete any comments and dependencies on the large object + * There are no comments, security labels or dependencies to remove: an + * object in lolor storage is not a catalog object, so nothing can be + * attached to it. See the note in lolor_inv_create(). The performDeletion() + * call that stood here searched pg_depend under a classId that is an + * ordinary relation OID and could never match. */ - object.classId = get_LOLOR_LargeObjectRelationId(); - object.objectId = lobjId; - object.objectSubId = 0; - performDeletion(&object, DROP_CASCADE, PERFORM_DELETION_SKIP_ORIGINAL); - LOLOR_LargeObjectDrop(object.objectId); + LOLOR_LargeObjectDrop(lobjId); /* * Advance command counter so that tuple removal will be seen by later