Обсуждение: RLS related docs
Please see attached two proposed patches for the docs related to RLS: 1) Correction to pg_restore 2) Additional mentions that "COPY FROM" does not allow RLS to be enabled Comments? Related question: I believe COPY tbl TO ... is internally converted to COPY (select * FROM tbl) TO ... when RLS is involved. Do we want to document that? Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
Вложения
On 25 May 2016 at 02:04, Joe Conway <mail@joeconway.com> wrote: > Please see attached two proposed patches for the docs related to RLS: > > 1) Correction to pg_restore > 2) Additional mentions that "COPY FROM" does not allow RLS to be enabled > > Comments? > The pg_restore change looks good -- that was clearly wrong. Also, +1 for the new note in pg_dump. For COPY, I think perhaps it would be more logical to put the new note immediately after the third note which describes the privileges required, since it's kind of related, and then we can talk about the RLS policies required, e.g.: If row-level security is enabled for the table, COPY table TO is internally converted to COPY (SELECT * FROM table)TO, and the relevant security policies are applied. Currently, COPY FROM is not supported for tables with row-levelsecurity. > Related question: I believe > > COPY tbl TO ... > > is internally converted to > > COPY (select * FROM tbl) TO ... > > when RLS is involved. Do we want to document that? > I think so, yes, because that makes it clearer what policies will be applied. Regards, Dean
On 05/26/2016 12:26 AM, Dean Rasheed wrote: > On 25 May 2016 at 02:04, Joe Conway <mail@joeconway.com> wrote: >> Please see attached two proposed patches for the docs related to RLS: >> >> 1) Correction to pg_restore >> 2) Additional mentions that "COPY FROM" does not allow RLS to be enabled >> >> Comments? >> > > The pg_restore change looks good -- that was clearly wrong. > > Also, +1 for the new note in pg_dump. Great, thanks! > For COPY, I think perhaps it would be more logical to put the new note > immediately after the third note which describes the privileges > required, since it's kind of related, and then we can talk about the > RLS policies required, e.g.: > > If row-level security is enabled for the table, COPY table TO is > internally converted to COPY (SELECT * FROM table) TO, and the > relevant security policies are applied. Currently, COPY FROM is not > supported for tables with row-level security. This sounds better than what I had, so I will do it that way. Thanks, Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
On 05/30/2016 01:56 PM, Joe Conway wrote: > On 05/26/2016 12:26 AM, Dean Rasheed wrote: >> On 25 May 2016 at 02:04, Joe Conway <mail@joeconway.com> wrote: >>> Please see attached two proposed patches for the docs related to RLS: >>> >>> 1) Correction to pg_restore >>> 2) Additional mentions that "COPY FROM" does not allow RLS to be enabled >>> >>> Comments? >>> >> >> The pg_restore change looks good -- that was clearly wrong. >> >> Also, +1 for the new note in pg_dump. > > Great, thanks! > >> For COPY, I think perhaps it would be more logical to put the new note >> immediately after the third note which describes the privileges >> required, since it's kind of related, and then we can talk about the >> RLS policies required, e.g.: >> >> If row-level security is enabled for the table, COPY table TO is >> internally converted to COPY (SELECT * FROM table) TO, and the >> relevant security policies are applied. Currently, COPY FROM is not >> supported for tables with row-level security. > > This sounds better than what I had, so I will do it that way. Apologies for the delay, but new patch attached. Assuming no more comments, will commit this, backpatched to 9.5, in a day or two. Thanks, Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
Вложения
On 28 August 2016 at 21:23, Joe Conway <mail@joeconway.com> wrote: > Apologies for the delay, but new patch attached. Assuming no more > comments, will commit this, backpatched to 9.5, in a day or two. > Looking at this again, I think there is something fishy about these dump/restore flags. If you do pg_dump --enable-row-security, then row_security is turned on during the dump and only the user-visible portions of the tables are dumped. But why does such a dump emit "SET row_security = on;" as part of the dump? There doesn't appear to be any reason for having row_security turned on during the restore just because it was on during the dump. The INSERT policies may well be different from the SELECT policies, and so this may lead to a dump that cannot be restored. ISTM that row_security should be off inside the dump, and only enabled during restore if the user explicitly asks for it, regardless of what setting was used to produce the dump. Also, isn't it the case that --enable-row-security during pg_restore is only relevant when performing a data-only restore (like --disable-triggers). Otherwise, it looks to me as though the restore will create the tables, restore the data, and then only at the end restore the table policies and enable row level security on the tables. So it looks like the flag would have no effect (and a COPY-format dump would work fine) for a non-data-only dump. I never really looked at the RLS dump/restore code. Am I missing something? Regards, Dean
On Tue, Aug 30, 2016 at 3:05 AM, Dean Rasheed <dean.a.rasheed@gmail.com> wrote: > On 28 August 2016 at 21:23, Joe Conway <mail@joeconway.com> wrote: >> Apologies for the delay, but new patch attached. Assuming no more >> comments, will commit this, backpatched to 9.5, in a day or two. > > Looking at this again, I think there is something fishy about these > dump/restore flags. > > If you do pg_dump --enable-row-security, then row_security is turned > on during the dump and only the user-visible portions of the tables > are dumped. But why does such a dump emit "SET row_security = on;" as > part of the dump? There doesn't appear to be any reason for having > row_security turned on during the restore just because it was on > during the dump. The INSERT policies may well be different from the > SELECT policies, and so this may lead to a dump that cannot be > restored. ISTM that row_security should be off inside the dump, and > only enabled during restore if the user explicitly asks for it, > regardless of what setting was used to produce the dump. I think you are right about this. > Also, isn't it the case that --enable-row-security during pg_restore > is only relevant when performing a data-only restore (like > --disable-triggers). Otherwise, it looks to me as though the restore > will create the tables, restore the data, and then only at the end > restore the table policies and enable row level security on the > tables. So it looks like the flag would have no effect (and a > COPY-format dump would work fine) for a non-data-only dump. Hmm. That seems odd. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Sun, Aug 28, 2016 at 4:23 PM, Joe Conway <mail@joeconway.com> wrote: >>> For COPY, I think perhaps it would be more logical to put the new note >>> immediately after the third note which describes the privileges >>> required, since it's kind of related, and then we can talk about the >>> RLS policies required, e.g.: >>> >>> If row-level security is enabled for the table, COPY table TO is >>> internally converted to COPY (SELECT * FROM table) TO, and the >>> relevant security policies are applied. Currently, COPY FROM is not >>> supported for tables with row-level security. >> >> This sounds better than what I had, so I will do it that way. > > Apologies for the delay, but new patch attached. Assuming no more > comments, will commit this, backpatched to 9.5, in a day or two. I don't think this was ever committed, but my comment is that it seems to be exposing rather more of the implementation than is probably wise. Can't we say that SELECT policies will apply rather than saying that it is internally converted to a SELECT? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 09/15/2016 01:33 PM, Robert Haas wrote: > On Sun, Aug 28, 2016 at 4:23 PM, Joe Conway <mail@joeconway.com> wrote: >>>> For COPY, I think perhaps it would be more logical to put the new note >>>> immediately after the third note which describes the privileges >>>> required, since it's kind of related, and then we can talk about the >>>> RLS policies required, e.g.: >>>> >>>> If row-level security is enabled for the table, COPY table TO is >>>> internally converted to COPY (SELECT * FROM table) TO, and the >>>> relevant security policies are applied. Currently, COPY FROM is not >>>> supported for tables with row-level security. >>> >>> This sounds better than what I had, so I will do it that way. >> >> Apologies for the delay, but new patch attached. Assuming no more >> comments, will commit this, backpatched to 9.5, in a day or two. > > I don't think this was ever committed, but my comment is that it seems > to be exposing rather more of the implementation than is probably > wise. Can't we say that SELECT policies will apply rather than saying > that it is internally converted to a SELECT? I've not committed it yet because I was going to look into the new info Dean mentioned first. Seems like your wording is fine, so will make that change. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development
On 09/15/2016 02:34 PM, Joe Conway wrote: > On 09/15/2016 01:33 PM, Robert Haas wrote: >> On Sun, Aug 28, 2016 at 4:23 PM, Joe Conway <mail@joeconway.com> wrote: >>>>> For COPY, I think perhaps it would be more logical to put the new note >>>>> immediately after the third note which describes the privileges >>>>> required, since it's kind of related, and then we can talk about the >>>>> RLS policies required, e.g.: >>>>> >>>>> If row-level security is enabled for the table, COPY table TO is >>>>> internally converted to COPY (SELECT * FROM table) TO, and the >>>>> relevant security policies are applied. Currently, COPY FROM is not >>>>> supported for tables with row-level security. >>>> >>>> This sounds better than what I had, so I will do it that way. >>> >>> Apologies for the delay, but new patch attached. Assuming no more >>> comments, will commit this, backpatched to 9.5, in a day or two. >> >> I don't think this was ever committed, but my comment is that it seems >> to be exposing rather more of the implementation than is probably >> wise. Can't we say that SELECT policies will apply rather than saying >> that it is internally converted to a SELECT? Committed that way, backpatched to 9.5. Joe -- Crunchy Data - http://crunchydata.com PostgreSQL Support for Secure Enterprises Consulting, Training, & Open Source Development