[PATCH] Reworks for Access Control facilities (r2277)

From: KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>
To: sfrost(at)snowman(dot)net
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: [PATCH] Reworks for Access Control facilities (r2277)
Date: 2009-09-03 06:50:28
Message-ID: 4A9F6734.9020705@ak.jp.nec.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

The attached patch is updated one for reworks of access control
facilities in PostgreSQL.

List of updates:
- Base tree was updated to the latest CVS HEAD
- ac_role_xxx() routines were added, which was forgotten to include.
- Code cleanups. A few compile time warnings were eliminated.
- Add source code comments to introduce why the security checks are
modified at some points.

BTW, IIRC, I remember you mentioned as follows:
| We're not afraid of making large scale changes to core. It just needs
| to be done incrementally and done *first*, before adding features and
| code which depends on it. We have to fix PG *first*, in this regard,
| before we can even start to look at SELinux hooks, etc.

This patch is indeed pure PostgreSQL feature, but a bit large.
| 54 files changed, 5313 insertions(+), 937 deletions(-), 1875 modifications(!)

I'm afraid of the patch is ignored due to the scale of changeset again.
But we also need to get succeeded to merge the access control reworks
at the next commit fest, because we have only three commit fest remained
and the last one automatically reject large patches, so I have to submit
SE-PostgreSQL/lite patch on the CF-Nov.

Is there any good idea to help reviewers?
Or, do you think it is enough to review in the CF-Sep?

One possible idea is to separate the current single patch into the several
shorter frames. For example, the first patch reworks access controls
corresponding to relations into ac_relation_xxxx(), the second patch
reworks access controls corresponding to procedures into ac_proc_xxx(),
and so on....
But, I'm not sure whether it is really helpful to review the patch.
(Needless to say, if we can conclude it is helpfull, I'll rework for
the patch prior to the beggining of the CF-Sep.)

Thanks,

KaiGai Kohei wrote:
> The attached patch reworks access control facilities in PostgreSQL.
>
> The current implementation does not have well separation in what
> to be controled and how to be controled. For example, when we create
> a new table, it requires users ACL_CREATE on the namespace and
> ACL_CREATE on the tablespace if necessary. These checks are methods
> to control whether he can create a new table, or not.
>
> This patch provides an abstraction layer of access controls to
> separate what to be controlsed and how to be controled.
> The abstraction layer is a set of functions to implement what
> to be controled.
> For example, ac_relation_create() checks user's privilege to
> create a new table. It internally calls pg_namespace_aclcheck()
> and pg_tablespace_aclcheck() to make its access control decision
> based on the security model in database ACLs.
>
> This abstraction layer functions have the following naming convension.
>
> ac_<object type>_<action>(args, ...)
>
> e.g) void ac_proc_execute(Oid proOid, Oid roleOid)
> It checks privilege to execute a certain procedure with
> the given database role. The caller gives all the necessary
> informations to make its decision.
>
> It replaces all the pg_xxx_aclcheck() and pg_xxx_ownercheck() invocations
> from the backend implementations, except for security/access_control.c.
> In this patch, these are used as helper functions to implement access
> control logic (in other word, how to be controled), invoked from the
> access control functions.
>
> These ac_xxx_xxx() routines will be entrypoints to invoke additional
> security checks (SE-PostgreSQL), rather than sepgsqlXXXX() hooks around
> the backend implementation.
>
> Thanks,

[kaigai(at)saba trunk]$ diffstat sepgsql-01-base-8.5devel-r2277.patch.gz
backend/Makefile | 2
backend/catalog/aclchk.c | 220 !
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 | 223 !
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 | 4513 ++++++++++++++++++++++++++++++++++++++
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 | 16
include/catalog/pg_proc_fn.h | 1
include/commands/defrem.h | 1
include/utils/security.h | 349 ++
54 files changed, 5313 insertions(+), 937 deletions(-), 1875 modifications(!)

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

Attachment Content-Type Size
sepgsql-01-base-8.5devel-r2277.patch.gz application/gzip 75.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message KaiGai Kohei 2009-09-03 07:10:33 Re: Triggers on columns
Previous Message Markus Wanner 2009-09-03 05:33:39 Re: combined indexes with Gist - planner issues?