Thread: 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@postgresql.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-suppliedquery. Unfortunately that would have a catastrophiceffect 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@archonet.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@postgresql.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@archonet.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@postgresql.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.linnakangas@enterprisedb.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@archonet.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@postgresql.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.linnakangas@enterprisedb.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@archonet.com>: > Pavel Stehule wrote: >> 2009/10/22 Heikki Linnakangas <heikki.linnakangas@enterprisedb.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 20a │ 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.linnakangas@enterprisedb.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@postgresql.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 ASSELECT * 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, 'selectparties') 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@2ndquadrant.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@postgresql.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@kaigai.gr.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@kaigai.gr.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@2ndQuadrant.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@2ndQuadrant.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@fetter.org> http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fetter@gmail.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@fetter.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@2ndQuadrant.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@2ndQuadrant.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@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.comPG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive,Christ can be your backup. +