AW: AW: Reimplementing permission checks for rules

Started by Zeugswetter Andreas SBover 25 years ago2 messages
#1Zeugswetter Andreas SB
ZeugswetterA@wien.spardat.at

Sorry for the late reply, but I was on vacation (my 2. daughter was born).

After looking at the rule rewriter some more, I realized that the only
way to push all permissions checks to execution time is not
only to keep
skipAcl, but to generalize it. The problem is with checks on the view
itself --- if you do INSERT INTO someView, which gets rewritten into
an insert to someRealTable, then what you want the executor
to check for
is
Write access on someView by current user
Write access on someRealTable by owner of rule
which is infeasible with the existing code because the executor only
checks for write access on the real target table (someRealTable).

Which is iirc incorrect. It should only check write access to someView.
The checks for the someRealTable should be done as view owner.
Views are often used for horizontal permissions.

What I have now got, and hope to commit today, is the following fields
in RangeTblEntry, replacing skipAcl:

* checkForRead, checkForWrite, and checkAsUser control
run-time access
* permissions checks. A rel will be checked for read
or write access
* (or both, or neither) per checkForRead and checkForWrite. If
* checkAsUser is not InvalidOid, then do the
permissions checks using
* the access rights of that user, not the current
effective user ID.
* (This allows rules to act as setuid gateways.)

bool checkForRead; /* check rel for read access */
bool checkForWrite; /* check rel for write access */
Oid checkAsUser; /* if not zero, check access
as this user */

I don't know, but imho one field for all permissions would have been better,
like we discussed for the permissions system table, since there are more rights
in SQL than read/write (e.g. write is separated into insert, update and delete)
Or did I not understand this correctly ?

NOTE: there is a subtle change here. A rule used to be taken as
executing setUID to the owner of the table the rule is attached to.
Now it is executed as if setUID to the person who created the rule,
who could be a different user if the table owner gave away
RULE rights.
I think this is a more correct behavior, but I'm open to argument.
It'd be easy to make CREATE RULE store the table owner's OID instead,
if anyone wants to argue that that was the right thing.

No, I think setUID to rule creator is correct.

Andreas

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zeugswetter Andreas SB (#1)
Re: AW: AW: Reimplementing permission checks for rules

Zeugswetter Andreas SB <ZeugswetterA@wien.spardat.at> writes:

I don't know, but imho one field for all permissions would have been
better, like we discussed for the permissions system table, since
there are more rights in SQL than read/write (e.g. write is separated
into insert, update and delete)

Not really necessary in the current implementation. checkForWrite
essentially identifies the target table for the operation, and then
the query's commandType is used to decide exactly which flavor of
write access to check for.

IIRC, the ACL code doesn't have the right set of primitive access types
anyway to match the SQL spec's requirements, but that's a task for
another day.

regards, tom lane