[PATCH] Reworks for Access Control facilities (r2277) - Mailing list pgsql-hackers

From KaiGai Kohei
Subject [PATCH] Reworks for Access Control facilities (r2277)
Date
Msg-id 4A9F6734.9020705@ak.jp.nec.com
Whole thread Raw
In response to [PATCH] Reworks for Access Control facilities (r2251)  (KaiGai Kohei <kaigai@ak.jp.nec.com>)
List 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@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@ak.jp.nec.com>

Attachment

pgsql-hackers by date:

Previous
From: Markus Wanner
Date:
Subject: Re: combined indexes with Gist - planner issues?
Next
From: KaiGai Kohei
Date:
Subject: Re: Triggers on columns