Largeobject Access Controls (r2460)
in [PgSql]
|
Prev: explain output infelicity in psql
Next: Winflex
From: Takahiro Itagaki on 10 Dec 2009 00:56 Hi, I'm reviewing LO-AC patch. KaiGai Kohei <kaigai(a)ak.jp.nec.com> wrote: > Nothing are changed in other codes, including something corresponding to > in-place upgrading. I'm waiting for suggestion. I have a question about the behavior -- the patch adds ownership management of large objects. Non-privileged users cannot read, write, or drop othere's LOs. But they can read the contents of large object when they read pg_catalog.pg_largeobject directly. Even if the patch is applied, we still allow "SELECT * FROM pg_largeobject" ...right? This issue might be solved by the core SE-PgSQL patch, but what should we do fo now? Other changes in the patch seem to be reasonable. "GRANT/REVOKE ON LARGE OBJECT <number>" might be hard to use if used alone, but we can use the commands as dynamic SQLs in DO statements if we want to grant or revoke privileges in bulk. "SELECT oid FROM pg_largeobject_metadata" is used in some places instead of "SELECT DISTINCT loid FROM pg_largeobject". They return the same result, but the former will be faster because we don't use DISTINCT. pg_dump will be slightly accelerated by the new query. Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: KaiGai Kohei on 10 Dec 2009 01:46 Takahiro Itagaki wrote: > Hi, I'm reviewing LO-AC patch. > > KaiGai Kohei <kaigai(a)ak.jp.nec.com> wrote: >> Nothing are changed in other codes, including something corresponding to >> in-place upgrading. I'm waiting for suggestion. > > I have a question about the behavior -- the patch adds ownership > management of large objects. Non-privileged users cannot read, write, > or drop othere's LOs. But they can read the contents of large object > when they read pg_catalog.pg_largeobject directly. Even if the patch > is applied, we still allow "SELECT * FROM pg_largeobject" ...right? > > This issue might be solved by the core SE-PgSQL patch, > but what should we do fo now? Oops, I forgot to fix it. It is a misconfiguration on database initialization, and not related issue with SE-PgSQL feature. It can be solved with revoking any privileges from anybody in the initdb phase. So, we should inject the following statement for setup_privileges(). REVOKE ALL ON pg_largeobject FROM PUBLIC; In the default PG model, database superuser is an exception in access controls, so he can bypass the checks eventually. Here is no difference, even if he can see pg_largeobject. For unprivileged users, this configuration restricts the way to access large object into lo_*() functions, so we can acquire all their accesses and apply permission checks comprehensively. When database superuser tries to consider something malicious, such as exposing any large object to public, we have to give up anything. > Other changes in the patch seem to be reasonable. > > "GRANT/REVOKE ON LARGE OBJECT <number>" might be hard to use if used alone, > but we can use the commands as dynamic SQLs in DO statements if we want to > grant or revoke privileges in bulk. We already have COMMENT ON LARGE OBJECT <number> IS <comment>; statement :-) > "SELECT oid FROM pg_largeobject_metadata" is used in some places instead of > "SELECT DISTINCT loid FROM pg_largeobject". They return the same result, > but the former will be faster because we don't use DISTINCT. pg_dump will > be slightly accelerated by the new query. > > Regards, > --- > Takahiro Itagaki > NTT Open Source Software Center > > > -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai(a)ak.jp.nec.com>
From: Takahiro Itagaki on 10 Dec 2009 20:16 KaiGai Kohei <kaigai(a)ak.jp.nec.com> wrote: > > we still allow "SELECT * FROM pg_largeobject" ...right? > > It can be solved with revoking any privileges from anybody in the initdb > phase. So, we should inject the following statement for setup_privileges(). > > REVOKE ALL ON pg_largeobject FROM PUBLIC; OK, I'll add the following description in the documentation of pg_largeobject. <structname>pg_largeobject</structname> should not be readable by the public, since the catalog contains data in large objects of all users. <structname>pg_largeobject_metadata</> is a publicly readable catalog that only contains identifiers of large objects. > {"lo_compat_privileges", PGC_SUSET, COMPAT_OPTIONS_PREVIOUS, > gettext_noop("Turn on/off privilege checks on large objects."), The description is true, but gives a confusion because "lo_compat_privileges = on" means "privilege checks are turned off". short desc: Enables backward compatibility in privilege checks on large objects long desc: When turned on, privilege checks on large objects are disabled. Are those descriptions appropriate? Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: KaiGai Kohei on 10 Dec 2009 22:02 Takahiro Itagaki wrote: > KaiGai Kohei <kaigai(a)ak.jp.nec.com> wrote: > >>> we still allow "SELECT * FROM pg_largeobject" ...right? >> It can be solved with revoking any privileges from anybody in the initdb >> phase. So, we should inject the following statement for setup_privileges(). >> >> REVOKE ALL ON pg_largeobject FROM PUBLIC; > > OK, I'll add the following description in the documentation of pg_largeobject. > > <structname>pg_largeobject</structname> should not be readable by the > public, since the catalog contains data in large objects of all users. > <structname>pg_largeobject_metadata</> is a publicly readable catalog > that only contains identifiers of large objects. It makes sense to me. >> {"lo_compat_privileges", PGC_SUSET, COMPAT_OPTIONS_PREVIOUS, >> gettext_noop("Turn on/off privilege checks on large objects."), > > The description is true, but gives a confusion because > "lo_compat_privileges = on" means "privilege checks are turned off". > > short desc: Enables backward compatibility in privilege checks on large objects > long desc: When turned on, privilege checks on large objects are disabled. > > Are those descriptions appropriate? The long description is a bit confusable, because it does not disable all the privilege checks, such as lo_export/lo_import which already checks superuser privilege on execution in the earlier releases. What's your opinion about: long desc: When turned on, privilege checks on large objects perform with backward compatibility as 8.4.x or earlier releases. Thanks, -- OSS Platform Development Division, NEC KaiGai Kohei <kaigai(a)ak.jp.nec.com> -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Takahiro Itagaki on 10 Dec 2009 22:41
KaiGai Kohei <kaigai(a)ak.jp.nec.com> wrote: > What's your opinion about: > long desc: When turned on, privilege checks on large objects perform with > backward compatibility as 8.4.x or earlier releases. I updated the description as your suggest. Applied with minor editorialization, mainly around tab-completion support in psql. # This is my first commit :) Regards, --- Takahiro Itagaki NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |