From 3e997da1472d4c7bc067a2178227c346bee91bfb Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Thu, 16 Mar 2023 11:46:08 -0700 Subject: [PATCH v3 2/2] pg_dump: skip lock for extension tables without policies If a user without SELECT permissions on an internal extension table tries to dump the extension, the dump will fail while trying to lock the table with ACCESS SHARE, even though the user doesn't want or need to dump the table in question. (The lock is taken to allow later pg_get_expr() calls on pg_policy to remain consistent in the face of concurrent schema changes.) It'd be ideal not to require SELECT permissions on a table to be able to dump its policies, but I don't have a great idea for how to implement that without races. As a workaround, skip the policy queries entirely if we can determine that no policies exist for a table at the time of getTables(). Fixes the previous commit's failing test. --- src/bin/pg_dump/common.c | 54 +++++++++++++++++++++++++++++++++++ src/bin/pg_dump/pg_dump.c | 59 +++++++++++++++++++++++++++++++++++++++ src/bin/pg_dump/pg_dump.h | 4 +++ 3 files changed, 117 insertions(+) diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index 5d988986ed..69df3567f9 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -59,6 +59,7 @@ typedef struct _catalogIdMapEntry uint32 hashval; /* hash code for the CatalogId */ DumpableObject *dobj; /* the associated DumpableObject, if any */ ExtensionInfo *ext; /* owning extension, if any */ + bool has_policies; /* referenced by pg_policy? */ } CatalogIdMapEntry; #define SH_PREFIX catalogid @@ -135,6 +136,13 @@ getSchemaData(Archive *fout, int *numTablesPtr) pg_log_info("identifying extension members"); getExtensionMembership(fout, extinfo, numExtensions); + /* + * Similarly, the existence of RLS policies influences whether some tables + * need to be locked. + */ + pg_log_info("checking for row-level security policies"); + getTablesWithPolicies(fout); + pg_log_info("reading schemas"); (void) getNamespaces(fout, &numNamespaces); @@ -686,6 +694,7 @@ AssignDumpId(DumpableObject *dobj) { entry->dobj = NULL; entry->ext = NULL; + entry->has_policies = false; } Assert(entry->dobj == NULL); entry->dobj = dobj; @@ -995,6 +1004,7 @@ recordExtensionMembership(CatalogId catId, ExtensionInfo *ext) { entry->dobj = NULL; entry->ext = NULL; + entry->has_policies = false; } Assert(entry->ext == NULL); entry->ext = ext; @@ -1019,6 +1029,50 @@ findOwningExtension(CatalogId catalogId) } +/* + * recordPoliciesExist + * Record that the object identified by the given catalog ID has RLS policies + */ +void +recordPoliciesExist(CatalogId catId) +{ + CatalogIdMapEntry *entry; + bool found; + + /* Initialize CatalogId hash table if not done yet */ + if (catalogIdHash == NULL) + catalogIdHash = catalogid_create(CATALOGIDHASH_INITIAL_SIZE, NULL); + + /* Add reference to CatalogId hash */ + entry = catalogid_insert(catalogIdHash, catId, &found); + if (!found) + { + entry->dobj = NULL; + entry->ext = NULL; + entry->has_policies = false; + } + entry->has_policies = true; +} + +/* + * hasPolicies + * return whether the specified catalog ID has RLS policies + */ +bool +hasPolicies(CatalogId catId) +{ + CatalogIdMapEntry *entry; + + if (catalogIdHash == NULL) + return false; /* no objects exist yet */ + + entry = catalogid_lookup(catalogIdHash, catId); + if (entry == NULL) + return false; + return entry->has_policies; +} + + /* * parseOidArray * parse a string of numbers delimited by spaces into a character array diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 5dab1ba9ea..6add01ca73 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -3742,6 +3742,54 @@ dumpLOs(Archive *fout, const void *arg) return 1; } +/* + * getTablesWithPolicies + * retrieve the IDs of all tables with pg_policy entries + */ +void +getTablesWithPolicies(Archive *fout) +{ + PQExpBuffer query; + PGresult *res; + int i_classid; + int i_polrelid; + int i, + ntups; + + /* No policies before 9.5 */ + if (fout->remoteVersion < 90500) + return; + + query = createPQExpBuffer(); + + /* Figure out which tables have RLS policies. */ + printfPQExpBuffer(query, + "SELECT DISTINCT 'pg_class'::regclass::oid AS classid, " + " polrelid " + "FROM pg_catalog.pg_policy"); + + res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK); + + ntups = PQntuples(res); + + i_classid = PQfnumber(res, "classid"); + i_polrelid = PQfnumber(res, "polrelid"); + + for (i = 0; i < ntups; i++) + { + CatalogId objId; + + objId.tableoid = atooid(PQgetvalue(res, i, i_classid)); + objId.oid = atooid(PQgetvalue(res, i, i_polrelid)); + + recordPoliciesExist(objId); + } + + PQclear(res); + + destroyPQExpBuffer(query); +} + /* * getPolicies * get information about all RLS policies on dumpable tables. @@ -6658,6 +6706,17 @@ getTables(Archive *fout, int *numTables) else selectDumpableTable(&tblinfo[i], fout); + /* + * If the table has no policies, we don't need to worry about those. + * + * For tables internal to an extension, this may mean we don't need to + * take an ACCESS SHARE lock, which in turn allows less privileged users + * to successfully perform a dump if they don't have SELECT access to + * those tables (which they weren't trying to dump in the first place). + */ + if (!hasPolicies(tblinfo[i].dobj.catId)) + tblinfo[i].dobj.dump &= ~DUMP_COMPONENT_POLICY; + /* * Now, consider the table "interesting" if we need to dump its * definition or its data. Later on, we'll skip a lot of data diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index bc8f2ec36d..5dea0b63d6 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -695,6 +695,9 @@ extern PublicationInfo *findPublicationByOid(Oid oid); extern void recordExtensionMembership(CatalogId catId, ExtensionInfo *ext); extern ExtensionInfo *findOwningExtension(CatalogId catalogId); +extern void recordPoliciesExist(CatalogId catId); +extern bool hasPolicies(CatalogId catId); + extern void parseOidArray(const char *str, Oid *array, int arraysize); extern void sortDumpableObjects(DumpableObject **objs, int numObjs, @@ -743,6 +746,7 @@ extern void getExtensionMembership(Archive *fout, ExtensionInfo extinfo[], extern void processExtensionTables(Archive *fout, ExtensionInfo extinfo[], int numExtensions); extern EventTriggerInfo *getEventTriggers(Archive *fout, int *numEventTriggers); +extern void getTablesWithPolicies(Archive *fout); extern void getPolicies(Archive *fout, TableInfo tblinfo[], int numTables); extern PublicationInfo *getPublications(Archive *fout, int *numPublications); -- 2.25.1