Greetings, While working through some pg_dump regression tests, I came across a bug in 9.6+ due to the changes I made to how GRANT/REVOKEs were handled.
The attached describes and fixes the bug, which only impacts dumping from 9.6+ clusters, so hopefully not too many people have been impacted by it (it's also a bit of an odd case anyway, as noted). I'm still reviewing and playing with it, but anticipate pushing it sometime tomorrow, along with a bunch of additional pg_dump regression tests, built on the prior framework (as an independent patch, and just for PG10, whereas the attached will be back-patched to 9.6). Thoughts and comments welcome, of course, but it's a relatively mechanical set of changes to match how other the ACLs for everything else are handled (I checked all other callers of buildACLCommands()). Thanks! Stephen
From 77956955561711cf8e96b23cdbbee400586f5440 Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfr...@snowman.net>
Date: Mon, 30 Jan 2017 13:22:17 -0500
Subject: [PATCH] pg_dump: Fix handling of ALTER DEFAULT PRIVILEGES
In commit 23f34fa, we changed how ACLs were handled to use the new
pg_init_privs catalog and to dump out the ACL commands as REVOKE+GRANT
combinations instead of trying to REVOKE all rights always and then
GRANT back just the ones which were in place.
Unfortunately, the DEFAULT PRIVILEGES system didn't quite get the
correct treatment with this change and ended up (incorrectly) only
including positive GRANTs instead of both the REVOKEs and GRANTs
necessary to preserve the correct privileges.
There are only a couple cases where such REVOKEs are possible because,
generally speaking, there's few rights which exist on objects by
default to be revoked.
Examples of REVOKEs which weren't being correctly preserved are when
privileges are REVOKE'd from the creator/owner, like so:
ALTER DEFAULT PRIVILEGES
FOR ROLE myrole
REVOKE SELECT ON TABLES FROM myrole;
or when other default privileges are being revoked, such as EXECUTE
rights granted to public for functions:
ALTER DEFAULT PRIVILEGES
FOR ROLE myrole
REVOKE EXECUTE ON FUNCTIONS FROM PUBLIC;
Fix this by correctly working out what the correct REVOKE statements are
(if any) and dump them out, just as we do for everything else.
Noticed while developing additional regression tests for pg_dump, which
will be landing shortly.
Back-patch to 9.6 where the bug was introduced.
---
src/bin/pg_dump/dumputils.c | 23 ++++++++++++-----
src/bin/pg_dump/dumputils.h | 4 ++-
src/bin/pg_dump/pg_dump.c | 63 ++++++++++++++++++++++++++++++++++++++++-----
src/bin/pg_dump/pg_dump.h | 3 +++
4 files changed, 78 insertions(+), 15 deletions(-)
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c
index 81ec650..b41f2b9 100644
--- a/src/bin/pg_dump/dumputils.c
+++ b/src/bin/pg_dump/dumputils.c
@@ -364,11 +364,12 @@ buildACLCommands(const char *name, const char *subname,
*/
bool
buildDefaultACLCommands(const char *type, const char *nspname,
- const char *acls, const char *owner,
+ const char *acls, const char *racls,
+ const char *initacls, const char *initracls,
+ const char *owner,
int remoteVersion,
PQExpBuffer sql)
{
- bool result;
PQExpBuffer prefix;
prefix = createPQExpBuffer();
@@ -384,14 +385,22 @@ buildDefaultACLCommands(const char *type, const char *nspname,
if (nspname)
appendPQExpBuffer(prefix, "IN SCHEMA %s ", fmtId(nspname));
- result = buildACLCommands("", NULL,
- type, acls, "", owner,
- prefix->data, remoteVersion,
- sql);
+ if (strlen(initacls) != 0 || strlen(initracls) != 0)
+ {
+ appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(true);\n");
+ if (!buildACLCommands("", NULL, type, initacls, initracls, owner,
+ prefix->data, remoteVersion, sql))
+ return false;
+ appendPQExpBuffer(sql, "SELECT pg_catalog.binary_upgrade_set_record_init_privs(false);\n");
+ }
+
+ if (!buildACLCommands("", NULL, type, acls, racls, owner,
+ prefix->data, remoteVersion, sql))
+ return false;
destroyPQExpBuffer(prefix);
- return result;
+ return true;
}
/*
diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h
index ba61fa9..d7eecdf 100644
--- a/src/bin/pg_dump/dumputils.h
+++ b/src/bin/pg_dump/dumputils.h
@@ -41,7 +41,9 @@ extern bool buildACLCommands(const char *name, const char *subname,
const char *owner, const char *prefix, int remoteVersion,
PQExpBuffer sql);
extern bool buildDefaultACLCommands(const char *type, const char *nspname,
- const char *acls, const char *owner,
+ const char *acls, const char *racls,
+ const char *initacls, const char *initracls,
+ const char *owner,
int remoteVersion,
PQExpBuffer sql);
extern void buildShSecLabelQuery(PGconn *conn, const char *catalog_name,
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a6de9d7..35ac05e 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -8683,6 +8683,9 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
int i_defaclnamespace;
int i_defaclobjtype;
int i_defaclacl;
+ int i_rdefaclacl;
+ int i_initdefaclacl;
+ int i_initrdefaclacl;
int i,
ntups;
@@ -8697,13 +8700,50 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
/* Make sure we are in proper schema */
selectSourceSchema(fout, "pg_catalog");
- appendPQExpBuffer(query, "SELECT oid, tableoid, "
- "(%s defaclrole) AS defaclrole, "
- "defaclnamespace, "
- "defaclobjtype, "
- "defaclacl "
- "FROM pg_default_acl",
- username_subquery);
+ if (fout->remoteVersion >= 90600)
+ {
+ PQExpBuffer acl_subquery = createPQExpBuffer();
+ PQExpBuffer racl_subquery = createPQExpBuffer();
+ PQExpBuffer initacl_subquery = createPQExpBuffer();
+ PQExpBuffer initracl_subquery = createPQExpBuffer();
+
+ buildACLQueries(acl_subquery, racl_subquery, initacl_subquery,
+ initracl_subquery, "defaclacl", "defaclrole",
+ "CASE WHEN defaclobjtype = 'S' THEN 's' ELSE defaclobjtype END::\"char\"",
+ dopt->binary_upgrade);
+
+ appendPQExpBuffer(query, "SELECT d.oid, d.tableoid, "
+ "(%s d.defaclrole) AS defaclrole, "
+ "d.defaclnamespace, "
+ "d.defaclobjtype, "
+ "%s AS defaclacl, "
+ "%s AS rdefaclacl, "
+ "%s AS initdefaclacl, "
+ "%s AS initrdefaclacl "
+ "FROM pg_default_acl d "
+ "LEFT JOIN pg_init_privs pip ON "
+ "(d.oid = pip.objoid "
+ "AND pip.classoid = 'pg_default_acl'::regclass "
+ "AND pip.objsubid = 0) ",
+ username_subquery,
+ acl_subquery->data,
+ racl_subquery->data,
+ initacl_subquery->data,
+ initracl_subquery->data);
+ }
+ else
+ {
+ appendPQExpBuffer(query, "SELECT oid, tableoid, "
+ "(%s defaclrole) AS defaclrole, "
+ "defaclnamespace, "
+ "defaclobjtype, "
+ "defaclacl, "
+ "NULL AS rdefaclacl, "
+ "NULL AS initdefaclacl, "
+ "NULL AS initrdefaclacl "
+ "FROM pg_default_acl",
+ username_subquery);
+ }
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
@@ -8718,6 +8758,9 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
i_defaclnamespace = PQfnumber(res, "defaclnamespace");
i_defaclobjtype = PQfnumber(res, "defaclobjtype");
i_defaclacl = PQfnumber(res, "defaclacl");
+ i_rdefaclacl = PQfnumber(res, "rdefaclacl");
+ i_initdefaclacl = PQfnumber(res, "initdefaclacl");
+ i_initrdefaclacl = PQfnumber(res, "initrdefaclacl");
for (i = 0; i < ntups; i++)
{
@@ -8738,6 +8781,9 @@ getDefaultACLs(Archive *fout, int *numDefaultACLs)
daclinfo[i].defaclrole = pg_strdup(PQgetvalue(res, i, i_defaclrole));
daclinfo[i].defaclobjtype = *(PQgetvalue(res, i, i_defaclobjtype));
daclinfo[i].defaclacl = pg_strdup(PQgetvalue(res, i, i_defaclacl));
+ daclinfo[i].rdefaclacl = pg_strdup(PQgetvalue(res, i, i_rdefaclacl));
+ daclinfo[i].initdefaclacl = pg_strdup(PQgetvalue(res, i, i_initdefaclacl));
+ daclinfo[i].initrdefaclacl = pg_strdup(PQgetvalue(res, i, i_initrdefaclacl));
/* Decide whether we want to dump it */
selectDumpableDefaultACL(&(daclinfo[i]), dopt);
@@ -14038,6 +14084,9 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
daclinfo->dobj.namespace != NULL ?
daclinfo->dobj.namespace->dobj.name : NULL,
daclinfo->defaclacl,
+ daclinfo->rdefaclacl,
+ daclinfo->initdefaclacl,
+ daclinfo->initrdefaclacl,
daclinfo->defaclrole,
fout->remoteVersion,
q))
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 77de22f..a466527 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -539,6 +539,9 @@ typedef struct _defaultACLInfo
char *defaclrole;
char defaclobjtype;
char *defaclacl;
+ char *rdefaclacl;
+ char *initdefaclacl;
+ char *initrdefaclacl;
} DefaultACLInfo;
typedef struct _blobInfo
--
2.7.4
signature.asc
Description: Digital signature
