Обсуждение: CREATE POLICY and RETURNING
Hi, While I was checking the behavior of RLS, I found that the policy for SELECT doesn't seem to be applied to RETURNING. Is this intentional? Please see the following example. CREATE ROLE foo LOGIN NOSUPERUSER; CREATE TABLE hoge AS SELECT col FROM generate_series(1,10) col; ALTER TABLE hoge ENABLE ROW LEVEL SECURITY; GRANT SELECT, DELETE ON hoge TO foo; CREATE POLICY hoge_select_policy ON hoge FOR SELECT TO foo USING (col < 4); CREATE POLICY hoge_delete_policy ON hoge FOR DELETE TO foo USING (col < 8); \c - foo DELETE FROM hoge WHERE col = 6 RETURNING *; The policy "hoge_select_policy" should disallow the user "foo" to see the row with "col = 6". But the last DELETE RETURNING returns that row. One minor suggestion is: what about changing the message as follows? There are two more similar messages in policy.c, and they use the word "row-policy" instead of "policy". For the consistency, I think that the following also should use the word "row-policy". diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index 3627539..df45385 100644 --- a/src/backend/commands/policy.c +++ b/src/backend/commands/policy.c @@ -551,7 +551,7 @@ CreatePolicy(CreatePolicyStmt *stmt) if (HeapTupleIsValid(rsec_tuple)) ereport(ERROR, (errcode(ERRCODE_DUPLICATE_OBJECT), - errmsg("policy \"%s\" for relation \"%s\" already exists", + errmsg("row-policy \"%s\" for relation \"%s\" already exists", Regards, -- Fujii Masao
On 10/16/2014 12:25 PM, Fujii Masao wrote: > Hi, > > While I was checking the behavior of RLS, I found that the policy for SELECT > doesn't seem to be applied to RETURNING. Is this intentional? This is why I was opposed to having a "SELECT" policy at all. It should be "VISIBLE", "INSERT", "UPDATE", "DELETE". I say "VISIBLE" instead of "READ" because I don't think the rows affected by an UPDATE or DELETE should be affected by whether or not they have a RETURNING clause. That's IMO nonsensical.and violates the usual expectations about which clauses can have filtering effects. So the read-filtering policy should apply to all statements. Not just SELECT. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 10/16/2014 01:44 PM, Craig Ringer wrote: > So the read-filtering policy should apply to all statements. Not just > SELECT. Oh, IIRC one wrinkle in the prior discussion about this was that doing this will prevent the implementation of policies that permit users to update/delete rows they cannot otherwise see. That's an argument in favour of only applying a read-filtering policy where a RETURNING clause is present, but that introduces the "surprise! the effects of your DELETE changed based on an unrelated clause!" issue. Keep in mind, when considering RETURNING, that users don't always add this clause directly. PgJDBC will tack a RETURNING clause on the end of a statement if the user requests generated keys, for example. They will be very surprised if the behaviour of their DML changes based on whether or not they asked to get generated keys. To my mind having behaviour change based on RETURNING is actively wrong, wheras policies that permit rows to be updated/deleted but not selected are a nice-to-have at most. I'd really like to see some more coverage of the details of how these policies apply to inheritance, both the read- and write- sides of DML with RETURNING clauses, etc. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Fujii, * Fujii Masao (masao.fujii@gmail.com) wrote: > While I was checking the behavior of RLS, I found that the policy for SELECT > doesn't seem to be applied to RETURNING. Is this intentional? Please see > the following example. Yes, it was intentional. That said, I'm not against changing it. > CREATE ROLE foo LOGIN NOSUPERUSER; > CREATE TABLE hoge AS SELECT col FROM generate_series(1,10) col; > ALTER TABLE hoge ENABLE ROW LEVEL SECURITY; > GRANT SELECT, DELETE ON hoge TO foo; > CREATE POLICY hoge_select_policy ON hoge FOR SELECT TO foo USING (col < 4); > CREATE POLICY hoge_delete_policy ON hoge FOR DELETE TO foo USING (col < 8); > \c - foo > DELETE FROM hoge WHERE col = 6 RETURNING *; > > The policy "hoge_select_policy" should disallow the user "foo" to see the row > with "col = 6". But the last DELETE RETURNING returns that row. The DELETE USING policy allows DELETE to see the record and therefore it's available for RETURNING. > One minor suggestion is: what about changing the message as follows? > There are two more similar messages in policy.c, and they use the word > "row-policy" instead of "policy". For the consistency, I think that > the following also should use the word "row-policy". I was looking at these while going over the larger "try to be more consistent" concerns, but that was leading me towards 'policy' instead of 'row-policy', as the commands are 'CREATE POLICY', etc. Not against going the other way, but it seems more consistent to do 'policy' everywhere.. Thanks! Stephen
* Craig Ringer (craig@2ndquadrant.com) wrote: > On 10/16/2014 01:44 PM, Craig Ringer wrote: > > So the read-filtering policy should apply to all statements. Not just > > SELECT. > > Oh, IIRC one wrinkle in the prior discussion about this was that doing > this will prevent the implementation of policies that permit users to > update/delete rows they cannot otherwise see. Yeah. > That's an argument in favour of only applying a read-filtering policy > where a RETURNING clause is present, but that introduces the "surprise! > the effects of your DELETE changed based on an unrelated clause!" issue. No- if we were going to do this, I wouldn't want to change the existing structure but rather provide either: a) a way to simply disable RETURNING if the policy is in effect and the policy creator doesn't wish to allow it b) allow the user to define another clause which would be applied to the rows in the RETURNING set > Keep in mind, when considering RETURNING, that users don't always add > this clause directly. PgJDBC will tack a RETURNING clause on the end of > a statement if the user requests generated keys, for example. They will > be very surprised if the behaviour of their DML changes based on whether > or not they asked to get generated keys. Right- that consideration was one of the reasons to not mess with RETURNING, in my view. > To my mind having behaviour change based on RETURNING is actively wrong, > wheras policies that permit rows to be updated/deleted but not selected > are a nice-to-have at most. > > I'd really like to see some more coverage of the details of how these > policies apply to inheritance, both the read- and write- sides of DML > with RETURNING clauses, etc. I assume you mean with regard to documentation..? I'll work on improving that. Thanks! Stephen
>> That's an argument in favour of only applying a read-filtering policy >> where a RETURNING clause is present, but that introduces the "surprise! >> the effects of your DELETE changed based on an unrelated clause!" issue. > > No- if we were going to do this, I wouldn't want to change the existing > structure but rather provide either: > > a) a way to simply disable RETURNING if the policy is in effect and the > policy creator doesn't wish to allow it > b) allow the user to define another clause which would be applied to the > rows in the RETURNING set I think you could probably make the DELETE policy control what can get deleted, but then have the SELECT policy further filter what gets returned. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 10/17/2014 02:49 AM, Robert Haas wrote: > I think you could probably make the DELETE policy control what can get > deleted, but then have the SELECT policy further filter what gets > returned. That seems like the worst of both worlds to me. Suddenly DELETE ... RETURNING might delete more rows than it reports a resultset for. As well as being potentially dangerous for people using it in wCTEs, etc, to me that's the most astonishing possible outcome of all. I'd be much happier with even: ERROR: RETURNING not permitted with SELECT row-security policy than this. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Robert Haas wrote >>> That's an argument in favour of only applying a read-filtering policy >>> where a RETURNING clause is present, but that introduces the "surprise! >>> the effects of your DELETE changed based on an unrelated clause!" issue. >> >> No- if we were going to do this, I wouldn't want to change the existing >> structure but rather provide either: >> >> a) a way to simply disable RETURNING if the policy is in effect and the >> policy creator doesn't wish to allow it >> b) allow the user to define another clause which would be applied to the >> rows in the RETURNING set > > I think you could probably make the DELETE policy control what can get > deleted, but then have the SELECT policy further filter what gets > returned. Without commenting on the usefulness of blind deletes... How about returning a placeholder row but with all the values replaced with NULL? In the absence of returning does the delete count show the total number of rows deleted or only the number of rows deleted that the user would be aware of if they issued a select with the same criteria? Whatever the answer the number of rows returned with returning should match the row count normally noted. David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/CREATE-POLICY-and-RETURNING-tp5823192p5823374.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
On 17 October 2014 07:57, Craig Ringer <craig@2ndquadrant.com> wrote:
On 10/17/2014 02:49 AM, Robert Haas wrote:
> I think you could probably make the DELETE policy control what can get
> deleted, but then have the SELECT policy further filter what gets
> returned.
That seems like the worst of both worlds to me.
Suddenly DELETE ... RETURNING might delete more rows than it reports a
resultset for. As well as being potentially dangerous for people using
it in wCTEs, etc, to me that's the most astonishing possible outcome of all.
I'd be much happier with even:
ERROR: RETURNING not permitted with SELECT row-security policy
than this.
+1
This suggestion is most in line with what I would expect to occur.
On Fri, Oct 17, 2014 at 3:49 AM, Robert Haas <robertmhaas@gmail.com> wrote: >>> That's an argument in favour of only applying a read-filtering policy >>> where a RETURNING clause is present, but that introduces the "surprise! >>> the effects of your DELETE changed based on an unrelated clause!" issue. >> >> No- if we were going to do this, I wouldn't want to change the existing >> structure but rather provide either: >> >> a) a way to simply disable RETURNING if the policy is in effect and the >> policy creator doesn't wish to allow it >> b) allow the user to define another clause which would be applied to the >> rows in the RETURNING set > > I think you could probably make the DELETE policy control what can get > deleted, but then have the SELECT policy further filter what gets > returned. +1 That's more intuitive to me because another security feature "privilege" works in that way, i.e., SELECT privilege not DELETE controls RETURNING. Another minor problem that I found is that pg_dump always fails when there is a row-level policy for update. I think that the attached patch should be applied. Regards, -- Fujii Masao
Вложения
* Fujii Masao (masao.fujii@gmail.com) wrote: > Another minor problem that I found is that pg_dump always fails when > there is a row-level policy for update. I think that the attached patch > should be applied. Urgh. That was tested before but we switched the characters used and evidently missed that. :( Will fix, thanks! Stephen
* Thom Brown (thom@linux.com) wrote: > On 17 October 2014 07:57, Craig Ringer <craig@2ndquadrant.com> wrote: > > On 10/17/2014 02:49 AM, Robert Haas wrote: > > > I think you could probably make the DELETE policy control what can get > > > deleted, but then have the SELECT policy further filter what gets > > > returned. > > > > That seems like the worst of both worlds to me. > > > > Suddenly DELETE ... RETURNING might delete more rows than it reports a > > resultset for. As well as being potentially dangerous for people using > > it in wCTEs, etc, to me that's the most astonishing possible outcome of > > all. > > > > I'd be much happier with even: > > > > ERROR: RETURNING not permitted with SELECT row-security policy > > > > than this. > > +1 > > This suggestion is most in line with what I would expect to occur. This was along the lines that I've been thinking for how to address this also and I think it's the least surprising- but I want it controllable.. Thoughts on 'WITH RETURNING' / 'WITHOUT RETURNING' and what the default should be? Thanks! Stephen
* David G Johnston (david.g.johnston@gmail.com) wrote: > How about returning a placeholder row but with all the values replaced with > NULL? I don't think that would be a good approach.. A user actually looking at those rows would be highly confused. > In the absence of returning does the delete count show the total number of > rows deleted or only the number of rows deleted that the user would be aware > of if they issued a select with the same criteria? Whatever the answer the > number of rows returned with returning should match the row count normally > noted. Today, everything matches up, yes. Having rows which are deleted but which don't show up in RETURNING could certainly surprise people and applications, which is why I tend to favor the 'all-or-error' approach that others have also suggested. Adding that wouldn't be difficult, though we'd need to decide which should be the default. Thanks! Stephen
On Fri, Oct 17, 2014 at 7:46 AM, Stephen Frost <sfrost@snowman.net> wrote: > Thoughts on 'WITH RETURNING' / 'WITHOUT RETURNING' and what the default > should be? That sounds like a horrendous pile of nasty complication for no good purpose. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2014-10-17 14:57:03 +0800, Craig Ringer wrote: > On 10/17/2014 02:49 AM, Robert Haas wrote: > > I think you could probably make the DELETE policy control what can get > > deleted, but then have the SELECT policy further filter what gets > > returned. > > That seems like the worst of both worlds to me. > > Suddenly DELETE ... RETURNING might delete more rows than it reports a > resultset for. As well as being potentially dangerous for people using > it in wCTEs, etc, to me that's the most astonishing possible outcome of all. > > I'd be much happier with even: > > ERROR: RETURNING not permitted with SELECT row-security policy FWIW, that doesn't sound acceptable to me. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Fri, Oct 17, 2014 at 5:34 AM, Andres Freund <andres@2ndquadrant.com> wrote: > On 2014-10-17 14:57:03 +0800, Craig Ringer wrote: >> On 10/17/2014 02:49 AM, Robert Haas wrote: >> > I think you could probably make the DELETE policy control what can get >> > deleted, but then have the SELECT policy further filter what gets >> > returned. >> >> That seems like the worst of both worlds to me. >> >> Suddenly DELETE ... RETURNING might delete more rows than it reports a >> resultset for. As well as being potentially dangerous for people using >> it in wCTEs, etc, to me that's the most astonishing possible outcome of all. >> >> I'd be much happier with even: >> >> ERROR: RETURNING not permitted with SELECT row-security policy > > FWIW, that doesn't sound acceptable to me. This is more or less what ended up happening with UPSERT and USING security barrier quals on UPDATE/ALL policies. Realistically, the large majority of use cases don't involve a user being able to INSERT/DELETE tuples, but not SELECT them, and those that do should not be surprised to have a RETURNING fail (it's an odd enough union of different features that this seems acceptable to me). Like Fujii, I think that RETURNING with RLS should not get to avoid SELECT policies. I agree with the concern about not seeing affected rows with a DELETE (which, as I said, is very similar to UPSERT + WCO_RLS_CONFLICT_CHECK policies), so an error seems like the only alternative. The argument against not requiring SELECT *column* privilege on the EXCLUDED.* pseudo relation for UPSERT might have been: "well, what can be the harm of allowing the user to see what they themselves might have inserted?". But that would have been a bad argument then had anyone made it, because RETURNING with a (vanilla) INSERT requires SELECT privilege, and that's also what the user then actually inserted (as distinct from what the user *would have* inserted had the insert path been taking, representing as the EXCLUDED.* pseudo relation -- for security purposes, ISTM that this is really no distinction at all). Consider before row insert triggers that can modify EXCLUDED.* tuples in a privileged way. So, the only logical reason that INSERT with RETURNING requires SELECT column privilege that I can see is that a before row INSERT trigger could modify the tuple inserted in a way that the inserter role should not know the details of. This long standing convention was reason enough to mandate that SELECT column privilege be required for the EXCLUDED.* pseudo relation for UPSERT. And so, I think it isn't too much of a jump to also say that we should do the same for RLS (for INSERTs for the reason I state, but also for UPDATEs and DELETEs for a far more obvious reason: the *existing* tuple can be projected, and the updater/deleter might well have no business seeing its contents). In short: I think we should be tracking a new WCOKind (perhaps WCO_RLS_RETURNING_CHECK?), that independently holds the security barrier quals as WCO-style checks when that's recognized as being necessary. For INSERT, these WCOs must be enforced against the target tuple projected by RETURNING. For UPDATEs and DELETEs, FROM/USING relations must also have SELECT privilege enforced against the projected target tuple, as well as the non-target relation -- apparently the latter isn't currently happening, although Dean has tried to address this with his recent patch [1]. That is, even non-target relations (UPDATE ... FROM relations, or DELETE ... USING relations) do not have SELECT policy enforcement, but rather have UPDATE or DELETE policy enforcement only. I must admit that I was rather surprised at that; it has to be a bug. [1] http://www.postgresql.org/message-id/CAEZATCVE7hdtfZGCJN-oevVaWBtBGG8-fBCh9VhDBHuZrsWY5w@mail.gmail.com -- Peter Geoghegan
On 6 June 2015 at 22:16, Peter Geoghegan <pg@heroku.com> wrote: > On Fri, Oct 17, 2014 at 5:34 AM, Andres Freund <andres@2ndquadrant.com> wrote: >> On 2014-10-17 14:57:03 +0800, Craig Ringer wrote: >>> On 10/17/2014 02:49 AM, Robert Haas wrote: >>> > I think you could probably make the DELETE policy control what can get >>> > deleted, but then have the SELECT policy further filter what gets >>> > returned. >>> >>> That seems like the worst of both worlds to me. >>> >>> Suddenly DELETE ... RETURNING might delete more rows than it reports a >>> resultset for. As well as being potentially dangerous for people using >>> it in wCTEs, etc, to me that's the most astonishing possible outcome of all. >>> >>> I'd be much happier with even: >>> >>> ERROR: RETURNING not permitted with SELECT row-security policy >> >> FWIW, that doesn't sound acceptable to me. > > This is more or less what ended up happening with UPSERT and USING > security barrier quals on UPDATE/ALL policies. Realistically, the > large majority of use cases don't involve a user being able to > INSERT/DELETE tuples, but not SELECT them, and those that do should > not be surprised to have a RETURNING fail (it's an odd enough union of > different features that this seems acceptable to me). > > Like Fujii, I think that RETURNING with RLS should not get to avoid > SELECT policies. I agree with the concern about not seeing affected > rows with a DELETE (which, as I said, is very similar to UPSERT + > WCO_RLS_CONFLICT_CHECK policies), so an error seems like the only > alternative. > > The argument against not requiring SELECT *column* privilege on the > EXCLUDED.* pseudo relation for UPSERT might have been: "well, what can > be the harm of allowing the user to see what they themselves might > have inserted?". But that would have been a bad argument then had > anyone made it, because RETURNING with a (vanilla) INSERT requires > SELECT privilege, and that's also what the user then actually inserted > (as distinct from what the user *would have* inserted had the insert > path been taking, representing as the EXCLUDED.* pseudo relation -- > for security purposes, ISTM that this is really no distinction at > all). Consider before row insert triggers that can modify EXCLUDED.* > tuples in a privileged way. > > So, the only logical reason that INSERT with RETURNING requires SELECT > column privilege that I can see is that a before row INSERT trigger > could modify the tuple inserted in a way that the inserter role should > not know the details of. This long standing convention was reason > enough to mandate that SELECT column privilege be required for the > EXCLUDED.* pseudo relation for UPSERT. And so, I think it isn't too > much of a jump to also say that we should do the same for RLS (for > INSERTs for the reason I state, but also for UPDATEs and DELETEs for a > far more obvious reason: the *existing* tuple can be projected, and > the updater/deleter might well have no business seeing its contents). > > In short: I think we should be tracking a new WCOKind (perhaps > WCO_RLS_RETURNING_CHECK?), that independently holds the security > barrier quals as WCO-style checks when that's recognized as being > necessary. For INSERT, these WCOs must be enforced against the target > tuple projected by RETURNING. For UPDATEs and DELETEs, FROM/USING > relations must also have SELECT privilege enforced against the > projected target tuple, as well as the non-target relation -- > apparently the latter isn't currently happening, although Dean has > tried to address this with his recent patch [1]. That is, even > non-target relations (UPDATE ... FROM relations, or DELETE ... USING > relations) do not have SELECT policy enforcement, but rather have > UPDATE or DELETE policy enforcement only. I must admit that I was > rather surprised at that; it has to be a bug. > Yes, I think a query with a RETURNING clause ought to either succeed and return everything the user asked for, or error out if some/all of the data isn't visible to the user according to SELECT policies in effect. I think not applying the SELECT policies is wrong, and returning a portion of the data updated would just be confusing. My previous patch causes the SELECT policies for all the non-target relations to be applied, but not the SELECT policies for the target relation. So I think it would suffice to just add another check to apply them to the target tuple, using something like WCO_RLS_RETURNING_CHECK, as you suggested. However, I think it must be applied to the target tuple *before* projecting it because the RETURNING expressions might contain malicious leaky functions. Actually I'm not sure the policy can be enforced against the projected target tuple, since it might not contain all the necessary columns to do the check. In principle, it sounds easy to do, but I think it will be much simpler against my previous patch. Regards, Dean
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote: > On 6 June 2015 at 22:16, Peter Geoghegan <pg@heroku.com> wrote: > > On Fri, Oct 17, 2014 at 5:34 AM, Andres Freund <andres@2ndquadrant.com> wrote: > >> On 2014-10-17 14:57:03 +0800, Craig Ringer wrote: > >>> On 10/17/2014 02:49 AM, Robert Haas wrote: > >>> > I think you could probably make the DELETE policy control what can get > >>> > deleted, but then have the SELECT policy further filter what gets > >>> > returned. > >>> > >>> That seems like the worst of both worlds to me. > >>> > >>> Suddenly DELETE ... RETURNING might delete more rows than it reports a > >>> resultset for. As well as being potentially dangerous for people using > >>> it in wCTEs, etc, to me that's the most astonishing possible outcome of all. > >>> > >>> I'd be much happier with even: > >>> > >>> ERROR: RETURNING not permitted with SELECT row-security policy > >> > >> FWIW, that doesn't sound acceptable to me. > > > > This is more or less what ended up happening with UPSERT and USING > > security barrier quals on UPDATE/ALL policies. Realistically, the > > large majority of use cases don't involve a user being able to > > INSERT/DELETE tuples, but not SELECT them, and those that do should > > not be surprised to have a RETURNING fail (it's an odd enough union of > > different features that this seems acceptable to me). > > > > Like Fujii, I think that RETURNING with RLS should not get to avoid > > SELECT policies. I agree with the concern about not seeing affected > > rows with a DELETE (which, as I said, is very similar to UPSERT + > > WCO_RLS_CONFLICT_CHECK policies), so an error seems like the only > > alternative. > > > > The argument against not requiring SELECT *column* privilege on the > > EXCLUDED.* pseudo relation for UPSERT might have been: "well, what can > > be the harm of allowing the user to see what they themselves might > > have inserted?". But that would have been a bad argument then had > > anyone made it, because RETURNING with a (vanilla) INSERT requires > > SELECT privilege, and that's also what the user then actually inserted > > (as distinct from what the user *would have* inserted had the insert > > path been taking, representing as the EXCLUDED.* pseudo relation -- > > for security purposes, ISTM that this is really no distinction at > > all). Consider before row insert triggers that can modify EXCLUDED.* > > tuples in a privileged way. > > > > So, the only logical reason that INSERT with RETURNING requires SELECT > > column privilege that I can see is that a before row INSERT trigger > > could modify the tuple inserted in a way that the inserter role should > > not know the details of. This long standing convention was reason > > enough to mandate that SELECT column privilege be required for the > > EXCLUDED.* pseudo relation for UPSERT. And so, I think it isn't too > > much of a jump to also say that we should do the same for RLS (for > > INSERTs for the reason I state, but also for UPDATEs and DELETEs for a > > far more obvious reason: the *existing* tuple can be projected, and > > the updater/deleter might well have no business seeing its contents). > > > > In short: I think we should be tracking a new WCOKind (perhaps > > WCO_RLS_RETURNING_CHECK?), that independently holds the security > > barrier quals as WCO-style checks when that's recognized as being > > necessary. For INSERT, these WCOs must be enforced against the target > > tuple projected by RETURNING. For UPDATEs and DELETEs, FROM/USING > > relations must also have SELECT privilege enforced against the > > projected target tuple, as well as the non-target relation -- > > apparently the latter isn't currently happening, although Dean has > > tried to address this with his recent patch [1]. That is, even > > non-target relations (UPDATE ... FROM relations, or DELETE ... USING > > relations) do not have SELECT policy enforcement, but rather have > > UPDATE or DELETE policy enforcement only. I must admit that I was > > rather surprised at that; it has to be a bug. > > > > Yes, I think a query with a RETURNING clause ought to either succeed > and return everything the user asked for, or error out if some/all of > the data isn't visible to the user according to SELECT policies in > effect. I think not applying the SELECT policies is wrong, and > returning a portion of the data updated would just be confusing. I definitely don't like the idea of returning a portion of the data in a RETURNING clause. Returning an error if the RETURNING clause ends up not passing the SELECT policy is an interesting idea, but I do have doubts about how often that will be a useful exercise. Another issue to note is that, currently, SELECT policies never cause errors to be returned, and this would change that. There was discussion about a VISIBILITY policy instead of a SELECT policy at one point. What I think we're seeing here is confusion about the various policies which exist, because the USING policy of an UPDATE is precisely its VISIBILITY policy, in my view, which is why I wasn't bothered by the RETURNING clause being able to be used in that case. I recall working on the documentation to make this clear, but it clearly needs work. The primary use-case for having a different VISIBILITY for UPDATE (or DELETE) than for SELECT is that you wish to allow users to only modify a subset of the rows in the table which they can see. I agree that the current arrangement is that this can be used to allow users to UPDATE records which they can't see (except through a RETURNING). Perhaps that's a mistake and we should, instead, force the SELECT policy to be included for the UPDATE and DELETE policies and have the USING clauses from those be AND'd with the SELECT policy. That strikes me as a much simpler approach than applying the SELECT policy against the RETURNING clause and then throwing an error if it fails, though this would remove a bit of flexibility in the system since we would no longer allow blind UPDATEs or DELETEs. I'm not sure if that's really an issue though- to compare it to our GRANT-based system, the only blind UPDATE allowed there is one which goes across the entire table- any WHERE clause used results in a check to ensure you have SELECT rights. Ultimately, we're trying to provide as much flexibility as possible while having the system be easily understandable by the policy authors and allowing them to write simple policies to enforce the necessary constraints. Perhaps allowing the open-ended USING clauses for UPDATE and DELETE is simply making for a more complicated system as policy authors then have to make sure they embed whatever SELECT policy they have into their UPDATE and DELETE policies explicitly- and the result if they don't do that correctly is that users may be able to update/delete things they really shouldn't be able to. Now, as we've discussed previously, users should be expected to test their systems and make sure that they are behaving as they expect, but we don't want to put landmines in either or have ways which they can be easily tripped up. > My previous patch causes the SELECT policies for all the non-target > relations to be applied, but not the SELECT policies for the target > relation. So I think it would suffice to just add another check to > apply them to the target tuple, using something like > WCO_RLS_RETURNING_CHECK, as you suggested. However, I think it must be > applied to the target tuple *before* projecting it because the > RETURNING expressions might contain malicious leaky functions. > Actually I'm not sure the policy can be enforced against the projected > target tuple, since it might not contain all the necessary columns to > do the check. Agreed- the SELECT policies for the non-target relations should be used, that's clearly the right thing to do. Regarding the rest, I'd certainly welcome your thoughts on my above response. > In principle, it sounds easy to do, but I think it will be much > simpler against my previous patch. I'm planning to review your latest prior to PGCon. I really like that you were able to eliminate the "extra" places we were adding the default-deny policy and believe that will make the code a lot simpler to follow. Thanks! Stephen
On 8 June 2015 at 16:53, Stephen Frost <sfrost@snowman.net> wrote: > I definitely don't like the idea of returning a portion of the data in a > RETURNING clause. Returning an error if the RETURNING clause ends up > not passing the SELECT policy is an interesting idea, but I do have > doubts about how often that will be a useful exercise. Another issue to > note is that, currently, SELECT policies never cause errors to be > returned, and this would change that. > True. Before UPSERT was added, it was the case that USING clauses from all kinds of policies didn't cause errors, and CHECK clauses did, but that has now changed for UPDATE, and I don't think it's necessarily a problem to change it for SELECT, if we decide that it makes sense to apply SELECT policies to rows returned by RETURNING clauses. > There was discussion about a VISIBILITY policy instead of a SELECT > policy at one point. What I think we're seeing here is confusion about > the various policies which exist, because the USING policy of an UPDATE > is precisely its VISIBILITY policy, in my view, which is why I wasn't > bothered by the RETURNING clause being able to be used in that case. I > recall working on the documentation to make this clear, but it clearly > needs work. > Yeah, perhaps there's scope for improving the documentation, regardless of whether or not we change the current behaviour. One place that we could add additional documentation is the pages describing the INSERT, UPDATE and DELETE commands. These currently each have a paragraph in their main description sections describing what privileges they require, so perhaps we should add similar paragraphs describing what RLS policies are checked, if RLS is enabled on the table. > The primary use-case for having a different VISIBILITY for UPDATE (or > DELETE) than for SELECT is that you wish to allow users to only modify a > subset of the rows in the table which they can see. I agree that the > current arrangement is that this can be used to allow users to UPDATE > records which they can't see (except through a RETURNING). Perhaps > that's a mistake and we should, instead, force the SELECT policy to be > included for the UPDATE and DELETE policies and have the USING clauses > from those be AND'd with the SELECT policy. That strikes me as a much > simpler approach than applying the SELECT policy against the RETURNING > clause and then throwing an error if it fails, I don't think that quite addresses the concern with RETURNING though because the resulting clauses would only be applied to the old (pre-update) data, whereas a RETURNING clause might still return new data that you couldn't otherwise see. > though this would remove > a bit of flexibility in the system since we would no longer allow blind > UPDATEs or DELETEs. I'm not sure if that's really an issue though- to > compare it to our GRANT-based system, the only blind UPDATE allowed > there is one which goes across the entire table- any WHERE clause used > results in a check to ensure you have SELECT rights. > That's not quite the extent of it. Column-level privileges might allow you to SELECT the PK column of a table, and then you might be able to UPDATE or DELETE specific rows despite not being able to see their entire contents. I can see cases where that might be useful, but I have to admit that I'm struggling to think of any case where the row-level equivalent would make sense (although that, by itself is not a good argument for not allowing it). > Ultimately, we're trying to provide as much flexibility as possible > while having the system be easily understandable by the policy authors > and allowing them to write simple policies to enforce the necessary > constraints. Perhaps allowing the open-ended USING clauses for UPDATE > and DELETE is simply making for a more complicated system as policy > authors then have to make sure they embed whatever SELECT policy they > have into their UPDATE and DELETE policies explicitly- and the result if > they don't do that correctly is that users may be able to update/delete > things they really shouldn't be able to. Now, as we've discussed > previously, users should be expected to test their systems and make sure > that they are behaving as they expect, but we don't want to put > landmines in either or have ways which they can be easily tripped up. > Well I'm all for keeping this as simple and easily understandable as possible. Right now the USING clauses of UPDATE policies control which existing rows can be updated and the CHECK clauses control what new data can go in. That's about as simple as it could be IMO, and trying to merge SELECT policies with either of those would only make it more complicated, and hence make it more likely for users to get it wrong. OTOH, when you tag a RETURNING clause onto the UPDATE, it's kind of like tying an UPDATE and a SELECT together, and it does make sense to me that the new rows returned by the SELECT part of the command be constrained by any SELECT policies on the table. That's consistent with what happens with GRANT-based privileges, so I don't think that it should be that surprising or difficult for users to understand. I also think that if it ever were the case that a row resulting from an UPDATE were not visible to the user who made that update, then that would most likely indicate that the policies were poorly-defined. Ideally the UPDATE policy's CHECK clause would prevent the user from making such an update. Having a RETURNING clause that was checked against the table's SELECT policy would then actually provide the user with a way to validate the consistency of their policy clauses. Hitting this new RETURNING-SELECT-POLICY error would be a sign that perhaps the WITH CHECK clauses on your policies aren't properly constraining new data. So I actually think this additional check would increase the chances of policies being defined correctly, and I think that consistency with GRANT-based privileges will make it easier to understand. Regards, Dean
Dean, * Dean Rasheed (dean.a.rasheed@gmail.com) wrote: > On 8 June 2015 at 16:53, Stephen Frost <sfrost@snowman.net> wrote: > > I definitely don't like the idea of returning a portion of the data in a > > RETURNING clause. Returning an error if the RETURNING clause ends up > > not passing the SELECT policy is an interesting idea, but I do have > > doubts about how often that will be a useful exercise. Another issue to > > note is that, currently, SELECT policies never cause errors to be > > returned, and this would change that. > > True. Before UPSERT was added, it was the case that USING clauses from > all kinds of policies didn't cause errors, and CHECK clauses did, but > that has now changed for UPDATE, and I don't think it's necessarily a > problem to change it for SELECT, if we decide that it makes sense to > apply SELECT policies to rows returned by RETURNING clauses. Fair enough. > > There was discussion about a VISIBILITY policy instead of a SELECT > > policy at one point. What I think we're seeing here is confusion about > > the various policies which exist, because the USING policy of an UPDATE > > is precisely its VISIBILITY policy, in my view, which is why I wasn't > > bothered by the RETURNING clause being able to be used in that case. I > > recall working on the documentation to make this clear, but it clearly > > needs work. > > Yeah, perhaps there's scope for improving the documentation, > regardless of whether or not we change the current behaviour. One > place that we could add additional documentation is the pages > describing the INSERT, UPDATE and DELETE commands. These currently > each have a paragraph in their main description sections describing > what privileges they require, so perhaps we should add similar > paragraphs describing what RLS policies are checked, if RLS is enabled > on the table. Agreed. > > The primary use-case for having a different VISIBILITY for UPDATE (or > > DELETE) than for SELECT is that you wish to allow users to only modify a > > subset of the rows in the table which they can see. I agree that the > > current arrangement is that this can be used to allow users to UPDATE > > records which they can't see (except through a RETURNING). Perhaps > > that's a mistake and we should, instead, force the SELECT policy to be > > included for the UPDATE and DELETE policies and have the USING clauses > > from those be AND'd with the SELECT policy. That strikes me as a much > > simpler approach than applying the SELECT policy against the RETURNING > > clause and then throwing an error if it fails, > > I don't think that quite addresses the concern with RETURNING though > because the resulting clauses would only be applied to the old > (pre-update) data, whereas a RETURNING clause might still return new > data that you couldn't otherwise see. This part doesn't make sense to me. You can issue a SELECT statement which will return records that are not visible to you also, by simply running some transformation of the appropriate column as it comes out. I don't believe it makes any sense to try and protect the results of a transformation from the user who is defining what the transformations are. Worse, with this approach, it might *look* like RETURNING results are being filtered in a way that prevents users from being able to see data they shouldn't be allowed to see in the table via the SELECT policy when they can- they just have to transform the appropriate column during the UPDATE to get it to pass the UPDATE CHECK and SELECT USING policies. Here is an example of what I mean: CREATE TABLE my_data ( color text, secret text ); CREATE POLICY only_green ON my_data FOR SELECT USING (color = 'green'); CREATE POLICY allow_update ON my_data FOR UPDATE USING (true) -- a mistake WITH CHECK (color = 'green'); If the attacker was interested in rows which had 'red' for the color, they could simply run: UPDATE my_data SET color = 'green' WHERE color = 'red' RETURNING *; The above command returns all the 'secret' data for rows which had a color of 'red' in the table, even though the SELECT policy clearly doesn't allow the user to see those rows and the UPDATE WITH CHECK policy only allows the user to create records with the color 'green', and this would still be the case even with the proposed changes. Note that the above WHERE clause isn't actually necessary at all, but the 'color' column would need to be set to 'green' to pass the SELECT and UPDATE policies, and therefore the attacker would lose the data in that column- but we're not trying to hide just the data in that particular column, nor do we want to have to define policies for every column. Basically, I don't think it makes much sense to try and enforce a SELECT policy on a value which an attacker can set. This is very different from enforcing a policy about adding rows to a table or enforcing a policy about what rows are visible to a user based on the data already in the table. Ultimately, we're not actually protecting any data by filtering or erroring based on the results of a transformation that the attacker gets to define. Worse, with this check, users might feel that they have their policies correctly defined based on testing a command such as: UPDATE my_data SET color = 'red' WHERE color = 'red' RETURNING *; (or perhaps setting some other, unrelated, column but trying to pull out the 'red' secret data) As that would then result in an error from the new RETURNING-SELECT-POLICY check, but the data wouldn't actually be protected, as illustrated above. > > though this would remove > > a bit of flexibility in the system since we would no longer allow blind > > UPDATEs or DELETEs. I'm not sure if that's really an issue though- to > > compare it to our GRANT-based system, the only blind UPDATE allowed > > there is one which goes across the entire table- any WHERE clause used > > results in a check to ensure you have SELECT rights. > > That's not quite the extent of it. Column-level privileges might allow > you to SELECT the PK column of a table, and then you might be able to > UPDATE or DELETE specific rows despite not being able to see their > entire contents. I can see cases where that might be useful, but I > have to admit that I'm struggling to think of any case where the > row-level equivalent would make sense (although that, by itself is not > a good argument for not allowing it). That's a good point that you could use the PK in the WHERE of an UPDATE or DELETE and not have rights to SELECT the other columns. I can imagine cases where users would want to be able to 'give away' rows they can currently see, but I have a hard time coming up with a reason to allow them to acquire rows they are not currently allowed to see. I see that as an argument for including the SELECT policy implicitly though, as I suggested above, but I take it that you're not advocating for that, per your comments below. > > Ultimately, we're trying to provide as much flexibility as possible > > while having the system be easily understandable by the policy authors > > and allowing them to write simple policies to enforce the necessary > > constraints. Perhaps allowing the open-ended USING clauses for UPDATE > > and DELETE is simply making for a more complicated system as policy > > authors then have to make sure they embed whatever SELECT policy they > > have into their UPDATE and DELETE policies explicitly- and the result if > > they don't do that correctly is that users may be able to update/delete > > things they really shouldn't be able to. Now, as we've discussed > > previously, users should be expected to test their systems and make sure > > that they are behaving as they expect, but we don't want to put > > landmines in either or have ways which they can be easily tripped up. > > Well I'm all for keeping this as simple and easily understandable as > possible. Right now the USING clauses of UPDATE policies control which > existing rows can be updated and the CHECK clauses control what new > data can go in. That's about as simple as it could be IMO, and trying > to merge SELECT policies with either of those would only make it more > complicated, and hence make it more likely for users to get it wrong. I like the flexibility and simplicity of having SELECT be independent of the UPDATE and other policies and the ALL option for defining policies certainly helps with that, when the policies for all operations have the same clauses. That said, I can see an argument for having SELECT policies enforced on the UPDATE USING clause as actually being simpler as it means you don't have to think about the UPDATE USING clause independently, unless you want to further reduce the set of rows available through the policy. Still, I'm not going to argue if we end up agreeing that what we have already makes sense. > OTOH, when you tag a RETURNING clause onto the UPDATE, it's kind of > like tying an UPDATE and a SELECT together, and it does make sense to > me that the new rows returned by the SELECT part of the command be > constrained by any SELECT policies on the table. That's consistent > with what happens with GRANT-based privileges, so I don't think that > it should be that surprising or difficult for users to understand. There is a big difference here though- there's no such thing as a 'new' or 'old' row when talking about the GRANT system. In the GRANT system, everything is checked at the outset of the command and the RETURNING clause's columns are checked for SELECT rights, even if the entire column has been replaced during the UPDATE statement with a constant or other value defined by the user. Therefore, I don't believe you can make the argument that the GRANT system supports this notion of checking the SELECT policy against the 'new' rows being returned. > I also think that if it ever were the case that a row resulting from > an UPDATE were not visible to the user who made that update, then that > would most likely indicate that the policies were poorly-defined. This is a case that I disagree with. As mentioned above, I can imagine environments where it's acceptable to be able to "give away" rows. Moving away from the security arena for a moment, consider a 'queue' table where the rows move through different states. RLS might be a very handy way to implement that queue by having different processes only able to see the rows which have a particular state, but they can update the rows to give them a different state which then moves them to another processes' queue. Bringing it back to the security side, consider a hospital where there can only be one 'responsible' physician assigned, but they are allowed to trade cases around based on some external criteria. Or the case files for a judge. Perhaps these change operations should require an 'administrator' user to implement, but perhaps not. Allowing users to acquire rows which they can't currently see is a much more questionable action, in my view, as it implies independent knowledge that such data exists to be acquired. > Ideally the UPDATE policy's CHECK clause would prevent the user from > making such an update. Having a RETURNING clause that was checked > against the table's SELECT policy would then actually provide the user > with a way to validate the consistency of their policy clauses. > Hitting this new RETURNING-SELECT-POLICY error would be a sign that > perhaps the WITH CHECK clauses on your policies aren't properly > constraining new data. Agreed, the CHECK clause should prevent users from giving away records in environments where that isn't appropriate. I'm not convinced that adding a RETURNING-SELECT-POLICY error would actually be terribly useful to users for checking that their policies are constructed correctly, see above regarding my concerns about enforcing policies against the results of the transformations requested by the user. > So I actually think this additional check would increase the chances > of policies being defined correctly, and I think that consistency with > GRANT-based privileges will make it easier to understand. I'm afraid it wouldn't actually prevent attackers from getting access to data they shouldn't have access to and I worry that it'd introduce complications that would mask incorrect policy definitions, as discussed above. Thanks! Stephen
Hi, This thread has a pretty thorough discussion of pros and cons of applying SELECT policy to other commands. Besides what have been mentioned, I think there is another potential pro: we can enable references to pseudorelations OLD and NEW in predicates. Now, for UPDATE, the references to the target table in USING clause are actually references to OLD and the references in WITH CHECK clause are references to NEW. Logically now USING and WITH CHECK are combined by AND, so we cannot have predicates like foo(NEW) > 1 OR bar(OLD) > 1 (combine predicates referencing OLD and NEW by an operation other than AND) NEW.id <> OLD.id (reference both in the same expression) If we apply SELECT policy to other commands, we only need one predicate for INSERT/UPDATE/DELETE. That predicate can reference to OLD and NEW, just like predicates for triggers and rules. For UPDATE and DELETE, the predicate of SELECT will be applied first (when the target table is scanned) to ensure no information leakage and their own predicate will be applied later. This doesn't change much for INSERT and DELETE, but it gives users more flexibility when they set predicates for UPDATE. Thanks, Zhaomo -- View this message in context: http://postgresql.nabble.com/CREATE-POLICY-and-RETURNING-tp5823192p5861550.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
In case you missed the link to the previous discussion at the bottom,
Zhaomo, * Zhaomo Yang (zmpgzm@gmail.com) wrote: > This thread has a pretty thorough discussion of pros and cons of applying > SELECT policy to other commands. Besides what have been mentioned, I think > there is another potential pro: we can enable references to pseudorelations > OLD and NEW in predicates. Now, for UPDATE, the references to the target > table in USING clause are actually references to OLD and the references in > WITH CHECK clause are references to NEW. Logically now USING and WITH CHECK > are combined by AND, so we cannot have predicates like For my part, I find that the simplicity of having USING only ever refer to existing records and WITH CHECK only ever refer to records being added to be good and I'm concerned that this approach would be confusing. If no NEW or OLD is used, what happens? Or would you have to always specify OLD/NEW for UPDATE, and then what about for the other policies, and the FOR ALL policies? > foo(NEW) > 1 OR bar(OLD) > 1 (combine predicates referencing OLD > and NEW by an operation other than AND) Your statement that this can't be done with the existing policy approach is incorrect, or I've misunderstood what you mean above. This could be accomplished with "USING (bar > 1)" and "WITH CHECK (foo > 1)", no? Your sentence above that "USING and WITH CHECK are combined by AND" isn't correct either- they're independent and are therefore really OR'd. If they were AND'd then the new record would have to pass both USING and WITH CHECK policies. > NEW.id <> OLD.id (reference both in the same expression) Here you're correct that this isn't something the existing approach to UPDATE policies can support. I don't intend to simply punt on this, but I will point out that this particular requirement can be handled in a trigger, if desired. Further, I'm not sure that I see how this would work in a case where you have the SELECT policy (which clearly could only refer to OLD) applied first, as you suggest? > If we apply SELECT policy to other commands, we only need one predicate for > INSERT/UPDATE/DELETE. That predicate can reference to OLD and NEW, just like > predicates for triggers and rules. For UPDATE and DELETE, the predicate of > SELECT will be applied first (when the target table is scanned) to ensure no > information leakage and their own predicate will be applied later. This > doesn't change much for INSERT and DELETE, but it gives users more > flexibility when they set predicates for UPDATE. OLD and NEW are only applicable for the UPDATE case and requiring those to be used for the other policies is adding complexity for a pretty narrow use-case, as is combining the SELECT USING policy with the USING (or any) policy for the other commands. Further, it clearly reduces the range of options for UPDATE as it means that you can't do blind updates or deletes. Perhaps that's sensible, but we could do that by simply AND'ing the SELECT USING policy with the other command USING policy, but Dean was against that idea up-thread, as am I, because it adds complexity, and it would further mean that neither of your suggested predicates above would be supported, as I explained above. Also, as far as I see, none of this really amounts to anything different with regard to RETURNING than the previous proposal of simply applying the SELECT USING policy to the records first, and the records returned could still not be allowed by the SELECT policy as they would be the results of the update, which could still pass the UPDATE policy for new records. Applying the SELECT USING policy against the RETURNING records strikes me, more and more, as just complexity which will cause more confusion than it helps anyone. We definitely need to explain how the USING clause works for the commands and how that impacts RETURNING, as I've mentioned in the 9.5 open items list, and I'm planning to tackle that this week. Thanks! Stephen
Stephen,
If no NEW or OLD is used, what happens? Or would you have
to always specify OLD/NEW for UPDATE, and then what about for the other
policies, and the FOR ALL policies?
I should be clearer with references to OLD/NEW. SELECT Predicates cannot reference any of them.
INSERT predicates cannot refer to OLD and DELETE predicates cannot refer to NEW. Basically,
for INSERT/UPDATE/DELETE, we specify predicates the same way as we do for triggers' WHEN condition.
As for FOR ALL, I think we will abandon it if we apply SELECT policy to other commands, since SELECT predicate
will be the new universally applicable read policy, which makes the FOR ALL USING clause much less useful. Of course users may need to specify separate predicates for different commands, but I think it is fine. How often do users want the same predicate for all the commands?
This could be accomplished with "USING (bar > 1)" and "WITH CHECK (foo > 1)", no?
Your sentence above that "USING and WITH CHECK are combined by AND"
isn't correct either- they're independent and are therefore really OR'd.
If they were AND'd then the new record would have to pass both USING and
WITH CHECK policies.
No, it is impossible with the current implementation.
CREATE TABLE test {
id int,
v1 int,
v2 int
};
Suppose that the user wants an update policy which is OLD.v1 > 10 OR NEW.v2 < 10.
As you suggested, we use the following policy
CREATE update_p ON test
FOR UPDATE TO test_user
USING v1 > 10
WITH CHECK v2 < 10;
(1) Assume there is only one row in the table
id | v1 | v2 |
1 | 11 | 20 |
Now we execute UPDATE test SET v2 = 100.
this query is allowed by the policy and the only row should be updated since v1's old value > 10, but will trigger an error because it violates the WITH CHECK clause.
(2) Again assume there is only one row in the table
id | v1 | v2 |
1 | 9 | 20 |
Now we execute UPDATE test SET v2 = 7.
this query is allowed by the policy and the only row should be updated since v2's new value < 10, nothing will be updated because the only row will be filtered out before update happens.
This is why I said USING and WITH CHECK are combined by AND. In order to update an row, first the row needs to be visible, which meaning it needs to pass the USING check, then it needs to pass the WITH CHECK.
Further, I'm not sure that I see how this would work in a case where you
have the SELECT policy (which clearly could only refer to OLD) applied
first, as you suggest?
We use SELECT policy to filter the table when we scan it (just like how we use USING clause now). The predicate of UPDATE will be checked later (probably similar to how we handle trigger's WHEN clause which can also reference OLD and NEW).
Thanks,
Zhaomo
Zhaomo, * Zhaomo Yang (zmpgzm@gmail.com) wrote: > > If no NEW or OLD is used, what happens? Or would you have > > to always specify OLD/NEW for UPDATE, and then what about for the other > > policies, and the FOR ALL policies? > > I should be clearer with references to OLD/NEW. SELECT Predicates cannot > reference any of them. > INSERT predicates cannot refer to OLD and DELETE predicates cannot refer to > NEW. Basically, > for INSERT/UPDATE/DELETE, we specify predicates the same way as we do for > triggers' WHEN condition. > > As for FOR ALL, I think we will abandon it if we apply SELECT policy to > other commands, since SELECT predicate > will be the new universally applicable read policy, which makes the FOR ALL > USING clause much less useful. Of course users may need to specify separate > predicates for different commands, but I think it is fine. How often do > users want the same predicate for all the commands? I can certainly see use-cases where you'd want to apply the same policy to all new records, regardless of how they're being added, and further, the use-case where you want the same policy for records which are visible and those which are added. In fact, I'd expect that to be one of the most common use-cases as it maps directly to a set of rows which are owned by one user, where that user can see/modify/delete their own records but not impact other users. So, I don't think it would be odd at all for users to want the same predicate for all of the commands. > > This could be accomplished with "USING (bar > 1)" and "WITH CHECK (foo > > > 1)", no? > > Your sentence above that "USING and WITH CHECK are combined by AND" > > isn't correct either- they're independent and are therefore really OR'd. > > If they were AND'd then the new record would have to pass both USING and > > WITH CHECK policies. > > No, it is impossible with the current implementation. > > CREATE TABLE test { > id int, > v1 int, > v2 int > }; > > Suppose that the user wants an update policy which is OLD.v1 > 10 OR NEW.v2 > < 10. > As you suggested, we use the following policy > > CREATE update_p ON test > FOR UPDATE TO test_user > USING v1 > 10 > WITH CHECK v2 < 10; > > (1) Assume there is only one row in the table > id | v1 | v2 | > 1 | 11 | 20 | > > Now we execute UPDATE test SET v2 = 100. > this query is allowed by the policy and the only row should be updated > since v1's old value > 10, but will trigger an error because it violates > the WITH CHECK clause. In this scenario, you don't care what the value of the NEW record is, at all? As long as the old record had 'v1 > 10', then the resulting row can be anything? I have to admit, I have a hard timing seeing the usefulness of that, but it could be allowed by having a 'true' WITH CHECK policy. > (2) Again assume there is only one row in the table > id | v1 | v2 | > 1 | 9 | 20 | > > Now we execute UPDATE test SET v2 = 7. > this query is allowed by the policy and the only row should be updated > since v2's new value < 10, nothing will be updated because the only row > will be filtered out before update happens. Again, in this case, you could have a 'USING' policy which is simply 'true', if you wish to allow any row to be updated, provided the result is v2 < 10 (and a WITH CHECK clause to enforce that). > This is why I said USING and WITH CHECK are combined by AND. In order to > update an row, first the row needs to be visible, which meaning it needs to > pass the USING check, then it needs to pass the WITH CHECK. That's correct, and very simple to reason about. I really don't like the approach you're suggesting above where an 'OR' inside of such a clause could mean that users can arbitrarly change any existing row without any further check on that row and I have a hard time seeing the use-case which justifies the additional complexity and user confusion. > > Further, I'm not sure that I see how this would work in a case where you > > have the SELECT policy (which clearly could only refer to OLD) applied > > first, as you suggest? > > > We use SELECT policy to filter the table when we scan it (just like how we > use USING clause now). The predicate of UPDATE will be checked later > (probably similar to how we handle trigger's WHEN clause which can also > reference OLD and NEW). So there would also be a SELECT policy anyway, which is just like the existing UPDATE USING policy is today and what you're really asking for is the ability to have the WITH CHECK policy reference both the OLD and NEW records. I might be able to get behind supporting that, but I'm not terribly excited about it and you've not provided any real use-cases for it that I've seen, and it still doesn't really change anything regarding RETURNING any differently than the earlier suggestions did about having the SELECT policy applied to all commands. Thanks, Stephen
I really don't like the approach you're suggesting above where an 'OR' inside of
such a clause could mean that users can arbitrarly change any existing row
without any further check on that row and I have a hard time seeing the
use-case which justifies the additional complexity and user confusion.
I admit that I gave some bad examples in the previous email, and it is fair to say
this (Being able to have something like NEW.value > 10 OR OLD.id = 1) is not a advantage of what I proposed
before I can come up with any real-world examples.
So there would also be a SELECT policy anyway, which is just like the
existing UPDATE USING policy is today and what you're really asking for
is the ability to have the WITH CHECK policy reference both the OLD and
NEW records.
Yes. Then we won't need any USING clauses for UPDATE/DELETE. For UPDATE/DELETE, we only need
one predicate which can reference both OLD and NEW.
I might be able to get behind supporting that, but I'm not
terribly excited about it and you've not provided any real use-cases for
it that I've seen
I think that there are two major advantages:
1)
As many folks have pointed out in this and other threads, this will makes information leakage less likely.
Now a permissive USING clause for UPDATE/DELETE can give an attacker chance to read rows he
is not allowed to SELECT. Even without leaky functions, an attacker can easily figure out the rows by doing a
binary search with tricks like division by zero.
2)
This proposal allows a user to reference both the OLD and NEW records in the same clause. For example,
NEW.id == OLD.id , or NEW.value <= OLD.value + 10. I think this should be useful for users since they may often
need to check the new value against the old one.
it still doesn't really change anything regarding
RETURNING any differently than the earlier suggestions did about having
the SELECT policy applied to all commands.
No, it doesn't. I proposed it here because there are some related discussions (applying SELECT policy to other commands).
Thanks,
Zhaomo
On Tue, Aug 25, 2015 at 8:17 AM, Stephen Frost <sfrost@snowman.net> wrote:
Zhaomo,
* Zhaomo Yang (zmpgzm@gmail.com) wrote:
> > If no NEW or OLD is used, what happens? Or would you have
> > to always specify OLD/NEW for UPDATE, and then what about for the other
> > policies, and the FOR ALL policies?
>
> I should be clearer with references to OLD/NEW. SELECT Predicates cannot
> reference any of them.
> INSERT predicates cannot refer to OLD and DELETE predicates cannot refer to
> NEW. Basically,
> for INSERT/UPDATE/DELETE, we specify predicates the same way as we do for
> triggers' WHEN condition.
>
> As for FOR ALL, I think we will abandon it if we apply SELECT policy to
> other commands, since SELECT predicate
> will be the new universally applicable read policy, which makes the FOR ALL
> USING clause much less useful. Of course users may need to specify separate
> predicates for different commands, but I think it is fine. How often do
> users want the same predicate for all the commands?
I can certainly see use-cases where you'd want to apply the same policy
to all new records, regardless of how they're being added, and further,
the use-case where you want the same policy for records which are
visible and those which are added. In fact, I'd expect that to be one
of the most common use-cases as it maps directly to a set of rows which
are owned by one user, where that user can see/modify/delete their own
records but not impact other users.
So, I don't think it would be odd at all for users to want the same
predicate for all of the commands.In this scenario, you don't care what the value of the NEW record is, at
> > This could be accomplished with "USING (bar > 1)" and "WITH CHECK (foo >
> > 1)", no?
> > Your sentence above that "USING and WITH CHECK are combined by AND"
> > isn't correct either- they're independent and are therefore really OR'd.
> > If they were AND'd then the new record would have to pass both USING and
> > WITH CHECK policies.
>
> No, it is impossible with the current implementation.
>
> CREATE TABLE test {
> id int,
> v1 int,
> v2 int
> };
>
> Suppose that the user wants an update policy which is OLD.v1 > 10 OR NEW.v2
> < 10.
> As you suggested, we use the following policy
>
> CREATE update_p ON test
> FOR UPDATE TO test_user
> USING v1 > 10
> WITH CHECK v2 < 10;
>
> (1) Assume there is only one row in the table
> id | v1 | v2 |
> 1 | 11 | 20 |
>
> Now we execute UPDATE test SET v2 = 100.
> this query is allowed by the policy and the only row should be updated
> since v1's old value > 10, but will trigger an error because it violates
> the WITH CHECK clause.
all? As long as the old record had 'v1 > 10', then the resulting row
can be anything? I have to admit, I have a hard timing seeing the
usefulness of that, but it could be allowed by having a 'true' WITH
CHECK policy.
> (2) Again assume there is only one row in the table
> id | v1 | v2 |
> 1 | 9 | 20 |
>
> Now we execute UPDATE test SET v2 = 7.
> this query is allowed by the policy and the only row should be updated
> since v2's new value < 10, nothing will be updated because the only row
> will be filtered out before update happens.
Again, in this case, you could have a 'USING' policy which is simply
'true', if you wish to allow any row to be updated, provided the result
is v2 < 10 (and a WITH CHECK clause to enforce that).
> This is why I said USING and WITH CHECK are combined by AND. In order to
> update an row, first the row needs to be visible, which meaning it needs to
> pass the USING check, then it needs to pass the WITH CHECK.
That's correct, and very simple to reason about. I really don't like
the approach you're suggesting above where an 'OR' inside of such a
clause could mean that users can arbitrarly change any existing row
without any further check on that row and I have a hard time seeing the
use-case which justifies the additional complexity and user confusion.
> > Further, I'm not sure that I see how this would work in a case where you
> > have the SELECT policy (which clearly could only refer to OLD) applied
> > first, as you suggest?
>
>
> We use SELECT policy to filter the table when we scan it (just like how we
> use USING clause now). The predicate of UPDATE will be checked later
> (probably similar to how we handle trigger's WHEN clause which can also
> reference OLD and NEW).
So there would also be a SELECT policy anyway, which is just like the
existing UPDATE USING policy is today and what you're really asking for
is the ability to have the WITH CHECK policy reference both the OLD and
NEW records. I might be able to get behind supporting that, but I'm not
terribly excited about it and you've not provided any real use-cases for
it that I've seen, and it still doesn't really change anything regarding
RETURNING any differently than the earlier suggestions did about having
the SELECT policy applied to all commands.
Thanks,
Stephen
Zhaomo, * Zhaomo Yang (zmpgzm@gmail.com) wrote: > I admit that I gave some bad examples in the previous email, and it is fair > to say > this (Being able to have something like NEW.value > 10 OR OLD.id = 1) is > not a advantage of what I proposed > before I can come up with any real-world examples. I do believe that real-world examples would be helpful here. > > So there would also be a SELECT policy anyway, which is just like the > > existing UPDATE USING policy is today and what you're really asking for > > is the ability to have the WITH CHECK policy reference both the OLD and > > NEW records. > > Yes. Then we won't need any USING clauses for UPDATE/DELETE. For > UPDATE/DELETE, we only need > one predicate which can reference both OLD and NEW. I agree that if we force a single visibility policy for all commands then we wouldn't need the USING clauses for UPDATE and DELETE, but we would certainly need *some* policy for DELETE to prevent users from being able to delete records that they aren't supposed to be allowed to. Therefore, we'd just be replacing the USING policy with a 'WITH CHECK' policy, no? I'm not against supporting a 'WITH CHECK' policy for DELETE, as outlined nearby in the discussion with Robert, but that strikes me as a new feature rather than an issue with the current system. Removing the existing ability to control the visibility on a per-command basis is pretty clearly a reduction in the overall flexibility of the system without a clear gain to me. As a thought experiment which might prove helpful, consider if we decided to implement a single visibility policy but then support the "error-out instead" options being proposed here first. Would we then, later, be against a patch which proposed adding the flexibility to control the visibility on a per-command basis? I tend to doubt it. In hindsight, perhaps that ordering would have been better, or it would have been better to get everything in all at once, though that has drawbacks (I seriously doubt we would have made the progress we have thus far towards BDR if it had to be done all at once). > > I might be able to get behind supporting that, but I'm not > > terribly excited about it and you've not provided any real use-cases for > > it that I've seen > > I think that there are two major advantages: > > 1) > As many folks have pointed out in this and other threads, this will makes > information leakage less likely. > Now a permissive USING clause for UPDATE/DELETE can give an attacker chance > to read rows he > is not allowed to SELECT. Even without leaky functions, an attacker can > easily figure out the rows by doing a > binary search with tricks like division by zero. Using a single USING policy for all commands rather than per-command USING policies does precisely the same thing as what is being proposed here. I'm certainly all for documenting the dangers of the per-command USING policies and the risks outlined regarding RETURNING but I dislike the general notion that we have to protect users from themselves as there are use-cases where blind updates/deletes could be quite handy, as I outlined over the summer in prior discussions (queueing systems come to mind, in particular). That's using the visibility controls of RLS in a different way than users familiar with the limitations of RLS in other systems might ever consider, but that doesn't make the use-case unreasonable. > 2) > This proposal allows a user to reference both the OLD and NEW records in > the same clause. For example, > NEW.id == OLD.id , or NEW.value <= OLD.value + 10. I think this should be > useful for users since they may often > need to check the new value against the old one. As I've said up-thread, this is something which I'm not against, but it doesn't depend on #1 in any way, nor does #1 depend on #2, and so I believe they should be independently discussed and this brought up as a new feature capability rather than a concern about the existing implementation. > > it still doesn't really change anything regarding > > RETURNING any differently than the earlier suggestions did about having > > the SELECT policy applied to all commands. > > No, it doesn't. I proposed it here because there are some related > discussions (applying SELECT policy to other commands). Yet, the ability to refer to OLD from 'WITH CHECK' policies isn't particularly related to limiting the visibility policies which are supported to a single SELECT USING policy. Hopefully I've outlined why I feel that to be the case above. Thanks! Stephen
Stephen, > I agree that if we force a single visibility policy for all commands > then we wouldn't need the USING clauses for UPDATE and DELETE, but we > would certainly need *some* policy for DELETE to prevent users from > being able to delete records that they aren't supposed to be allowed to. > Therefore, we'd just be replacing the USING policy with a 'WITH CHECK' > policy, no? If we force a single visibility policy (SELECT policy), then we will need a command-specific policy for each of UPDATE/DELETE/INSERT. A command-specific policy may be a writing policy (as for INSERT), a reading policy (as for DELETE), or a hybrid policy (as for UPDATE). For DELETE we can either combine the visibility policy (SELECT policy) with the DELETE policy using AND and then scan the table, or just attach the DELETE policy to the WHERE clause after the visibility policy has been enforced. I don't see why we need to replace USING policy with a "WITH CHECK". BTW, what is the fundamental difference between a USING predicate and a WITH CHECK predicate? Is it that which phase they are applied (read or write)? Or is it that how they handle violations (nothing-happens or error-out)? > Removing the existing ability to control the visibility on a > per-command basis is pretty clearly a reduction in the overall > flexibility of the system without a clear gain to me. I think there is a clear gain: security. One interesting issue related to this discussion is that how violations are handled. Now reading violations fail silently (nothing-happens result) while writing violations cause errors (throw-error result). In the paper named "Extending Query Rewriting Techniques for Fine-Grained Access Control" [1], Rizvi et al. added row level access control to DBMSes using an interesting syntax: GRANT-WHERE. They added a WHERE predicate to the SQL GRANT statement to achieve row-level access control. Besides the interesting syntax, they brought up the two possible models of handling violations in the paper. One model is "nothing-happens" model (they call it Truman's world model) and another is "error out" model (they call it Non-Truman model). The authors discussed the pros and cons of both models: the "nothing-happens" model is more secure since it leaks less information but a user may get surprised by the results; the "error-out" model leaks information but may be more convenient when a user is debugging his queries. I curious about our community's take on this issue. Thanks, Zhaomo [1] http://avid.cs.umass.edu/courses/645/s2006/645-paper5.pdf
Zhaomo, * Zhaomo Yang (zmpgzm@gmail.com) wrote: > > I agree that if we force a single visibility policy for all commands > > then we wouldn't need the USING clauses for UPDATE and DELETE, but we > > would certainly need *some* policy for DELETE to prevent users from > > being able to delete records that they aren't supposed to be allowed to. > > Therefore, we'd just be replacing the USING policy with a 'WITH CHECK' > > policy, no? > > If we force a single visibility policy (SELECT policy), then we will > need a command-specific policy for each of UPDATE/DELETE/INSERT. A > command-specific policy may be a writing policy (as for INSERT), a > reading policy (as for DELETE), or a hybrid policy (as for UPDATE). > > For DELETE we can either combine the visibility policy (SELECT policy) > with the DELETE policy using AND and then scan the table, or just > attach the DELETE policy to the WHERE clause after the visibility > policy has been enforced. I don't see why we need to replace USING > policy with a "WITH CHECK". We are certainly interested in supporting restrictive policies, which is what you're asking for here. I don't have any problem with that and have written a fair bit of code to help it happen. It'd be great if others who are interested can help define the grammar changes necessary and perhaps even help with the code aspect of it. > BTW, what is the fundamental difference between a USING predicate and > a WITH CHECK predicate? Is it that which phase they are applied (read > or write)? Or is it that how they handle violations (nothing-happens > or error-out)? Yes, USING is a filter, WITH CHECK throws an error. > > Removing the existing ability to control the visibility on a > > per-command basis is pretty clearly a reduction in the overall > > flexibility of the system without a clear gain to me. > > I think there is a clear gain: security. I don't buy that argument. > One interesting issue related to this discussion is that how > violations are handled. Now reading violations fail silently > (nothing-happens result) while writing violations cause errors > (throw-error result). The proposal which I made, to which you are responding, was specifically to provide users with the ability to specify which kind of handling they want. I'm firmly of the opinion that there are perfectly valid and secure use-cases of both permissive and restrictive policies, for all commands. > In the paper named "Extending Query Rewriting Techniques for > Fine-Grained Access Control" [1], Rizvi et al. added row level access > control to DBMSes using an interesting syntax: GRANT-WHERE. They added > a WHERE predicate to the SQL GRANT statement > to achieve row-level access control. Besides the interesting syntax, > they brought up the two possible models of handling violations in the > paper. One model is "nothing-happens" model (they call it Truman's > world model) and another is "error out" model (they call it Non-Truman > model). The authors discussed the pros and cons of both models: the > "nothing-happens" model is more secure since it leaks less information > but a user may get surprised by the results; the "error-out" model > leaks information but may be more convenient when a user is debugging > his queries. I curious about our community's take on this issue. That grammar was considered and I recall that paper being specifically discussed, but it wasn't what we decided to go with and I don't see that changing at this point. In the end, I believe we will provide support for both models (ideally as early as 9.6, if we can begin making progress towards that goal now). Thanks! Stephen
Stephen,
It'd be great if others who are interested can help define the grammar changes necessary
and perhaps even help with the code aspect of it.
I'd like to help on both. Can you elaborate a little bit more, especially on the code aspect?
I don't buy that argument.
It is agreed that blind updates and deletes with RETURNING clause are dangerous. It is quite similar here.
Instead of using
BEGIN
UPDATE-or-DELETE-with-RETURNING
ROLLBACK
as a substitute for SELECT, a malicious user can do a binary search with some trick like divide-by-zero
to figure out rows he is not allowed to access. Of course, this is not as serious as RETURNING, but it is still quite convenient for attackers.
Thanks,
Zhaomo
Zhaomo, Just a side-note, but your mail client doesn't seem to get the quoting quite right sometimes, which can be confusing. Not sure if there's anything you can do about it but wanted to let you know in case there is. * Zhaomo Yang (zmpgzm@gmail.com) wrote: > > It'd be great if others who are interested can help define the grammar > > changes necessary > > and perhaps even help with the code aspect of it. > > I'd like to help on both. Can you elaborate a little bit more, especially > on the code aspect? There's a fair bit of information available regarding how to contribute code on the wiki; here's a few links: https://wiki.postgresql.org/wiki/Development_information https://wiki.postgresql.org/wiki/Developer_FAQ https://wiki.postgresql.org/wiki/Submitting_a_Patch Regarding this, specifically, we'd need to first decide on what the syntax/grammar should be. Right now we have, at a basic level: CREATE POLICY <name> ON <table> USING (<expression>); To define restrictive policies we'd need a new bit of syntax which says "this policy should be restrictive instead of permissive." One approach to that would be: CREATE RESTRICTIVE POLICY ... but my initial hunch is that we'd rather have something like: CREATE POLICY <name> ON <table> RESTRICTIVE USING (<expression); or something along those lines. At some point, the actual grammar (expressed in gram.y) has to be modified. One other point to be aware of is that the grammar can't be ambiguous or we can end up with shift/shift or shift/reduce conflicts when building the actual code that runs the grammar. There's a whole lot which could be explained about just what that means, but a quick summary is "avoid things like this:" Having productions which are like this: CREATE POLICY <p1> ON <table> USING (<expression>); CREATE POLICY RESTRICTIVE <p1> ON <table> USING (<expression>); as that means that we have to make RESTRICTIVE be reserved, to make sure we know if it's a keyword (as in the second example above) or actually a policy name (as in the first example above). This is a pretty gross over-simplification, and the above might even work (I've not tried it), but hopefully it gets the point across. > > I don't buy that argument. > > It is agreed that blind updates and deletes with RETURNING clause are > dangerous. It is quite similar here. Right, and we adressed the concerns with RETURNING. Regarding the non-RETURNING case, The same concerns about blind updates and deletes already exist with the GRANT permission system; it's not anything new. > Instead of using > BEGIN > UPDATE-or-DELETE-with-RETURNING > ROLLBACK > as a substitute for SELECT, a malicious user can do a binary search with > some trick like divide-by-zero > to figure out rows he is not allowed to access. Of course, this is not as > serious as RETURNING, but it is still quite convenient for attackers. This is true of the current GRANT system. Thanks! Stephen
Stephen, > Just a side-note, but your mail client doesn't seem to get the quoting > quite right sometimes, which can be confusing. Not sure if there's > anything you can do about it but wanted to let you know in case there > is. Sorry about this. From now on I'll use the plain text mode for msgs I send to the mailing list. Please let me know if this happens also in this email. > Regarding this, specifically, we'd need to first decide on what the > syntax/grammar should be. I'll think about it. Also, thanks for the pointers. > Right, and we adressed the concerns with RETURNING. Regarding the > non-RETURNING case, The same concerns about blind updates and deletes > already exist with the GRANT permission system; it's not anything new. I think they are different. In the current GRANT permission system, one can do blind updates but he cannot refer to any existing values in either the expressions or the condition if he doesn't have SELECT privilege on the table (or the columns), thus the tricks like divide-by-zero cannot be used and a malicious user cannot get information out of blind updates. Thanks, Zhaomo
* Zhaomo Yang (zmpgzm@gmail.com) wrote: > > Just a side-note, but your mail client doesn't seem to get the quoting > > quite right sometimes, which can be confusing. Not sure if there's > > anything you can do about it but wanted to let you know in case there > > is. > > Sorry about this. From now on I'll use the plain text mode for msgs I > send to the mailing list. > Please let me know if this happens also in this email. Looks like this one has all of the quoting correct- thanks! > > Regarding this, specifically, we'd need to first decide on what the > > syntax/grammar should be. > > I'll think about it. Also, thanks for the pointers. Sure, no problem. > > Right, and we adressed the concerns with RETURNING. Regarding the > > non-RETURNING case, The same concerns about blind updates and deletes > > already exist with the GRANT permission system; it's not anything new. > > I think they are different. In the current GRANT permission system, > one can do blind updates but he > cannot refer to any existing values in either the expressions or the > condition if he doesn't have > SELECT privilege on the table (or the columns), thus the tricks like > divide-by-zero cannot be used and a malicious > user cannot get information out of blind updates. Ok, I see what you're getting at with that and I believe it'll be a pretty straight-forward change, thanks to Dean's recent rework. I'll take a look at making that happens. Thanks! Stephen
Stephen, I just tried a little bit your patch for applying SELECT policies to DELETE/UPDATE. It is consistent with the GRANT system so it looks really good. I'll test it more thoroughly later. Also, I guess we don't need to worry about the syntax of "restrictive policies" you mentioned in the upthread since SELECT policies are essentially restrictive now. Since that work has already been done, I'm wondering if I can take the task of allowing policies to reference both the 'old' and 'new' versions of the row. I understand that this feature won't be considered for 9.5 but I'd like to implement it and hopefully get it incorporated into 9.6. Thanks, Zhaomo On Wed, Sep 23, 2015 at 11:54 AM, Stephen Frost <sfrost@snowman.net> wrote: > * Zhaomo Yang (zmpgzm@gmail.com) wrote: >> > Just a side-note, but your mail client doesn't seem to get the quoting >> > quite right sometimes, which can be confusing. Not sure if there's >> > anything you can do about it but wanted to let you know in case there >> > is. >> >> Sorry about this. From now on I'll use the plain text mode for msgs I >> send to the mailing list. >> Please let me know if this happens also in this email. > > Looks like this one has all of the quoting correct- thanks! > >> > Regarding this, specifically, we'd need to first decide on what the >> > syntax/grammar should be. >> >> I'll think about it. Also, thanks for the pointers. > > Sure, no problem. > >> > Right, and we adressed the concerns with RETURNING. Regarding the >> > non-RETURNING case, The same concerns about blind updates and deletes >> > already exist with the GRANT permission system; it's not anything new. >> >> I think they are different. In the current GRANT permission system, >> one can do blind updates but he >> cannot refer to any existing values in either the expressions or the >> condition if he doesn't have >> SELECT privilege on the table (or the columns), thus the tricks like >> divide-by-zero cannot be used and a malicious >> user cannot get information out of blind updates. > > Ok, I see what you're getting at with that and I believe it'll be a > pretty straight-forward change, thanks to Dean's recent rework. I'll > take a look at making that happens. > > Thanks! > > Stephen
Zhaomo, * Zhaomo Yang (zmpgzm@gmail.com) wrote: > I just tried a little bit your patch for applying SELECT policies to > DELETE/UPDATE. It is consistent with the GRANT system so it looks > really good. I'll test it more thoroughly later. Great! Glad to hear it. > Also, I guess we don't need to worry about the syntax of "restrictive > policies" you mentioned in the upthread since SELECT policies are > essentially restrictive now. They are when it comes to applying them on top of other policies to match the permissions system, but what I believe we'd like is the ability to *explicitly* make policies both restrictive and permissive. That would allow a user to create a set of permissive SELECT policies and than a set of restrictive SELECT policies, which might be much simpler to manage for their particular use-case. > Since that work has already been done, > I'm wondering if I can take the task of allowing policies to reference > both the 'old' and 'new' versions of the row. I understand that this > feature won't be considered for 9.5 but I'd like to implement it and > hopefully get it incorporated into 9.6. I'd love to see a patch for that for 9.6. Feel free to work on it and ping me with any questions you have. Once you have a patch, please make sure to add it to the appropriate commitfest (via http://commitfest.postgresql.org), so it won't be lost. Thanks! Stephen