Re: row_security GUC, BYPASSRLS - Mailing list pgsql-hackers

* Noah Misch (noah@leadboat.com) wrote:
> On Tue, Sep 22, 2015 at 10:38:53AM -0400, Stephen Frost wrote:
> > This would lead to trivial to cause (and likely hard to diagnose)
> > referential integrity data corruption issues.  I find that a very hard
> > pill to swallow in favor of a theoretical concern that new code may open
> > avenues of exploit for a new security context mode to bypass RLS when
> > doing internal referential integrity checks.  Further, it changes this
> > additional capability, which was agreed would be added to offset removal
> > of the 'row_security = force' option, from useful to downright
> > dangerous.
>
> In schema reviews, I will raise a red flag for use of this feature; the best
> designs will instead use additional roles.  I forecast that PostgreSQL would
> fare better with no owner-constrained-by-RLS capability.  Even so, others want
> it, and FORCE ROW SECURITY would deliver it with an acceptable risk profile.

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.

> "SET row_security=force" was too risky, and not in a way particular to foreign
> key constraints, because the session user chose row_security=force independent
> of object owners.  With FORCE ROW SECURITY, each table owner would make both
> decisions.  A foreign key constraint, plus a SELECT policy hiding rows from
> the table owner, plus FORCE ROW SECURITY is one example of self-contradictory
> policy design.  That example is unexceptional amidst the countless ways a
> table owner can get security policy wrong.

I agree that a table owner can get security policy wrong in any number
of ways and that having a FORCE RLS option at the table level is better
than the GUC.

> SECURITY_ROW_LEVEL_DISABLED could have been okay.  I removed an incomplete
> implementation (e.g. didn't affect CASCADE constraints).  Writing a full one
> would be a mammoth job, and for what?  Setting the wrong SELECT policies can
> disrupt _any_ application logic; no foreign keys or FORCE ROW SECURITY need be
> involved.  Protecting just foreign keys brings some value, but it will not
> materially reduce the vigilance demanded of RLS policy authors and reviewers.

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.

Thanks!

Stephen