Re: API change advice: Passing plan invalidation info from the rewriter into the planner?

* Kevin Grittner (kgrittn(at)ymail(dot)com) wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> > Any dump not run by a superuser is already in doubt, imv.  That
> > is a problem we already have which really needs to be addressed,
> > but I view that as an independent issue.
>
> I'm not seeing that.  If the user can't dump, you get an error and
> pg_dump returns something other than SUCCESS.

We've outlined an approach with RLS which would do the same.

I'm still of the opinion that, today, we have a problem that only a
superuser-run dump has any chance of success (and even if you get it
working today it'll probably break tomorrow, and you had better be
paying attention). I'd like to fix that situation, but it's an
independent effort from this. We've had issues in the past with pg_dump
creating things that can't be restored and they're certainly bugs but
trying to make that work with a regular user as a whole system backup
strategy, today, is just asking for trouble.

> > I agree with avoiding adding another superuser-only capability;
> > see the other sub-thread about making this a per-user capability.
>
> It should be possible to design something which does not have this
> risk. 

The risk that pg_dump might create a dump which can't be restored?
Agreed, and I'd love to hear your thoughts on the proposal.

> What I was saying was that what was being described at that
> point wasn't it, and IMV was not acceptable.  I think that there
> should never by any doubt that a pg_dump run which completes
> without error copied all requested tables in their entirety, not a
> subset of the rows in the tables.

pg_dump needs to be able to have an option to go either way on this
case, as I can see value in running pg_dump in "RLS-enforcing" mode, but
it could default to "error-if-RLS".

> A GUC which only caused an error on the attempt to actually read
> specific rows which the user does not have permission to see would
> leak too much information.  A GUC which caused a SELECT or COPY
> from a table to throw an error if the user was not entitled to see
> all rows in the table could work.

Right- this would be the 'DIRECT SELECT' which would allow bypassing all
RLS and therefore mean that the user is allowed to see ALL rows of a
table. That's one of the reasons why I agree with Dean's approach,
because we really need to know at the outset if the calling user is
allowed to extract all rows from a table or not- we can't go looking
through the entire table testing each row before we start running the
query.

>   Another thing which could work,
> if it can be coded, would be a GUC which would throw an error if
> the there were not quals on the query to prohibit seeing rows which
> the security conditions would prohibit, whether or not any matching
> rows actually existed. 

If I'm following you correctly, this would be an optimization that
allows avoiding RLS in the case where some information about the user
causes the overall qual to always return 'true', correct? I'd certainly
like to see what happens in that case today and agree that it'd be great
to optimize for and perhaps even allow a user for which that is true to
not need the 'DIRECT SELECT' privilege, but in practice, I don't think
it'll be possible in most cases (certainly not in the case where an
external security module is deciding the access) and the optimization
may not be worth it.

> The latter would match the behavior of
> column level security -- you get an error when trying to select a
> prohibited column even if there are no rows in the table.

Agreed, but that would be a relaxation of the proposed approach and
therefore something which could be added later, if it's deemed
worthwhile.

Thanks,

Stephen