Row Level Security Documentation
I think the biggest piece missing is something to summarize the giant
blocks of text.
Attached is a table that has commands and policy types, and a "yes" if it
applies.
--
Rod Taylor
Attachments:
create_policy.sgml.patchtext/x-patch; charset=US-ASCII; name=create_policy.sgml.patchDownload
diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml
index 3b24e5e95e..c737f9e884 100644
--- a/doc/src/sgml/ref/create_policy.sgml
+++ b/doc/src/sgml/ref/create_policy.sgml
@@ -389,6 +389,96 @@ CREATE POLICY <replaceable class="parameter">name</replaceable> ON <replaceable
</varlistentry>
</variablelist>
+
+ <table><title>Policies Applied During Statement</title>
+ <tgroup cols='8'>
+ <thead>
+ <row>
+ <entry>Policy</entry>
+ <entry><literal>SELECT</literal></entry>
+ <entry><literal>INSERT</literal></entry>
+ <entry><literal>INSERT RETURNING</literal></entry>
+ <entry><literal>UPDATE WHERE</literal></entry>
+ <entry><literal>UPDATE RETURNING</literal></entry>
+ <entry><literal>DELETE WHERE</literal></entry>
+ <entry><literal>DELETE RETURNING</literal></entry>
+ </row>
+ </thead>
+ <tbody>
+ <row>
+ <entry><literal>FOR ALL ... USING</literal></entry>
+ <entry>yes</entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ </row>
+ <row>
+ <entry><literal>FOR ALL ... WITH CHECK</literal></entry>
+ <entry></entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry></entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry><literal>FOR SELECT ... USING</literal></entry>
+ <entry>yes</entry>
+ <entry></entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ </row>
+ <row>
+ <entry><literal>FOR INSERT ... WITH CHECK</literal></entry>
+ <entry></entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry><literal>FOR UPDATE ... USING</literal></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry></entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry><literal>FOR UPDATE ... WITH CHECK</literal></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ <entry></entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry><literal>FOR DELETE ... USING</literal></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>yes</entry>
+ <entry>yes</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
</refsect2>
</refsect1>
Of course, better thoughts appear immediately after hitting the send button.
This version of the table attempts to stipulate which section of the
process the rule applies to.
On Thu, May 11, 2017 at 8:40 PM, Rod Taylor <rod.taylor@gmail.com> wrote:
I think the biggest piece missing is something to summarize the giant
blocks of text.Attached is a table that has commands and policy types, and a "yes" if it
applies.--
Rod Taylor
--
Rod Taylor
Attachments:
create_policy.sgml_v2.patchtext/x-patch; charset=US-ASCII; name=create_policy.sgml_v2.patchDownload
diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml
index 3b24e5e95e..4c997a956d 100644
--- a/doc/src/sgml/ref/create_policy.sgml
+++ b/doc/src/sgml/ref/create_policy.sgml
@@ -389,6 +389,72 @@ CREATE POLICY <replaceable class="parameter">name</replaceable> ON <replaceable
</varlistentry>
</variablelist>
+
+ <table><title>Policies Applied During Statement</title>
+ <tgroup cols='5'>
+ <thead>
+ <row>
+ <entry>Policy</entry>
+ <entry><literal>SELECT</literal></entry>
+ <entry><literal>INSERT</literal></entry>
+ <entry><literal>UPDATE</literal></entry>
+ <entry><literal>DELETE</literal></entry>
+ </row>
+ </thead>
+ <tbody>
+ <row>
+ <entry><literal>FOR ALL ... USING</literal></entry>
+ <entry><literal>WHERE</literal> clause</entry>
+ <entry><literal>RETURNING</literal> clause</entry>
+ <entry><literal>WHERE</literal> and <literal>RETURNING</literal> clause</entry>
+ <entry><literal>WHERE</literal> and <literal>RETURNING</literal> clause</entry>
+ </row>
+ <row>
+ <entry><literal>FOR ALL ... WITH CHECK</literal></entry>
+ <entry></entry>
+ <entry>new tuple</entry>
+ <entry>new tuple</entry>
+ <entry>new tuple</entry>
+ </row>
+ <row>
+ <entry><literal>FOR SELECT ... USING</literal></entry>
+ <entry><literal>WHERE</literal> clause</entry>
+ <entry><literal>RETURNING</literal> clause</entry>
+ <entry><literal>WHERE</literal> and <literal>RETURNING</literal> clause</entry>
+ <entry><literal>WHERE</literal> and <literal>RETURNING</literal> clause</entry>
+ </row>
+ <row>
+ <entry><literal>FOR INSERT ... WITH CHECK</literal></entry>
+ <entry></entry>
+ <entry>new tuple</entry>
+ <entry></entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry><literal>FOR UPDATE ... USING</literal></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry><literal>WHERE</literal> clause</entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry><literal>FOR UPDATE ... WITH CHECK</literal></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>new tuple</entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry><literal>FOR DELETE ... USING</literal></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry><literal>WHERE</literal> clause</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
</refsect2>
</refsect1>
Hello Rod,
This version of the table attempts to stipulate which section of the
process the rule applies to.
A few comments about this patch. It applies cleanly, make html is ok.
It adds a summary table which shows for each case what happens. Although
the information can be guessed/infered from the text, I think that a
summary table is a good thing, at least because it breaks an otherwise
dense presentation.
I would suggest the following changes:
The table should be referenced from the description, something like "Table
xxx summarizes the ..."
ISTM that it would be clearer to split the Policy column into "FOR xxx
..." and "USING" or "WITH CHECK", and to merge the rows which have the
same "FOR xxx ..." contents, something like:
POLICY |
---------------+------------+-----
| USING | ...
FOR ALL ... +------------+-----
| WITH CHECK | ...
---------------+------------+-----
FOR SELECT ... | USING | ...
So that it is clear that only ALL & UPDATE can get both USING & WITH
CHECK. This can be done with "morerows=1" on an entry so that it spans
more rows.
For empty cells, maybe a dash would be clearer. Not sure.
--
Fabien
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Thu, Jul 13, 2017 at 5:49 AM, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
Hello Rod,
This version of the table attempts to stipulate which section of the
process the rule applies to.
The table should be referenced from the description, something like "Table
xxx summarizes the ..."
Added the below which seemed consistent with other "see something else"
messages.
A summary of the application of policies to a command is found in <xref
linkend="SQL-CREATEPOLICY-SUMMARY">.
ISTM that it would be clearer to split the Policy column into "FOR xxx
..." and "USING" or "WITH CHECK", and to merge the rows which have the same
"FOR xxx ..." contents, something like:POLICY |
---------------+------------+-----
| USING | ...
FOR ALL ... +------------+-----
| WITH CHECK | ...
---------------+------------+-----
FOR SELECT ... | USING | ...So that it is clear that only ALL & UPDATE can get both USING & WITH
CHECK. This can be done with "morerows=1" on an entry so that it spans more
rows.
Done. I couldn't figure out a morecols=1 equivalent to keep everything
under the Policy heading without a full colspec.
For empty cells, maybe a dash would be clearer. Not sure.
Looked cluttered to me. Tried N/A first which was even worse.
--
Rod Taylor
Attachments:
create_policy.sgml_v3.patchtext/x-patch; charset=US-ASCII; name=create_policy.sgml_v3.patchDownload
diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml
index c0dfe1ea4b..52a868e65d 100644
--- a/doc/src/sgml/ref/create_policy.sgml
+++ b/doc/src/sgml/ref/create_policy.sgml
@@ -94,6 +94,11 @@ CREATE POLICY <replaceable class="parameter">name</replaceable> ON <replaceable
exist, a <quote>default deny</> policy is assumed, so that no rows will
be visible or updatable.
</para>
+
+ <para>
+ A summary of the application of policies to a command is found
+ in <xref linkend="SQL-CREATEPOLICY-SUMMARY">.
+ </para>
</refsect1>
<refsect1>
@@ -389,6 +394,80 @@ CREATE POLICY <replaceable class="parameter">name</replaceable> ON <replaceable
</varlistentry>
</variablelist>
+
+ <table id="SQL-CREATEPOLICY-SUMMARY"><title>Policies Applied During Statement</title>
+ <tgroup cols='6'>
+ <colspec colname="pol1">
+ <colspec colname="pol2">
+ <colspec colname="select">
+ <colspec colname="insert">
+ <colspec colname="update">
+ <colspec colname="delete">
+ <thead>
+ <row>
+ <entry namest="pol1" nameend="pol2">Policy</entry>
+ <entry><literal>SELECT</literal></entry>
+ <entry><literal>INSERT</literal></entry>
+ <entry><literal>UPDATE</literal></entry>
+ <entry><literal>DELETE</literal></entry>
+ </row>
+ </thead>
+ <tbody>
+ <row>
+ <entry morerows="1"><literal>FOR ALL ...</literal></entry>
+ <entry><literal>USING</literal></entry>
+ <entry><literal>WHERE</literal> clause</entry>
+ <entry><literal>RETURNING</literal> clause</entry>
+ <entry><literal>WHERE</literal> and <literal>RETURNING</literal> clause</entry>
+ <entry><literal>WHERE</literal> and <literal>RETURNING</literal> clause</entry>
+ </row>
+ <row>
+ <entry><literal>WITH CHECK</literal></entry>
+ <entry></entry>
+ <entry>new tuple</entry>
+ <entry>new tuple</entry>
+ <entry>new tuple</entry>
+ </row>
+ <row>
+ <entry namest="pol1" nameend="pol2"><literal>FOR SELECT ... USING</literal></entry>
+ <entry><literal>WHERE</literal> clause</entry>
+ <entry><literal>RETURNING</literal> clause</entry>
+ <entry><literal>WHERE</literal> and <literal>RETURNING</literal> clause</entry>
+ <entry><literal>WHERE</literal> and <literal>RETURNING</literal> clause</entry>
+ </row>
+ <row>
+ <entry namest="pol1" nameend="pol2"><literal>FOR INSERT ... WITH CHECK</literal></entry>
+ <entry></entry>
+ <entry>new tuple</entry>
+ <entry></entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry morerows="1"><literal>FOR UPDATE ...</literal></entry>
+ <entry><literal>USING</literal></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry><literal>WHERE</literal> clause</entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry><literal>WITH CHECK</literal></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry>new tuple</entry>
+ <entry></entry>
+ </row>
+ <row>
+ <entry namest="pol1" nameend="pol2"><literal>FOR DELETE ... USING</literal></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry></entry>
+ <entry><literal>WHERE</literal> clause</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
</refsect2>
</refsect1>
Hello Rod,
Patch applies cleanly, make html ok, new table looks good to me.
I've turned it "Ready for Committer".
Thanks!
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 5 August 2017 at 10:03, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
Patch applies cleanly, make html ok, new table looks good to me.
So I started looking at this patch, but before even considering the
new table proposed, I think there are multiple issues that need to be
resolved with the current docs:
Firstly, there are 4 separate places in the current CREATE POLICY docs
that say that multiple policies are combined using OR, which is of
course no longer correct, since PG10 added RESTRICTIVE policy support.
In fact, it wasn't even strictly correct in 9.5/9.6, because if
multiple different types of policy apply to a single command (e.g.
SELECT and UPDATE policies, being applied to an UPDATE command), then
they are combined using AND. Rather than trying to make this correct
in 4 different places, I think there should be a single new section of
the docs that explains how multiple policies are combined. This will
also reduce the number of places where the pre- and post-v10 docs
differ, making them easier to maintain.
In the 6th paragraph of the description section, the terminology used
is mixed up and does not match the terminology used elsewhere
("command" instead of "policy" and "policy" instead of "expression").
The description of UPDATE policies lists the commands that the policy
applies to (UPDATE and INSERT ... ON CONFLICT DO UPDATE), but fails to
mention SELECT FOR UPDATE and SELECT FOR SHARE.
The proposed patch distinguishes between UPDATE/DELETE commands that
have WHERE or RETURNING clauses, and those that don't, claiming that
the former require SELECT permissions and the latter don't. That's
based on a couple of statements from the current docs, but it's not
entirely accurate. For example, "UPDATE foo SET a=NULL WHERE true
RETURNING now()" does not require SELECT permissions despite having
both WHERE and RETURNING clauses (OK, I admit that's a rather
contrived example, but still...), and conversely (a more realistic
example) "UPDATE foo SET a=b+c" does require SELECT permissions
despite not having WHERE or RETURNING clauses. I think we need to try
to be more precise about when SELECT permissions are required.
In the notes section, there is a note about there needing to be at
least one permissive policy for restrictive policies to take effect.
That's well worth pointing out, however, I fear that this note is
buried so far down the page (amongst some other very wordy notes) that
readers will miss it. I suggest moving it to the parameters section,
where permissive and restrictive policies are first introduced,
because it's a pretty crucial fact to be aware of when using these new
types of policy.
Attached is a proposed patch to address these issues, along with a few
other minor tidy-ups.
Regards,
Dean
Attachments:
create-policy-docs.patchapplication/octet-stream; name=create-policy-docs.patchDownload
diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml
new file mode 100644
index c0dfe1e..0f833c2
--- a/doc/src/sgml/ref/create_policy.sgml
+++ b/doc/src/sgml/ref/create_policy.sgml
@@ -73,20 +73,17 @@ CREATE POLICY <replaceable class="parame
<para>
Policies can be applied for specific commands or for specific roles. The
default for newly created policies is that they apply for all commands and
- roles, unless otherwise specified. If multiple policies apply to a given
- statement, they will be combined using OR (although <literal>ON CONFLICT DO
- UPDATE</> and <literal>INSERT</> policies are not combined in this way, but
- rather enforced as noted at each stage of <literal>ON CONFLICT</> execution).
+ roles, unless otherwise specified.
</para>
<para>
- For commands that can have both <literal>USING</literal>
- and <literal>WITH CHECK</literal> policies (<literal>ALL</literal>
+ For policies that can have both <literal>USING</literal>
+ and <literal>WITH CHECK</literal> expressions (<literal>ALL</literal>
and <literal>UPDATE</literal>), if no <literal>WITH CHECK</literal>
- policy is defined, then the <literal>USING</literal> policy will be used
- both for which rows are visible (normal <literal>USING</literal> case)
- and for which rows will be allowed to be added (<literal>WITH
- CHECK</literal> case).
+ expression is defined, then the <literal>USING</literal> expression will be
+ used both to determine which rows are visible (normal
+ <literal>USING</literal> case) and which new rows will be allowed to be
+ added (<literal>WITH CHECK</literal> case).
</para>
<para>
@@ -144,6 +141,16 @@ CREATE POLICY <replaceable class="parame
which can be accessed as all restrictive policies must be passed for
each record.
</para>
+
+ <para>
+ Note that there needs to be at least one permissive policy to grant
+ access to records before restrictive policies can be usefully used to
+ reduce that access. If only restrictive policies exist, then no records
+ will be accessible. When a mix of permissive and restrictive policies
+ are present, a record is only accessible if at least one of the
+ permissive policies passes, in addition to all the restrictive
+ policies.
+ </para>
</listitem>
</varlistentry>
@@ -210,7 +217,7 @@ CREATE POLICY <replaceable class="parame
</variablelist>
- <refsect2>
+ <refsect2>
<title>Per-Command Policies</title>
<variablelist>
@@ -223,8 +230,7 @@ CREATE POLICY <replaceable class="parame
to all commands, regardless of the type of command. If an
<literal>ALL</literal> policy exists and more specific policies
exist, then both the <literal>ALL</literal> policy and the more
- specific policy (or policies) will be combined using
- OR, as usual for overlapping policies.
+ specific policy (or policies) will be applied.
Additionally, <literal>ALL</literal> policies will be applied to
both the selection side of a query and the modification side, using
the <literal>USING</literal> expression for both cases if only
@@ -293,11 +299,12 @@ CREATE POLICY <replaceable class="parame
<listitem>
<para>
Using <literal>UPDATE</literal> for a policy means that it will apply
- to <literal>UPDATE</literal> commands (or auxiliary <literal>ON
- CONFLICT DO UPDATE</literal> clauses of <literal>INSERT</literal>
- commands). Since <literal>UPDATE</literal> involves pulling an
- existing record and then making changes to some portion (but
- possibly not all) of the record, <literal>UPDATE</literal>
+ to <literal>UPDATE</literal>, <literal>SELECT FOR UPDATE</literal>
+ and <literal>SELECT FOR SHARE</literal> commands, as well as
+ auxiliary <literal>ON CONFLICT DO UPDATE</literal> clauses of
+ <literal>INSERT</literal> commands. Since <literal>UPDATE</literal>
+ involves pulling an existing record and replacing it with a new
+ modified record, <literal>UPDATE</literal>
policies accept both a <literal>USING</literal> expression and
a <literal>WITH CHECK</literal> expression.
The <literal>USING</literal> expression determines which records
@@ -307,22 +314,6 @@ CREATE POLICY <replaceable class="parame
</para>
<para>
- When an <literal>UPDATE</literal> command is used with a
- <literal>WHERE</literal> clause or a <literal>RETURNING</literal>
- clause, <literal>SELECT</literal> rights are also required on the
- relation being updated and the appropriate <literal>SELECT</literal>
- and <literal>ALL</literal> policies will be combined (using OR for any
- overlapping <literal>SELECT</literal> related policies found) with the
- <literal>USING</literal> clause of the <literal>UPDATE</literal> policy
- using AND. Therefore, in order for a user to be able to
- <literal>UPDATE</literal> specific rows, the user must have access
- to the row(s) through a <literal>SELECT</literal>
- or <literal>ALL</literal> policy and the row(s) must pass
- the <literal>UPDATE</literal> policy's <literal>USING</>
- expression.
- </para>
-
- <para>
Any rows whose updated values do not pass the
<literal>WITH CHECK</literal> expression will cause an error, and the
entire command will be aborted. If only a <literal>USING</literal>
@@ -331,21 +322,33 @@ CREATE POLICY <replaceable class="parame
</para>
<para>
- Note, however, that <literal>INSERT</literal> with <literal>ON CONFLICT
- DO UPDATE</literal> requires that an <literal>UPDATE</literal> policy
- <literal>USING</literal> expression always be enforced as a
- <literal>WITH CHECK</literal> expression. This
- <literal>UPDATE</literal> policy must always pass when the
- <literal>UPDATE</literal> path is taken. Any existing row that
- necessitates that the <literal>UPDATE</literal> path be taken must
- pass the (<literal>UPDATE</literal> or <literal>ALL</literal>)
- <literal>USING</literal> qualifications (combined using OR), which
- are always enforced as <literal>WITH CHECK</literal>
- options in this context. (The <literal>UPDATE</literal> path will
- <emphasis>never</> be silently avoided; an error will be thrown
- instead.) Finally, the final row appended to the relation must pass
- any <literal>WITH CHECK</literal> options that a conventional
- <literal>UPDATE</literal> is required to pass.
+ Typically an <literal>UPDATE</literal> command also needs to read
+ data from columns in the relation being updated (e.g., in a
+ <literal>WHERE</literal> clause or a <literal>RETURNING</literal>
+ clause, or in an expression on the right hand side of the
+ <literal>SET</literal> clause). In this case,
+ <literal>SELECT</literal> rights are also required on the relation
+ being updated, and the appropriate <literal>SELECT</literal> or
+ <literal>ALL</literal> policies will be applied in addition to
+ the <literal>UPDATE</literal> policies. Thus the user must have
+ access to the row(s) being updated through a <literal>SELECT</literal>
+ or <literal>ALL</literal> policy in addition to being granted
+ permission to update the row(s) via an <literal>UPDATE</literal>
+ or <literal>All</literal> policy.
+ </para>
+
+ <para>
+ When an <literal>INSERT</literal> command has an auxiliary
+ <literal>ON CONFLICT DO UPDATE</literal> clause, if the
+ <literal>UPDATE</literal> path is taken, the row to be updated is
+ first checked against the <literal>USING</literal> expressions of
+ any <literal>UPDATE</literal> policies, and then the new updated row
+ is checked against the <literal>WITH CHECK</literal> expressions.
+ Note, however, that unlike a standalone <literal>UPDATE</literal>
+ command, if the existing row does not pass the
+ <literal>USING</literal> expressions, an error will be thrown (the
+ <literal>UPDATE</literal> path will <emphasis>never</> be silently
+ avoided).
</para>
</listitem>
</varlistentry>
@@ -364,19 +367,18 @@ CREATE POLICY <replaceable class="parame
</para>
<para>
- When a <literal>DELETE</literal> command is used with a
- <literal>WHERE</literal> clause or a <literal>RETURNING</literal>
- clause, <literal>SELECT</literal> rights are also required on the
- relation being updated and the appropriate <literal>SELECT</literal>
- and <literal>ALL</literal> policies will be combined (using OR for any
- overlapping <literal>SELECT</literal> related policies found) with the
- <literal>USING</literal> clause of the <literal>DELETE</literal> policy
- using AND. Therefore, in order for a user to be able to
- <literal>DELETE</literal> specific rows, the user must have access
- to the row(s) through a <literal>SELECT</literal>
- or <literal>ALL</literal> policy and the row(s) must pass
- the <literal>DELETE</literal> policy's <literal>USING</>
- expression.
+ In most cases a <literal>DELETE</literal> command also needs to read
+ data from columns in the relation that it is deleting from (e.g.,
+ in a <literal>WHERE</literal> clause or a
+ <literal>RETURNING</literal> clause). In this case,
+ <literal>SELECT</literal> rights are also required on the relation,
+ and the appropriate <literal>SELECT</literal> or
+ <literal>ALL</literal> policies will be applied in addition to
+ the <literal>DELETE</literal> policies. Thus the user must have
+ access to the row(s) being deleted through a <literal>SELECT</literal>
+ or <literal>ALL</literal> policy in addition to being granted
+ permission to delete the row(s) via a <literal>DELETE</literal> or
+ <literal>All</literal> policy.
</para>
<para>
@@ -390,6 +392,76 @@ CREATE POLICY <replaceable class="parame
</variablelist>
</refsect2>
+
+ <refsect2>
+ <title>Application of Multiple Policies</title>
+
+ <para>
+ When multiple policies of different command types apply to the same command
+ (for example <literal>SELECT</literal> and <literal>UPDATE</literal>
+ policies applied to an <literal>UPDATE</literal> command), then the
+ expressions for one type of policy are combined with the expressions for
+ the other type of policy using <literal>AND</literal>. This ensures that
+ the user has both types of permissions (for example, both permission to
+ select rows from the relation and to update them).
+ </para>
+
+ <para>
+ When multiple policies of the same command type apply to the same command,
+ then all <literal>PERMISSIVE</literal> policy expressions are combined
+ using <literal>OR</literal>, all <literal>RESTRICTIVE</literal> policy
+ expressions are combined using <literal>AND</literal>, and the results are
+ combined using <literal>AND</literal>. Thus the overall result is that
+ the combined set of policies for that command type will pass if at least
+ one of the <literal>PERMISSIVE</literal> policies passes, and all of the
+ <literal>RESTRICTIVE</literal> policies pass. If there are no
+ <literal>PERMISSIVE</literal> policies, then access is denied.
+ </para>
+
+ <para>
+ Note that, for the purposes of combining multiple policies,
+ <literal>ALL</literal> policies are treated as having the same type as
+ whichever other type of policy is being applied.
+ </para>
+
+ <para>
+ For example, in an <literal>UPDATE</literal> command requiring both
+ <literal>SELECT</literal> and <literal>UPDATE</literal> permissions, if
+ there are multiple applicable policies of each type, they will be combined
+ as follows:
+
+<programlisting>
+<replaceable>expression</replaceable> from RESTRICTIVE SELECT/ALL policy 1
+AND
+<replaceable>expression</replaceable> from RESTRICTIVE SELECT/ALL policy 2
+AND
+...
+AND
+(
+ <replaceable>expression</replaceable> from PERMISSIVE SELECT/ALL policy 1
+ OR
+ <replaceable>expression</replaceable> from PERMISSIVE SELECT/ALL policy 2
+ OR
+ ...
+)
+AND
+<replaceable>expression</replaceable> from RESTRICTIVE UPDATE/ALL policy 1
+AND
+<replaceable>expression</replaceable> from RESTRICTIVE UPDATE/ALL policy 2
+AND
+...
+AND
+(
+ <replaceable>expression</replaceable> from PERMISSIVE UPDATE/ALL policy 1
+ OR
+ <replaceable>expression</replaceable> from PERMISSIVE UPDATE/ALL policy 2
+ OR
+ ...
+)
+</programlisting>
+ </para>
+
+ </refsect2>
</refsect1>
<refsect1>
@@ -419,16 +491,6 @@ CREATE POLICY <replaceable class="parame
</para>
<para>
- Note that there needs to be at least one permissive policy to grant
- access to records before restrictive policies can be usefully used to
- reduce that access. If only restrictive policies exist, then no records
- will be accessible. When a mix of permissive and restrictive policies
- are present, a record is only accessible if at least one of the
- permissive policies passes, in addition to all the restrictive
- policies.
- </para>
-
- <para>
Generally, the system will enforce filter conditions imposed using
security policies prior to qualifications that appear in user queries,
in order to prevent inadvertent exposure of the protected data to
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
On 5 August 2017 at 10:03, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
Patch applies cleanly, make html ok, new table looks good to me.
So I started looking at this patch, but before even considering the
new table proposed, I think there are multiple issues that need to be
resolved with the current docs:Firstly, there are 4 separate places in the current CREATE POLICY docs
that say that multiple policies are combined using OR, which is of
course no longer correct, since PG10 added RESTRICTIVE policy support.
Ah, indeed, you're right there, those should have been updated to
indicate that it was the default or perhaps clarify the difference, but
I agree that doing so all over the place isn't ideal.
In fact, it wasn't even strictly correct in 9.5/9.6, because if
multiple different types of policy apply to a single command (e.g.
SELECT and UPDATE policies, being applied to an UPDATE command), then
they are combined using AND. Rather than trying to make this correct
in 4 different places, I think there should be a single new section of
the docs that explains how multiple policies are combined. This will
also reduce the number of places where the pre- and post-v10 docs
differ, making them easier to maintain.
Agreed, that makes a lot of sense to me.
The proposed patch distinguishes between UPDATE/DELETE commands that
have WHERE or RETURNING clauses, and those that don't, claiming that
the former require SELECT permissions and the latter don't. That's
based on a couple of statements from the current docs, but it's not
entirely accurate. For example, "UPDATE foo SET a=NULL WHERE true
RETURNING now()" does not require SELECT permissions despite having
both WHERE and RETURNING clauses (OK, I admit that's a rather
contrived example, but still...), and conversely (a more realistic
example) "UPDATE foo SET a=b+c" does require SELECT permissions
despite not having WHERE or RETURNING clauses. I think we need to try
to be more precise about when SELECT permissions are required.
Agreed.
In the notes section, there is a note about there needing to be at
least one permissive policy for restrictive policies to take effect.
That's well worth pointing out, however, I fear that this note is
buried so far down the page (amongst some other very wordy notes) that
readers will miss it. I suggest moving it to the parameters section,
where permissive and restrictive policies are first introduced,
because it's a pretty crucial fact to be aware of when using these new
types of policy.
Ok.
Attached is a proposed patch to address these issues, along with a few
other minor tidy-ups.
I've taken a look through this and generally agree with it. I'll note
that the bits inside <literal> ... </literal> tags should be
consistently capitalized (you had one case of 'All' that I noticed) and
I wonder if it'd be better to have the "simple" description *first*
instead of later on, eg, where you have "Thus the overall result is
that" we might want to try and reword things to decribe it as "Overall,
it works thusly, ..., and the specifics are ...".
That's a relatively minor point, however, and I do feel that this patch
is a definite improvement. Were you thinking of just applying this for
v10, or back-patching all or part of it..? For my 2c, at least, I'm
pretty open to clarifying things in the back-branches (and we have
technically had restrictive policies for a while, they just required
using an extension, so even those pieces are relevant for older
versions, but might need additional caveats...).
Thanks!
Stephen
On 26 September 2017 at 00:42, Stephen Frost <sfrost@snowman.net> wrote:
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
Attached is a proposed patch...
I've taken a look through this and generally agree with it.
Thanks for looking at this.
the bits inside <literal> ... </literal> tags should be
consistently capitalized (you had one case of 'All' that I noticed)
Yes, that's a typo. Will fix.
I wonder if it'd be better to have the "simple" description *first*
instead of later on, eg, where you have "Thus the overall result is
that" we might want to try and reword things to decribe it as "Overall,
it works thusly, ..., and the specifics are ...".
OK, that makes sense. I'll try it that way round.
That's a relatively minor point, however, and I do feel that this patch
is a definite improvement. Were you thinking of just applying this for
v10, or back-patching all or part of it..?
I was planning on back-patching it to 9.5, taking out the parts
relating the restrictive policies as appropriate. Currently the 9.5
and 9.6 docs are identical, as are 10 and HEAD, and 9.5/9.6 only
differs from 10/HEAD in a couple of places where they mention
restrictive policies. IMO we should stick to that, making any
improvements available in the back-branches. I was also thinking the
same about the new summary table, although I haven't properly reviewed
that yet.
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
Dean,
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
On 26 September 2017 at 00:42, Stephen Frost <sfrost@snowman.net> wrote:
That's a relatively minor point, however, and I do feel that this patch
is a definite improvement. Were you thinking of just applying this for
v10, or back-patching all or part of it..?I was planning on back-patching it to 9.5, taking out the parts
relating the restrictive policies as appropriate. Currently the 9.5
and 9.6 docs are identical, as are 10 and HEAD, and 9.5/9.6 only
differs from 10/HEAD in a couple of places where they mention
restrictive policies. IMO we should stick to that, making any
improvements available in the back-branches. I was also thinking the
same about the new summary table, although I haven't properly reviewed
that yet.
Makes sense to me.
+1
Thanks!
Stephen
On 5 August 2017 at 10:03, Fabien COELHO <coelho@cri.ensmp.fr> wrote:
Hello Rod,
Patch applies cleanly, make html ok, new table looks good to me.
I've turned it "Ready for Committer".
I didn't really finish committing this in the last commitfest, before
getting distracted by a security issue, so returning to it now...
I like Rod's original idea of a summary table here, to save reading
through a lot of text, and I think it's handy to be able to see at a
glance which polices apply to which commands, and vice versa. However,
I'm not entirely convinced by the contents of the table, as proposed.
The meaning of the contents of the table cells isn't entirely clear.
For example, the SELECT/FOR ALL USING cell contains "WHERE clause",
which I presume means the policy expressions are added to the query's
WHERE clause. But the INSERT/FOR ALL USING cell contains "RETURNING
clause", meaning the policy is used if there is a RETURNING clause.
Then the INSERT/FOR ALL WITH CHECK cell contains "new tuple". So the
table cells seem to be providing answers to different orthogonal
questions.
I think the contents of each cell should provide the answer to a
single question, or a consistent set of questions. IMO the principal
question is "does this policy apply to this command?", although that
can be expanded to include "... and if so, which tuple is tested?" so
then the principal content of each cell would be "Existing row" or
"New row", or blank if the policy doesn't apply.
There's a minor complication that some cases like SELECT policies for
UPDATE commands don't always apply. I think that can be covered by a
suitable footnote.
The set of table columns (commands) currently only includes SELECT,
INSERT, UPDATE and DELETE. I think that should be expanded to include
SELECT FOR UPDATE/SHARE and ON CONFLICT DO UPDATE. I think it also
makes sense to include INSERT ... RETURNING as a separate command,
since it almost always requires SELECT permissions, whereas a bare
INSERT never does. I don't think it's worth including the RETURNING
variants of the other commands, since they typically require SELECT
permissions even without a RETURNING clause.
The set of table rows (policy types) currently includes FOR ALL as a
separate policy type. Whilst that might be technically correct, I
think it's more useful to think of the set of policy types as
SELECT/ALL, INSERT/ALL, UPDATE/ALL and DELETE/ALL because ALL policies
effectively just behave like one or more of the other policy types
depending on the context. Doing it that way then ties in better with
the way multiple policies are combined. For example, for an UPDATE
command, there are 2 sets of policies that get applied (combined using
AND) -- the SELECT/ALL policies and the UPDATE/ALL policies. It's not
so useful to regards ALL as a separate type, and it would be wrong to
AND together the ALL policies with the SELECT policies and the UPDATE
policies.
With the above changes, there are more command types than policy
types, so it also makes sense to swap the orientation of the table.
Here's an updated patch, with the table done that way. Comments?
Regards,
Dean
Attachments:
rls-summary-table.patchtext/x-patch; charset=US-ASCII; name=rls-summary-table.patchDownload
diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml
new file mode 100644
index 64d3a6b..b477958
--- a/doc/src/sgml/ref/create_policy.sgml
+++ b/doc/src/sgml/ref/create_policy.sgml
@@ -73,7 +73,10 @@ CREATE POLICY <replaceable class="parame
<para>
Policies can be applied for specific commands or for specific roles. The
default for newly created policies is that they apply for all commands and
- roles, unless otherwise specified.
+ roles, unless otherwise specified. Multiple policies may apply to a single
+ command; see below for more details.
+ <xref linkend="sql-createpolicy-summary"> summarizes how the different types
+ of policy apply to specific commands.
</para>
<para>
@@ -391,6 +394,105 @@ CREATE POLICY <replaceable class="parame
</varlistentry>
</variablelist>
+
+ <table id="sql-createpolicy-summary">
+ <title>Policies Applied by Command Type</title>
+ <tgroup cols="6">
+ <colspec colnum="4" colname="update-using">
+ <colspec colnum="5" colname="update-check">
+ <spanspec namest="update-using" nameend="update-check" spanname="update">
+ <thead>
+ <row>
+ <entry morerows="1">Command</entry>
+ <entry><literal>SELECT/ALL policy</literal></entry>
+ <entry><literal>INSERT/ALL policy</literal></entry>
+ <entry spanname="update"><literal>UPDATE/ALL policy</literal></entry>
+ <entry><literal>DELETE/ALL policy</literal></entry>
+ </row>
+ <row>
+ <entry><literal>USING expr</literal></entry>
+ <entry><literal>WITH CHECK expr</literal></entry>
+ <entry><literal>USING expr</literal></entry>
+ <entry><literal>WITH CHECK expr</literal></entry>
+ <entry><literal>USING expr</literal></entry>
+ </row>
+ </thead>
+ <tbody>
+ <row>
+ <entry><command>SELECT</command></entry>
+ <entry>Existing row</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ </row>
+ <row>
+ <entry><command>SELECT FOR UPDATE/SHARE</command></entry>
+ <entry>Existing row</entry>
+ <entry>—</entry>
+ <entry>Existing row</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ </row>
+ <row>
+ <entry><command>INSERT</command></entry>
+ <entry>—</entry>
+ <entry>New row</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ </row>
+ <row>
+ <entry><command>INSERT ... RETURNING</command></entry>
+ <entry>
+ New row
+ <footnote id="rls-select-priv">
+ <para>
+ If read access is required to the existing or new row (for example,
+ a <literal>WHERE</literal> or <literal>RETURNING</literal> clause
+ that refers to columns from the relation).
+ </para>
+ </footnote>
+ </entry>
+ <entry>New row</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ </row>
+ <row>
+ <entry><command>UPDATE</command></entry>
+ <entry>
+ Existing & new rows
+ <footnoteref linkend="rls-select-priv">
+ </entry>
+ <entry>—</entry>
+ <entry>Existing row</entry>
+ <entry>New row</entry>
+ <entry>—</entry>
+ </row>
+ <row>
+ <entry><command>DELETE</command></entry>
+ <entry>
+ Existing row
+ <footnoteref linkend="rls-select-priv">
+ </entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ <entry>—</entry>
+ <entry>Existing row</entry>
+ </row>
+ <row>
+ <entry><command>ON CONFLICT DO UPDATE</command></entry>
+ <entry>Existing & new rows</entry>
+ <entry>—</entry>
+ <entry>Existing row</entry>
+ <entry>New row</entry>
+ <entry>—</entry>
+ </row>
+ </tbody>
+ </tgroup>
+ </table>
+
</refsect2>
<refsect2>