[v9.1] access control reworks in ALTER TABLE

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: [v9.1] access control reworks in ALTER TABLE
Date: 2010-03-17 07:26:49
Message-ID: 4BA08439.6010204@ak.jp.nec.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(at)ak(dot)jp(dot)nec(dot)com>

Attachment Content-Type Size
pgsql-at-reworks.1.patch application/octect-stream 42.7 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dmitry Fefelov 2010-03-17 07:54:48 Re: Partitioning syntax
Previous Message Merlin Moncure 2010-03-17 03:08:08 Re: Dyamic updates of NEW with pl/pgsql