Re: leaky views, yet again
От | KaiGai Kohei |
---|---|
Тема | Re: leaky views, yet again |
Дата | |
Msg-id | 4CB3FBBA.5030504@ak.jp.nec.com обсуждение исходный текст |
Ответ на | Re: leaky views, yet again (KaiGai Kohei <kaigai@ak.jp.nec.com>) |
Ответы |
Re: leaky views, yet again
(Robert Haas <robertmhaas@gmail.com>)
|
Список | pgsql-hackers |
I noticed the previous patch has an obvious conflict to the latest git mater, and it does not have any documentation updates. So, I rebased the patch, and added descriptions about secure views. Rest of parts are unchanged. Thanks, (2010/10/06 17:01), KaiGai Kohei wrote: > The attached patch is a revised version of the fix-leaky-view patch. > > List of updates: > - It was rebased to the latest git master. > - It revised pointer castings in rewriteHandler.c. > - It added a new regression test: 'security_views'. > - It added SECURITY VIEW support to pg_dump. > - It modified the definition of pg_views to show whether it is > a security view, or not. > - It revised psql to show SECURITY VIEW attribute of views in > verbose mode. > - I tried to add a new field to pg_class, but scale of code changes > are eventually similar to previous version. So, all I revised was > to define StdViewOptions, instead of abuse of StdRdOptions. > > > To avoid confusion, I'd like to make clear what is the matter this > patch tries to tackle. > As we have discussed for a long time, an interaction between functions > with side-effects and query optimization on views may cause information > leaks, even if owner of the views intend to use them for row-level > security. > In this context, we define the term of 'leak' as system can expose > content of tuples to be invisible unexpectedly, then people can see > the content directly without any inferences. > > Thus, if and when people supplied a function that writes out given > arguments into other tables on the WHERE clause of views, it should > be invoked after all the conditions within view definition. > > E.g) CREATE OR REPLACE FUNCTION f_malicious(text) > RETURNS bool COST 0.01 LANGUAGE 'sql' > AS 'INSERT INTO my_private VALUES($1); SELECT true;'; > SELECT * FROM secret_view v WHERE f_malicious(v); > > In addition, some of functions (including built-in functions) raise > an error with message that may contain given arguments. It also allows > to expose content of invisible tuples (although throughput of the > channel is relatively slow), if it would be pushed down into inside > of join-loop. > > E.g) postgres=# select * from v2; > a | b | x | y > ---+-----+---+----- > 2 | bbb | 2 | www > 4 | eee | 4 | yyy > (2 rows) > > postgres=# select * from v2 where y::int = 1; > ERROR: invalid input syntax for integer: "vvv" > > However, it is too expensive to prohibit pushing down all the function > calls into join-loops. So, we need allow to push down a part of harmless > functions. In this patch, I don't change the logic to decide it. > Thus, only operators implemented by functions with INTERNALlanguage > are allowed to push down into SECURITY VIEWs. > > On the other hand, we can infer a certain value of hidden tuple with > iteration of probes. In this scenario, people never see the hidden > values directly. So, it is not a matter this patch tries to tackle. > > E.g) postgres=# select * from v1; > a | b > ---+----- > 2 | bbb > 4 | eee > (2 rows) > > postgres=# insert into t1 values (3, 'xyz'); > ERROR: duplicate key value violates unique constraint "t1_pkey" > DETAIL: Key (a)=(3) already exists. > postgres=# select * from v1 where 1/(3-a)> 0; > ERROR: division by zero > > As long as I know, commercial RDBMSs with RLS also don't tackle such > kind of information leaks via side-channels. At least, I don't think > it is a matter that we should tackle with first priority. > > Thanks, > > (2010/10/05 18:01), Itagaki Takahiro wrote: >> 2010/9/6 KaiGai Kohei<kaigai@ak.jp.nec.com>: >>> The attached patch tackles both of the above two points, and allows to >>> control this deoptimization when CREATE SECURITY VIEW is used. >> >> I'll send a review for the patch. >> >> Contents& Purpose >> ================== >> The patch adds a new SECURITY option for VIEWs. Views defined with >> CREATE SECURITY >> VIEW command are not merged with external WHERE-clauses including >> security-leakable >> functions in queries. However, since internal functions and operators are not >> considered as leakable functions, the planner still works well for >> security views >> unless we use user-defined functions in the WHERE-clause. >> >> Initial Run >> =========== >> * Because of the delayed review, the patch has one hunk: >> 1 out of 5 hunks FAILED -- saving rejects to file >> src/backend/commands/tablecmds.c.rej >> but it is not serious at all, and can be easily fixed. >> >> * It can be compiled, but has two warnings: >> rewriteHandler.c: In function 'MarkFuncExprDepthWalker': >> rewriteHandler.c:1155: warning: cast from pointer to integer of different size >> rewriteHandler.c:1227: warning: cast to pointer from integer of different size >> >> They should be harmless, but casting intptr_t is often used to avoid warnings: >> - L1155: (int) (intptr_t) context; >> - L1227: (void *) (intptr_t) (depth + 1) >> >> * It passes all regression tests, but doesn't have own new tests. >> >> Remaining works >> =============== >> The patch might include only core code, but I'll let you know additional >> sub-works are still required to complete the feature. Also, I've not measured >> the performance yet, though performance might not be so critical for the patch. >> >> * Regression tests and documentation for the feature are required. >> * pg_dump must support to dump SECURITY VIEWs. >> They are dumped as normal views for now. >> * pg_views can have "security" column. >> * psql's \dv can show security attributes of views. >> >> Questions >> ========= >>> postgres=# EXPLAIN select * from v1 where f_malicious(b) and a = 2; >>> ==> We assume operators implemented with built-in functions are safe. >>> So, we don't prevent this pushing-down, if the clause does not >>> contains something leakable. >> >> The term "built-in functions" means functions written in INTERNAL language >> here. But we also have SQL functions by default. Some of them are just a >> wrapper to internal functions. I'm not sure the checking of INTERNAL language >> is the best way for the purpose. Did you compare it with other methods? >> For example, "oid< FirstNormalObjectId" looks workable for me. >> >>> Instead of modifying the structure of pg_class, this patch stores a flag of >>> security view on the reloption. So, it needed to modify routines about >>> reloptions because it is the first case to store reloptions of views. >> >> Why did you need to extend StdRdOptions strucuture? Since other fields in >> the structure are not used by views at all, I think adding a new structure >> struct ViewOptions { vl_len, security_view } >> would be better than extending StdRdOptions. >> > > > > > -- KaiGai Kohei <kaigai@ak.jp.nec.com>
Вложения
В списке pgsql-hackers по дате отправления: