Re: leaky views, yet again
От | Itagaki Takahiro |
---|---|
Тема | Re: leaky views, yet again |
Дата | |
Msg-id | AANLkTinX=HxYjRox-oE3PzcuSurpdUZ0iqnPSvq315=o@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: leaky views, yet again (KaiGai Kohei <kaigai@ak.jp.nec.com>) |
Ответы |
Re: leaky views, yet again
(KaiGai Kohei <kaigai@ak.jp.nec.com>)
Re: leaky views, yet again (KaiGai Kohei <kaigai@ak.jp.nec.com>) |
Список | pgsql-hackers |
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. -- Itagaki Takahiro
В списке pgsql-hackers по дате отправления: