Re: [HACKERS] Possible typo in create_policy.sgml
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: Don't forget the ALTER POLICY page. This and some of the other things being discussed on this thread ought to be copied there too. Thanks, I've fixed this also. Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: On 30 January 2015 at 03:40, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost sfr...@snowman.net wrote: A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. When a USING expression returns false for a given row, that row is not visible to the user. When a WITH CHECK expression returns false for a row which is to be added, an error occurs. Yeah, that's not bad. I think it's an improvement, in fact. Yes I like that too. My main concern was that we should be describing policies in terms of permitting access to the table, not limiting access, because of the default-deny policy, and this new text clears that up. Great, thanks, pushed. One additional quibble -- it's misleading to say expression returns false here (and later in the check_expression parameter description) because if the expression returns null, that's also a failure. So it ought to be false or null, but perhaps it could just be described in terms of rows matching the expression, with a separate note to say that a row only matches a policy expression if that expression returns true, not false or null. Good point, I've made a few minor changes to address that also, please let me know if you see any issus. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
On 30 January 2015 at 03:40, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost sfr...@snowman.net wrote: A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. When a USING expression returns false for a given row, that row is not visible to the user. When a WITH CHECK expression returns false for a row which is to be added, an error occurs. Yeah, that's not bad. I think it's an improvement, in fact. Yes I like that too. My main concern was that we should be describing policies in terms of permitting access to the table, not limiting access, because of the default-deny policy, and this new text clears that up. One additional quibble -- it's misleading to say expression returns false here (and later in the check_expression parameter description) because if the expression returns null, that's also a failure. So it ought to be false or null, but perhaps it could just be described in terms of rows matching the expression, with a separate note to say that a row only matches a policy expression if that expression returns true, not false or null. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
On 29 January 2015 at 04:00, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Wed, Jan 7, 2015 at 3:06 PM, Stephen Frost sfr...@snowman.net wrote: If I'm following correctly, Peter's specifically talking about: [ USING ( replaceable class=parameterexpression/replaceable ) ] [ WITH CHECK ( replaceable class=parametercheck_expression/replaceable ) ] Where the USING parameter is 'expression' but the WITH CHECK parameter is 'check_expression'. He makes a good point, I believe, as expression is overly generic. I don't like the idea of using barrier_expression though as that ends up introducing a new term- how about using_expression? I've gone ahead and made this minor change. Don't forget the ALTER POLICY page. This and some of the other things being discussed on this thread ought to be copied there too. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
On Wed, Jan 28, 2015 at 10:45 PM, Stephen Frost sfr...@snowman.net wrote: I agree, especially after going back and re-reading this while fixing the issue mentioned earlier by Peter (which was an orthogonal complaint about the shadowing of WITH CHECK by USING, if WITH CHECK isn't specified). We really need a paragraph on USING policies and another on WITH CHECK policies. How about a reword along these lines: When row level security is enabled on a table, all access to that table by users other than the owner or a superuser must be through a policy. This requirement applies to both selecting out existing rows from the table and to adding rows to the table (through either INSERT or UPDATE). Granting access to existing rows in a table is done by specifying a USING expression which will be added to queries which reference the table. Every row in the table which a USING expression returns true will be visible. Granting access to add rows to a table is done by specifying a WITH CHECK expressison. A WITH CHECK expression must return true for every row being added to the table or an error will be returned and the command will be aborted. I hate to be a critic, but this existing sentence in the documentation seems to explain nearly all of the above: A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows in a table to those rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. The only thing I can see we might need to add is a sentence that says something like If the USING clause returns false for a particular row, that row will not be visible to the user; if a WITH CHECK expression does not return true, an error occurs. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
* Dean Rasheed (dean.a.rash...@gmail.com) wrote: On 29 January 2015 at 04:00, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: On Wed, Jan 7, 2015 at 3:06 PM, Stephen Frost sfr...@snowman.net wrote: If I'm following correctly, Peter's specifically talking about: [ USING ( replaceable class=parameterexpression/replaceable ) ] [ WITH CHECK ( replaceable class=parametercheck_expression/replaceable ) ] Where the USING parameter is 'expression' but the WITH CHECK parameter is 'check_expression'. He makes a good point, I believe, as expression is overly generic. I don't like the idea of using barrier_expression though as that ends up introducing a new term- how about using_expression? I've gone ahead and made this minor change. Don't forget the ALTER POLICY page. This and some of the other things being discussed on this thread ought to be copied there too. Ah, yeah, good point, will address soon. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
* Robert Haas (robertmh...@gmail.com) wrote: On Wed, Jan 28, 2015 at 10:45 PM, Stephen Frost sfr...@snowman.net wrote: I agree, especially after going back and re-reading this while fixing the issue mentioned earlier by Peter (which was an orthogonal complaint about the shadowing of WITH CHECK by USING, if WITH CHECK isn't specified). We really need a paragraph on USING policies and another on WITH CHECK policies. How about a reword along these lines: When row level security is enabled on a table, all access to that table by users other than the owner or a superuser must be through a policy. This requirement applies to both selecting out existing rows from the table and to adding rows to the table (through either INSERT or UPDATE). Granting access to existing rows in a table is done by specifying a USING expression which will be added to queries which reference the table. Every row in the table which a USING expression returns true will be visible. Granting access to add rows to a table is done by specifying a WITH CHECK expressison. A WITH CHECK expression must return true for every row being added to the table or an error will be returned and the command will be aborted. I hate to be a critic, but this existing sentence in the documentation seems to explain nearly all of the above: A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows in a table to those rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. The only thing I can see we might need to add is a sentence that says something like If the USING clause returns false for a particular row, that row will not be visible to the user; if a WITH CHECK expression does not return true, an error occurs. Well, I agree (I suppose perhaps that I have to, since I'm pretty sure I wrote that.. :D), but what Dean was suggesting was a reword that approached it from the other direction- that is, talk about policies as granting access, instead of limiting it. I thought that was an excellent approach which will hopefully reduce confusion. To that end, how about this reword: - A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. When a USING expression returns false for a given row, that row is not visible to the user. When a WITH CHECK expression returns false for a row which is to be added, an error occurs. - This would need to be after we talk about row level security being enabled for a table and the default-deny policy or it might not make sense, but otherwise I'm thinking it works. Thoughts? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
On Thu, Jan 29, 2015 at 9:04 PM, Stephen Frost sfr...@snowman.net wrote: A policy grants the ability to SELECT, INSERT, UPDATE, or DELETE rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. When a USING expression returns false for a given row, that row is not visible to the user. When a WITH CHECK expression returns false for a row which is to be added, an error occurs. Yeah, that's not bad. I think it's an improvement, in fact. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
* Robert Haas (robertmh...@gmail.com) wrote: On Fri, Jan 9, 2015 at 3:46 PM, Stephen Frost sfr...@snowman.net wrote: A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows in a table that has row level security enabled. Access to existing table rows is granted if they match a policy expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK. For policy expressions specified via USING which grant access to existing rows, the system will generally test the policy expressions prior to any qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. I think that sticking while new rows that would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK into the middle of this is horribly confusing, as it's a completely separate mechanism from the rest of what's being discussed here. I think there needs to be some initial language that clarifies that USING expressions apply to old rows and WITH CHECK expressions to new rows, and then you can go into more detail. But mentioning WITH CHECK parenthetically in the middle of the rest of this I think will not lead to clarity. I agree, especially after going back and re-reading this while fixing the issue mentioned earlier by Peter (which was an orthogonal complaint about the shadowing of WITH CHECK by USING, if WITH CHECK isn't specified). We really need a paragraph on USING policies and another on WITH CHECK policies. How about a reword along these lines: When row level security is enabled on a table, all access to that table by users other than the owner or a superuser must be through a policy. This requirement applies to both selecting out existing rows from the table and to adding rows to the table (through either INSERT or UPDATE). Granting access to existing rows in a table is done by specifying a USING expression which will be added to queries which reference the table. Every row in the table which a USING expression returns true will be visible. Granting access to add rows to a table is done by specifying a WITH CHECK expressison. A WITH CHECK expression must return true for every row being added to the table or an error will be returned and the command will be aborted. For policy expressions specified via USING which grant access to existing rows, the system will generally test the policy expressions prior to any qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. Thoughts? Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
* Robert Haas (robertmh...@gmail.com) wrote: On Wed, Jan 7, 2015 at 3:06 PM, Stephen Frost sfr...@snowman.net wrote: If I'm following correctly, Peter's specifically talking about: [ USING ( replaceable class=parameterexpression/replaceable ) ] [ WITH CHECK ( replaceable class=parametercheck_expression/replaceable ) ] Where the USING parameter is 'expression' but the WITH CHECK parameter is 'check_expression'. He makes a good point, I believe, as expression is overly generic. I don't like the idea of using barrier_expression though as that ends up introducing a new term- how about using_expression? Oh. Well, I guess we could change that. I don't think it's a problem, myself. I thought he was talking about something in the SGML markup. I agree that it's not a big deal, but I agree with Peter that it's worthwhile to clarify, especially since this will be seen in psql's \h w/o the rest of the context of the CREATE POLICY documentation. I've gone ahead and made this minor change. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: [There's also a typo further down -- filter out the records which are visible, should be not visible] I agree, that's not really worded quite right. I've reworded this along the lines of what you suggested (though not exactly- if you get a chance to review it, that'd be great) and pushed it, so we don't forget to do so later. What do you think of the attached rewording? Regarding the larger/earlier paragraph, Would be great if you could comment on my latest suggestion. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
Peter, * Peter Geoghegan (p...@heroku.com) wrote: I also don't see this behavior documented (this is from process_policies()): [...] But is that really the right place for it? Does it not equally well apply to FOR UPDATE policies, that can on their own have both barriers quals and WITH CHECK options? Basically, that seems to me like a *generic* property of policies (it's a generic thing that the WITH CHECK options/expressions shadow the USING security barrier quals as check options), and so should be documented as such. Thanks, you're right, the documentation there can be improved. I've pushed up a change to clarify that the USING quals will be used for the WITH CHECK case if no WITH CHECK quals are specified. Hopefully that's clear now, but please let me know if you feel further improvements to this would help. I do think we will be making additional changes in this area before too long, but good to get it cleaned up now anyway, so we don't forget to do so later. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
On Fri, Jan 9, 2015 at 3:46 PM, Stephen Frost sfr...@snowman.net wrote: A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows in a table that has row level security enabled. Access to existing table rows is granted if they match a policy expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK. For policy expressions specified via USING which grant access to existing rows, the system will generally test the policy expressions prior to any qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. I think that sticking while new rows that would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK into the middle of this is horribly confusing, as it's a completely separate mechanism from the rest of what's being discussed here. I think there needs to be some initial language that clarifies that USING expressions apply to old rows and WITH CHECK expressions to new rows, and then you can go into more detail. But mentioning WITH CHECK parenthetically in the middle of the rest of this I think will not lead to clarity. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
On 9 January 2015 at 20:46, Stephen Frost sfr...@snowman.net wrote: I'd suggest we further clarify with: The commandCREATE POLICY/command command defines a new policy for a table. Note that row level security must also be enabled on the table using commandALTER TABLE/command in order for created policies to be applied. Once row level security has been enabled, a default-deny policy is used and no rows in the table are visible, except to the table owner or superuser, unless permitted by a specific policy. A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows in a table that has row level security enabled. Access to existing table rows is granted if they match a policy expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK. For policy expressions specified via USING which grant access to existing rows, the system will generally test the policy expressions prior to any qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. Looks good to me. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
On 8 January 2015 at 18:57, Stephen Frost sfr...@snowman.net wrote: What do you think of the attached rewording? Rewording it this way is a great idea. Hopefully that will help address the confusion which we've seen. The only comment I have offhand is: should we should add a sentence to this paragraph about the default-deny policy? Yes, good idea, although I think perhaps that sentence should be added to the preceding paragraph, after noting that RLS has to be enabled on the table for the policies to be applied: The commandCREATE POLICY/command command defines a new policy for a table. Note that row level security must also be enabled on the table using commandALTER TABLE/command in order for created policies to be applied. Once row level security has been enabled, a default-deny policy is used and no rows in the table are visible unless permitted by a specific policy. A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows in a table that has row level security enabled. Access to existing table rows is granted if they match a policy expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK. For policy expressions specified via USING which grant access to existing rows, the system will generally test the policy expressions prior to any qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. Also, perhaps the ALTER TABLE in the first paragraph should be turned into a link. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: On 8 January 2015 at 18:57, Stephen Frost sfr...@snowman.net wrote: What do you think of the attached rewording? Rewording it this way is a great idea. Hopefully that will help address the confusion which we've seen. The only comment I have offhand is: should we should add a sentence to this paragraph about the default-deny policy? Yes, good idea, although I think perhaps that sentence should be added to the preceding paragraph, after noting that RLS has to be enabled on the table for the policies to be applied: I'm a bit on the fence about these ending up as different paragraphs then, but ignoring that for the moment, I'd suggest we further clarify with: The commandCREATE POLICY/command command defines a new policy for a table. Note that row level security must also be enabled on the table using commandALTER TABLE/command in order for created policies to be applied. Once row level security has been enabled, a default-deny policy is used and no rows in the table are visible, except to the table owner or superuser, unless permitted by a specific policy. A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows in a table that has row level security enabled. Access to existing table rows is granted if they match a policy expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against policy expressions specified via WITH CHECK. For policy expressions specified via USING which grant access to existing rows, the system will generally test the policy expressions prior to any qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. Also, perhaps the ALTER TABLE in the first paragraph should be turned into a link. Ah, yes, agreed. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
On 8 January 2015 at 08:30, Dean Rasheed dean.a.rash...@gmail.com wrote: I have a wider concern about the wording on this page - both the rewritten paragraph and elsewhere talk about policies in terms of limiting access to or filtering out rows. However, since policy expressions are OR'ed together and there is a default-deny policy when RLS is enabled, I think it should be talking about policies in terms of permitting access to tables that have row security enabled. [There's also a typo further down -- filter out the records which are visible, should be not visible] What do you think of the attached rewording? Regards, Dean diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml new file mode 100644 index 8ef8556..066aa76 *** a/doc/src/sgml/ref/create_policy.sgml --- b/doc/src/sgml/ref/create_policy.sgml *** CREATE POLICY replaceable class=parame *** 39,56 /para para !A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows !in a table to those rows which match the relevant policy expression. !Existing table rows are checked against the expression specified via !USING, while new rows that would be created via INSERT or UPDATE are !checked against the expression specified via WITH CHECK. Generally, !the system will enforce filter conditions imposed using security !policies prior to qualifications that appear in the query itself, in !order to the prevent the inadvertent exposure of the protected data to !user-defined functions which might not be trustworthy. However, !functions and operators marked by the system (or the system !administrator) as LEAKPROOF may be evaluated before policy !expressions, as they are assumed to be trustworthy. /para para --- 39,56 /para para !A policy permits SELECT, INSERT, UPDATE or DELETE commands to access rows !in a table that has row level security enabled. Access to existing table !rows is granted if they match a policy expression specified via USING, !while new rows that would be created via INSERT or UPDATE are checked !against policy expressions specified via WITH CHECK. For policy !expressions specified via USING which grant access to existing rows, the !system will generally test the policy expressions prior to any !qualifications that appear in the query itself, in order to the prevent the !inadvertent exposure of the protected data to user-defined functions which !might not be trustworthy. However, functions and operators marked by the !system (or the system administrator) as LEAKPROOF may be evaluated before !policy expressions, as they are assumed to be trustworthy. /para para *** CREATE POLICY replaceable class=parame *** 154,160 Any acronymSQL/acronym conditional expression (returning typeboolean/type). The conditional expression cannot contain any aggregate or window functions. This expression will be added ! to queries to filter out the records which are visible to the query. /para /listitem /varlistentry --- 154,161 Any acronymSQL/acronym conditional expression (returning typeboolean/type). The conditional expression cannot contain any aggregate or window functions. This expression will be added ! to queries that refer to the table if row level security is enabled, ! and will allow access to rows matching the expression. /para /listitem /varlistentry *** CREATE POLICY replaceable class=parame *** 164,174 listitem para Any acronymSQL/acronym conditional expression (returning ! typeboolean/type). The condition expression cannot contain ! any aggregate or window functions. This expression will be added ! to queries which are attempting to add records to the table as ! with-check options, and an error will be thrown if this condition ! returns false for any records being added. /para /listitem /varlistentry --- 165,176 listitem para Any acronymSQL/acronym conditional expression (returning ! typeboolean/type). The conditional expression cannot contain ! any aggregate or window functions. This expression will be used in ! commandINSERT/command and commandUPDATE/command queries on ! the table if row level security is enabled, and an error will be thrown ! if the expression evaluates to false for any of the new records added or ! updated. /para /listitem /varlistentry -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
On 6 January 2015 at 19:25, Stephen Frost sfr...@snowman.net wrote: Robert, Amit, * Robert Haas (robertmh...@gmail.com) wrote: I don't think that's a typo, although it's not particularly well-worded IMHO. I might rewrite the whole paragraph like this: A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows in a table to those rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. Generally, the system will enforce filter conditions imposed using security policies prior to qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. Looks reasonable to me. Amit, does this read better for you? If so, I can handle making the change to the docs. I have a wider concern about the wording on this page - both the rewritten paragraph and elsewhere talk about policies in terms of limiting access to or filtering out rows. However, since policy expressions are OR'ed together and there is a default-deny policy when RLS is enabled, I think it should be talking about policies in terms of permitting access to tables that have row security enabled. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
Dean, * Dean Rasheed (dean.a.rash...@gmail.com) wrote: [There's also a typo further down -- filter out the records which are visible, should be not visible] What do you think of the attached rewording? Rewording it this way is a great idea. Hopefully that will help address the confusion which we've seen. The only comment I have offhand is: should we should add a sentence to this paragraph about the default-deny policy? I feel like that would help explain why the policies are allowing access to rows. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
* Robert Haas (robertmh...@gmail.com) wrote: On Tue, Jan 6, 2015 at 4:07 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Jan 6, 2015 at 1:03 PM, Robert Haas robertmh...@gmail.com wrote: I thought my rewrite clarified this distinction pretty well. Maybe I'm wrong? We're talking about the same paragraph. Sorry, I didn't express myself clearly. I think that you did get it right, but I would like to see that distinction also reflected in the actual sgml PARAMETER class tag. expression is way too generic here. I'm happy to see us change that if it makes sense, but I'm not sure what that actually does. If I'm following correctly, Peter's specifically talking about: [ USING ( replaceable class=parameterexpression/replaceable ) ] [ WITH CHECK ( replaceable class=parametercheck_expression/replaceable ) ] Where the USING parameter is 'expression' but the WITH CHECK parameter is 'check_expression'. He makes a good point, I believe, as expression is overly generic. I don't like the idea of using barrier_expression though as that ends up introducing a new term- how about using_expression? This would need to be reflected below in the documentation which talks about expression as the parameter to USING, but I can certainly handle cleaning that up. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
On Tue, Jan 6, 2015 at 4:07 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Jan 6, 2015 at 1:03 PM, Robert Haas robertmh...@gmail.com wrote: I thought my rewrite clarified this distinction pretty well. Maybe I'm wrong? We're talking about the same paragraph. Sorry, I didn't express myself clearly. I think that you did get it right, but I would like to see that distinction also reflected in the actual sgml PARAMETER class tag. expression is way too generic here. I'm happy to see us change that if it makes sense, but I'm not sure what that actually does. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
* Peter Geoghegan (p...@heroku.com) wrote: I also don't see this behavior documented (this is from process_policies()): /* * If we end up with only USING quals, then use those as * WITH CHECK quals also. */ if (with_check_quals == NIL) with_check_quals = copyObject(quals); Now, I do see a reference to it under Per-Command policies - ALL. It says: If an INSERT or UPDATE command attempts to add rows to the table which do not pass the ALL WITH CHECK (or USING, if no WITH CHECK expression is defined) expression, the command will error. But is that really the right place for it? Does it not equally well apply to FOR UPDATE policies, that can on their own have both barriers quals and WITH CHECK options? Basically, that seems to me like a *generic* property of policies (it's a generic thing that the WITH CHECK options/expressions shadow the USING security barrier quals as check options), and so should be documented as such. Ah, yes, good point, I can add more documentation around that. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
On Wed, Jan 7, 2015 at 3:06 PM, Stephen Frost sfr...@snowman.net wrote: If I'm following correctly, Peter's specifically talking about: [ USING ( replaceable class=parameterexpression/replaceable ) ] [ WITH CHECK ( replaceable class=parametercheck_expression/replaceable ) ] Where the USING parameter is 'expression' but the WITH CHECK parameter is 'check_expression'. He makes a good point, I believe, as expression is overly generic. I don't like the idea of using barrier_expression though as that ends up introducing a new term- how about using_expression? Oh. Well, I guess we could change that. I don't think it's a problem, myself. I thought he was talking about something in the SGML markup. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
On Tue, Jan 6, 2015 at 11:25 AM, Stephen Frost sfr...@snowman.net wrote: Looks reasonable to me. Amit, does this read better for you? If so, I can handle making the change to the docs. The docs also prominently say: The security-barrier qualifications will always be evaluated prior to any user-defined functions or user-provided WHERE clauses, while the with-check expression will be evaluated against the rows which are going to be added to the table. By adding policies to a table, a user can limit the rows which a given user can select, insert, update, or delete. This capability is also known as Row Level Security or RLS. I would prefer it if it was clearer based on the syntax description which qual is which. The security barrier qual expression should have an identifier/name in the syntax description that is more suggestive of security barrier qual, emphasizing its distinctness from check_expression. For example, I think barrier_expression would be clearer. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
Robert, Amit, * Robert Haas (robertmh...@gmail.com) wrote: I don't think that's a typo, although it's not particularly well-worded IMHO. I might rewrite the whole paragraph like this: A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows in a table to those rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. Generally, the system will enforce filter conditions imposed using security policies prior to qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. Looks reasonable to me. Amit, does this read better for you? If so, I can handle making the change to the docs. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Possible typo in create_policy.sgml
On Tue, Jan 6, 2015 at 1:03 PM, Robert Haas robertmh...@gmail.com wrote: I thought my rewrite clarified this distinction pretty well. Maybe I'm wrong? We're talking about the same paragraph. Sorry, I didn't express myself clearly. I think that you did get it right, but I would like to see that distinction also reflected in the actual sgml PARAMETER class tag. expression is way too generic here. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
I also don't see this behavior documented (this is from process_policies()): /* * If we end up with only USING quals, then use those as * WITH CHECK quals also. */ if (with_check_quals == NIL) with_check_quals = copyObject(quals); Now, I do see a reference to it under Per-Command policies - ALL. It says: If an INSERT or UPDATE command attempts to add rows to the table which do not pass the ALL WITH CHECK (or USING, if no WITH CHECK expression is defined) expression, the command will error. But is that really the right place for it? Does it not equally well apply to FOR UPDATE policies, that can on their own have both barriers quals and WITH CHECK options? Basically, that seems to me like a *generic* property of policies (it's a generic thing that the WITH CHECK options/expressions shadow the USING security barrier quals as check options), and so should be documented as such. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
On Tue, Jan 6, 2015 at 2:48 PM, Peter Geoghegan p...@heroku.com wrote: On Tue, Jan 6, 2015 at 11:25 AM, Stephen Frost sfr...@snowman.net wrote: Looks reasonable to me. Amit, does this read better for you? If so, I can handle making the change to the docs. The docs also prominently say: The security-barrier qualifications will always be evaluated prior to any user-defined functions or user-provided WHERE clauses, while the with-check expression will be evaluated against the rows which are going to be added to the table. By adding policies to a table, a user can limit the rows which a given user can select, insert, update, or delete. This capability is also known as Row Level Security or RLS. I would prefer it if it was clearer based on the syntax description which qual is which. The security barrier qual expression should have an identifier/name in the syntax description that is more suggestive of security barrier qual, emphasizing its distinctness from check_expression. For example, I think barrier_expression would be clearer. I thought my rewrite clarified this distinction pretty well. Maybe I'm wrong? We're talking about the same paragraph. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Possible typo in create_policy.sgml
On 07-01-2015 AM 04:25, Stephen Frost wrote: Robert, Amit, * Robert Haas (robertmh...@gmail.com) wrote: I don't think that's a typo, although it's not particularly well-worded IMHO. I might rewrite the whole paragraph like this: A policy limits the ability to SELECT, INSERT, UPDATE, or DELETE rows in a table to those rows which match the relevant policy expression. Existing table rows are checked against the expression specified via USING, while new rows that would be created via INSERT or UPDATE are checked against the expression specified via WITH CHECK. Generally, the system will enforce filter conditions imposed using security policies prior to qualifications that appear in the query itself, in order to the prevent the inadvertent exposure of the protected data to user-defined functions which might not be trustworthy. However, functions and operators marked by the system (or the system administrator) as LEAKPROOF may be evaluated before policy expressions, as they are assumed to be trustworthy. Looks reasonable to me. Amit, does this read better for you? If so, I can handle making the change to the docs. Yes, it looks reasonable to me to. Thanks, Amit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Possible typo in create_policy.sgml
Hi, Following is perhaps a typo: - qualifications of queries which are run against the table the policy is on, + qualifications of queries which are run against the table if the policy is on, Attached fixes it if so. Thanks, Amit diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml index 4c8c001..c28ba2c 100644 --- a/doc/src/sgml/ref/create_policy.sgml +++ b/doc/src/sgml/ref/create_policy.sgml @@ -40,7 +40,7 @@ CREATE POLICY replaceable class=parametername/replaceable ON replaceable para A policy is an expression which is added to the security-barrier - qualifications of queries which are run against the table the policy is on, + qualifications of queries which are run against the table if the policy is on, or an expression which is added to the with-check options for a table and which is applied to rows which would be added to the table. The security-barrier qualifications will always be evaluated prior to any -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
