Using views for row-level access control is leaky

In chapter "36.4 Rules and Privileges" we show an example of using a
view to expose part of a table to other users, keeping other rows private:

> For example: A user has a list of phone numbers where some of them are
private, the others are of interest for the secretary of the office. He
can construct the following:
>
> CREATE TABLE phone_data (person text, phone text, private boolean);
> CREATE VIEW phone_number AS
> SELECT person, phone FROM phone_data WHERE NOT private;
> GRANT SELECT ON phone_number TO secretary;

While it works for this example, if the WHERE clause in the view is more
complex, it is possible for the secretary to circumvent the protection
by filtering rows in a function used in the WHERE clause. If the
function has a lower cost than the restriction in the view, it will be
executed first and will see all the rows, even though they won't be
present in the final result set.

For example:

CREATE TABLE phone_data (person text, phone text);
CREATE VIEW phone_number AS
SELECT person, phone FROM phone_data WHERE phone NOT LIKE '6%';
GRANT SELECT ON phone_number TO secretary;

-- secretary should only see the first row
INSERT INTO phone_data VALUES ('public person', '12345');
INSERT INTO phone_data VALUES ('secret person', '67890');

\c - secretary

CREATE OR REPLACE FUNCTION expose_person (person text, phone text)
RETURNS bool AS $$
begin
RAISE NOTICE 'person: % number: %', person, phone;
RETURN true;
END; $$ LANGUAGE plpgsql COST 0.000001;

postgres=> SELECT * FROM phone_number WHERE expose_person(person, phone);
NOTICE: person: public person number: 12345
NOTICE: person: secret person number: 67890
person | phone
---------------+-------
public person | 12345
(1 row)

Using views for access control like this is what we've been suggesting
to implement row-level access control for a long time. For example, Veil
depends heavily on this.

A related problem is that EXPLAIN ANALYZE too can reveal information
about the underlying tables that the view doesn't reveal, even if
there's no functions involved.

I posted this to security(at)postgresql(dot)org first, and the conclusion was
to take the discussion to pgsql-hackers since there's no easy, robust
and back-patchable solution in sight. Options discussed include:

1. Change the planner so that conditions (and join!) in the view are
always enforced first, before executing any quals from the user-supplied
query. Unfortunately that would have a catastrophic effect on performance.

2. As an optimization, we could keep the current behavior if the user
has access to all the underlying tables anyway, but that's nontrivial
because permission checks are supposed to be executed at runtime, not
plan time.

3. Label every function as safe or unsafe, depending on whether it can
leak information about the arguments. Classifying functions correctly
can be a bit tricky; e.g functions that throw an error on some input
values could be exploited. And it's not clear how a user could label
user-defined functions in a secure way. We'd still need the
infrastructure of point 1 to delay evaluation of non-safe quals, so this
is really just another optimization of it.

4. Make the behavior user-controllable, something along the lines of
"CREATE RESTRICTED VIEW ...", to avoid the performance impact when views
are not used for access control.

Thoughts?

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas wrote:
> CREATE VIEW phone_number AS
> SELECT person, phone FROM phone_data WHERE phone NOT LIKE '6%';

> CREATE OR REPLACE FUNCTION expose_person (person text, phone text)
> RETURNS bool AS $$
> begin
> RAISE NOTICE 'person: % number: %', person, phone;
> RETURN true;
> END; $$ LANGUAGE plpgsql COST 0.000001;
>
> postgres=> SELECT * FROM phone_number WHERE expose_person(person, phone);
> NOTICE: person: public person number: 12345
> NOTICE: person: secret person number: 67890
> person | phone
> ---------------+-------
> public person | 12345

Ouch!

> 1. Change the planner so that conditions (and join!) in the view are
> always enforced first, before executing any quals from the user-supplied
> query. Unfortunately that would have a catastrophic effect on performance.

I have the horrible feeling that you're going to end up doing this
(possibly in conjunction with #4). Once you've executed a user-defined
function on a "hidden" row I think the game is lost. That might even
apply to non-trivial expressions too.

> 2. As an optimization, we could keep the current behavior if the user
> has access to all the underlying tables anyway, but that's nontrivial
> because permission checks are supposed to be executed at runtime, not
> plan time.
>
> 3. Label every function as safe or unsafe, depending on whether it can
> leak information about the arguments. Classifying functions correctly
> can be a bit tricky; e.g functions that throw an error on some input
> values could be exploited.
[snip]

I'm sure there's a way to generate an error on-demand for rows with
specific numbers. That opens you up to fishing for hidden rows.

It might be possible to label a subset of operators etc as safe. I'd
guess that would exclude any casts in it, and perhaps CASE. Hmm - you
could probably generate a divide-by-zero or overflow error or some such
for any targetted numeric value though.

> 4. Make the behavior user-controllable, something along the lines of
> "CREATE RESTRICTED VIEW ...", to avoid the performance impact when views
> are not used for access control.

Not pretty, but solves the problem.

--
Richard Huxton
Archonet Ltd

What version do you have?

I am cannot repeat it.

Regards
Pavel Stehule

2009/10/22 Richard Huxton <dev(at)archonet(dot)com>:
> Heikki Linnakangas wrote:
>> CREATE VIEW phone_number AS
>>     SELECT person, phone FROM phone_data WHERE phone NOT LIKE '6%';
>
>> CREATE OR REPLACE FUNCTION expose_person (person text, phone text)
>> RETURNS bool AS $$
>> begin
>>   RAISE NOTICE 'person: % number: %', person, phone;
>>   RETURN true;
>> END; $$ LANGUAGE plpgsql COST 0.000001;
>>
>> postgres=>  SELECT * FROM phone_number WHERE expose_person(person, phone);
>> NOTICE:  person: public person number: 12345
>> NOTICE:  person: secret person number: 67890
>>     person     | phone
>> ---------------+-------
>>  public person | 12345
>
> Ouch!
>
>> 1. Change the planner so that conditions (and join!) in the view are
>> always enforced first, before executing any quals from the user-supplied
>>  query. Unfortunately that would have a catastrophic effect on performance.
>
> I have the horrible feeling that you're going to end up doing this
> (possibly in conjunction with #4). Once you've executed a user-defined
> function on a "hidden" row I think the game is lost. That might even
> apply to non-trivial expressions too.
>
>> 2. As an optimization, we could keep the current behavior if the user
>> has access to all the underlying tables anyway, but that's nontrivial
>> because permission checks are supposed to be executed at runtime, not
>> plan time.
>>
>> 3. Label every function as safe or unsafe, depending on whether it can
>> leak information about the arguments. Classifying functions correctly
>> can be a bit tricky; e.g functions that throw an error on some input
>> values could be exploited.
> [snip]
>
> I'm sure there's a way to generate an error on-demand for rows with
> specific numbers. That opens you up to fishing for hidden rows.
>
> It might be possible to label a subset of operators etc as safe. I'd
> guess that would exclude any casts in it, and perhaps CASE. Hmm - you
> could probably generate a divide-by-zero or overflow error or some such
> for any targetted numeric value though.
>
>> 4. Make the behavior user-controllable, something along the lines of
>> "CREATE RESTRICTED VIEW ...", to avoid the performance impact when views
>> are not used for access control.
>
> Not pretty, but solves the problem.
>
> --
>  Richard Huxton
>  Archonet Ltd
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

That example I ran on CVS HEAD, but it's a generic problem on all versions.

Pavel Stehule wrote:
> What version do you have?
>
> I am cannot repeat it.
>
> Regards
> Pavel Stehule
>
> 2009/10/22 Richard Huxton <dev(at)archonet(dot)com>:
>> Heikki Linnakangas wrote:
>>> CREATE VIEW phone_number AS
>>> SELECT person, phone FROM phone_data WHERE phone NOT LIKE '6%';
>>> CREATE OR REPLACE FUNCTION expose_person (person text, phone text)
>>> RETURNS bool AS $$
>>> begin
>>> RAISE NOTICE 'person: % number: %', person, phone;
>>> RETURN true;
>>> END; $$ LANGUAGE plpgsql COST 0.000001;
>>>
>>> postgres=> SELECT * FROM phone_number WHERE expose_person(person, phone);
>>> NOTICE: person: public person number: 12345
>>> NOTICE: person: secret person number: 67890
>>> person | phone
>>> ---------------+-------
>>> public person | 12345
>> Ouch!
>>
>>> 1. Change the planner so that conditions (and join!) in the view are
>>> always enforced first, before executing any quals from the user-supplied
>>> query. Unfortunately that would have a catastrophic effect on performance.
>> I have the horrible feeling that you're going to end up doing this
>> (possibly in conjunction with #4). Once you've executed a user-defined
>> function on a "hidden" row I think the game is lost. That might even
>> apply to non-trivial expressions too.
>>
>>> 2. As an optimization, we could keep the current behavior if the user
>>> has access to all the underlying tables anyway, but that's nontrivial
>>> because permission checks are supposed to be executed at runtime, not
>>> plan time.
>>>
>>> 3. Label every function as safe or unsafe, depending on whether it can
>>> leak information about the arguments. Classifying functions correctly
>>> can be a bit tricky; e.g functions that throw an error on some input
>>> values could be exploited.
>> [snip]
>>
>> I'm sure there's a way to generate an error on-demand for rows with
>> specific numbers. That opens you up to fishing for hidden rows.
>>
>> It might be possible to label a subset of operators etc as safe. I'd
>> guess that would exclude any casts in it, and perhaps CASE. Hmm - you
>> could probably generate a divide-by-zero or overflow error or some such
>> for any targetted numeric value though.
>>
>>> 4. Make the behavior user-controllable, something along the lines of
>>> "CREATE RESTRICTED VIEW ...", to avoid the performance impact when views
>>> are not used for access control.
>> Not pretty, but solves the problem.
>>
>> --
>> Richard Huxton
>> Archonet Ltd
>>
>> --
>> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
>> To make changes to your subscription:
>> http://www.postgresql.org/mailpref/pgsql-hackers
>>

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Pavel Stehule wrote:
> What version do you have?
>
> I am cannot repeat it.

It will depend on the relative cost of the clauses (though 0.0001 should
have been enough to force it). Try:

CREATE OR REPLACE FUNCTION row_hidden (phone text) RETURNS bool AS $$
BEGIN
RETURN phone LIKE '6%';
END;
$$ LANGUAGE plpgsql COST 999;

CREATE VIEW phone_number AS
SELECT person, phone FROM phone_data WHERE NOT row_hidden(phone);

--
Richard Huxton
Archonet Ltd

2009/10/22 Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>:
> That example I ran on CVS HEAD, but it's a generic problem on all versions.
postgres=# select version();
version
────────────────────────────────────────────────────────────────────────────────────
PostgreSQL 8.5devel on i686-pc-linux-gnu, compiled by GCC gcc (GCC)
4.4.1 20090725
(1 row)

postgres=# select * from x;
a │ b
────┼────
10 │ 20
(1 row)

postgres=# create view v as select * from x where b <> 20;
CREATE VIEW
postgres=# create function vv(int, int) returns bool as $$begin raise
notice '% %', $1, $2; return true; end$$ language plpgsql;
CREATE FUNCTION
postgres=# select * from v where vv(a,b);
a │ b
───┼───
(0 rows)

postgres=# create or replace function vv(int, int) returns bool as
$$begin raise notice '% %', $1, $2; return true; end$$ language
plpgsql COST 999;
CREATE FUNCTION

postgres=# select * from v where vv(a,b); a │ b
───┼───
(0 rows)

it is ok
Pavel

>
> Pavel Stehule wrote:
>> What version do you have?
>>
>> I am cannot repeat it.
>>
>> Regards
>> Pavel Stehule
>>
>> 2009/10/22 Richard Huxton <dev(at)archonet(dot)com>:
>>> Heikki Linnakangas wrote:
>>>> CREATE VIEW phone_number AS
>>>>     SELECT person, phone FROM phone_data WHERE phone NOT LIKE '6%';
>>>> CREATE OR REPLACE FUNCTION expose_person (person text, phone text)
>>>> RETURNS bool AS $$
>>>> begin
>>>>   RAISE NOTICE 'person: % number: %', person, phone;
>>>>   RETURN true;
>>>> END; $$ LANGUAGE plpgsql COST 0.000001;
>>>>
>>>> postgres=>  SELECT * FROM phone_number WHERE expose_person(person, phone);
>>>> NOTICE:  person: public person number: 12345
>>>> NOTICE:  person: secret person number: 67890
>>>>     person     | phone
>>>> ---------------+-------
>>>>  public person | 12345
>>> Ouch!
>>>
>>>> 1. Change the planner so that conditions (and join!) in the view are
>>>> always enforced first, before executing any quals from the user-supplied
>>>>  query. Unfortunately that would have a catastrophic effect on performance.
>>> I have the horrible feeling that you're going to end up doing this
>>> (possibly in conjunction with #4). Once you've executed a user-defined
>>> function on a "hidden" row I think the game is lost. That might even
>>> apply to non-trivial expressions too.
>>>
>>>> 2. As an optimization, we could keep the current behavior if the user
>>>> has access to all the underlying tables anyway, but that's nontrivial
>>>> because permission checks are supposed to be executed at runtime, not
>>>> plan time.
>>>>
>>>> 3. Label every function as safe or unsafe, depending on whether it can
>>>> leak information about the arguments. Classifying functions correctly
>>>> can be a bit tricky; e.g functions that throw an error on some input
>>>> values could be exploited.
>>> [snip]
>>>
>>> I'm sure there's a way to generate an error on-demand for rows with
>>> specific numbers. That opens you up to fishing for hidden rows.
>>>
>>> It might be possible to label a subset of operators etc as safe. I'd
>>> guess that would exclude any casts in it, and perhaps CASE. Hmm - you
>>> could probably generate a divide-by-zero or overflow error or some such
>>> for any targetted numeric value though.
>>>
>>>> 4. Make the behavior user-controllable, something along the lines of
>>>> "CREATE RESTRICTED VIEW ...", to avoid the performance impact when views
>>>> are not used for access control.
>>> Not pretty, but solves the problem.
>>>
>>> --
>>>  Richard Huxton
>>>  Archonet Ltd
>>>
>>> --
>>> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgsql-hackers
>>>
>
>
> --
>  Heikki Linnakangas
>  EnterpriseDB   http://www.enterprisedb.com
>

Pavel Stehule wrote:
> 2009/10/22 Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>:
>> That example I ran on CVS HEAD, but it's a generic problem on all versions.
> postgres=# select version();
> version
> ────────────────────────────────────────────────────────────────────────────────────
> PostgreSQL 8.5devel on i686-pc-linux-gnu, compiled by GCC gcc (GCC)
> 4.4.1 20090725
> (1 row)
>
> postgres=# select * from x;
> a │ b
> ────┼────
> 10 │ 20
> (1 row)
>
> postgres=# create view v as select * from x where b <> 20;
^^^^^^^
This is the expression that needs to be expensive. Then the exposing
function needs to be cheap. That makes the planner run the exposing
function first.

--
Richard Huxton
Archonet Ltd

2009/10/22 Richard Huxton <dev(at)archonet(dot)com>:
> Pavel Stehule wrote:
>> 2009/10/22 Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>:
>>> That example I ran on CVS HEAD, but it's a generic problem on all versions.
>> postgres=# select version();
>>                                                    version
>> ────────────────────────────────────────────────────────────────────────────────────
>>  PostgreSQL 8.5devel on i686-pc-linux-gnu, compiled by GCC gcc (GCC)
>> 4.4.1 20090725
>> (1 row)
>>
>> postgres=# select * from x;
>>  a  │ b
>> ────┼────
>>  10 │ 20
>> (1 row)
>>
>> postgres=# create view v as select * from x where b <> 20;
>                                                    ^^^^^^^
> This is the expression that needs to be expensive. Then the exposing
> function needs to be cheap. That makes the planner run the exposing
> function first.
>

postgres=# create or replace function vv(int, int) returns bool as
$$begin raise notice '% %', $1, $2; return true; end$$ language
plpgsql COST 0.000001;
CREATE FUNCTION
postgres=# select * from v where vv(a,b);NOTICE: 10 20
a │ b
───┼───
(0 rows)

still I have not bad result, but, yes, I see what I could not to see.

Pavel

> --
>  Richard Huxton
>  Archonet Ltd
>

Richard Huxton wrote:
> Heikki Linnakangas wrote:
>> CREATE VIEW phone_number AS
>> SELECT person, phone FROM phone_data WHERE phone NOT LIKE '6%';
>
>> CREATE OR REPLACE FUNCTION expose_person (person text, phone text)
>> RETURNS bool AS $$
>> begin
>> RAISE NOTICE 'person: % number: %', person, phone;
>> RETURN true;
>> END; $$ LANGUAGE plpgsql COST 0.000001;
>>
>> postgres=> SELECT * FROM phone_number WHERE expose_person(person, phone);
>> NOTICE: person: public person number: 12345
>> NOTICE: person: secret person number: 67890
>> person | phone
>> ---------------+-------
>> public person | 12345

Hmm - just using SQL (but with an expensive view filtering function):

SELECT * FROM phone_number WHERE (CASE WHEN phone = '67890' THEN
person::int ELSE 2 END)=2;
ERROR: invalid input syntax for integer: "secret person"

You could get a related problem where a view exposes a text column full
of valid dates which the user then tries to cast to date. If the
underlying table contains non-dates you could still get an error.
Arguably the view should have handled the cast in this case though.

--
Richard Huxton
Archonet Ltd

Pavel Stehule wrote:
>
> postgres=# create or replace function vv(int, int) returns bool as
> $$begin raise notice '% %', $1, $2; return true; end$$ language
> plpgsql COST 0.000001;
> CREATE FUNCTION
> postgres=# select * from v where vv(a,b);NOTICE: 10 20
> a │ b
> ───┼───
> (0 rows)
>
> still I have not bad result, but, yes, I see what I could not to see.

Ah - that's the problem. It's not possible to get the "hidden" values
into the result set, but it is possible to see them. It only matters if
you are using the view to prevent access to certain rows.

--
Richard Huxton
Archonet Ltd

On Thu, Oct 22, 2009 at 6:03 AM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> In chapter "36.4 Rules and Privileges" we show an example of using a
> view to expose part of a table to other users, keeping other rows private:
>
>> For example: A user has a list of phone numbers where some of them are
> private, the others are of interest for the secretary of the office. He
> can construct the following:
>>
>> CREATE TABLE phone_data (person text, phone text, private boolean);
>> CREATE VIEW phone_number AS
>>     SELECT person, phone FROM phone_data WHERE NOT private;
>> GRANT SELECT ON phone_number TO secretary;
>
> While it works for this example, if the WHERE clause in the view is more
> complex, it is possible for the secretary to circumvent the protection
> by filtering rows in a function used in the WHERE clause. If the
> function has a lower cost than the restriction in the view, it will be
> executed first and will see all the rows, even though they won't be
> present in the final result set.
>
> For example:
>
> CREATE TABLE phone_data (person text, phone text);
> CREATE VIEW phone_number AS
>    SELECT person, phone FROM phone_data WHERE phone NOT LIKE '6%';
> GRANT SELECT ON phone_number TO secretary;
>
> -- secretary should only see the first row
> INSERT INTO phone_data VALUES ('public person', '12345');
> INSERT INTO phone_data VALUES ('secret person', '67890');
>
> \c - secretary
>
> CREATE OR REPLACE FUNCTION expose_person (person text, phone text)
> RETURNS bool AS $$
> begin
>  RAISE NOTICE 'person: % number: %', person, phone;
>  RETURN true;
> END; $$ LANGUAGE plpgsql COST 0.000001;
>
> postgres=>  SELECT * FROM phone_number WHERE expose_person(person, phone);
> NOTICE:  person: public person number: 12345
> NOTICE:  person: secret person number: 67890
>    person     | phone
> ---------------+-------
>  public person | 12345
> (1 row)
>
>
> Using views for access control like this is what we've been suggesting
> to implement row-level access control for a long time. For example, Veil
> depends heavily on this.
>
>
> A related problem is that EXPLAIN ANALYZE too can reveal information
> about the underlying tables that the view doesn't reveal, even if
> there's no functions involved.
>
>
> I posted this to security(at)postgresql(dot)org first, and the conclusion was
> to take the discussion to pgsql-hackers since there's no easy, robust
> and back-patchable solution in sight. Options discussed include:
>
> 1. Change the planner so that conditions (and join!) in the view are
> always enforced first, before executing any quals from the user-supplied
>  query. Unfortunately that would have a catastrophic effect on performance.
>
> 2. As an optimization, we could keep the current behavior if the user
> has access to all the underlying tables anyway, but that's nontrivial
> because permission checks are supposed to be executed at runtime, not
> plan time.
>
> 3. Label every function as safe or unsafe, depending on whether it can
> leak information about the arguments. Classifying functions correctly
> can be a bit tricky; e.g functions that throw an error on some input
> values could be exploited. And it's not clear how a user could label
> user-defined functions in a secure way. We'd still need the
> infrastructure of point 1 to delay evaluation of non-safe quals, so this
> is really just another optimization of it.
>
> 4. Make the behavior user-controllable, something along the lines of
> "CREATE RESTRICTED VIEW ...", to avoid the performance impact when views
> are not used for access control.

Well, I think #4 is a good start (though I don't like CREATE x VIEW -
I think that keyword should appear somewhere lower down in the
syntax), but I'm not sure where to go with it after that. I'm not
sure you're going to be able to make it secure short of leaving the
view as an unflattened subquery. "Catastrophic for performance" is a
charitable description...

...Robert

>
> \c - secretary
>
> CREATE OR REPLACE FUNCTION expose_person (person text, phone text)
> RETURNS bool AS $$
> begin
>  RAISE NOTICE 'person: % number: %', person, phone;
>  RETURN true;
> END; $$ LANGUAGE plpgsql COST 0.000001;
>
> postgres=>  SELECT * FROM phone_number WHERE expose_person(person, phone);
> NOTICE:  person: public person number: 12345
> NOTICE:  person: secret person number: 67890
>    person     | phone
> ---------------+-------
>  public person | 12345
> (1 row)
>

Given RAISE is easily replaced with INSERT into a logging table or
another recording mechanism, it needs to be something to push back
execution of user based parameters OR something to push forward
security clauses.

Is there any way of exposing the information using standard SQL or is
a procedure required?

If a procedure is required, then we simply need a way of ensuring the
SECURITY clauses or functions run before all of the things which an
expose information (procedures at the moment).

How about some kind of a marker on which allows security based
constraints to be pushed forward rather than the entire view?

CREATE VIEW phone_number AS
SELECT person, phone FROM phone_data WHERE SECURITY(phone NOT LIKE '6%');

This still allows complex views and queries to be mostly optimized
with a few filters that run very early and in the order they are
defined in.

Perhaps we go one step further and encourage security filters to be
applied to the table directly where possible:

CREATE VIEW phone_number AS
SELECT person, phone
FROM phone_data USING SECURITY FILTER(phone NOT LIKE '6%');

This still allow many optimizations to be applied in complex cases. The planner

CREATE VIEW phone_number AS
SELECT person, phone, company
FROM phone_data USING SECURITY FILTER(phone NOT LIKE '6%')
JOIN person USING (person_id)
JOIN company USING (company_id)
AND person.active AND company.active;

\c - secretary
SELECT * FROM phone_number WHERE company = 'Frankies Co.';

This still allows a query against phone_number to use the company data
first, find the single person (Frankie) within that company, then get
his phone number out.

The scan against phone_data would be an index scan for person_id BUT
applies the SECURITY FILTER as the node immediately around the index
scan as a Recheck Condition, similar to how bitmap scans ensure they
got the correct and only the correct information.

person.active and company.active, and the joins can still be optimized
in standard ways.

More complex SECURITY FILTER clauses might be applied in the where clause. I.e.

CREATE VIEW phone_number AS
SELECT person, phone, company
FROM phone_data USING SECURITY CLAUSE (phone NOT LIKE '6%')
JOIN person USING (person_id)
JOIN company USING (company_id)
WHERE SECURITY CLAUSE (person.status = company.status)
AND person.active AND company.active;

This would result in the security check (person.status =
company.status) occurring as a filter tied to the join node for person
and company which cannot be moved around.

Layering is tricky, using the above view:

\c - secretary
CREATE VIEW company_number AS
SELECT * FROM phone_number SECURITY CLAUSE (expose_person(person, phone));

SELECT * FROM company_number;

The security clauses are bound to run in the order they are found in
the node closes to the data they use.

phone_data is immediately run through a Recheck Cond. person/company
join node is checked immediately after. Finally, the expose_person()
function is run against the now clean data.

Oh, This all has the nice side effect of knowing what to hide in
explain analyze as well since the specific clauses are marked up. If
the user running the query is super user or owner of the view, they
see the security clause filters. If they are not, then they get a line
like this:

SELECT * FROM phone_number WHERE phone = '555-555-5555';

Bitmap Heap Scan on phone_data (cost=14.25..61.47 rows=258 width=185)
Security Cond: ** Hidden due to permissions **
-> Bitmap Index Scan on phone_data_index (cost=0.00..14.19
rows=258 width=0)
Index Cond: (phone = '555-555-5555')

Just to intoduce myself, I'm Marc Munro the developer of Veil, a
postgres add-in that allows you to implement virtual private databases
using views.

The problem we are discussing here is the existence of covert or
side-channels being available from functions that can leak information
about the rows they see, even though the end-user may not see those
rows. These can be built-ins such as set_config() (thanks, Heikki) or
user-defined.

I assert that any attempt to use a secured-view's where-clause
conditions before any other conditions are applied will lead to poor
performance.

Here is a typical veil secured view definition:

create view parties as
SELECT party_id, client_id, party_type_id, username, party_name
FROM parties.parties
WHERE api.user_has_client_or_personal_privilege(client_id,
party_id, 'select parties')
OR api.user_has_client_privilege(party_id, 'select clients');

A typical query against this would be:

select * from parties where party_id = 42;

The conditions in the view's where clause cannot generally be indexed.
Applying those conditions before the user-supplied conditions would mean
that a full-table scan would be required and performance would suck. In
fact, this very suckiness also exposes a covert channel in that now we
can use the performance of the query to estimate the number of party
records.

The most acceptable solution I have heard so far for this issue, is to
identify those functions which can leak information as 'insecure', and
those views which are for security purpose as 'secured'. Then it is
simply (hah!) a matter of planning the query of secured views so that
all insecure functions are called after all secure functions. In this
way, they will only be able to leak what the user is entitled to see,
and performance will only be as badly affected as is strictly necessary.

__
Marc

Marc Munro wrote:
> Here is a typical veil secured view definition:
>
> create view parties as
> SELECT party_id, client_id, party_type_id, username, party_name
> FROM parties.parties
> WHERE api.user_has_client_or_personal_privilege(client_id,
> party_id, 'select parties')
> OR api.user_has_client_privilege(party_id, 'select clients');
>
> A typical query against this would be:
>
> select * from parties where party_id = 42;
>
> The conditions in the view's where clause cannot generally be indexed.
> Applying those conditions before the user-supplied conditions would mean
> that a full-table scan would be required and performance would suck. In
> fact, this very suckiness also exposes a covert channel in that now we
> can use the performance of the query to estimate the number of party
> records.

I'm not too worried about that as anyone can do "SELECT reltuples FROM
pg_class where relname='parties'". An index-scan, however, would let you
estimate the number of records with party_id=42, whether or not you have
privileges on them, by timing how long the query takes. I don't think we
can avoid that.

> The most acceptable solution I have heard so far for this issue, is to
> identify those functions which can leak information as 'insecure', and
> those views which are for security purpose as 'secured'. Then it is
> simply (hah!) a matter of planning the query of secured views so that
> all insecure functions are called after all secure functions. In this
> way, they will only be able to leak what the user is entitled to see,
> and performance will only be as badly affected as is strictly necessary.

The most user-friendly and backwards-compatible (though not necessarily
back-patchable) approach I can see is:

1. If the user has read access to all the underlying tables, plan it
like we do today.

2. If the view refers only one table (as a typical Veil view does), plan
it like we do today but enforce that view conditions are evaluated first
in the Filter. Notably, allow using any user-supplied conditions as
index quals if there's a matching index.

3. Otherwise fully materialize the view.

I expect 1 & 2 to catch most real-life scenarios. 1 ensures that we
don't degrade performance of (potentially very complex) views that are
not used for access control, and 2 should apply to most views used for
row-level access control. Note that if you have an access-control view
that has a join, you can often split it into two access-control views
over the underlying tables with another view that joins the two views.
Such a construct would be quite well

Tom didn't like the idea of performing permission checks at plan time,
because they're currently always done at execution time. If we plan a
query on the assumption that you have access to the underlying table, I
think we could perform permission checks on the underlying tables at
runtime to check that the user still has access. It would mean that a
previously prepared query might start to fail with "permission denied"
errors if someone revokes your privileges on the tables, but I think we
could live with that. Plan invalidation would cover the case where
permissions on the tables are changed, but would not cover indirect
cases like removing the user from a group that had the required permissions.

Whatever we do, we probably should add a caveat in the manual that
EXPLAIN or timing attacks can let you deduce some (statistical)
information you won't have direct access to.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Fri, 2009-10-23 at 11:30 +0300, Heikki Linnakangas wrote:

> The most user-friendly and backwards-compatible (though not necessarily
> back-patchable) approach I can see is:
>
> 1. If the user has read access to all the underlying tables, plan it
> like we do today.

For me, it would make most sense to explicitly mark Views as being
security views. That way planning need only change when we are
optimizing a query that accesses a view with plan security enabled.

ALTER VIEW foo ENABLE PLAN SECURITY;

That is much clearer and easily to verify/audit, so more secure.

Also, we should presume that any function created with SECURITY DEFINER
and created by a superuser would have plan security, so we don't need to
annotate lots of old code to work securely. Annotating the built-in
functions is a lot easier.

> 2. If the view refers only one table (as a typical Veil view does), plan
> it like we do today but enforce that view conditions are evaluated first
> in the Filter. Notably, allow using any user-supplied conditions as
> index quals if there's a matching index.
>
> 3. Otherwise fully materialize the view.

So if we join a normal table or a view to a secure view then only the
secure view part would be materialized? Or do you mean the whole query
would be materialized?

--
Simon Riggs www.2ndQuadrant.com

2009/10/23 Simon Riggs <simon(at)2ndquadrant(dot)com>:
> On Fri, 2009-10-23 at 11:30 +0300, Heikki Linnakangas wrote:
>
>> The most user-friendly and backwards-compatible (though not necessarily
>> back-patchable) approach I can see is:
>>
>> 1. If the user has read access to all the underlying tables, plan it
>> like we do today.
>
> For me, it would make most sense to explicitly mark Views as being
> security views. That way planning need only change when we are
> optimizing a query that accesses a view with plan security enabled.
>
> ALTER VIEW foo ENABLE PLAN SECURITY;

+1

Pavel

>
> That is much clearer and easily to verify/audit, so more secure.
>
> Also, we should presume that any function created with SECURITY DEFINER
> and created by a superuser would have plan security, so we don't need to
> annotate lots of old code to work securely. Annotating the built-in
> functions is a lot easier.
>
>> 2. If the view refers only one table (as a typical Veil view does), plan
>> it like we do today but enforce that view conditions are evaluated first
>> in the Filter. Notably, allow using any user-supplied conditions as
>> index quals if there's a matching index.
>>
>> 3. Otherwise fully materialize the view.
>
> So if we join a normal table or a view to a secure view then only the
> secure view part would be materialized? Or do you mean the whole query
> would be materialized?
>
> --
>  Simon Riggs           www.2ndQuadrant.com
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Simon Riggs wrote:
> On Fri, 2009-10-23 at 11:30 +0300, Heikki Linnakangas wrote:
>
>> The most user-friendly and backwards-compatible (though not necessarily
>> back-patchable) approach I can see is:
>>
>> 1. If the user has read access to all the underlying tables, plan it
>> like we do today.
>
> For me, it would make most sense to explicitly mark Views as being
> security views. That way planning need only change when we are
> optimizing a query that accesses a view with plan security enabled.
>
> ALTER VIEW foo ENABLE PLAN SECURITY;
>
> That is much clearer and easily to verify/audit, so more secure.

The point is that only owner knows intention of the view correctly.
It seems to me reasonable to explicitly mark whether the major purpose
of view is security, or not.

> Also, we should presume that any function created with SECURITY DEFINER
> and created by a superuser would have plan security, so we don't need to
> annotate lots of old code to work securely. Annotating the built-in
> functions is a lot easier.

Sorry, what is happen if function is marked as "plan security"?

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

On Fri, 2009-10-23 at 19:38 +0900, KaiGai Kohei wrote:
> > Also, we should presume that any function created with SECURITY DEFINER
> > and created by a superuser would have plan security, so we don't need to
> > annotate lots of old code to work securely. Annotating the built-in
> > functions is a lot easier.
>
> Sorry, what is happen if function is marked as "plan security"?

I was suggesting an intelligent default by which we could determine
function marking implicitly, if it was not explicitly stated on the
CREATE FUNCTION.

--
Simon Riggs www.2ndQuadrant.com

Simon Riggs wrote:
> On Fri, 2009-10-23 at 19:38 +0900, KaiGai Kohei wrote:
>>> Also, we should presume that any function created with SECURITY DEFINER
>>> and created by a superuser would have plan security, so we don't need to
>>> annotate lots of old code to work securely. Annotating the built-in
>>> functions is a lot easier.
>> Sorry, what is happen if function is marked as "plan security"?
>
> I was suggesting an intelligent default by which we could determine
> function marking implicitly, if it was not explicitly stated on the
> CREATE FUNCTION.

How to handle a (corner) case when the function owner was changed to non
privileged user and its definition is replaced later?

Even if someone malicious gives leakage condition on the view, possible
leakable infotmation is restricted to where the owner of view can access.
So, it seems to me the security mark on views by owner are sufficient.

Thanks,
--
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Simon Riggs wrote:
> Also, we should presume that any function created with SECURITY DEFINER
> and created by a superuser would have plan security, so we don't need to
> annotate lots of old code to work securely. Annotating the built-in
> functions is a lot easier.

SECURITY DEFINER is an orthogonal aspect. Consider something as
innocent-looking as this:

CREATE FUNCTION secdeffunc(text) RETURNS boolean AS $$ SELECT
$1::integer < 10; $$ LANGUAGE SQL SECURITY DEFINER;

The text-to-integer cast throws an error and reveals the argument as
someone pointed out earlier in this thread. Creating such a function
shouldn't open the door to information leaks in views elsewhere.

The most useful "automatic" annotation I can see is to treat functions
implementing B-tree operators as safe. I *think* that's safe, anyway.

>> 2. If the view refers only one table (as a typical Veil view does), plan
>> it like we do today but enforce that view conditions are evaluated first
>> in the Filter. Notably, allow using any user-supplied conditions as
>> index quals if there's a matching index.
>>
>> 3. Otherwise fully materialize the view.
>
> So if we join a normal table or a view to a secure view then only the
> secure view part would be materialized? Or do you mean the whole query
> would be materialized?

Just the secure view. Materializing the result of the overall query
wouldn't help.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Rod Taylor wrote:
> This still allow many optimizations to be applied in complex cases. The planner
>
> CREATE VIEW phone_number AS
> SELECT person, phone, company
> FROM phone_data USING SECURITY FILTER(phone NOT LIKE '6%')
> JOIN person USING (person_id)
> JOIN company USING (company_id)
> AND person.active AND company.active;

Well, you can also achieve that by creating two views, one to hide the
sensitive data and another to do the join:

CREATE VIEW not6_numbers AS
SELECT phone FROM phone_data WHERE phone NOT LIKE '6%';

CREATE VIEW phone_number AS
SELECT person, phone, company FROM not6_numbers
JOIN person USING (person_id)
JOIN company USING (company_id)
WHERE person.active AND company.active;

So I don't think we should invent new syntax for that. The 1st view
would be marked with SECURE if we end up using that explicit annotation
in CREATE VIEW.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas wrote:
> The most useful "automatic" annotation I can see is to treat functions
> implementing B-tree operators as safe. I *think* that's safe, anyway.

Index lookups and single-type comparisons were the only things I could
come up with as safe. Unless there is some way to generate an error from
geometric ops (overflow or some such).

Anything involving a type-cast can obviously be finessed. If you allow
arithmetic then you could trigger an overflow or divide-by-zero error.

Hmm - you can probably do something evil with non-UTF8 characters if you
allow string operations. Would string comparisons be safe (because a
literal would be caught before the view gets evaluated)?

--
Richard Huxton
Archonet Ltd

Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> On Fri, 2009-10-23 at 19:38 +0900, KaiGai Kohei wrote:
>> Sorry, what is happen if function is marked as "plan security"?

> I was suggesting an intelligent default by which we could determine
> function marking implicitly, if it was not explicitly stated on the
> CREATE FUNCTION.

The thought that's been in the back of my mind is that you could solve
99% of the performance problem if you trusted all builtin functions and
nothing else. This avoids the question of who gets to mark functions
as trustable.

regards, tom lane

On Fri, Oct 23, 2009 at 10:04:29AM -0400, Tom Lane wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> > On Fri, 2009-10-23 at 19:38 +0900, KaiGai Kohei wrote:
> >> Sorry, what is happen if function is marked as "plan security"?
>
> > I was suggesting an intelligent default by which we could
> > determine function marking implicitly, if it was not explicitly
> > stated on the CREATE FUNCTION.
>
> The thought that's been in the back of my mind is that you could
> solve 99% of the performance problem if you trusted all builtin
> functions and nothing else. This avoids the question of who gets to
> mark functions as trustable.

Great idea!

One of the things the security community has learned is that the only
way it's even possible to get an information leak rate of zero is to
have a system which does nothing at all. It's a fact we need to bear
in mind when addressing this or any other issue of access control.

Cheers,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Tom Lane wrote:
> The thought that's been in the back of my mind is that you could solve
> 99% of the performance problem if you trusted all builtin functions and
> nothing else. This avoids the question of who gets to mark functions
> as trustable.

Except that all builtin functions are not trustworthy. set_config and
int->text cast are two examples mentioned this far, and I'm sure there's
a boatload of others.

Trusting only index operators seems more and more attractive to me. That
won't limit us to built-in datatypes, requires no explicit user action
to categorize functions. They're also the most significant functions
from a performance point-of-view, allowing use of indexes instead of
forcing a seqscan of all tables.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

David Fetter <david(at)fetter(dot)org> wrote:

> One of the things the security community has learned is that the
> only way it's even possible to get an information leak rate of zero
> is to have a system which does nothing at all. It's a fact we need
> to bear in mind when addressing this or any other issue of access
> control.

And to get all old-school about it, I tend to go with the position put
forward by Admiral Grace Hopper[1] when I heard her speak at an ACM
meeting here. She said that *any* security could be broken, and that
the goal should be to put the cost of creating the breach higher for
the perpetrators than the benefits which would accrue to them. That
informs my perspective, anyway.

So, one of the first questions I ask about an information leak is
"what good would it do someone to know that?" So I don't worry too
much about someone knowing the size of my database or the number of
rows in a table, or for that matter whether county 12 has a
2009GN000317 case or how many party records have a Social Security
Number stored. I care very much that the SSN associated with a person
or a document flagged as confidential doesn't leak to unauthorized
viewers, because that information could benefit someone who obtains it
and harm others.

Perspective is more important that purity here.

-Kevin

[1] http://en.wikipedia.org/wiki/Grace_Hopper

On Fri, 2009-10-23 at 10:04 -0400, Tom Lane wrote:
> Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> > On Fri, 2009-10-23 at 19:38 +0900, KaiGai Kohei wrote:
> >> Sorry, what is happen if function is marked as "plan security"?
>
> > I was suggesting an intelligent default by which we could determine
> > function marking implicitly, if it was not explicitly stated on the
> > CREATE FUNCTION.
>
> The thought that's been in the back of my mind is that you could solve
> 99% of the performance problem if you trusted all builtin functions and
> nothing else. This avoids the question of who gets to mark functions
> as trustable.

That is a very good default. My experience is that those 1% of cases are
responsible for 99% of wasted time, so the ability to specify things for
user functions is critical. If we make user extensibility second rate we
will force solutions to be second rate also. (e.g. where would PostGIS
be without type-specific analyze functions?).

--
Simon Riggs www.2ndQuadrant.com

Simon Riggs wrote:
> On Fri, 2009-10-23 at 10:04 -0400, Tom Lane wrote:
> > Simon Riggs <simon(at)2ndQuadrant(dot)com> writes:
> > > On Fri, 2009-10-23 at 19:38 +0900, KaiGai Kohei wrote:
> > >> Sorry, what is happen if function is marked as "plan security"?
> >
> > > I was suggesting an intelligent default by which we could determine
> > > function marking implicitly, if it was not explicitly stated on the
> > > CREATE FUNCTION.
> >
> > The thought that's been in the back of my mind is that you could solve
> > 99% of the performance problem if you trusted all builtin functions and
> > nothing else. This avoids the question of who gets to mark functions
> > as trustable.
>
> That is a very good default. My experience is that those 1% of cases are
> responsible for 99% of wasted time, so the ability to specify things for
> user functions is critical. If we make user extensibility second rate we
> will force solutions to be second rate also. (e.g. where would PostGIS
> be without type-specific analyze functions?).

Added to TODO:

Prevent low-cost functions from seeing unauthorized view rows

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +