Reimplementing permission checks for rules
I'm thinking about changing the way that access permission checks are
handled for rules. The rule mechanism provides that accesses to tables
that are mentioned within rules are done with the permissions of the
rule owner, not the invoking user. The way this is implemented is that
when a rule is substituted into a query, the rule rewriter
(a) does its own permission checking on the newly-added rangetable
entries, and
(b) sets a "skipAcl" flag in each such RTE to prevent the executor
from doing normal permissions checking on that RTE.
This is pretty ugly. For one thing, it means near-duplicate code that
has to be kept in sync between the executor and the rewriter. For
another, it's not good that rule-related permissions checks happen at
rewrite time instead of execution time. That means that a cached
execution plan will not respond to later changes in table permissions,
if the access comes via a rule rather than a direct reference.
What I'm thinking about doing is eliminating the "skipAcl" RTE field
and instead adding an Oid field named something like "checkAclAs".
The semantics of this field would be "if zero, check access permissions
for this table using the current effective userID; but if not zero,
check access permissions as if you are this userID". Then the rule
rewriter would do no access permission checks of its own, but would
set this field appropriately in RTEs that it adds to queries. All the
actual permissions checking would happen in one place in the executor.
Comments? Is this a general enough mechanism, and does it fit well
with the various setUID tricks that people are thinking about?
regards, tom lane
At 10:54 26/09/00 -0400, Tom Lane wrote:
Comments? Is this a general enough mechanism, and does it fit well
with the various setUID tricks that people are thinking about?
Didn't Peter & Jan have a rewrite of the permissions system in the pipeline
- or has that disappeared? What Jan was proposing was rather more
substantial than just the setuid stuff, I *think*.
----------------------------------------------------------------
Philip Warner | __---_____
Albatross Consulting Pty. Ltd. |----/ - \
(A.B.N. 75 008 659 498) | /(@) ______---_
Tel: (+61) 0500 83 82 81 | _________ \
Fax: (+61) 0500 83 82 82 | ___________ |
Http://www.rhyme.com.au | / \|
| --________--
PGP key available upon request, | /
and from pgp5.ai.mit.edu:11371 |/
Tom Lane writes:
What I'm thinking about doing is eliminating the "skipAcl" RTE field
and instead adding an Oid field named something like "checkAclAs".
The semantics of this field would be "if zero, check access permissions
for this table using the current effective userID; but if not zero,
check access permissions as if you are this userID". Then the rule
rewriter would do no access permission checks of its own, but would
set this field appropriately in RTEs that it adds to queries. All the
actual permissions checking would happen in one place in the executor.
I like it.
--
Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/
Philip Warner writes:
Didn't Peter & Jan have a rewrite of the permissions system in the pipeline
- or has that disappeared? What Jan was proposing was rather more
substantial than just the setuid stuff, I *think*.
If I had known that we wouldn't beta until October I probably would have
started on it. But there's a technical incompatibility issue at the core
of my idea that would have forced a postpone until 7.2 for some parts
anyway.
--
Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/
Peter Eisentraut <peter_e@gmx.net> writes:
Tom Lane writes:
What I'm thinking about doing is eliminating the "skipAcl" RTE field
and instead adding an Oid field named something like "checkAclAs".
The semantics of this field would be "if zero, check access permissions
for this table using the current effective userID; but if not zero,
check access permissions as if you are this userID". Then the rule
rewriter would do no access permission checks of its own, but would
set this field appropriately in RTEs that it adds to queries. All the
actual permissions checking would happen in one place in the executor.
I like it.
OK. BTW, what is the status of the changeover you proposed re using
OIDs instead of int4 userids as the unique identifiers for users?
In other words, should my field be type Oid or type int4?
regards, tom lane
Tom Lane writes:
OK. BTW, what is the status of the changeover you proposed re using
OIDs instead of int4 userids as the unique identifiers for users?
Because of the pg_dumpall thing that had to be postponed for another
release, otherwise the users would be associated to the wrong groups on
restore.
In other words, should my field be type Oid or type int4?
Interesting question, actually, because the master uid global variable has
always been a Oid type but it was mostly referenced as int4. Considering
that we have a whole oid/int4 mess and that you can't have negative uid's
anyway, you might as well go for the Oid now if you don't mind.
--
Peter Eisentraut peter_e@gmx.net http://yi.org/peter-e/
What I'm thinking about doing is eliminating the "skipAcl" RTE field
and instead adding an Oid field named something like "checkAclAs".
The semantics of this field would be "if zero, check access
permissions
for this table using the current effective userID; but if not zero,
check access permissions as if you are this userID". Then the rule
rewriter would do no access permission checks of its own, but would
set this field appropriately in RTEs that it adds to queries. All the
actual permissions checking would happen in one place in the executor.Comments? Is this a general enough mechanism, and does it fit well
with the various setUID tricks that people are thinking about?
Sounds good, and a step in the right direction.
Andreas
Import Notes
Resolved by subject fallback
Zeugswetter Andreas SB <ZeugswetterA@wien.spardat.at> writes:
What I'm thinking about doing is eliminating the "skipAcl" RTE field
and instead adding an Oid field named something like "checkAclAs".Comments? Is this a general enough mechanism, and does it fit well
with the various setUID tricks that people are thinking about?
Sounds good, and a step in the right direction.
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).
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 */
The parser always sets checkAsUser = 0; it sets checkForRead if the rel
is referenced explicitly anywhere in the query; and it sets
checkForWrite on the target table of INSERT/UPDATE/DELETE. It was a
little tricky to get it to do the right thing when the same table is
both a source and target, eg in INSERT ... SELECT, but not too bad.
When a rule is created, the stored parsetree has the rule creator's OID
assigned into each RTE's checkAsUser field, except for the dummy *NEW*
and *OLD* rtable entries.
The rewriter leaves the check fields on a view's RTE as they are, and
just copies the check fields from the rule for the rtable entries it
adds. No rewrite-time permission checks anywhere.
The executor just carries out the indicated checks. execMain's checking
got a *lot* simpler, too, because it doesn't have to carry around a lot
of baggage to determine whether to check a given RTE for read, write,
or both.
Seems like a big win ...
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.
regards, tom lane