Re: ExecutorCheckPerms() hook

(2010/05/21 1:14), Robert Haas wrote:
> In yesterday's development meeting, we talked about the possibility of
> a basic SE-PostgreSQL implementation that checks permissions only for
> DML. Greg Smith offered the opinion that this could provide much of
> the benefit of SE-PostgreSQL for many users, while being much simpler.
> In fact, SE-PostgreSQL would need to get control in just one place:
> ExecCheckRTPerms. This morning, Stephen Frost and I worked up a quick
> patch showing how we could add a hook here to let a hypothetical
> SE-PostgreSQL module get control in the relevant place. The attached
> patch also includes a toy contrib module showing how it could be used
> to enforce arbitrary security policy.
>
> I don't think that this by itself would be quite enough framework for
> a minimal SE-PostgreSQL implementation - for that, you'd probably need
> an object-labeling facility in core which SE-PostgreSQL could leverage
> - or else some other way to determine which the label associated with
> a given object - but I think that plus this would be enough.

I'd like to point out two more points are necessary to be considered
for DML permission checks in addition to ExecCheckRTPerms().

* DoCopy()

Although DoCopy() is called from standard_ProcessUtility(), it performs
as DML statement, rather than DDL. It check ACL_SELECT or ACL_INSERT on
the copied table or attributes, similar to what ExecCheckRTEPerms() doing.

* RI_Initial_Check()

RI_Initial_Check() is a function called on ALTER TABLE command to add FK
constraints between two relations. The permission to execute this ALTER TABLE
command itself is checked on ATPrepCmd() and ATAddForeignKeyConstraint(),
so it does not affect anything on the DML permission reworks.

When we add a new FK constraint, both of the existing FK and PK relations have
to satify the new constraint. So, RI_Initial_Check() tries to check whether the
PK relation has corresponding tuples to FK relation, or not.
Then, it tries to execute a secondary query using SPI_*() functions, if no
access violations are expected. Otherwise, it scans the FK relation with
per tuple checks sequentionally (see, validateForeignKeyConstraint()), but slow.

If we have an external security provider which will deny accesses on the FK/PK
relation, but the default PG checks allows it, the RI_Initial_Check() tries to
execute secondary SELECT statement, then it raises an access violation error,
although we are already allowed to execute ALTER TABLE statement.

Therefore, we also need to check DML permissions at RI_Initial_Check() to avoid
unexpected access violation error, prior to the secondary query.

BTW, I guess the reason why permissions on attributes are not checked here is
that we missed it at v8.4 development.

The attached patch provides a common checker function of DML, and modifies
ExecCheckRTPerms(), CopyTo() and RI_Initial_Check() to call the checker
function instead of individual ACL checks.

The most part of the checker function is cut & paste from ExecCheckRTEPerms(),
but its arguments are modified for easy invocation from other functions.

extern bool check_dml_permissions(Oid relOid,
Oid userId,
AclMode requiredPerms,
Bitmapset *selCols,
Bitmapset *modCols,
bool abort);

Thanks,
--
KaiGai Kohei <kaigai(at)ak(dot)jp(dot)nec(dot)com>