[v9.1] access control reworks in ALTER TABLE
Started by KaiGai Koheialmost 16 years ago2 messages
The attached patch reworks access control logics corresponding
to ALTER TABLE statement, as a groundwork for the upcoming
enhanced security features.
As we discussed in the last commit fest, it shall be a preferable
way to wrap up a unit of access control logics into some functions
which are categorized by the actions to be checked. These functions
will be able to perform as entrypoints of existing permission checks
and the upcoming security features, like SELinux.
The first step of this efforts is to consolidate existing access
control codes into a unit for each operations. Once we reworks them,
it is obvious to replace these unit by access control functions.
ALTER TABLE is the most functional command in PostgreSQL, so it may
be a good stater for this efforts.
--------
In this patch, I put access control codes in the execution stage
after all needed information getting gathered. Then, privileges
are checked at once.
The ALTER TABLE implementation, exceptionally, has a few stages.
The preparation stage applies sanity and permission checks, and
expand the given command to child relations. Then, the execution
stage updates system catalogs according to the given command.
However, we don't have multiple stages in most of ddl statements.
So, even if we adopt a basis to apply permission checks in the
preparation stage, we cannot reuse this basis for others.
Most of AT commands checks ownerships of the relation using
ATSimplePermissions() in the preparation stage, but now a few number
of commands also needs to check privileges in the execution stage
dues to some reasons.
(1) commands with self recursion
ATExecDropColumn(), ATAddCheckConstraint() and ATExecDropConstraint()
implements its recursive calls by itself. It means we cannot know
child relations to be altered in the preparation stage, so it has
the following code typically.
| /* At top level, permission check was done in ATPrepCmd, else do it */
| if (recursing)
| ATSimplePermissions(rel, false);
(2) commands directly called from alter.c
RenameRelation(), renameatt() and AlterTableNamespace() are directly
called from alter.c, so they have no preparation stage. Instead of
the ATSimplePermissions(), it calls CheckRelationOwnership() in the
caller. Also, renameatt() checks ownership of the relation during
its recursive calls.
(3) commands need privilges to other objects
- AlterTableNamespace() checks ACL_CREATE on the specified namespace
in the LookupCreationNamespace().
- ATPrepSetTableSpace() checks ACL_CREATE on the specified tablespace.
- ATExecAddIndex() checks ACL_CREATE on the namespace and ACL_CREATE
on the specified tablespace (if exist) in DefineIndex().
- ATExecAddInherit() checks ownership of the specified parent relation.
- ATAddForeignKeyConstraint() checks ACL_REFERENCES on the both of
specified table/columns.
(4) commands needs its own sanity checks
ATSimplePermissions() also ensures the relation is table (or view),
and not a system catalog, not only checks its ownership.
However, some of commands need to apply an exceptional sanity checks.
ATSimplePermissionsRelationOrIndex() is used for sanity checks of the
AT commands which is allowed on indexes. ATPrepSetStatistics() skips
checks to ensure the relation is not system catalogs.
At first, this patch broke down the ATSimplePermissions() into sanity
checks and permission checks.
This patch break down the ATSimplePermissions() into sanity checks
and permissions checks, then later part is moved to the execution
stage.
- ATCheckSanity()
- ATCheckOwnership()
It enables to eliminate (1) special case handling on the commands with
self recursion, and (4) exceptional sanity checks because ATCheckSanity()
takes three arguments to control sanity checks behavior.
It also means we don't need to consider (2) functions are exceptional,
because all the commands apply checks in the execution stage.
For the (3) functions, it moved ownership checks nearby the permission
checks to other object, according to the basis: access control codes in
the execution stage after all needed information getting gathered, and
these are checked at once.
ATExecAddIndex() is a wrapper function of the DefineIndex(). It requires
caller checks ownership of the relation to be indexed, then it checks
rest of permissions for namespace and tablespace. I moved checks for
ownership of the relation into DefineIndex(), and also eliminated the
CheckRelationOwnership() from standard_ProcessUtility() that is another
caller of DefineIndex().
Any comments?
Thanks,
--
KaiGai Kohei <kaigai@ak.jp.nec.com>
Attachments:
pgsql-at-reworks.1.patchapplication/octect-stream; name=pgsql-at-reworks.1.patchDownload
*** a/src/backend/commands/alter.c
--- b/src/backend/commands/alter.c
***************
*** 41,46 ****
--- 41,49 ----
void
ExecRenameStmt(RenameStmt *stmt)
{
+ Oid relOid;
+ bool inhopt;
+
switch (stmt->renameType)
{
case OBJECT_AGGREGATE:
***************
*** 87,143 **** ExecRenameStmt(RenameStmt *stmt)
case OBJECT_SEQUENCE:
case OBJECT_VIEW:
case OBJECT_INDEX:
case OBJECT_COLUMN:
case OBJECT_TRIGGER:
! {
! Oid relid;
!
! CheckRelationOwnership(stmt->relation, true);
!
! relid = RangeVarGetRelid(stmt->relation, false);
!
! switch (stmt->renameType)
! {
! case OBJECT_TABLE:
! case OBJECT_SEQUENCE:
! case OBJECT_VIEW:
! case OBJECT_INDEX:
! {
! /*
! * RENAME TABLE requires that we (still) hold
! * CREATE rights on the containing namespace, as
! * well as ownership of the table.
! */
! Oid namespaceId = get_rel_namespace(relid);
! AclResult aclresult;
!
! aclresult = pg_namespace_aclcheck(namespaceId,
! GetUserId(),
! ACL_CREATE);
! if (aclresult != ACLCHECK_OK)
! aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
! get_namespace_name(namespaceId));
!
! RenameRelation(relid, stmt->newname, stmt->renameType);
! break;
! }
! case OBJECT_COLUMN:
! renameatt(relid,
! stmt->subname, /* old att name */
! stmt->newname, /* new att name */
! interpretInhOption(stmt->relation->inhOpt), /* recursive? */
! 0); /* expected inhcount */
! break;
! case OBJECT_TRIGGER:
! renametrig(relid,
! stmt->subname, /* old att name */
! stmt->newname); /* new att name */
! break;
! default:
! /* can't happen */ ;
! }
! break;
! }
case OBJECT_TSPARSER:
RenameTSParser(stmt->object, stmt->newname);
--- 90,115 ----
case OBJECT_SEQUENCE:
case OBJECT_VIEW:
case OBJECT_INDEX:
+ relOid = RangeVarGetRelid(stmt->relation, false);
+ RenameRelation(relOid, stmt->newname, stmt->renameType);
+ break;
+
case OBJECT_COLUMN:
+ relOid = RangeVarGetRelid(stmt->relation, false);
+ inhopt = interpretInhOption(stmt->relation->inhOpt);
+ renameatt(relOid,
+ stmt->subname, /* old att name */
+ stmt->newname, /* new att name */
+ inhopt, /* recursive? */
+ 0); /* expected inhcount */
+ break;
+
case OBJECT_TRIGGER:
! relOid = RangeVarGetRelid(stmt->relation, false);
! renametrig(relOid,
! stmt->subname, /* old att name */
! stmt->newname); /* new att name */
! break;
case OBJECT_TSPARSER:
RenameTSParser(stmt->object, stmt->newname);
***************
*** 187,193 **** ExecAlterObjectSchemaStmt(AlterObjectSchemaStmt *stmt)
case OBJECT_SEQUENCE:
case OBJECT_TABLE:
case OBJECT_VIEW:
- CheckRelationOwnership(stmt->relation, true);
AlterTableNamespace(stmt->relation, stmt->newschema,
stmt->objectType);
break;
--- 159,164 ----
*** a/src/backend/commands/indexcmds.c
--- b/src/backend/commands/indexcmds.c
***************
*** 195,217 **** DefineIndex(RangeVar *heapRelation,
errmsg("cannot create indexes on temporary tables of other sessions")));
/*
- * Verify we (still) have CREATE rights in the rel's namespace.
- * (Presumably we did when the rel was created, but maybe not anymore.)
- * Skip check if caller doesn't want it. Also skip check if
- * bootstrapping, since permissions machinery may not be working yet.
- */
- if (check_rights && !IsBootstrapProcessingMode())
- {
- AclResult aclresult;
-
- aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(),
- ACL_CREATE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
- get_namespace_name(namespaceId));
- }
-
- /*
* Select tablespace to use. If not specified, use default tablespace
* (which may in turn default to database's default).
*/
--- 195,200 ----
***************
*** 230,235 **** DefineIndex(RangeVar *heapRelation,
--- 213,245 ----
/* note InvalidOid is OK in this case */
}
+ /*
+ * Permission checks to create a new index.
+ * Skip check if caller doesn't want it. It should not be skipped
+ * except for bootstraping mode and internal index rebuiling.
+ */
+ if (check_rights)
+ {
+ AclResult aclresult;
+
+ /*
+ * Verify we have ownership of the relation to be indexed on.
+ */
+ if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+ RelationGetRelationName(rel));
+
+ /*
+ * Verify we (still) have CREATE rights in the rel's namespace.
+ * (Presumably we did when the rel was created, but maybe not anymore.)
+ */
+ aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(),
+ ACL_CREATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
+ get_namespace_name(namespaceId));
+ }
+
/* Check permissions except when using database's default */
if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
{
***************
*** 243,248 **** DefineIndex(RangeVar *heapRelation,
--- 253,269 ----
}
/*
+ * Ensure the new index is not on system catalogs, except for
+ * the case when bootstraping mode
+ */
+ if (!IsBootstrapProcessingMode() &&
+ !allowSystemTableMods && IsSystemRelation(rel))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a system catalog",
+ RelationGetRelationName(rel))));
+
+ /*
* Force shared indexes into the pg_global tablespace. This is a bit of a
* hack but seems simpler than marking them in the BKI commands. On the
* other hand, if it's not shared, don't allow it to be placed there.
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 262,269 **** static void ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
static void ATRewriteTables(List **wqueue);
static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap);
static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel);
! static void ATSimplePermissions(Relation rel, bool allowView);
! static void ATSimplePermissionsRelationOrIndex(Relation rel);
static void ATSimpleRecursion(List **wqueue, Relation rel,
AlterTableCmd *cmd, bool recurse);
static void ATOneLevelRecursion(List **wqueue, Relation rel,
--- 262,269 ----
static void ATRewriteTables(List **wqueue);
static void ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap);
static AlteredTableInfo *ATGetQueueEntry(List **wqueue, Relation rel);
! static void ATCheckSanity(Relation rel, bool viewOK, bool indexOK, bool catalogOK);
! static void ATCheckOwnership(Relation rel);
static void ATSimpleRecursion(List **wqueue, Relation rel,
AlterTableCmd *cmd, bool recurse);
static void ATOneLevelRecursion(List **wqueue, Relation rel,
***************
*** 280,287 **** static void ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
const char *colName);
static void ATExecColumnDefault(Relation rel, const char *colName,
Node *newDefault);
- static void ATPrepSetStatistics(Relation rel, const char *colName,
- Node *newValue);
static void ATExecSetStatistics(Relation rel, const char *colName,
Node *newValue);
static void ATExecSetOptions(Relation rel, const char *colName,
--- 280,285 ----
***************
*** 389,410 **** DefineRelation(CreateStmt *stmt, char relkind)
/*
* Look up the namespace in which we are supposed to create the relation.
- * Check we have permission to create there. Skip check if bootstrapping,
- * since permissions machinery may not be working yet.
*/
namespaceId = RangeVarGetCreationNamespace(stmt->relation);
- if (!IsBootstrapProcessingMode())
- {
- AclResult aclresult;
-
- aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(),
- ACL_CREATE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
- get_namespace_name(namespaceId));
- }
-
/*
* Select tablespace to use. If not specified, use default tablespace
* (which may in turn default to database's default).
--- 387,395 ----
***************
*** 424,441 **** DefineRelation(CreateStmt *stmt, char relkind)
/* note InvalidOid is OK in this case */
}
- /* Check permissions except when using database's default */
- if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
- {
- AclResult aclresult;
-
- aclresult = pg_tablespace_aclcheck(tablespaceId, GetUserId(),
- ACL_CREATE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, ACL_KIND_TABLESPACE,
- get_tablespace_name(tablespaceId));
- }
-
/* In all cases disallow placing user relations in pg_global */
if (tablespaceId == GLOBALTABLESPACE_OID)
ereport(ERROR,
--- 409,414 ----
***************
*** 474,479 **** DefineRelation(CreateStmt *stmt, char relkind)
--- 447,492 ----
descriptor->tdhasoid = (localHasOids || parentOidCount > 0);
/*
+ * Check we have permission to create there. Skip check if bootstrapping,
+ * since permissions machinery may not be working yet.
+ */
+ if (!IsBootstrapProcessingMode())
+ {
+ AclResult aclresult;
+
+ aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(),
+ ACL_CREATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
+ get_namespace_name(namespaceId));
+ }
+
+ /* Check permissions except when using database's default */
+ if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
+ {
+ AclResult aclresult;
+
+ aclresult = pg_tablespace_aclcheck(tablespaceId, GetUserId(),
+ ACL_CREATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_TABLESPACE,
+ get_tablespace_name(tablespaceId));
+ }
+
+ /*
+ * We should have an UNDER permission flag for this, but for now,
+ * demand that creator of a child table own the parent.
+ */
+ foreach(listptr, inheritOids)
+ {
+ Oid parentId = lfirst_oid(listptr);
+
+ if (!pg_class_ownercheck(parentId, GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+ get_rel_name(parentId));
+ }
+
+ /*
* Find columns with default values and prepare for insertion of the
* defaults. Pre-cooked (that is, inherited) defaults go into a list of
* CookedConstraint structs that we'll pass to heap_create_with_catalog,
***************
*** 1303,1317 **** MergeAttributes(List *schema, List *supers, bool istemp,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot inherit from temporary relation \"%s\"",
parent->relname)));
-
- /*
- * We should have an UNDER permission flag for this, but for now,
- * demand that creator of a child table own the parent.
- */
- if (!pg_class_ownercheck(RelationGetRelid(relation), GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
- RelationGetRelationName(relation));
-
/*
* Reject duplications in the list of parents.
*/
--- 1316,1321 ----
***************
*** 1956,1966 **** renameatt(Oid myrelid,
errmsg("cannot rename column of typed table")));
/*
! * permissions checking. only the owner of a class can change its schema.
*/
! if (!pg_class_ownercheck(myrelid, GetUserId()))
! aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
! RelationGetRelationName(targetrelation));
if (!allowSystemTableMods && IsSystemRelation(targetrelation))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
--- 1960,1969 ----
errmsg("cannot rename column of typed table")));
/*
! * permissions checking.
*/
! ATCheckOwnership(targetrelation);
!
if (!allowSystemTableMods && IsSystemRelation(targetrelation))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
***************
*** 2092,2097 **** RenameRelation(Oid myrelid, const char *newrelname, ObjectType reltype)
--- 2095,2101 ----
Relation targetrelation;
Oid namespaceId;
char relkind;
+ AclResult aclresult;
/*
* Grab an exclusive lock on the target table, index, sequence or view,
***************
*** 2119,2124 **** RenameRelation(Oid myrelid, const char *newrelname, ObjectType reltype)
--- 2123,2145 ----
RelationGetRelationName(targetrelation))));
/*
+ * Permission checks
+ */
+ ATCheckOwnership(targetrelation);
+
+ aclresult = pg_namespace_aclcheck(namespaceId, GetUserId(), ACL_CREATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_NAMESPACE,
+ get_namespace_name(namespaceId));
+
+ /* disallow to rename system relations */
+ if (!allowSystemTableMods && IsSystemRelation(targetrelation))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a system catalog",
+ RelationGetRelationName(targetrelation))));
+
+ /*
* Don't allow ALTER TABLE on composite types. We want people to use ALTER
* TYPE for that.
*/
***************
*** 2424,2437 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
switch (cmd->subtype)
{
case AT_AddColumn: /* ADD COLUMN */
! ATSimplePermissions(rel, false);
/* Performs own recursion */
ATPrepAddColumn(wqueue, rel, recurse, cmd);
pass = AT_PASS_ADD_COL;
break;
case AT_AddColumnToView: /* add column via CREATE OR REPLACE
* VIEW */
! ATSimplePermissions(rel, true);
/* Performs own recursion */
ATPrepAddColumn(wqueue, rel, recurse, cmd);
pass = AT_PASS_ADD_COL;
--- 2445,2458 ----
switch (cmd->subtype)
{
case AT_AddColumn: /* ADD COLUMN */
! ATCheckSanity(rel, false, false, false);
/* Performs own recursion */
ATPrepAddColumn(wqueue, rel, recurse, cmd);
pass = AT_PASS_ADD_COL;
break;
case AT_AddColumnToView: /* add column via CREATE OR REPLACE
* VIEW */
! ATCheckSanity(rel, true, false, false);
/* Performs own recursion */
ATPrepAddColumn(wqueue, rel, recurse, cmd);
pass = AT_PASS_ADD_COL;
***************
*** 2444,2486 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
* substitutes default values into INSERTs before it expands
* rules.
*/
! ATSimplePermissions(rel, true);
ATSimpleRecursion(wqueue, rel, cmd, recurse);
/* No command-specific prep needed */
pass = cmd->def ? AT_PASS_ADD_CONSTR : AT_PASS_DROP;
break;
case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */
! ATSimplePermissions(rel, false);
ATSimpleRecursion(wqueue, rel, cmd, recurse);
/* No command-specific prep needed */
pass = AT_PASS_DROP;
break;
case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */
! ATSimplePermissions(rel, false);
ATSimpleRecursion(wqueue, rel, cmd, recurse);
/* No command-specific prep needed */
pass = AT_PASS_ADD_CONSTR;
break;
case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */
ATSimpleRecursion(wqueue, rel, cmd, recurse);
- /* Performs own permission checks */
- ATPrepSetStatistics(rel, cmd->name, cmd->def);
pass = AT_PASS_COL_ATTRS;
break;
case AT_SetOptions: /* ALTER COLUMN SET ( options ) */
case AT_ResetOptions: /* ALTER COLUMN RESET ( options ) */
! ATSimplePermissionsRelationOrIndex(rel);
/* This command never recurses */
pass = AT_PASS_COL_ATTRS;
break;
case AT_SetStorage: /* ALTER COLUMN SET STORAGE */
! ATSimplePermissions(rel, false);
ATSimpleRecursion(wqueue, rel, cmd, recurse);
/* No command-specific prep needed */
pass = AT_PASS_COL_ATTRS;
break;
case AT_DropColumn: /* DROP COLUMN */
! ATSimplePermissions(rel, false);
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
--- 2465,2506 ----
* substitutes default values into INSERTs before it expands
* rules.
*/
! ATCheckSanity(rel, true, false, false);
ATSimpleRecursion(wqueue, rel, cmd, recurse);
/* No command-specific prep needed */
pass = cmd->def ? AT_PASS_ADD_CONSTR : AT_PASS_DROP;
break;
case AT_DropNotNull: /* ALTER COLUMN DROP NOT NULL */
! ATCheckSanity(rel, false, false, false);
ATSimpleRecursion(wqueue, rel, cmd, recurse);
/* No command-specific prep needed */
pass = AT_PASS_DROP;
break;
case AT_SetNotNull: /* ALTER COLUMN SET NOT NULL */
! ATCheckSanity(rel, false, false, false);
ATSimpleRecursion(wqueue, rel, cmd, recurse);
/* No command-specific prep needed */
pass = AT_PASS_ADD_CONSTR;
break;
case AT_SetStatistics: /* ALTER COLUMN SET STATISTICS */
+ ATCheckSanity(rel, false, true, true);
ATSimpleRecursion(wqueue, rel, cmd, recurse);
pass = AT_PASS_COL_ATTRS;
break;
case AT_SetOptions: /* ALTER COLUMN SET ( options ) */
case AT_ResetOptions: /* ALTER COLUMN RESET ( options ) */
! ATCheckSanity(rel, false, true, true);
/* This command never recurses */
pass = AT_PASS_COL_ATTRS;
break;
case AT_SetStorage: /* ALTER COLUMN SET STORAGE */
! ATCheckSanity(rel, false, false, false);
ATSimpleRecursion(wqueue, rel, cmd, recurse);
/* No command-specific prep needed */
pass = AT_PASS_COL_ATTRS;
break;
case AT_DropColumn: /* DROP COLUMN */
! ATCheckSanity(rel, false, false, false);
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
***************
*** 2488,2500 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_DROP;
break;
case AT_AddIndex: /* ADD INDEX */
! ATSimplePermissions(rel, false);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_ADD_INDEX;
break;
case AT_AddConstraint: /* ADD CONSTRAINT */
! ATSimplePermissions(rel, false);
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
--- 2508,2520 ----
pass = AT_PASS_DROP;
break;
case AT_AddIndex: /* ADD INDEX */
! /* Performs own sanity checks in the execution stage */
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_ADD_INDEX;
break;
case AT_AddConstraint: /* ADD CONSTRAINT */
! /* Performs own sanity checks in the execution stage */
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
***************
*** 2502,2508 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_ADD_CONSTR;
break;
case AT_DropConstraint: /* DROP CONSTRAINT */
! ATSimplePermissions(rel, false);
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
--- 2522,2528 ----
pass = AT_PASS_ADD_CONSTR;
break;
case AT_DropConstraint: /* DROP CONSTRAINT */
! ATCheckSanity(rel, false, false, false);
/* Recursion occurs during execution phase */
/* No command-specific prep needed except saving recurse flag */
if (recurse)
***************
*** 2510,2516 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_DROP;
break;
case AT_AlterColumnType: /* ALTER COLUMN TYPE */
! ATSimplePermissions(rel, false);
/* Performs own recursion */
ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd);
pass = AT_PASS_ALTER_TYPE;
--- 2530,2536 ----
pass = AT_PASS_DROP;
break;
case AT_AlterColumnType: /* ALTER COLUMN TYPE */
! ATCheckSanity(rel, false, false, false);
/* Performs own recursion */
ATPrepAlterColumnType(wqueue, tab, rel, recurse, recursing, cmd);
pass = AT_PASS_ALTER_TYPE;
***************
*** 2522,2541 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_ClusterOn: /* CLUSTER ON */
case AT_DropCluster: /* SET WITHOUT CLUSTER */
! ATSimplePermissions(rel, false);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_AddOids: /* SET WITH OIDS */
! ATSimplePermissions(rel, false);
/* Performs own recursion */
if (!rel->rd_rel->relhasoids || recursing)
ATPrepAddOids(wqueue, rel, recurse, cmd);
pass = AT_PASS_ADD_COL;
break;
case AT_DropOids: /* SET WITHOUT OIDS */
! ATSimplePermissions(rel, false);
/* Performs own recursion */
if (rel->rd_rel->relhasoids)
{
--- 2542,2561 ----
break;
case AT_ClusterOn: /* CLUSTER ON */
case AT_DropCluster: /* SET WITHOUT CLUSTER */
! ATCheckSanity(rel, false, false, false);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
case AT_AddOids: /* SET WITH OIDS */
! ATCheckSanity(rel, false, false, false);
/* Performs own recursion */
if (!rel->rd_rel->relhasoids || recursing)
ATPrepAddOids(wqueue, rel, recurse, cmd);
pass = AT_PASS_ADD_COL;
break;
case AT_DropOids: /* SET WITHOUT OIDS */
! ATCheckSanity(rel, false, false, false);
/* Performs own recursion */
if (rel->rd_rel->relhasoids)
{
***************
*** 2549,2562 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
pass = AT_PASS_DROP;
break;
case AT_SetTableSpace: /* SET TABLESPACE */
! ATSimplePermissionsRelationOrIndex(rel);
/* This command never recurses */
ATPrepSetTableSpace(tab, rel, cmd->name);
pass = AT_PASS_MISC; /* doesn't actually matter */
break;
case AT_SetRelOptions: /* SET (...) */
case AT_ResetRelOptions: /* RESET (...) */
! ATSimplePermissionsRelationOrIndex(rel);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
--- 2569,2582 ----
pass = AT_PASS_DROP;
break;
case AT_SetTableSpace: /* SET TABLESPACE */
! ATCheckSanity(rel, false, true, false);
/* This command never recurses */
ATPrepSetTableSpace(tab, rel, cmd->name);
pass = AT_PASS_MISC; /* doesn't actually matter */
break;
case AT_SetRelOptions: /* SET (...) */
case AT_ResetRelOptions: /* RESET (...) */
! ATCheckSanity(rel, false, true, false);
/* This command never recurses */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
***************
*** 2575,2581 **** ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_DisableRule:
case AT_AddInherit: /* INHERIT / NO INHERIT */
case AT_DropInherit:
! ATSimplePermissions(rel, false);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
--- 2595,2601 ----
case AT_DisableRule:
case AT_AddInherit: /* INHERIT / NO INHERIT */
case AT_DropInherit:
! ATCheckSanity(rel, false, false, false);
/* These commands never recurse */
/* No command-specific prep needed */
pass = AT_PASS_MISC;
***************
*** 2754,2763 **** ATExecCmd(List **wqueue, AlteredTableInfo *tab, Relation rel,
*/
break;
case AT_SetTableSpace: /* SET TABLESPACE */
-
/*
! * Nothing to do here; Phase 3 does the work
*/
break;
case AT_SetRelOptions: /* SET (...) */
ATExecSetRelOptions(rel, (List *) cmd->def, false);
--- 2774,2794 ----
*/
break;
case AT_SetTableSpace: /* SET TABLESPACE */
/*
! * Nothing to do here except for permission checks;
! * Phase 3 does the work
*/
+ {
+ AclResult aclresult;
+
+ ATCheckOwnership(rel);
+
+ aclresult = pg_tablespace_aclcheck(tab->newTableSpace,
+ GetUserId(), ACL_CREATE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_TABLESPACE,
+ get_tablespace_name(tab->newTableSpace));
+ }
break;
case AT_SetRelOptions: /* SET (...) */
ATExecSetRelOptions(rel, (List *) cmd->def, false);
***************
*** 3299,3336 **** ATGetQueueEntry(List **wqueue, Relation rel)
}
/*
! * ATSimplePermissions
*
! * - Ensure that it is a relation (or possibly a view)
! * - Ensure this user is the owner
* - Ensure that it is not a system table
*/
static void
! ATSimplePermissions(Relation rel, bool allowView)
{
! if (rel->rd_rel->relkind != RELKIND_RELATION)
! {
! if (allowView)
! {
! if (rel->rd_rel->relkind != RELKIND_VIEW)
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("\"%s\" is not a table or view",
! RelationGetRelationName(rel))));
! }
! else
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("\"%s\" is not a table",
! RelationGetRelationName(rel))));
! }
! /* Permissions checks */
! if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
! aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
! RelationGetRelationName(rel));
! if (!allowSystemTableMods && IsSystemRelation(rel))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied: \"%s\" is a system catalog",
--- 3330,3356 ----
}
/*
! * ATCheckSanity
*
! * - Ensure that it is a relation (or possibly a view or index)
* - Ensure that it is not a system table
*/
static void
! ATCheckSanity(Relation rel, bool viewOK, bool indexOK, bool catalogOK)
{
! char relkind = RelationGetForm(rel)->relkind;
! if (relkind != RELKIND_RELATION &&
! (!viewOK || relkind != RELKIND_VIEW) &&
! (!indexOK || relkind != RELKIND_INDEX))
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("\"%s\" is not a table%s%s",
! RelationGetRelationName(rel),
! !viewOK ? "" : " or view",
! !indexOK ? "" : " or index")));
! if (!catalogOK && !allowSystemTableMods && IsSystemRelation(rel))
ereport(ERROR,
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
errmsg("permission denied: \"%s\" is a system catalog",
***************
*** 3338,3369 **** ATSimplePermissions(Relation rel, bool allowView)
}
/*
! * ATSimplePermissionsRelationOrIndex
*
- * - Ensure that it is a relation or an index
* - Ensure this user is the owner
- * - Ensure that it is not a system table
*/
static void
! ATSimplePermissionsRelationOrIndex(Relation rel)
{
! if (rel->rd_rel->relkind != RELKIND_RELATION &&
! rel->rd_rel->relkind != RELKIND_INDEX)
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("\"%s\" is not a table or index",
! RelationGetRelationName(rel))));
!
! /* Permissions checks */
if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
RelationGetRelationName(rel));
-
- if (!allowSystemTableMods && IsSystemRelation(rel))
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied: \"%s\" is a system catalog",
- RelationGetRelationName(rel))));
}
/*
--- 3358,3374 ----
}
/*
! * ATCheckOwnership
*
* - Ensure this user is the owner
*/
static void
! ATCheckOwnership(Relation rel)
{
! /* Ownership checks */
if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
RelationGetRelationName(rel));
}
/*
***************
*** 3605,3610 **** ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
--- 3610,3618 ----
Form_pg_type tform;
Expr *defval;
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
if (rel->rd_rel->reloftype)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
***************
*** 3906,3911 **** ATExecDropNotNull(Relation rel, const char *colName)
--- 3914,3922 ----
List *indexoidlist;
ListCell *indexoidscan;
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
/*
* lookup the attribute
*/
***************
*** 3996,4001 **** ATExecSetNotNull(AlteredTableInfo *tab, Relation rel,
--- 4007,4015 ----
AttrNumber attnum;
Relation attr_rel;
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
/*
* lookup the attribute
*/
***************
*** 4046,4051 **** ATExecColumnDefault(Relation rel, const char *colName,
--- 4060,4068 ----
{
AttrNumber attnum;
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
/*
* get the number of the attribute
*/
***************
*** 4091,4118 **** ATExecColumnDefault(Relation rel, const char *colName,
* ALTER TABLE ALTER COLUMN SET STATISTICS
*/
static void
- ATPrepSetStatistics(Relation rel, const char *colName, Node *newValue)
- {
- /*
- * We do our own permission checking because (a) we want to allow SET
- * STATISTICS on indexes (for expressional index columns), and (b) we want
- * to allow SET STATISTICS on system catalogs without requiring
- * allowSystemTableMods to be turned on.
- */
- if (rel->rd_rel->relkind != RELKIND_RELATION &&
- rel->rd_rel->relkind != RELKIND_INDEX)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("\"%s\" is not a table or index",
- RelationGetRelationName(rel))));
-
- /* Permissions checks */
- if (!pg_class_ownercheck(RelationGetRelid(rel), GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
- RelationGetRelationName(rel));
- }
-
- static void
ATExecSetStatistics(Relation rel, const char *colName, Node *newValue)
{
int newtarget;
--- 4108,4113 ----
***************
*** 4120,4125 **** ATExecSetStatistics(Relation rel, const char *colName, Node *newValue)
--- 4115,4123 ----
HeapTuple tuple;
Form_pg_attribute attrtuple;
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
Assert(IsA(newValue, Integer));
newtarget = intVal(newValue);
***************
*** 4186,4191 **** ATExecSetOptions(Relation rel, const char *colName, Node *options,
--- 4184,4192 ----
bool repl_null[Natts_pg_attribute];
bool repl_repl[Natts_pg_attribute];
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
***************
*** 4245,4250 **** ATExecSetStorage(Relation rel, const char *colName, Node *newValue)
--- 4246,4254 ----
HeapTuple tuple;
Form_pg_attribute attrtuple;
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
Assert(IsA(newValue, String));
storagemode = strVal(newValue);
***************
*** 4333,4339 **** ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
! ATSimplePermissions(rel, false);
/*
* get the number of the attribute
--- 4337,4346 ----
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
! ATCheckSanity(rel, false, false, false);
!
! /* Permission checks */
! ATCheckOwnership(rel);
/*
* get the number of the attribute
***************
*** 4533,4539 **** ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
quiet = is_rebuild;
/* The IndexStmt has already been through transformIndexStmt */
!
DefineIndex(stmt->relation, /* relation */
stmt->idxname, /* index name */
InvalidOid, /* no predefined OID */
--- 4540,4546 ----
quiet = is_rebuild;
/* The IndexStmt has already been through transformIndexStmt */
! /* All the permission checks are applied in DefineIndex() */
DefineIndex(stmt->relation, /* relation */
stmt->idxname, /* index name */
InvalidOid, /* no predefined OID */
***************
*** 4635,4643 **** ATAddCheckConstraint(List **wqueue, AlteredTableInfo *tab, Relation rel,
List *children;
ListCell *child;
! /* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
! ATSimplePermissions(rel, false);
/*
* Call AddRelationNewConstraints to do the work, making sure it works on
--- 4642,4653 ----
List *children;
ListCell *child;
! /* At top level, sanity check was done in ATPrepCmd, else do it*/
if (recursing)
! ATCheckSanity(rel, false, false, false);
!
! /* Permission checks */
! ATCheckOwnership(rel);
/*
* Call AddRelationNewConstraints to do the work, making sure it works on
***************
*** 4755,4771 **** ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
* Validity checks (permission checks wait till we have the column
* numbers)
*/
! if (pkrel->rd_rel->relkind != RELKIND_RELATION)
! ereport(ERROR,
! (errcode(ERRCODE_WRONG_OBJECT_TYPE),
! errmsg("referenced relation \"%s\" is not a table",
! RelationGetRelationName(pkrel))));
!
! if (!allowSystemTableMods && IsSystemRelation(pkrel))
! ereport(ERROR,
! (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
! errmsg("permission denied: \"%s\" is a system catalog",
! RelationGetRelationName(pkrel))));
/*
* Disallow reference from permanent table to temp table or vice versa.
--- 4765,4771 ----
* Validity checks (permission checks wait till we have the column
* numbers)
*/
! ATCheckSanity(pkrel, false, false, false);
/*
* Disallow reference from permanent table to temp table or vice versa.
***************
*** 4833,4838 **** ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
--- 4833,4839 ----
/*
* Now we can check permissions.
*/
+ ATCheckOwnership(rel);
checkFkeyPermissions(pkrel, pkattnum, numpks);
checkFkeyPermissions(rel, fkattnum, numfks);
***************
*** 5579,5585 **** ATExecDropConstraint(Relation rel, const char *constrName,
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
! ATSimplePermissions(rel, false);
conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
--- 5580,5589 ----
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
! ATCheckSanity(rel, false, false, false);
!
! /* Permission checks */
! ATCheckOwnership(rel);
conrel = heap_open(ConstraintRelationId, RowExclusiveLock);
***************
*** 5913,5918 **** ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel,
--- 5917,5925 ----
SysScanDesc scan;
HeapTuple depTup;
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
/* Look up the target column */
***************
*** 6691,6696 **** ATExecClusterOn(Relation rel, const char *indexName)
--- 6698,6706 ----
{
Oid indexOid;
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
indexOid = get_relname_relid(indexName, rel->rd_rel->relnamespace);
if (!OidIsValid(indexOid))
***************
*** 6715,6720 **** ATExecClusterOn(Relation rel, const char *indexName)
--- 6725,6733 ----
static void
ATExecDropCluster(Relation rel)
{
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
mark_index_clustered(rel, InvalidOid);
}
***************
*** 6725,6731 **** static void
ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, char *tablespacename)
{
Oid tablespaceId;
- AclResult aclresult;
/* Check that the tablespace exists */
tablespaceId = get_tablespace_oid(tablespacename);
--- 6738,6743 ----
***************
*** 6734,6744 **** ATPrepSetTableSpace(AlteredTableInfo *tab, Relation rel, char *tablespacename)
(errcode(ERRCODE_UNDEFINED_OBJECT),
errmsg("tablespace \"%s\" does not exist", tablespacename)));
- /* Check its permissions */
- aclresult = pg_tablespace_aclcheck(tablespaceId, GetUserId(), ACL_CREATE);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, ACL_KIND_TABLESPACE, tablespacename);
-
/* Save info for Phase 3 to do the real work */
if (OidIsValid(tab->newTableSpace))
ereport(ERROR,
--- 6746,6751 ----
***************
*** 6765,6770 **** ATExecSetRelOptions(Relation rel, List *defList, bool isReset)
--- 6772,6780 ----
bool repl_repl[Natts_pg_class];
static char *validnsps[] = HEAP_RELOPT_NAMESPACES;
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
if (defList == NIL)
return; /* nothing to do */
***************
*** 7097,7102 **** static void
--- 7107,7115 ----
ATExecEnableDisableTrigger(Relation rel, char *trigname,
char fires_when, bool skip_system)
{
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
EnableDisableTrigger(rel, trigname, fires_when, skip_system);
}
***************
*** 7109,7114 **** static void
--- 7122,7130 ----
ATExecEnableDisableRule(Relation rel, char *trigname,
char fires_when)
{
+ /* Permission checks */
+ ATCheckOwnership(rel);
+
EnableDisableRule(rel, trigname, fires_when);
}
***************
*** 7136,7146 **** ATExecAddInherit(Relation child_rel, RangeVar *parent)
*/
parent_rel = heap_openrv(parent, AccessShareLock);
/*
! * Must be owner of both parent and child -- child was checked by
! * ATSimplePermissions call in ATPrepCmd
*/
! ATSimplePermissions(parent_rel, false);
/* Permanent rels cannot inherit from temporary ones */
if (parent_rel->rd_istemp && !child_rel->rd_istemp)
--- 7152,7164 ----
*/
parent_rel = heap_openrv(parent, AccessShareLock);
+ ATCheckSanity(parent_rel, false, false, false);
+
/*
! * Must be owner of both parent and child
*/
! ATCheckOwnership(child_rel);
! ATCheckOwnership(parent_rel);
/* Permanent rels cannot inherit from temporary ones */
if (parent_rel->rd_istemp && !child_rel->rd_istemp)
***************
*** 7494,7499 **** ATExecDropInherit(Relation rel, RangeVar *parent)
--- 7512,7520 ----
List *connames;
bool found = false;
+ /* Permission check */
+ ATCheckOwnership(rel);
+
/*
* AccessShareLock on the parent is probably enough, seeing that DROP
* TABLE doesn't lock parent tables at all. We need some lock since we'll
***************
*** 7787,7792 **** AlterTableNamespace(RangeVar *relation, const char *newschema,
--- 7808,7815 ----
errmsg("\"%s\" is not a table, view, or sequence",
RelationGetRelationName(rel))));
}
+ /* Must be owner of the relation */
+ ATCheckOwnership(rel);
/* get schema OID and check its permissions */
nspOid = LookupCreationNamespace(newschema);
***************
*** 7798,7803 **** AlterTableNamespace(RangeVar *relation, const char *newschema,
--- 7821,7833 ----
RelationGetRelationName(rel),
newschema)));
+ /* disallow renaming the system relations */
+ if (!allowSystemTableMods && IsSystemRelation(rel))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a system catalog",
+ RelationGetRelationName(rel))));
+
/* disallow renaming into or out of temp schemas */
if (isAnyTempNamespace(nspOid) || isAnyTempNamespace(oldNspOid))
ereport(ERROR,
*** a/src/backend/commands/trigger.c
--- b/src/backend/commands/trigger.c
***************
*** 1117,1128 **** renametrig(Oid relid,
--- 1117,1144 ----
ScanKeyData key[2];
/*
+ * Permission checks
+ */
+ if (!pg_class_ownercheck(relid, GetUserId()))
+ aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
+ get_rel_name(relid));
+
+ /*
* Grab an exclusive lock on the target table, which we will NOT release
* until end of transaction.
*/
targetrel = heap_open(relid, AccessExclusiveLock);
/*
+ * Ensure target relation is not catalog
+ */
+ if (!allowSystemTableMods && IsSystemRelation(targetrel))
+ ereport(ERROR,
+ (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ errmsg("permission denied: \"%s\" is a system catalog",
+ RelationGetRelationName(targetrel))));
+
+ /*
* Scan pg_trigger twice for existing triggers on relation. We do this in
* order to ensure a trigger does not exist with newname (The unique index
* on tgrelid/tgname would complain anyway) and to ensure a trigger does
*** a/src/backend/rewrite/rewriteDefine.c
--- b/src/backend/rewrite/rewriteDefine.c
***************
*** 670,676 **** EnableDisableRule(Relation rel, const char *rulename,
{
Relation pg_rewrite_desc;
Oid owningRel = RelationGetRelid(rel);
- Oid eventRelationOid;
HeapTuple ruletup;
bool changed = false;
--- 670,675 ----
***************
*** 688,702 **** EnableDisableRule(Relation rel, const char *rulename,
rulename, get_rel_name(owningRel))));
/*
- * Verify that the user has appropriate permissions.
- */
- eventRelationOid = ((Form_pg_rewrite) GETSTRUCT(ruletup))->ev_class;
- Assert(eventRelationOid == owningRel);
- if (!pg_class_ownercheck(eventRelationOid, GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
- get_rel_name(eventRelationOid));
-
- /*
* Change ev_enabled if it is different from the desired new state.
*/
if (DatumGetChar(((Form_pg_rewrite) GETSTRUCT(ruletup))->ev_enabled) !=
--- 687,692 ----
*** a/src/backend/tcop/utility.c
--- b/src/backend/tcop/utility.c
***************
*** 61,102 ****
/* Hook for plugins to get control in ProcessUtility() */
ProcessUtility_hook_type ProcessUtility_hook = NULL;
-
- /*
- * Verify user has ownership of specified relation, else ereport.
- *
- * If noCatalogs is true then we also deny access to system catalogs,
- * except when allowSystemTableMods is true.
- */
- void
- CheckRelationOwnership(RangeVar *rel, bool noCatalogs)
- {
- Oid relOid;
- HeapTuple tuple;
-
- relOid = RangeVarGetRelid(rel, false);
- tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relOid));
- if (!HeapTupleIsValid(tuple)) /* should not happen */
- elog(ERROR, "cache lookup failed for relation %u", relOid);
-
- if (!pg_class_ownercheck(relOid, GetUserId()))
- aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
- rel->relname);
-
- if (noCatalogs)
- {
- if (!allowSystemTableMods &&
- IsSystemClass((Form_pg_class) GETSTRUCT(tuple)))
- ereport(ERROR,
- (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- errmsg("permission denied: \"%s\" is a system catalog",
- rel->relname)));
- }
-
- ReleaseSysCache(tuple);
- }
-
-
/*
* CommandIsReadOnly: is an executable query read-only?
*
--- 61,66 ----
***************
*** 865,873 **** standard_ProcessUtility(Node *parsetree,
if (stmt->concurrent)
PreventTransactionChain(isTopLevel,
"CREATE INDEX CONCURRENTLY");
-
- CheckRelationOwnership(stmt->relation, true);
-
/* Run parse analysis ... */
stmt = transformIndexStmt(stmt, queryString);
--- 829,834 ----
*** a/src/include/tcop/utility.h
--- b/src/include/tcop/utility.h
***************
*** 40,45 **** extern LogStmtLevel GetCommandLogLevel(Node *parsetree);
extern bool CommandIsReadOnly(Node *parsetree);
- extern void CheckRelationOwnership(RangeVar *rel, bool noCatalogs);
-
#endif /* UTILITY_H */
--- 40,43 ----
Re: [v9.1] access control reworks in ALTER TABLE
At Ottawa, I had a talk with Robert Haas about this reworks.
Because a part of ALTER TABLE options need information which can
be gathered at execution stage, not preparation stage, the patch
tried to move all the access control stuff into execution stage.
However, he pointed out it has a matter the tables tried to be
altered may acquire locks on the table without permissions.
It can allow DoS attacks, so we agreed we should not move these
checks into execution stage.
Fortunately, only three ALTER TABLE options need information
gathered at execution stage (add inheritance; add FK constraints;
add index); So, we can rework them using separated checker functions,
so it is not a matter.
I marked it as returned with feedback.
At the 1st CF, we focus on the reworks of DML permission checks.
So, the rest of DDL works should be done on the 2nd.
Thanks,
(2010/03/17 16:26), KaiGai Kohei wrote:
The attached patch reworks access control logics corresponding
to ALTER TABLE statement, as a groundwork for the upcoming
enhanced security features.As we discussed in the last commit fest, it shall be a preferable
way to wrap up a unit of access control logics into some functions
which are categorized by the actions to be checked. These functions
will be able to perform as entrypoints of existing permission checks
and the upcoming security features, like SELinux.The first step of this efforts is to consolidate existing access
control codes into a unit for each operations. Once we reworks them,
it is obvious to replace these unit by access control functions.
ALTER TABLE is the most functional command in PostgreSQL, so it may
be a good stater for this efforts.
--------In this patch, I put access control codes in the execution stage
after all needed information getting gathered. Then, privileges
are checked at once.The ALTER TABLE implementation, exceptionally, has a few stages.
The preparation stage applies sanity and permission checks, and
expand the given command to child relations. Then, the execution
stage updates system catalogs according to the given command.
However, we don't have multiple stages in most of ddl statements.
So, even if we adopt a basis to apply permission checks in the
preparation stage, we cannot reuse this basis for others.Most of AT commands checks ownerships of the relation using
ATSimplePermissions() in the preparation stage, but now a few number
of commands also needs to check privileges in the execution stage
dues to some reasons.(1) commands with self recursion
ATExecDropColumn(), ATAddCheckConstraint() and ATExecDropConstraint()
implements its recursive calls by itself. It means we cannot know
child relations to be altered in the preparation stage, so it has
the following code typically.| /* At top level, permission check was done in ATPrepCmd, else do it */
| if (recursing)
| ATSimplePermissions(rel, false);(2) commands directly called from alter.c
RenameRelation(), renameatt() and AlterTableNamespace() are directly
called from alter.c, so they have no preparation stage. Instead of
the ATSimplePermissions(), it calls CheckRelationOwnership() in the
caller. Also, renameatt() checks ownership of the relation during
its recursive calls.(3) commands need privilges to other objects
- AlterTableNamespace() checks ACL_CREATE on the specified namespace
in the LookupCreationNamespace().
- ATPrepSetTableSpace() checks ACL_CREATE on the specified tablespace.
- ATExecAddIndex() checks ACL_CREATE on the namespace and ACL_CREATE
on the specified tablespace (if exist) in DefineIndex().
- ATExecAddInherit() checks ownership of the specified parent relation.
- ATAddForeignKeyConstraint() checks ACL_REFERENCES on the both of
specified table/columns.(4) commands needs its own sanity checks
ATSimplePermissions() also ensures the relation is table (or view),
and not a system catalog, not only checks its ownership.
However, some of commands need to apply an exceptional sanity checks.
ATSimplePermissionsRelationOrIndex() is used for sanity checks of the
AT commands which is allowed on indexes. ATPrepSetStatistics() skips
checks to ensure the relation is not system catalogs.At first, this patch broke down the ATSimplePermissions() into sanity
checks and permission checks.This patch break down the ATSimplePermissions() into sanity checks
and permissions checks, then later part is moved to the execution
stage.
- ATCheckSanity()
- ATCheckOwnership()It enables to eliminate (1) special case handling on the commands with
self recursion, and (4) exceptional sanity checks because ATCheckSanity()
takes three arguments to control sanity checks behavior.It also means we don't need to consider (2) functions are exceptional,
because all the commands apply checks in the execution stage.For the (3) functions, it moved ownership checks nearby the permission
checks to other object, according to the basis: access control codes in
the execution stage after all needed information getting gathered, and
these are checked at once.ATExecAddIndex() is a wrapper function of the DefineIndex(). It requires
caller checks ownership of the relation to be indexed, then it checks
rest of permissions for namespace and tablespace. I moved checks for
ownership of the relation into DefineIndex(), and also eliminated the
CheckRelationOwnership() from standard_ProcessUtility() that is another
caller of DefineIndex().Any comments?
Thanks,
--
KaiGai Kohei <kaigai@ak.jp.nec.com>