Row-security writer-side checks proposal - Mailing list pgsql-hackers

Hi

I've been looking some more into write-side checks in row-security and
have a suggestion.

Even though write-side checks are actually fairly separate to read
checks, and can be done as another step, I'd like to think about them
before the catalog format and syntax are settled. I think we need fields
for write operations in pg_rowsecurity and the syntax to set them so
that the catalog information can be used by triggers to enforce write
checks. Even if, for the first cut, they're not supported by built-in
auto-created triggers.

Here's my proposal, let me know what you think:

SET ROW SECURITY FOR { ALL COMMANDS | {[SELECT,INSERT,UPDATE,DELETE}+}

in other words, you specify either:

SET ROW SECURITY FOR ALL COMMANDS

or a command-list like:

SET ROW SECURITY FOR INSERT OR UPDATE OR DELETE

(Intentionally the same as CREATE TRIGGER ... FOR INSERT OR UPDATE OR
DELETE ...)

The subtlety here is that the "SELECT" clause applies to the *read part*
of an UPDATE or DELETE too, just like the current implementation. That
protects us against leaks via RETURNING, and ensures that the
row-security policy is consistent. The "INSERT", "UPDATE" or "DELETE"
part of the policy would *only* be used by write checks that verify that
a new tuple being written meets the row-security criteria.

For INSERT, that's obvious: check the insert policy and see if the tuple
should be allowed; if not, raise permission denied. The SELECT predicate
doesn't matter since it's not reading from the target table (except
possibly via join/subquery, where it is already applied).

For UPDATE, we only ever try to update tuples the select policy allows
us to see. Row security already does this thanks to Kohei KaiGai's great
work. The write side check (just a trigger) only needs to make sure the
new tuple meets the UPDATE predicate.

For DELETE, the predicate controls whether the user can delete the
tuple, so it's possible to have row-security policies that let users
read but not delete some tuples.

If the catalog fields and syntax for setting them are included in the
patch the first time around then users can use that information in their
own triggers, and we can provide canned ones in the documentation if we
run out of time to write C triggers that are automatically created like
FK checks are.

That keeps the patch smaller, since it separates the write and read
row-security.

Opinions?

I'm cooking up an adjustment to Kohei KaiGai's RLS patch with this
change now. Meanwhile I'm attaching my most recent update of his patch,
which includes:


* Rebase on top of head

* Documentation updates and rewording

* Fixes some missed renaming of "rls" and variants to "rowsecurity"

* Additional regression tests demonstrating the problems with
  handling of portals (cursors and SECURITY DEFINER functions
  returning refcursor). These tests intentionally fail as their
  expected file contains what _should_ happen not what does.

* Additional regression tests demonstrating that foreign key
  enforcement is affected inconsistently by RLS, including the
  broken example with superuser I posted previously. Again
  it intentionally fails with expected containing what I think
  should happen.

* Test cases to demonstrate that RS leaks information via
  UNIQUE constraints and that this is expected.

* Documentation on use of pg_get_expr for decoding the
  rowsecurity expressions in pg_rowsecurity into readable
  SQL expressions usable via EXECUTE

The current tree is here, rebased on top of today's master:

  https://github.com/ringerc/postgres/tree/rls-9.4

(this branch is rebased regularly!)

I've attached an updated squashed patch against today's master/head for
anyone who wants to give it a go or take a look. "make check" is
supposed to fail, since what should happen isn't yet what the code
actually does.
--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services