Re: [HACKERS] [PATCH] Largeobject Access Controls (r2460)
KaiGai Kohei kai...@ak.jp.nec.com wrote: Apart from SELinux, it is quite natural to apply any access controls on binary data. If we could not have any valid access controls, users will not want to store their sensitive information, such as confidential PDF files, as a large object. Absolutely. The historical security issues for large objects immediately eliminated them as a possible place to store PDF files. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Largeobject Access Controls (r2460)
I just looked over the latest version of this patch and it seems to satisfy all the issues suggested by the initial review. This looks like it's ready for a committer from a quality perspective and I'm going to mark it as such. I have a guess what some of the first points of discussion are going to be though, so might as well raise them here. This patch is 2.8K lines of code that's in a lot of places: a mix of full new functions, tweaks to existing ones, docs, regression tests, it's a well structured but somewhat heavy bit of work. One obvious questions is whether there's enough demand for access controls on large objects to justify adding the complexity involved to do so. A second thing I'm concerned about is what implications this change would have for in-place upgrades. If there's demand and it's not going to cause upgrade issues, then we just need to find a committer willing to chew on it. I think those are the main hurdles left for this patch. -- Greg Smith2ndQuadrant Baltimore, MD PostgreSQL Training, Services and Support g...@2ndquadrant.com 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
Re: [HACKERS] [PATCH] Largeobject Access Controls (r2460)
Greg Smith wrote: I just looked over the latest version of this patch and it seems to satisfy all the issues suggested by the initial review. This looks like it's ready for a committer from a quality perspective and I'm going to mark it as such. Thanks for your efforts. I have a guess what some of the first points of discussion are going to be though, so might as well raise them here. This patch is 2.8K lines of code that's in a lot of places: a mix of full new functions, tweaks to existing ones, docs, regression tests, it's a well structured but somewhat heavy bit of work. One obvious questions is whether there's enough demand for access controls on large objects to justify adding the complexity involved to do so. At least, it is a todo item in the community: http://wiki.postgresql.org/wiki/Todo#Binary_Data Apart from SELinux, it is quite natural to apply any access controls on binary data. If we could not have any valid access controls, users will not want to store their sensitive information, such as confidential PDF files, as a large object. A second thing I'm concerned about is what implications this change would have for in-place upgrades. If there's demand and it's not going to cause upgrade issues, then we just need to find a committer willing to chew on it. I think those are the main hurdles left for this patch. I guess we need to create an empty entry with a given OID into the pg_largeobject_metadata for each large objects when we try to upgrade in-place from 8.4.x or earlier release to the upcoming release. However, no format changes in the pg_largeobject catalog, including an empty large object, so I guess we need a small amount of additional support in pg_dump to create empty metadata. I want any suggestion about here. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Largeobject Access Controls (r2460)
On Sun, Dec 6, 2009 at 11:19 PM, Greg Smith g...@2ndquadrant.com wrote: I just looked over the latest version of this patch and it seems to satisfy all the issues suggested by the initial review. This looks like it's ready for a committer from a quality perspective and I'm going to mark it as such. yes. i have just finished my tests and seems like the patch is working just fine... BTW, seems like KaiGai miss this comment in src/backend/catalog/pg_largeobject.c when renaming the parameter * large_object_privilege_checks is not refered here, i still doesn't like the name but we have changed it a lot of times so if anyone has a better idea now is when you have to speak -- Atentamente, Jaime Casanova Soporte y capacitación de PostgreSQL Asesoría y desarrollo de sistemas Guayaquil - Ecuador Cel. +59387171157 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Largeobject Access Controls (r2460)
Jaime Casanova wrote: On Sun, Dec 6, 2009 at 11:19 PM, Greg Smith g...@2ndquadrant.com wrote: I just looked over the latest version of this patch and it seems to satisfy all the issues suggested by the initial review. This looks like it's ready for a committer from a quality perspective and I'm going to mark it as such. yes. i have just finished my tests and seems like the patch is working just fine... BTW, seems like KaiGai miss this comment in src/backend/catalog/pg_largeobject.c when renaming the parameter * large_object_privilege_checks is not refered here, i still doesn't like the name but we have changed it a lot of times so if anyone has a better idea now is when you have to speak Oops, it should be fixed to lo_compat_privileges. This comment also have version number issue, so I fixed it as follows: BEFORE: /* * large_object_privilege_checks is not refered here, * because it is a compatibility option, but we don't * have ALTER LARGE OBJECT prior to the v8.5.0. */ AFTER: /* * The 'lo_compat_privileges' is not checked here, because we * don't have any access control features in the 8.4.x series * or earlier release. * So, it is not a place we can define a compatible behavior. */ Nothing are changed in other codes, including something corresponding to in-place upgrading. I'm waiting for suggestion. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com sepgsql-02-blob-8.5devel-r2461.patch.gz Description: application/gzip -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] Largeobject Access Controls (r2460)
The attached patch is an updated revision of Largeobject Access Controls. List of updates: * rebased to the latest CVS HEAD * SGML documentation fixes: - The future version number was replaced as: In the 8.4.x series and earlier release, ... - Other strange English representations and typoes were fixed. * Fixed OID conflicts in system catalog definition. The new TOAST relation for pg_trigger used same OID number with pg_largeobject_metadata. * Fixed incorrect error code in pg_largeobject_ownercheck(). It raised _UNDEFINED_FUNCTION, but should be _UNDEFINED_OBJECT. * Renamed GUC parameter to lo_compat_privileges from large_object_privilege_checks. * pg_largeobject_aclmask() and pg_largeobject_aclcheck(), not take an argument of snapshot, were removed. Currently, the caller provide an appropriate snapshot them. Thanks, Jaime Casanova wrote: 2009/11/12 KaiGai Kohei kai...@ak.jp.nec.com: The attached patch is a revised version of large object privileges based on the feedbacks at the last commit fest. please update the patch, it's giving an error when 'make check' is trying to create template1 in initdb: creating template1 database in /home/postgres/pg_releases/pgsql/src/test/regress/./tmp_check/data/base/1 ... TRAP: FailedAssertion(!(reln-md_fd[forkNum] == ((void *)0)), File: md.c, Line: 254) child process was terminated by signal 6: Aborted Meanwhile, i will make some comments: This manual will be specific for 8.5 so i think all mentions to the version should be removed for example; +In this version, a large object has OID of its owner, access permissions +and OID of the largeobject itself. + Prior to the version 8.5.x release does not have any privilege checks on + large objects. the parameter name (large_object_privilege_checks) is confusing enough that we have to make this statements to clarify... let's think in a better less confuse name + Please note that it is not equivalent to disable all the security + checks corresponding to large objects. + For example, the literallo_import()/literal and + literallo_export/literal need superuser privileges independent + from this setting as prior versions were doing. this will not be off by default? it should be for compatibility reasons... i remember there was a discussion about that but can't remember the conclusion Mmm... One of them? the first? + The one is literalSELECT/literal. + Even if a transaction modified access rights and commit it, it is + not invisible from other transaction which already opened the large + object. The other one, the second + The other is literalUPDATE/literal. it seems there is an are that should not be there :) + + These functions are originally requires database superuser privilege, + and it allows to bypass the default database privilege checks, so + we don't need to check an obvious test twice. a typo, obviously +For largeo bjects, this privilege also allows to read from +the target large object. We have two versions of these functions one that a recieve an SnapShot parameter and other that don't... what is the rationale of this? AFAIU, the one that doesn't receive SnapShot is calling the other one with SnapShotNow, can't we simply call it that way and drop the version of the functions that doesn't have that parameter? + pg_largeobject_aclmask(Oid lobj_oid, Oid roleid, + AclMode mask, AclMaskHow how) + pg_largeobject_aclcheck(Oid lobj_oid, Oid roleid, AclMode mode) -- OSS Platform Development Division, NEC KaiGai Kohei kai...@ak.jp.nec.com sepgsql-02-blob-8.5devel-r2460.patch.gz Description: application/gzip -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
