From: | Adam Brightwell <adam(dot)brightwell(at)crunchydatasolutions(dot)com> |
---|---|
To: | Stephen Frost <sfrost(at)snowman(dot)net> |
Cc: | Noah Misch <noah(at)leadboat(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, Joe Conway <mail(at)joeconway(dot)com> |
Subject: | Re: row_security GUC, BYPASSRLS |
Date: | 2015-09-29 14:56:44 |
Message-ID: | CAKRt6CSt=0d2UGajDSQYPqFTtmZZWBtz+0UAb6edzDDqUQg3Bg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> I've attached a patch to implement it. It's not fully polished but it's
> sufficient for comment, I believe. Additional comments, documentation
> and regression tests are to be added, if we have agreement on the
> grammer and implementation approach.
I have given the first patch a quick review. I think I agree with the
grammar, it makes sense to me. At first I didn't really like NO
FORCE, but I couldn't think of anything better.
One comment:
if (pg_class_ownercheck(relid, user_id))
- return RLS_NONE_ENV;
+ {
+ if (relforcerowsecurity)
+ return RLS_ENABLED;
+ else
+ return RLS_NONE_ENV;
+ }
Is the 'else' even necessary in this block?
Other than that, the approach looks good to me.
The patch applied cleanly against master (758fcfd). As well
'check-world' was successful (obviously understanding that more
related tests are necessary).
> I have a hard time with this. We're not talking about the application
> logic in this case, we're talking about the guarantees which the
> database is making to the user, be it an application or an individual.
>
> I've included a patch (the second in the set attached) which adds a
> SECURITY_NOFORCE_RLS bit which has a much tighter scope- it only applies
> after the regular owner check is done. This reduces the risk of it
> being set mistakenly dramatically, I believe. Further, the cascaded
> case is correctly handled. This also needs more polish and regression
> tests, but I wanted to solicit for comment before moving forward with
> that since we don't have a consensus about if this should be done.
I'm not sure that I understand the previous concerns around this bit
well enough to speak intelligently on this point. However, with that
said, I believe the changes made by this patch make sense.
-Adam
--
Adam Brightwell - adam(dot)brightwell(at)crunchydatasolutions(dot)com
Database Engineer - www.crunchydatasolutions.com
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2015-09-29 14:57:03 | Re: PATCH: index-only scans with partial indexes |
Previous Message | Steve Crawford | 2015-09-29 14:55:26 | Re: No Issue Tracker - Say it Ain't So! |