Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit b8daf89

Browse files
nmischnmisch
committed
Fix pg_dump for GRANT OPTION among initial privileges.
The context is an object that no longer bears some aclitem that it bore initially. (A user issued REVOKE or GRANT statements upon the object.) pg_dump is forming SQL to reproduce the object ACL. Since initdb creates no ACL bearing GRANT OPTION, reaching this bug requires an extension where the creation script establishes such an ACL. No PGXN extension does that. If an installation did reach the bug, pg_dump would have omitted a semicolon, causing a REVOKE and the next SQL statement to fail. Separately, since the affected code exists to eliminate an entire aclitem, it wants plain REVOKE, not REVOKE GRANT OPTION FOR. Back-patch to 9.6, where commit 23f34fa first appeared. Discussion: https://postgr.es/m/20210109102423.GA160022@rfd.leadboat.com
1 parent 6eb3fc7 commit b8daf89

File tree

3 files changed

+70
-39
lines changed

3 files changed

+70
-39
lines changed

‎src/bin/pg_dump/dumputils.c

+22-39
Original file line numberDiff line numberDiff line change
@@ -168,48 +168,28 @@ buildACLCommands(const char *name, const char *subname, const char *nspname,
168168
for (i = 0; i < nraclitems; i++)
169169
{
170170
if (!parseAclItem(raclitems[i], type, name, subname, remoteVersion,
171-
grantee, grantor, privs, privswgo))
171+
grantee, grantor, privs, NULL))
172172
{
173173
ok = false;
174174
break;
175175
}
176176

177-
if (privs->len > 0 || privswgo->len > 0)
177+
if (privs->len > 0)
178178
{
179-
if (privs->len > 0)
180-
{
181-
appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s ",
182-
prefix, privs->data, type);
183-
if (nspname && *nspname)
184-
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
185-
appendPQExpBuffer(firstsql, "%s FROM ", name);
186-
if (grantee->len == 0)
187-
appendPQExpBufferStr(firstsql, "PUBLIC;\n");
188-
else if (strncmp(grantee->data, "group ",
189-
strlen("group ")) == 0)
190-
appendPQExpBuffer(firstsql, "GROUP %s;\n",
191-
fmtId(grantee->data + strlen("group ")));
192-
else
193-
appendPQExpBuffer(firstsql, "%s;\n",
194-
fmtId(grantee->data));
195-
}
196-
if (privswgo->len > 0)
197-
{
198-
appendPQExpBuffer(firstsql,
199-
"%sREVOKE GRANT OPTION FOR %s ON %s ",
200-
prefix, privswgo->data, type);
201-
if (nspname && *nspname)
202-
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
203-
appendPQExpBuffer(firstsql, "%s FROM ", name);
204-
if (grantee->len == 0)
205-
appendPQExpBufferStr(firstsql, "PUBLIC");
206-
else if (strncmp(grantee->data, "group ",
207-
strlen("group ")) == 0)
208-
appendPQExpBuffer(firstsql, "GROUP %s",
209-
fmtId(grantee->data + strlen("group ")));
210-
else
211-
appendPQExpBufferStr(firstsql, fmtId(grantee->data));
212-
}
179+
appendPQExpBuffer(firstsql, "%sREVOKE %s ON %s ",
180+
prefix, privs->data, type);
181+
if (nspname && *nspname)
182+
appendPQExpBuffer(firstsql, "%s.", fmtId(nspname));
183+
appendPQExpBuffer(firstsql, "%s FROM ", name);
184+
if (grantee->len == 0)
185+
appendPQExpBufferStr(firstsql, "PUBLIC;\n");
186+
else if (strncmp(grantee->data, "group ",
187+
strlen("group ")) == 0)
188+
appendPQExpBuffer(firstsql, "GROUP %s;\n",
189+
fmtId(grantee->data + strlen("group ")));
190+
else
191+
appendPQExpBuffer(firstsql, "%s;\n",
192+
fmtId(grantee->data));
213193
}
214194
}
215195
}
@@ -462,8 +442,11 @@ buildDefaultACLCommands(const char *type, const char *nspname,
462442
* The returned grantee string will be the dequoted username or groupname
463443
* (preceded with "group " in the latter case). Note that a grant to PUBLIC
464444
* is represented by an empty grantee string. The returned grantor is the
465-
* dequoted grantor name. Privilege characters are decoded and split between
466-
* privileges with grant option (privswgo) and without (privs).
445+
* dequoted grantor name. Privilege characters are translated to GRANT/REVOKE
446+
* comma-separated privileges lists. If "privswgo" is non-NULL, the result is
447+
* separate lists for privileges with grant option ("privswgo") and without
448+
* ("privs"). Otherwise, "privs" bears every relevant privilege, ignoring the
449+
* grant option distinction.
467450
*
468451
* Note: for cross-version compatibility, it's important to use ALL to
469452
* represent the privilege sets whenever appropriate.
@@ -514,7 +497,7 @@ parseAclItem(const char *item, const char *type,
514497
do { \
515498
if ((pos = strchr(eqpos + 1, code))) \
516499
{ \
517-
if (*(pos + 1) == '*') \
500+
if (*(pos + 1) == '*' && privswgo != NULL) \
518501
{ \
519502
AddAcl(privswgo, keywd, subname); \
520503
all_without_go = false; \

‎src/test/modules/test_pg_dump/t/001_base.pl

+39
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,45 @@
348348
},
349349
},
350350
351+
'REVOKE ALL ON FUNCTION wgo_then_no_access' => {
352+
create_order => 3,
353+
create_sql => q{
354+
DO $$BEGIN EXECUTE format(
355+
'REVOKE ALL ON FUNCTION wgo_then_no_access()
356+
FROM pg_signal_backend, public, %I',
357+
(SELECT usename
358+
FROM pg_user JOIN pg_proc ON proowner = usesysid
359+
WHERE proname = 'wgo_then_no_access')); END$$;},
360+
regexp => qr/^
361+
\QREVOKE ALL ON FUNCTION public.wgo_then_no_access() FROM PUBLIC;\E
362+
\n\QREVOKE ALL ON FUNCTION public.wgo_then_no_access() FROM \E.*;
363+
\n\QREVOKE ALL ON FUNCTION public.wgo_then_no_access() FROM pg_signal_backend;\E
364+
/xm,
365+
like => {
366+
%full_runs,
367+
schema_only => 1,
368+
section_pre_data => 1,
369+
},
370+
unlike => { no_privs => 1, },
371+
},
372+
373+
'REVOKE GRANT OPTION FOR UPDATE ON SEQUENCE wgo_then_regular' => {
374+
create_order => 3,
375+
create_sql => 'REVOKE GRANT OPTION FOR UPDATE ON SEQUENCE
376+
wgo_then_regular FROM pg_signal_backend;',
377+
regexp => qr/^
378+
\QREVOKE ALL ON SEQUENCE public.wgo_then_regular FROM pg_signal_backend;\E
379+
\n\QGRANT SELECT,UPDATE ON SEQUENCE public.wgo_then_regular TO pg_signal_backend;\E
380+
\n\QGRANT USAGE ON SEQUENCE public.wgo_then_regular TO pg_signal_backend WITH GRANT OPTION;\E
381+
/xm,
382+
like => {
383+
%full_runs,
384+
schema_only => 1,
385+
section_pre_data => 1,
386+
},
387+
unlike => { no_privs => 1, },
388+
},
389+
351390
'CREATE ACCESS METHOD regress_test_am' => {
352391
regexp => qr/^
353392
\QCREATE ACCESS METHOD regress_test_am TYPE INDEX HANDLER bthandler;\E

‎src/test/modules/test_pg_dump/test_pg_dump--1.0.sql

+9
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ GRANT SELECT(col1) ON regress_pg_dump_table TO public;
2828
GRANT SELECT(col2) ON regress_pg_dump_table TO regress_dump_test_role;
2929
REVOKE SELECT(col2) ON regress_pg_dump_table FROM regress_dump_test_role;
3030

31+
CREATE FUNCTION wgo_then_no_access() RETURNS int LANGUAGE SQL AS 'SELECT 1';
32+
GRANT ALL ON FUNCTION wgo_then_no_access()
33+
TO pg_signal_backend WITH GRANT OPTION;
34+
35+
CREATE SEQUENCE wgo_then_regular;
36+
GRANT ALL ON SEQUENCE wgo_then_regular TO pg_signal_backend WITH GRANT OPTION;
37+
REVOKE GRANT OPTION FOR SELECT ON SEQUENCE wgo_then_regular
38+
FROM pg_signal_backend;
39+
3140
CREATE ACCESS METHOD regress_test_am TYPE INDEX HANDLER bthandler;
3241

3342
-- Create a set of objects that are part of the schema created by

0 commit comments

Comments
 (0)