From: | Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at> |
---|---|
To: | Christoph Heiss <christoph(dot)heiss(at)cybertec(dot)at>, pgsql-hackers(at)postgresql(dot)org |
Cc: | Hans-Jürgen Schönig <hs(at)cybertec(dot)at> |
Subject: | Re: [PATCH] Add reloption for views to enable RLS |
Date: | 2022-02-04 16:09:44 |
Message-ID: | aa49733ca567937b6b1e51951740b67f85a2c2fb.camel@cybertec.at |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, 2022-02-02 at 18:23 +0100, Christoph Heiss wrote:
> > Huh? "duff" has no permission to insert into "tab"!
> That really should not happen, thanks for finding that and helping me
> investigating on how to fix that!
>
> This is now solved by checking the security_invoker property on the view
> in rewriteTargetView().
>
> I've also added a testcase for this in v4 to catch that in future.
I tested it, and the patch works fine now.
Some little comments:
> --- a/src/backend/rewrite/rewriteHandler.c
> +++ b/src/backend/rewrite/rewriteHandler.c
> @@ -3242,9 +3243,13 @@ rewriteTargetView(Query *parsetree, Relation view)
> 0);
>
> /*
> - * Mark the new target RTE for the permissions checks that we want to
> - * enforce against the view owner, as distinct from the query caller. At
> - * the relation level, require the same INSERT/UPDATE/DELETE permissions
> + * If the view has security_invoker set, mark the new target RTE for the
> + * permissions checks that we want to enforce against the query caller, as
> + * distince from the view owner.
Typo: distince
diff --git a/src/test/regress/expected/create_view.out b/src/test/regress/expected/create_view.out
index 509e930fc7..fea893569f 100644
--- a/src/test/regress/expected/create_view.out
+++ b/src/test/regress/expected/create_view.out
@@ -261,15 +261,26 @@ CREATE VIEW mysecview3 WITH (security_barrier=false)
AS SELECT * FROM tbl1 WHERE a < 0;
CREATE VIEW mysecview4 WITH (security_barrier)
AS SELECT * FROM tbl1 WHERE a <> 0;
-CREATE VIEW mysecview5 WITH (security_barrier=100) -- Error
+CREATE VIEW mysecview5 WITH (security_invoker=true)
+ AS SELECT * FROM tbl1 WHERE a = 100;
+CREATE VIEW mysecview6 WITH (security_invoker=false)
AS SELECT * FROM tbl1 WHERE a > 100;
+CREATE VIEW mysecview7 WITH (security_invoker)
+ AS SELECT * FROM tbl1 WHERE a < 100;
+CREATE VIEW mysecview8 WITH (security_barrier=100) -- Error
+ AS SELECT * FROM tbl1 WHERE a <> 100;
ERROR: invalid value for boolean option "security_barrier": 100
-CREATE VIEW mysecview6 WITH (invalid_option) -- Error
+CREATE VIEW mysecview9 WITH (security_invoker=100) -- Error
+ AS SELECT * FROM tbl1 WHERE a = 100;
+ERROR: invalid value for boolean option "security_invoker": 100
+CREATE VIEW mysecview10 WITH (invalid_option) -- Error
I see no reasons to remove two of the existing tests.
+++ b/src/test/regress/expected/rowsecurity.out
@@ -8,9 +8,11 @@ DROP USER IF EXISTS regress_rls_alice;
DROP USER IF EXISTS regress_rls_bob;
DROP USER IF EXISTS regress_rls_carol;
DROP USER IF EXISTS regress_rls_dave;
+DROP USER IF EXISTS regress_rls_grace;
But the name has to start with "e"!
I also see no reason to split a small patch like this into three parts.
In the attached, I dealt with the above and went over the comments.
How do you like it?
Yours,
Laurenz Albe
>
Attachment | Content-Type | Size |
---|---|---|
v5-0001-Add-new-boolean-reloption-security_invoker-to-views.patch | text/x-patch | 28.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2022-02-04 16:18:43 | Re: Stats collector's idx_blks_hit value is highly misleading in practice |
Previous Message | Alvaro Herrera | 2022-02-04 15:36:19 | Re: [BUG]Update Toast data failure in logical replication |