Re: [RFC] Interface of Row Level Security

From: Florian Pflug <fgp(at)phlo(dot)org>
To: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PgHacker <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [RFC] Interface of Row Level Security
Date: 2012-05-24 17:31:37
Message-ID: C43CFF68-24F1-490E-826B-76F59626869E@phlo.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On May24, 2012, at 18:42 , Kohei KaiGai wrote:
> I'd like to summarize the current design being discussed.
>
> syntax:
> ALTER TABLE <tblname> WITH ROW LEVEL SECURITY
> ( <condition clause> ) [FOR (SELECT | UPDATE | DELETE)];
> ALTER TABLE <tblname> WITHOUT ROW LEVEL SECURITY;
>
> I tried to patch the parser/gram.y, but here was syntax conflicts
> on ADD / DROP sub-command.
> And, I noticed "ROW LEVEL SECURITY" allows to implement
> without adding new keyword, unlike "SECURITY POLICY".

Let the bike-shedding begin ;-)

ALTER TABLE … WITH sounds a bit weird. What about

ALTER TABLE <tblname> SET ROW POLICY <condition> FOR { SELECT | UPDATE | DELETE }
ALTER TABLE <tblname> RESET ROW POLICY

> As we discussed, it causes a problem with approach to append
> additional qualifiers to where clause implicitly, because it does
> not solve the matter corresponding to the order to execute
> qualifiers. So, I'm inclined to the approach to replace reference
> to tables with security policy by sub-queries with security barrier
> flag.

Since the security barrier flag carries a potentially hefty performance
penalty, I think it should be optional. Application which don't allow
SQL-level access to the database might still benefit from row-level security,
because it saves them from having to manually add the WHERE clause to every
statement, or having to wrap all their tables with views. Yet without direct
SQL-level access, the security barrier thing isn't really necessary, so
it'd be nice if they wouldn't have to pay for it. How about

ALTER TABLE … SET ROW POLICY … WITH (security_barrier)

> One thing still I'm thinking is whether the security policy should
> be provided as a function or a clause. Enough simple sql function
> is inlined at simplify_function(), so here is no meaningful difference.
> I was afraid of code complexity, but all we should do is to append
> configured clause on the where clause of sub-query inside.
>
> | ALTER TABLE <tblname> WITH ROW LEVEL SECURITY
> | ( <condition clause> ) [FOR (SELECT | UPDATE | DELETE)];
>
> So, I tried to put "condition clause" instead of a function, right now.

A single function seems much easier implementation-wise, since you wouldn't
need to store an arbitrary expression in the catalog, just an oid. It also
delegates the dependency tracking problem to the function. It also simplies
the grammar, because the "FOR … " clause cannot be mistaken to belong to
the condition clause.

> One other issue we didn't have discussed is table inheritance.
> In case when a table TBLP has a child table TBLC and only
> TBLC has its security policy, what security policy should be
> applied when we run "SELECT * FROM TBLP".
> My preference is, the security policy is only applied to scan on
> TBLC, not TBLP.

Agreed.

> It is not desirable behavior that visible tuples
> are different from way to reference a certain table.
> In addition, if and when TBLP and TBLC have their own policy
> individually, what is a desirable behavior?
> I think, the security policy of both TBLP and TBLC should be
> applied on TBLC; in other words, it applies the security policy
> of all the parent tables to scan on child table.

I think security policies should only apply to the table they're
declared for, not their child tables. Mostly because that is how
triggers operate, and security policies and triggers will often
be used together, so having their semantics regarding inheritance
be the same seems to be the least surprising option.

Also, if policies are inherited, how would you define a policy
which applies only to the parent, not to the child?

best regards,
Florian Pflug

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Sabino Mullane 2012-05-24 17:33:25 Backends stalled in 'startup' state: index corruption
Previous Message Alex Hunsaker 2012-05-24 17:31:03 Re: plperl_helpers.h fix for clang