8000 Make pg_dump's ACL, sec label, and comment entries reliably identifia… · postgres/postgres@52cc1b4 · GitHub
[go: up one dir, main page]
Skip to content

Commit 52cc1b4

Browse files
tglsfdctglsfdc
committed
Make pg_dump's ACL, sec label, and comment entries reliably identifiable.
_tocEntryRequired() expects that it can identify ACL, SECURITY LABEL, and COMMENT TOC entries that are for large objects by seeing whether the tag for them starts with "LARGE OBJECT ". While that works fine for actual large objects, which are indeed tagged that way, it's subject to false positives unless every such entry's tag starts with an appropriate type ID. And in fact it does not work for ACLs, because up to now we customarily tagged those entries with just the bare name of the object. This means that an ACL for an object named "LARGE OBJECT something" would be misclassified as data not schema, with undesirable results in a schema-only or data-only dump --- although pg_upgrade seems unaffected, due to the special case for binary-upgrade mode further down in _tocEntryRequired(). We can fix this by changing all the dumpACL calls to use the label strings already in use for comments and security labels, which do follow the convention of starting with an object type indicator. Well, mostly they follow it. dumpDatabase() got it wrong, using just the bare database name for those purposes, so that a database named "LARGE OBJECT something" would similarly be subject to having its comment or security label dropped or included when not wanted. Bring that into line too. (Note that up to now, database ACLs have not been processed by pg_dump, so that this issue doesn't affect them.) _tocEntryRequired() itself is not free of fault: it was overly liberal about matching object tags to "LARGE OBJECT " in binary-upgrade mode. This looks like it is probably harmless because there would be no data component to strip anyway in that mode, but at best it's trouble waiting to happen, so tighten that up too. The possible misclassification of SECURITY LABEL entries for databases is in principle a security problem, but the opportunities for actual exploits seem too narrow to be interesting. The other cases seem like just bugs, since an object owner can change its ACL or comment for himself, he needn't try to trick someone else into doing it by choosing a strange name. This has been broken since per-large-object TOC entries were introduced in 9.0, so back-patch to all supported branches. Discussion: https://postgr.es/m/21714.1516553459@sss.pgh.pa.us
1 parent d516aea commit 52cc1b4

File tree

2 files changed

+53
-35
lines changed

2 files changed

+53
-35
lines changed

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2903,8 +2903,14 @@ _tocEntryRequired(TocEntry *te, teSection curSection, RestoreOptions *ropt)
29032903
* other information should be generated in binary-upgrade mode (not
29042904
* the actual data).
29052905
*/
2906-
if (!(ropt->binary_upgrade && strcmp(te->desc,"BLOB") == 0) &&
2907-
!(ropt->binary_upgrade && strncmp(te->tag,"LARGE OBJECT ", 13) == 0))
2906+
if (!(ropt->binary_upgrade &&
2907+
(strcmp(te->desc, "BLOB") == 0 ||
2908+
(strcmp(te->desc, "ACL") == 0 &&
2909+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0) ||
2910+
(strcmp(te->desc, "COMMENT") == 0 &&
2911+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0) ||
2912+
(strcmp(te->desc, "SECURITY LABEL") == 0 &&
2913+
strncmp(te->tag, "LARGE OBJECT ", 13) == 0))))
29082914
res = res & REQ_SCHEMA;
29092915
}
29102916

‎src/bin/pg_dump/pg_dump.c

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2426,6 +2426,7 @@ dumpDatabase(Archive *fout)
24262426
PQExpBuffer dbQry = createPQExpBuffer();
24272427
PQExpBuffer delQry = createPQExpBuffer();
24282428
PQExpBuffer creaQry = createPQExpBuffer();
2429+
PQExpBuffer labelq = createPQExpBuffer();
24292430
PGconn *conn = GetConnection(fout);
24302431
PGresult *res;
24312432
int i_tableoid,
@@ -2712,16 +2713,20 @@ dumpDatabase(Archive *fout)
27122713
destroyPQExpBuffer(loOutQry);
27132714
}
27142715

2716+
/* Compute correct tag for comments etc */
2717+
appendPQExpBuffer(labelq, "DATABASE %s", fmtId(datname));
2718+
27152719
/* Dump DB comment if any */
27162720
if (fout->remoteVersion >= 80200)
27172721
{
27182722
/*
2719-
* 8.2 keeps comments on shared objects in a shared table, so we
2720-
* cannot use the dumpComment used for other database objects.
2723+
* 8.2 and up keep comments on shared objects in a shared table, so we
2724+
* cannot use the dumpComment() code used for other database objects.
2725+
* Be careful that the ArchiveEntry parameters match that function.
27212726
*/
27222727
char *comment = PQgetvalue(res, 0, PQfnumber(res, "description"));
27232728

2724-
if (comment && strlen(comment))
2729+
if (comment && *comment)
27252730
{
27262731
resetPQExpBuffer(dbQry);
27272732

@@ -2733,17 +2738,17 @@ dumpDatabase(Archive *fout)
27332738
appendStringLiteralAH(dbQry, comment, fout);
27342739
appendPQExpBufferStr(dbQry, ";\n");
27352740

2736-
ArchiveEntry(fout, dbCatId, createDumpId(), datname, NULL, NULL,
2737-
dba, false, "COMMENT", SECTION_NONE,
2741+
ArchiveEntry(fout, nilCatalogId, createDumpId(),
2742+
labelq->data, NULL, NULL, dba,
2743+
false, "COMMENT", SECTION_NONE,
27382744
dbQry->data, "", NULL,
2739-
&dbDumpId, 1, NULL, NULL);
2745+
&(dbDumpId), 1,
2746+
NULL, NULL);
27402747
}
27412748
}
27422749
else
27432750
{
2744-
resetPQExpBuffer(dbQry);
2745-
appendPQExpBuffer(dbQry, "DATABASE %s", fmtId(datname));
2746-
dumpComment(fout, dbQry->data, NULL, "",
2751+
dumpComment(fout, labelq->data, NULL, dba,
27472752
dbCatId, 0, dbDumpId);
27482753
}
27492754

@@ -2759,11 +2764,13 @@ dumpDatabase(Archive *fout)
27592764
shres = ExecuteSqlQuery(fout, seclabelQry->data, PGRES_TUPLES_OK);
27602765
resetPQExpBuffer(seclabelQry);
27612766
emitShSecLabels(conn, shres, seclabelQry, "DATABASE", datname);
2762-
if (strlen(seclabelQry->data))
2763-
ArchiveEntry(fout, dbCatId, createDumpId(), datname, NULL, NULL,
2764-
dba, false, "SECURITY LABEL", SECTION_NONE,
2767+
if (seclabelQry->len > 0)
2768+
ArchiveEntry(fout, nilCatalogId, createDumpId(),
2769+
labelq->data, NULL, NULL, dba,
2770+
false, "SECURITY LABEL", SECTION_NONE,
27652771
seclabelQry->data, "", NULL,
2766-
&dbDumpId, 1, NULL, NULL);
2772+
&(dbDumpId), 1,
2773+
NULL, NULL);
27672774
destroyPQExpBuffer(seclabelQry);
27682775
PQclear(shres);
27692776
}
@@ -2773,6 +2780,7 @@ dumpDatabase(Archive *fout)
27732780
destroyPQExpBuffer(dbQry);
27742781
destroyPQExpBuffer(delQry);
27752782
destroyPQExpBuffer(creaQry);
2783+
destroyPQExpBuffer(labelq);
27762784
}
27772785

27782786
/*
@@ -9643,7 +9651,7 @@ dumpNamespace(Archive *fout, NamespaceInfo *nspinfo)
96439651

96449652
if (nspinfo->dobj.dump & DUMP_COMPONENT_ACL)
96459653
dumpACL(fout, nspinfo->dobj.catId, nspinfo->dobj.dumpId, "SCHEMA",
9646-
qnspname, NULL, nspinfo->dobj.name, NULL,
9654+
qnspname, NULL, labelq->data, NULL,
96479655
nspinfo->rolname, nspinfo->nspacl, nspinfo->rnspacl,
96489656
nspinfo->initnspacl, nspinfo->initrnspacl);
96499657

@@ -9938,7 +9946,7 @@ dumpEnumType(Archive *fout, TypeInfo *tyinfo)
99389946

99399947
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
99409948
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
9941-
qtypname, NULL, tyinfo->dobj.name,
9949+
qtypname, NULL, labelq->data,
99429950
tyinfo->dobj.namespace->dobj.name,
99439951
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
99449952
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10077,7 +10085,7 @@ dumpRangeType(Archive *fout, TypeInfo *tyinfo)
1007710085

1007810086
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1007910087
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10080-
qtypname, NULL, tyinfo->dobj.name,
10088+
qtypname, NULL, labelq->data,
1008110089
tyinfo->dobj.namespace->dobj.name,
1008210090
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1008310091
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10153,7 +10161,7 @@ dumpUndefinedType(Archive *fout, TypeInfo *tyinfo)
1015310161

1015410162
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1015510163
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10156-
qtypname, NULL, tyinfo->dobj.name,
10164+
qtypname, NULL, labelq->data,
1015710165
tyinfo->dobj.namespace->dobj.name,
1015810166
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1015910167
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10548,7 +10556,7 @@ dumpBaseType(Archive *fout, TypeInfo *tyinfo)
1054810556

1054910557
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1055010558
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10551-
qtypname, NULL, tyinfo->dobj.name,
10559+
qtypname, NULL, labelq->data,
1055210560
tyinfo->dobj.namespace->dobj.name,
1055310561
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1055410562
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10717,7 +10725,7 @@ dumpDomain(Archive *fout, TypeInfo *tyinfo)
1071710725

1071810726
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1071910727
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10720-
qtypname, NULL, tyinfo->dobj.name,
10728+
qtypname, NULL, labelq->data,
1072110729
tyinfo->dobj.namespace->dobj.name,
1072210730
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1072310731
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -10953,7 +10961,7 @@ dumpCompositeType(Archive *fout, TypeInfo *tyinfo)
1095310961

1095410962
if (tyinfo->dobj.dump & DUMP_COMPONENT_ACL)
1095510963
dumpACL(fout, tyinfo->dobj.catId, tyinfo->dobj.dumpId, "TYPE",
10956-
qtypname, NULL, tyinfo->dobj.name,
10964+
qtypname, NULL, labelq->data,
1095710965
tyinfo->dobj.namespace->dobj.name,
1095810966
tyinfo->rolname, tyinfo->typacl, tyinfo->rtypacl,
1095910967
tyinfo->inittypacl, tyinfo->initrtypacl);
@@ -11272,7 +11280,7 @@ dumpProcLang(Archive *fout, ProcLangInfo *plang)
1127211280

1127311281
if (plang->lanpltrusted && plang->dobj.dump & DUMP_COMPONENT_ACL)
1127411282
dumpACL(fout, plang->dobj.catId, plang->dobj.dumpId, "LANGUAGE",
11275-
qlanname, NULL, plang->dobj.name,
11283+
qlanname, NULL, labelq->data,
1127611284
lanschema,
1127711285
plang->lanowner, plang->lanacl, plang->rlanacl,
1127811286
plang->initlanacl, plang->initrlanacl);
@@ -11944,7 +11952,7 @@ dumpFunc(Archive *fout, FuncInfo *finfo)
1194411952

1194511953
if (finfo->dobj.dump & DUMP_COMPONENT_ACL)
1194611954
dumpACL(fout, finfo->dobj.catId, finfo->dobj.dumpId, "FUNCTION",
11947-
funcsig, NULL, funcsig_tag,
11955+
funcsig, NULL, labelq->data,
1194811956
finfo->dobj.namespace->dobj.name,
1194911957
finfo->rolname, finfo->proacl, finfo->rproacl,
1195011958
finfo->initproacl, finfo->initrproacl);
@@ -13997,15 +14005,13 @@ dumpAgg(Archive *fout, AggInfo *agginfo)
1399714005
* syntax for zero-argument aggregates and ordered-set aggregates.
1399814006
*/
1399914007
free(aggsig);
14000-
free(aggsig_tag);
1400114008

1400214009
aggsig = format_function_signature(fout, &agginfo->aggfn, true);
14003-
aggsig_tag = format_function_signature(fout, &agginfo->aggfn, false);
1400414010

1400514011
if (agginfo->aggfn.dobj.dump & DUMP_COMPONENT_ACL)
1400614012
dumpACL(fout, agginfo->aggfn.dobj.catId, agginfo->aggfn.dobj.dumpId,
1400714013
"FUNCTION",
14008-
aggsig, NULL, aggsig_tag,
14014+
aggsig, NULL, labelq->data,
1400914015
agginfo->aggfn.dobj.namespace->dobj.name,
1401014016
agginfo->aggfn.rolname, agginfo->aggfn.proacl,
1401114017
agginfo->aggfn.rproacl,
@@ -14451,7 +14457,7 @@ dumpForeignDataWrapper(Archive *fout, FdwInfo *fdwinfo)
1445114457
if (fdwinfo->dobj.dump & DUMP_COMPONENT_ACL)
1445214458
dumpACL(fout, fdwinfo->dobj.catId, fdwinfo->dobj.dumpId,
1445314459
"FOREIGN DATA WRAPPER",
14454-
qfdwname, NULL, fdwinfo->dobj.name,
14460+
qfdwname, NULL, labelq->data,
1445514461
NULL, fdwinfo->rolname,
1445614462
fdwinfo->fdwacl, fdwinfo->rfdwacl,
1445714463
fdwinfo->initfdwacl, fdwinfo->initrfdwacl);
@@ -14548,7 +14554,7 @@ dumpForeignServer(Archive *fout, ForeignServerInfo *srvinfo)
1454814554
if (srvinfo->dobj.dump & DUMP_COMPONENT_ACL)
1454914555
dumpACL(fout, srvinfo->dobj.catId, srvinfo->dobj.dumpId,
1455014556
"FOREIGN SERVER",
14551-
qsrvname, NULL, srvinfo->dobj.name,
14557+
qsrvname, NULL, labelq->data,
1455214558
NULL, srvinfo->rolname,
1455314559
srvinfo->srvacl, srvinfo->rsrvacl,
1455414560
srvinfo->initsrvacl, srvinfo->initrsrvacl);
@@ -14756,7 +14762,8 @@ dumpDefaultACL(Archive *fout, DefaultACLInfo *daclinfo)
1475614762
* FOREIGN DATA WRAPPER, SERVER, or LARGE OBJECT.
1475714763
* 'name' is the formatted name of the object. Must be quoted etc. already.
1475814764
* 'subname' is the formatted name of the sub-object, if any. Must be quoted.
14759-
* 'tag' is the tag for the archive entry (typ. unquoted name of object).
14765+
* 'tag' is the tag for the archive entry (should be the same tag as would be
14766+
* used for comments etc; for example "TABLE foo").
1476014767
* 'nspname' is the namespace the object is in (NULL if none).
1476114768
* 'owner' is the owner, NULL if there is no owner (for languages).
1476214769
* 'acls' contains the ACL string of the object from the appropriate system
@@ -14866,7 +14873,7 @@ dumpSecLabel(Archive *fout, const char *target,
1486614873
if (dopt->no_security_labels)
1486714874
return;
1486814875

14869-
/* Comments are schema not data ... except blob comments are data */
14876+
/* Security labels are schema not data ... except blob labels are data */
1487014877
if (strncmp(target, "LARGE OBJECT ", 13) != 0)
1487114878
{
1487214879
if (dopt->dataOnly)
@@ -15160,13 +15167,18 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1516015167
/* Handle the ACL here */
1516115168
namecopy = pg_strdup(fmtId(tbinfo->dobj.name));
1516215169
if (tbinfo->dobj.dump & DUMP_COMPONENT_ACL)
15170+
{
15171+
const char *objtype =
15172+
(tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" : "TABLE";
15173+
char *acltag = psprintf("%s %s", objtype, namecopy);
15174+
1516315175
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId,
15164-
(tbinfo->relkind == RELKIND_SEQUENCE) ? "SEQUENCE" :
15165-
"TABLE",
15166-
namecopy, NULL, tbinfo->dobj.name,
15176+
objtype, namecopy, NULL, acltag,
1516715177
tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
1516815178
tbinfo->relacl, tbinfo->rrelacl,
1516915179
tbinfo->initrelacl, tbinfo->initrrelacl);
15180+
free(acltag);
15181+
}
1517015182

1517115183
/*
1517215184
* Handle column ACLs, if any. Note: we pull these with a separate query
@@ -15250,7 +15262,7 @@ dumpTable(Archive *fout, TableInfo *tbinfo)
1525015262
char *acltag;
1525115263

1525215264
attnamecopy = pg_strdup(fmtId(attname));
15253-
acltag = psprintf("%s.%s", tbinfo->dobj.name, attname);
15265+
acltag = psprintf("COLUMN %s.%s", namecopy, attnamecopy);
1525415266
/* Column's GRANT type is always TABLE */
1525515267
dumpACL(fout, tbinfo->dobj.catId, tbinfo->dobj.dumpId, "TABLE",
1525615268
namecopy, attnamecopy, acltag,

0 commit comments

Comments
 (0)
0