[PATCH] Reworks for Access Control facilities (r2311)

The attached patch is an updated version of reworks for access control
facilities, and a short introduction to help reviewing not-obvious part
of the patch.

List of updates:
- Rebased to the latest CVS HEAD.
- bugfix: ac_opfamily_create() was not called on DefineOpClass().
- bugfix: ac_trigger_create() was moved to the proper point.
- Add more source code comments.

== Purpose ==

The purpose of this patch is to provide a common abstraction layer for
various kind of upcoming access control facilities.
Access control is to make an access control decision whether the give
access can be allowed, or not. The way to decide it depends on the
security model. For example, when we create a new table, it should be
checked whether the client has enough privilege to create a new one.
The native database privilege stuff checks ACL_CREATE permission on
the namespace to be placed. It is the way to make a decision on access
controls.

Now PostgreSQL has only one access control mechanism, and its routines
to make access control decisions are invoked from all over the backend
implementation. (Please count pg_xxx_aclcheck() and pg_xxx_ownercheck().)
It also means we need to put a similar number of invocations of security
hooks, when we implement a new security mechanism in addition to the
native one.

The common abstraction layer performs as entry points for each security
features (including the native database privilege), and enables to minimize
the impact when a new security feature is added.

== Implementation ==

Basically, this patch replaces a series of pg_xxx_aclcheck() and
pg_xxx_ownercheck() invocations by the new abstraction function.
The abstraction functions invokes equivalent acl/owner check routines,
and it allows us to put other security checks within the abstraction
functions.

For example, when we want to check client's privilege to execute a certain
function, pg_proc_aclcheck(ACL_EXECUTE) was called from several points.
This patch replaces it by ac_proc_execute(procOid) which internally invokes
pg_proc_aclcheck() as follows:

------------------------------------------------
*** 1029,1040 ****
init_fcache(Oid foid, FuncExprState *fcache,
MemoryContext fcacheCxt, bool needDescForSets)
{
- AclResult aclresult;
-
/* Check permission to call function */
! aclresult = pg_proc_aclcheck(foid, GetUserId(), ACL_EXECUTE);
! if (aclresult != ACLCHECK_OK)
! aclcheck_error(aclresult, ACL_KIND_PROC, get_func_name(foid));

/*
* Safety check on nargs. Under normal circumstances this should never
--- 1029,1036 ----
init_fcache(Oid foid, FuncExprState *fcache,
MemoryContext fcacheCxt, bool needDescForSets)
{
/* Check permission to call function */
! ac_proc_execute(foid, GetUserId());

/*
* Safety check on nargs. Under normal circumstances this should never
------------------------------------------------

And,

------------------------------------------------
+ /*
+ * ac_proc_execute
+ *
+ * It checks privilege to execute a certain function.
+ *
+ * Note that it should be checked on the function runtime.
+ * Some of DDL statements requires ACL_EXECUTE on creation time, such as
+ * CreateConversionCommand(), however, these are individually checked on
+ * the ac_xxx_create() hook.
+ *
+ * [Params]
+ * proOID : OID of the function to be executed
+ * roleOid : OID of the database role to be evaluated
+ */
+ void
+ ac_proc_execute(Oid proOid, Oid roleOid)
+ {
+ AclResult aclresult;
+
+ aclresult = pg_proc_aclcheck(proOid, roleOid, ACL_EXECUTE);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, ACL_KIND_PROC, get_func_name(proOid));
+ }
------------------------------------------------

It is obvious that we can add invocations of different kind of security
checks at the tail of this abstraction function. It also can be a check
based on MAC security policy.

All the abstraction functions have the following convention.

ac_<object class>_<type of action>([args ...]);

The object class shows what kind of database object is checked. For example,
here is "relation", "proc", "database" and so on. Most of then are equivalent
to the name of system catalog.

The type of action shows what kind of action is required on the object.
It is specific for each object classes, but the "_create", "_alter" and
"_drop" are common for each object classes. (A few object classes without
its ALTER statement does not have "_alter" function.)

Most of abstraction functions will be obvious what does it replaces.

However, "ac_*_create" function tends not to be obvious, because some of
them requires multiple permissions to create a new database object.
For example, when we create a new function, the following permissions are
checked in the native database privilege mechanism.

- ACL_CREATE on the namespace.
- ACL_USAGE on the procedural language, or superuser privilege if untrusted.
- Ownership of the procedure to be replaced, if CREATE OR REPLACE is used.

The original implementation checks these permissions at the different points,
but this patch consolidated them within a ac_proc_create() function.

The "_alter" function may also seems a bit not-obvious, because required
permission depends on the alter option.
For example, ALTER TABLE ... RENAME TO statement required ACL_CREATE on
the current namespace, not only ownership of the target table. So, it
should be checked conditionally, if the ALTER TABLE statement tries to
alter the name.
In the typical "_alter" functions, it checks ownership of the target object,
and it also checks ACL_CREATE on the namespace if renamed, and it also checks
role membership and ACL_CREATE on the namespace with the new owner if owner
changed.

The restrict_and_check_grant() has checked client's privilege to grant/revoke
something on the given database object, and dropped permissions being
available to grant/revoke.
It is separated into two parts. The earlier part is replaced by
the ac_xxx_grant() function to check client's privilege to grant/revoke it.
The later part is still common to drop unavailable permissions to grant/reveke,
as follows:

------------------------------------------------
*************** ExecGrant_Function(InternalGrant *istmt)
*** 1562,1577 ****
old_acl, ownerId,
&grantorId, &avail_goptions);

/*
* Restrict the privileges to what we can actually grant, and emit the
* standards-mandated warning and error messages.
*/
this_privileges =
! restrict_and_check_grant(istmt->is_grant, avail_goptions,
! istmt->all_privs, istmt->privileges,
! funcId, grantorId, ACL_KIND_PROC,
! NameStr(pg_proc_tuple->proname),
! 0, NULL);

/*
* Generate new ACL.
--- 1508,1524 ----
old_acl, ownerId,
&grantorId, &avail_goptions);

+ /* Permission check to grant/revoke */
+ ac_proc_grant(funcId, grantorId, avail_goptions);
+
/*
* Restrict the privileges to what we can actually grant, and emit the
* standards-mandated warning and error messages.
*/
this_privileges =
! restrict_grant(istmt->is_grant, avail_goptions,
! istmt->all_privs, istmt->privileges,
! NameStr(pg_proc_tuple->proname));

/*
* Generate new ACL.
------------------------------------------------

== Needs any comments ==

Currently, all the abstraction functions are placed at
the src/backend/security/access_control.c, but it has about 4400 lines.

It might be preferable to deploy these abstraction functions categorized
by object classes, because it may help to lookup abstraction functions,
as follows:

src/backend/security/ac/ac_database.c
/ac_schema.c
/ac_relation.c
:

Which is preferable for reviewers?
We still have a day by the start of the CF#2.

Thanks,

[kaigai(at)saba ~]$ diffstat sepgsql-01-base-8.5devel-r2311.patch.gz
backend/Makefile | 2
backend/catalog/aclchk.c | 220 !
backend/catalog/dependency.c | 15
backend/catalog/namespace.c | 66
backend/catalog/pg_aggregate.c | 12
backend/catalog/pg_conversion.c | 33
backend/catalog/pg_operator.c | 42
backend/catalog/pg_proc.c | 22
backend/catalog/pg_shdepend.c | 18
backend/catalog/pg_type.c | 25
backend/commands/aggregatecmds.c | 42
backend/commands/alter.c | 72
backend/commands/analyze.c | 5
backend/commands/cluster.c | 9
backend/commands/comment.c | 125
backend/commands/conversioncmds.c | 71
backend/commands/copy.c | 40
backend/commands/dbcommands.c | 160 !
backend/commands/foreigncmds.c | 144
backend/commands/functioncmds.c | 127
backend/commands/indexcmds.c | 120
backend/commands/lockcmds.c | 17
backend/commands/opclasscmds.c | 236 !
backend/commands/operatorcmds.c | 70
backend/commands/proclang.c | 56
backend/commands/schemacmds.c | 60
backend/commands/sequence.c | 38
backend/commands/tablecmds.c | 355 -
backend/commands/tablespace.c | 46
backend/commands/trigger.c | 41
backend/commands/tsearchcmds.c | 176 !
backend/commands/typecmds.c | 137 !
backend/commands/user.c | 183 !
backend/commands/vacuum.c | 5
backend/commands/view.c | 7
backend/executor/execMain.c | 203 !
backend/executor/execQual.c | 16
backend/executor/nodeAgg.c | 24
backend/executor/nodeMergejoin.c | 8
backend/executor/nodeWindowAgg.c | 24
backend/optimizer/util/clauses.c | 6
backend/parser/parse_utilcmd.c | 13
backend/rewrite/rewriteDefine.c | 10
backend/rewrite/rewriteRemove.c | 6
backend/security/Makefile | 10
backend/security/access_control.c | 4472 ++++++++++++++++++++++++++++++++++++++
backend/tcop/fastpath.c | 15
backend/tcop/utility.c | 74
backend/utils/adt/dbsize.c | 25
backend/utils/adt/ri_triggers.c | 24
backend/utils/adt/tid.c | 18
backend/utils/init/postinit.c | 15
include/catalog/pg_proc_fn.h | 1
include/commands/defrem.h | 1
include/utils/security.h | 348 ++
55 files changed, 5294 insertions(+), 922 deletions(-), 1894 modifications(!)

--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>