Thread

1. sepgsql and materialized views

   Kevin Grittner <kgrittn@ymail.com> — 2013-02-03T20:38:39Z

   I'm hoping that someone familiar with sepgsql can review this
   portion of the materialized view patch and comment on whether it is
   the best approach for dealing with the integration of these two
   features.  Basically, the patch as it stands treats a materialized
   view as a table for purposes of sepgsql labels.  I initially
   invented new lables, but Robert suggested that this would make
   materialized views unusable in an SE environment until the
   corresponding labels were added at the OS level.  It seems sane to
   me because a materialized view exists on disk the same as a table,
   but is populated differently -- from a view-like rule.
   
   The portion of the patch which affects the contrib/sepgsql/ tree is
   attached.
   
   Thoughts?
   
   -Kevin

2. Re: sepgsql and materialized views

   Kohei KaiGai <kaigai@kaigai.gr.jp> — 2013-02-04T09:17:18Z

   2013/2/3 Kevin Grittner <kgrittn@ymail.com>:
   > I'm hoping that someone familiar with sepgsql can review this> portion of the materialized view patch and comment on whether it is
   > the best approach for dealing with the integration of these two
   > features.  Basically, the patch as it stands treats a materialized
   > view as a table for purposes of sepgsql labels.  I initially
   > invented new lables, but Robert suggested that this would make
   > materialized views unusable in an SE environment until the
   > corresponding labels were added at the OS level.  It seems sane to
   > me because a materialized view exists on disk the same as a table,
   > but is populated differently -- from a view-like rule.
   >
   > The portion of the patch which affects the contrib/sepgsql/ tree is
   > attached.
   >
   Hi Kevin,
   
   Sorry, I have not been involved this discussion.
   I briefly checked your patch. Indeed, it performs like a table, even though
   it is named materialized-view.
   
   Probably, we have two standpoints.
   
   First, materialized-view shall have a security label corresponding to table,
   and related checks handle references to materialized-views as if user
   references regular-tables. This is an idea.
   I briefly checked your latest patch. ExecRefreshMatView is a unique entry
   point to update a particular materialized-view, and REFRESH MATERIALIZED
   VIEW command is only way to kick this function. It also checks permissions to
   reference underlying tables. So, it means update of materialized-view is a stuff
   like writing a table with contents read from other tables by a particular users.
   
   However, I'm worried whether this design continues forever, or not.
   IIRC, you have a plan to refresh materialized-view asynchronously using
   background worker stuff, don't you? Once we support an internal stuff (thus,
   it can bypass valid security checks) to write out confidential contents into
   unconfidential zone, it does not make sense to keep data confidentiality.
   
   So, I'd like to suggest second way; that handles a materialized-view as a view.
   SELinux checks db_view:{expand} permissions and relevant permissions
   towards underlying tables. I don't think it is hard to implement because
   relation->rd_rules holds Query tree to reference underlying tables.
   
   Can you wait for a week? I'll adjust contrib/sepgsql portion to fit
   materialized-
   view with matter of existing view.
   
   Thanks,
   -- 
   KaiGai Kohei <kaigai@kaigai.gr.jp>
   
   
   

3. Re: sepgsql and materialized views

   Kevin Grittner <kgrittn@ymail.com> — 2013-02-04T15:58:03Z

   Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   > 2013/2/3 Kevin Grittner <kgrittn@ymail.com>:
   >> I'm hoping that someone familiar with sepgsql can review this
   >> portion of the materialized view patch and comment on whether it is
   >> the best approach for dealing with the integration of these two
   >> features.  Basically, the patch as it stands treats a materialized
   >> view as a table for purposes of sepgsql labels.  I initially
   >> invented new lables, but Robert suggested that this would make
   >> materialized views unusable in an SE environment until the
   >> corresponding labels were added at the OS level.  It seems sane to
   >> me because a materialized view exists on disk the same as a table,
   >> but is populated differently -- from a view-like rule.
   >>
   >> The portion of the patch which affects the contrib/sepgsql/ tree is
   >> attached.
   >>
   > Hi Kevin,
   >
   > Sorry, I have not been involved this discussion.
   > I briefly checked your patch. Indeed, it performs like a table, even though
   > it is named materialized-view.
   >
   > Probably, we have two standpoints.
   >
   > First, materialized-view shall have a security label corresponding to table,
   > and related checks handle references to materialized-views as if user
   > references regular-tables. This is an idea.
   > I briefly checked your latest patch. ExecRefreshMatView is a unique entry
   > point to update a particular materialized-view, and REFRESH MATERIALIZED
   > VIEW command is only way to kick this function. It also checks permissions to
   > reference underlying tables. So, it means update of materialized-view is a stuff
   > like writing a table with contents read from other tables by a particular users.
   >
   > However, I'm worried whether this design continues forever, or not.
   > IIRC, you have a plan to refresh materialized-view asynchronously using
   > background worker stuff, don't you?
   
   The goal is to build on this to support other timings of updates to
   the materialized view.  In general, I think this will take the form
   of identifying, for the given rewrite rules associated with the
   materialized view, what changes to the underlying data can affect
   the view and determining whether incremental updates can be
   supported for the MV.  If incremental update is possible, then
   various timings can be chosen for applying them.  I think that all
   timing should go through a queue of change requests, although that
   is not yet designed, and I'm just waving my hands around and
   speculating about any details.  Timings likely to be supported
   range from on-demand incremental updates (of all accumlated
   changes) to applying the changes from a transaction at COMMIT time,
   or possibly as the underlying changes are made within a
   transaction.  I suspect that the most popular timing will involve a
   background process working from the queue asynchronously to keep
   the MV updated asynchronously but without any artificial delay.
   
   In all cases, a query against the view does not reach below the MV
   table to the underlying tables or functions used to build the data
   -- it will always read the "materialized" data, so I"m not sure
   that the normal concerns about leaky views apply here.
   
   > Once we support an internal stuff (thus,
   > it can bypass valid security checks) to write out confidential contents into
   > unconfidential zone, it does not make sense to keep data confidentiality.
   
   If the person who creates the materialized view has authority to
   query the underlying tables, and grants access to the materialized
   view as desired, and those selecting from the materialized view
   only see the contents of the table which is being populated by the
   underlying pg_rewrite rule, I'm not understanding your concern. 
   Can you elaborate?
   
   > So, I'd like to suggest second way; that handles a materialized-view as a
   > view.
   
   I don't see why that should apply to anyone selecting from the MV. 
   Certainly it must apply to anyone creating the MV.  I'm not at all
   clear why anything special would be required for REFRESH or
   updating the underlying tables which will eventually generate
   incremental changes.  I might be missing something, but could you
   explain any risks you see?
   
   > SELinux checks db_view:{expand} permissions and relevant permissions
   > towards underlying tables. I don't think it is hard to implement because
   > relation->rd_rules holds Query tree to reference underlying tables.
   >
   > Can you wait for a week? I'll adjust contrib/sepgsql portion to fit
   > materialized-view with matter of existing view.
   
   Before we code a different alternative, I would like to understand
   why.  What risks do you see, exactly?
   
   -Kevin
   
   
   
   

4. Re: sepgsql and materialized views

   Kohei KaiGai <kaigai@kaigai.gr.jp> — 2013-02-04T17:45:58Z

   2013/2/4 Kevin Grittner <kgrittn@ymail.com>:
   > Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   >> 2013/2/3 Kevin Grittner <kgrittn@ymail.com>:
   >>> I'm hoping that someone familiar with sepgsql can review this
   >>> portion of the materialized view patch and comment on whether it is
   >>> the best approach for dealing with the integration of these two
   >>> features.  Basically, the patch as it stands treats a materialized
   >>> view as a table for purposes of sepgsql labels.  I initially
   >>> invented new lables, but Robert suggested that this would make
   >>> materialized views unusable in an SE environment until the
   >>> corresponding labels were added at the OS level.  It seems sane to
   >>> me because a materialized view exists on disk the same as a table,
   >>> but is populated differently -- from a view-like rule.
   >>>
   >>> The portion of the patch which affects the contrib/sepgsql/ tree is
   >>> attached.
   >>>
   >> Hi Kevin,
   >>
   >> Sorry, I have not been involved this discussion.
   >> I briefly checked your patch. Indeed, it performs like a table, even though
   >> it is named materialized-view.
   >>
   >> Probably, we have two standpoints.
   >>
   >> First, materialized-view shall have a security label corresponding to table,
   >> and related checks handle references to materialized-views as if user
   >> references regular-tables. This is an idea.
   >> I briefly checked your latest patch. ExecRefreshMatView is a unique entry
   >> point to update a particular materialized-view, and REFRESH MATERIALIZED
   >> VIEW command is only way to kick this function. It also checks permissions to
   >> reference underlying tables. So, it means update of materialized-view is a stuff
   >> like writing a table with contents read from other tables by a particular users.
   >>
   >> However, I'm worried whether this design continues forever, or not.
   >> IIRC, you have a plan to refresh materialized-view asynchronously using
   >> background worker stuff, don't you?
   >
   > The goal is to build on this to support other timings of updates to
   > the materialized view.  In general, I think this will take the form
   > of identifying, for the given rewrite rules associated with the
   > materialized view, what changes to the underlying data can affect
   > the view and determining whether incremental updates can be
   > supported for the MV.  If incremental update is possible, then
   > various timings can be chosen for applying them.  I think that all
   > timing should go through a queue of change requests, although that
   > is not yet designed, and I'm just waving my hands around and
   > speculating about any details.  Timings likely to be supported
   > range from on-demand incremental updates (of all accumlated
   > changes) to applying the changes from a transaction at COMMIT time,
   > or possibly as the underlying changes are made within a
   > transaction.  I suspect that the most popular timing will involve a
   > background process working from the queue asynchronously to keep
   > the MV updated asynchronously but without any artificial delay.
   >
   > In all cases, a query against the view does not reach below the MV
   > table to the underlying tables or functions used to build the data
   > -- it will always read the "materialized" data, so I"m not sure
   > that the normal concerns about leaky views apply here.
   >
   Let me confirm a significant point. Do you never plan a feature that
   allows to update/refresh materialized-views out of user session?
   I had an impression on asynchronous update of MV something like
   a feature that moves data from regular tables to MV with batch-jobs
   in mid-night, but under the privilege that bypass regular permission
   checks.
   It it is never planned, my concern was pointless.
   
   >> Once we support an internal stuff (thus,
   >> it can bypass valid security checks) to write out confidential contents into
   >> unconfidential zone, it does not make sense to keep data confidentiality.
   >
   > If the person who creates the materialized view has authority to
   > query the underlying tables, and grants access to the materialized
   > view as desired, and those selecting from the materialized view
   > only see the contents of the table which is being populated by the
   > underlying pg_rewrite rule, I'm not understanding your concern.
   > Can you elaborate?
   >
   My concern is future development that allows to update/refresh MV
   asynchronously, out of privilege control.
   As long as all the update/refresh operation is under privilege control
   with user-id/security label of the current session, here is no difference
   from regular writer operation of tables with contents read from other
   tables.
   Can I explain where is my concern about well?
   
   
   BTW, please clarify expected behavior in case when MV contains
   WHERE clause that returns different result depending on privilege
   of current session, such as:
     ... WHERE underlying_table.uname = CURRENT_USER
   
   It seems to me this MV saves just a snapshot from a standpoint of
   a particular user who refreshed this MV; typically its owner.
   If bob has privilege to reference this MV, he will see rows to be visible
   for alice. Of course, it does not contradictory, because all alice doing
   is just writing data she can see into a table being visible for public.
   
   >> So, I'd like to suggest second way; that handles a materialized-view as a
   >> view.
   >
   > I don't see why that should apply to anyone selecting from the MV.
   > Certainly it must apply to anyone creating the MV.  I'm not at all
   > clear why anything special would be required for REFRESH or
   > updating the underlying tables which will eventually generate
   > incremental changes.  I might be missing something, but could you
   > explain any risks you see?
   >
   Even if MV's contents were moved in out of privilege controls,
   we can ensure the current user has rights to reference data of MV,
   as long as he has privileges to reference underlying data source.
   On the other hand, it can make problems if some internal stuff
   moves data from regular tables with "confidential" label into MV
   with "unconfidential" label; that works official information leak channel.
   
   Only point I'm concerned about is whether we will support a feature
   that refresh materialized-view without appropriate privilege control,
   or not.
   
   Thanks,
   -- 
   KaiGai Kohei <kaigai@kaigai.gr.jp>
   
   
   

5. Re: sepgsql and materialized views

   Kevin Grittner <kgrittn@ymail.com> — 2013-02-05T01:57:50Z

   Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   
   > Let me confirm a significant point. Do you never plan a feature
   > that allows to update/refresh materialized-views out of user
   > session?
   
   Currently only the owner of the MV or a database superuser can
   refresh it, and the refresh is run with the permissions of the MV
   owner.  The main change to that I see is that the owner could
   establish a policy of automatic updates to the MV based on changes
   to the underlying tables, with a timing established by the MV owner
   or a database superuser.
   
   > I had an impression on asynchronous update of MV something like
   > a feature that moves data from regular tables to MV with
   > batch-jobs in mid-night, but under the privilege that bypass
   > regular permission checks.
   
   I would expect it to be more a matter of being based on the
   authority of the MV owner.  That raises interesting questions about
   what happens if a permission which allowed the MV to be defined is
   revoked from the owner, or if the MV is altered to have an owner
   without permission to access the underlying data.  With the current
   patch, if the owner is changed to a user who does not have rights
   to access the underlying table, a "permission denied" error is
   generated when that new owner tries to refresh the MV.
   
   > It it is never planned, my concern was pointless.
   
   I think these are issues require vigilance.  I hadn't really
   thought through all of this, but since the creation and refresh
   work off of the existing VIEW code, and the querying works off of
   the existing TABLE code, the existing security mechanisms tend to
   come into play by default.
   
   > My concern is future development that allows to update/refresh MV
   > asynchronously, out of privilege control.
   
   While it has not yet been defined, my first reaction is that it
   should happen under privileges of the MV owner.
   
   > As long as all the update/refresh operation is under privilege
   > control with user-id/security label of the current session, here
   > is no difference from regular writer operation of tables with
   > contents read from other tables.
   
   Again, it's just my first impression, but I think that the
   permissions of the current session would control whether the
   operation would be allowed on the underlying tables, but the
   permissions of the MV owner would control replication to the MV.
   
   > BTW, please clarify expected behavior in case when MV contains
   > WHERE clause that returns different result depending on privilege
   > of current session, such as:
   >   ... WHERE underlying_table.uname = CURRENT_USER
   
   Ah, good question.  Something else I hadn't thought about.  When I
   read that I was afraid that the current patch left a security hole
   where if the owner didn't have rights to populate the MV with
   something, it could still get there if a database superuser ran
   REFRESH, but it seems to do what I would think is the right thing. 
   With kgrittn as a superuser and bob as a normal user:
   
   test=# set role bob;
   SET
   test=> select * from t;
   ERROR:  permission denied for relation t
   test=> reset role;
   RESET
   test=# alter materialized view tm owner to bob;
   ALTER MATERIALIZED VIEW
   test=# set role bob;
   SET
   test=> refresh MATERIALIZED VIEW tm;
   ERROR:  permission denied for relation t
   test=> reset role;
   RESET
   test=# refresh MATERIALIZED VIEW tm;
   ERROR:  permission denied for relation t
   test=# alter materialized view tm owner to kgrittn;
   ALTER MATERIALIZED VIEW
   test=# refresh MATERIALIZED VIEW tm;
   REFRESH MATERIALIZED VIEW
   
   So it seems to run the refresh as the owner, not under authority of
   the session.
   
   > It seems to me this MV saves just a snapshot from a standpoint of
   > a particular user who refreshed this MV; typically its owner.
   
   Exactly.  Typically a summary of a snapshot of underlying detail.
   
   > If bob has privilege to reference this MV, he will see rows to be
   > visible for alice. Of course, it does not contradictory, because
   > all alice doing is just writing data she can see into a table
   > being visible for public.
   
   Right.  From a security perspective it is rather like alice running
   CREATE TABLE AS.  And again, it is worth remembering that the usual
   reason for creating one of these is to summarize data, to support
   quick generation of statistical reports, for example.
   
   > Even if MV's contents were moved in out of privilege controls,
   > we can ensure the current user has rights to reference data of
   > MV, as long as he has privileges to reference underlying data
   > source.
   
   I don't think it should have anything to do with authority to the
   underlying tables from which the data is selected.
   
   > On the other hand, it can make problems if some internal stuff
   > moves data from regular tables with "confidential" label into MV
   > with "unconfidential" label; that works official information leak
   > channel.
   
   I don't see that it opens any new vulnerabilities compared to
   INSERT ... SELECT or CREATE TABLE AS.
   
   > Only point I'm concerned about is whether we will support a
   > feature that refresh materialized-view without appropriate
   > privilege control, or not.
   
   I guess the question is whether it makes sense to support these
   under sepgsql using table access labels, or whether we need new
   labels and the feature is not usable in environments without those
   labels.  That's what I"m not clear on either.  I first did the
   patch assuming a new label, but changed it based on feedback from
   Robert suggesting we should use the table labels.  I can change it
   back if you like.
   
   -Kevin
   
   
   

6. Re: sepgsql and materialized views

   Kohei KaiGai <kaigai@kaigai.gr.jp> — 2013-02-07T11:06:15Z

   Thanks for your introduction. It made me almost clear.
   
   From selinux perspective, it does not switch user's privilege even when
   query scans underlying tables because it has no ownership concept.
   The current implementation does not make a problem because all the
   MV refresh shall be done in a particular user session, thus, it is also
   associated with a particular security label if sepgsql is enabled.
   So, as you introduced, all we need to do is identical with SELECT ...
   INTO and CREATE TABLE AS from selinux perspective also.
   
   > I think these are issues require vigilance.  I hadn't really
   > thought through all of this, but since the creation and refresh
   > work off of the existing VIEW code, and the querying works off of
   > the existing TABLE code, the existing security mechanisms tend to
   > come into play by default.
   >
   If we will try to support refresh MV out of user session like what
   autovacuum is doing towards vacuum by hand, probably,
   a reasonable design is applying a pseudo session user-id come
   from ownership of MV. I think, similar idea can be applied to
   apply a temporary pseudo security label, as long as it allows
   extensions to get control around these processed.
   Anyway, I'm inclined to be optimistic about this possible issue
   as long as extension can get necessary support such as new
   hooks.
   
   I think it is a reasonable alternative to apply db_table object
   class for materialized-view, right now, because it performs as
   if we are doing onto regular tables.
   However, one exception is here; how to control privilege to
   refresh the MV. Of course, db_table class does not have
   "refresh" operation. Even though the default permission checks
   its ownership (or superuser) at ExecRefreshMatView, but nothing
   are checked in extension side.
   (Of course, it will need a new hook around here, also.)
   
   So, an ideal solution is to add db_materialized_view
   (or db_matview in short) object class in selinux world, then
   we checks "refresh" permission of MV.
   However, it takes more time to have discussion in selinux
   community then shipped to distributions.
   
   So, I'd like to review two options.
   1) we uses db_table object class for materialized-views for
   a while, until selinux-side become ready. Probably, v9.3 will
   use db_table class then switched at v9.4.
   2) we uses db_materialized_view object class from the
   begining, but its permission checks are ignored because
   installed security policy does not support this class yet.
   
   My preference is 2), even though we cannot apply label
   based permission checks until selinux support it, because
   1) makes troubles when selinux-side become ready to
   support new db_materialized_view class. Even though
   policy support MV class, working v9.3 will ignore the policy.
   
   Let me ask selinux folks about this topic also.
   
   Thanks,
   
   2013/2/5 Kevin Grittner <kgrittn@ymail.com>:
   > Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   >
   >> Let me confirm a significant point. Do you never plan a feature
   >> that allows to update/refresh materialized-views out of user
   >> session?
   >
   > Currently only the owner of the MV or a database superuser can
   > refresh it, and the refresh is run with the permissions of the MV
   > owner.  The main change to that I see is that the owner could
   > establish a policy of automatic updates to the MV based on changes
   > to the underlying tables, with a timing established by the MV owner
   > or a database superuser.
   >
   >> I had an impression on asynchronous update of MV something like
   >> a feature that moves data from regular tables to MV with
   >> batch-jobs in mid-night, but under the privilege that bypass
   >> regular permission checks.
   >
   > I would expect it to be more a matter of being based on the
   > authority of the MV owner.  That raises interesting questions about
   > what happens if a permission which allowed the MV to be defined is
   > revoked from the owner, or if the MV is altered to have an owner
   > without permission to access the underlying data.  With the current
   > patch, if the owner is changed to a user who does not have rights
   > to access the underlying table, a "permission denied" error is
   > generated when that new owner tries to refresh the MV.
   >
   >> It it is never planned, my concern was pointless.
   >
   > I think these are issues require vigilance.  I hadn't really
   > thought through all of this, but since the creation and refresh
   > work off of the existing VIEW code, and the querying works off of
   > the existing TABLE code, the existing security mechanisms tend to
   > come into play by default.
   >
   >> My concern is future development that allows to update/refresh MV
   >> asynchronously, out of privilege control.
   >
   > While it has not yet been defined, my first reaction is that it
   > should happen under privileges of the MV owner.
   >
   >> As long as all the update/refresh operation is under privilege
   >> control with user-id/security label of the current session, here
   >> is no difference from regular writer operation of tables with
   >> contents read from other tables.
   >
   > Again, it's just my first impression, but I think that the
   > permissions of the current session would control whether the
   > operation would be allowed on the underlying tables, but the
   > permissions of the MV owner would control replication to the MV.
   >
   >> BTW, please clarify expected behavior in case when MV contains
   >> WHERE clause that returns different result depending on privilege
   >> of current session, such as:
   >>   ... WHERE underlying_table.uname = CURRENT_USER
   >
   > Ah, good question.  Something else I hadn't thought about.  When I
   > read that I was afraid that the current patch left a security hole
   > where if the owner didn't have rights to populate the MV with
   > something, it could still get there if a database superuser ran
   > REFRESH, but it seems to do what I would think is the right thing.
   > With kgrittn as a superuser and bob as a normal user:
   >
   > test=# set role bob;
   > SET
   > test=> select * from t;
   > ERROR:  permission denied for relation t
   > test=> reset role;
   > RESET
   > test=# alter materialized view tm owner to bob;
   > ALTER MATERIALIZED VIEW
   > test=# set role bob;
   > SET
   > test=> refresh MATERIALIZED VIEW tm;
   > ERROR:  permission denied for relation t
   > test=> reset role;
   > RESET
   > test=# refresh MATERIALIZED VIEW tm;
   > ERROR:  permission denied for relation t
   > test=# alter materialized view tm owner to kgrittn;
   > ALTER MATERIALIZED VIEW
   > test=# refresh MATERIALIZED VIEW tm;
   > REFRESH MATERIALIZED VIEW
   >
   > So it seems to run the refresh as the owner, not under authority of
   > the session.
   >
   >> It seems to me this MV saves just a snapshot from a standpoint of
   >> a particular user who refreshed this MV; typically its owner.
   >
   > Exactly.  Typically a summary of a snapshot of underlying detail.
   >
   >> If bob has privilege to reference this MV, he will see rows to be
   >> visible for alice. Of course, it does not contradictory, because
   >> all alice doing is just writing data she can see into a table
   >> being visible for public.
   >
   > Right.  From a security perspective it is rather like alice running
   > CREATE TABLE AS.  And again, it is worth remembering that the usual
   > reason for creating one of these is to summarize data, to support
   > quick generation of statistical reports, for example.
   >
   >> Even if MV's contents were moved in out of privilege controls,
   >> we can ensure the current user has rights to reference data of
   >> MV, as long as he has privileges to reference underlying data
   >> source.
   >
   > I don't think it should have anything to do with authority to the
   > underlying tables from which the data is selected.
   >
   >> On the other hand, it can make problems if some internal stuff
   >> moves data from regular tables with "confidential" label into MV
   >> with "unconfidential" label; that works official information leak
   >> channel.
   >
   > I don't see that it opens any new vulnerabilities compared to
   > INSERT ... SELECT or CREATE TABLE AS.
   >
   >> Only point I'm concerned about is whether we will support a
   >> feature that refresh materialized-view without appropriate
   >> privilege control, or not.
   >
   > I guess the question is whether it makes sense to support these
   > under sepgsql using table access labels, or whether we need new
   > labels and the feature is not usable in environments without those
   > labels.  That's what I"m not clear on either.  I first did the
   > patch assuming a new label, but changed it based on feedback from
   > Robert suggesting we should use the table labels.  I can change it
   > back if you like.
   >
   > -Kevin
   
   
   
   -- 
   KaiGai Kohei <kaigai@kaigai.gr.jp>
   
   
   

7. Re: sepgsql and materialized views

   Kevin Grittner <kgrittn@ymail.com> — 2013-02-07T22:13:22Z

   Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   
   > So, I'd like to review two options.
   > 1) we uses db_table object class for materialized-views for
   > a while, until selinux-side become ready. Probably, v9.3 will
   > use db_table class then switched at v9.4.
   > 2) we uses db_materialized_view object class from the
   > begining, but its permission checks are ignored because
   > installed security policy does not support this class yet.
   >
   > My preference is 2), even though we cannot apply label
   > based permission checks until selinux support it, because
   > 1) makes troubles when selinux-side become ready to
   > support new db_materialized_view class. Even though
   > policy support MV class, working v9.3 will ignore the policy.
   >
   > Let me ask selinux folks about this topic also.
   
   To make sure I understand, the current patch is consistent with
   option 1?  It sounds like I have code from a prior version of the
   patch pretty close to what you describe for option 2, so that can
   be put back in place if you confirm that as the preferred option.
   From what you describe, it sounds like the only thing it doesn't
   have is a new hook for REFRESH, but that doesn't sound like it
   would take that much to implement.
   
   Thanks for looking at this!
   
   -Kevin
   
   
   

8. Re: sepgsql and materialized views

   Kohei KaiGai <kaigai@kaigai.gr.jp> — 2013-02-08T13:51:40Z

   2013/2/7 Kevin Grittner <kgrittn@ymail.com>:
   > Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   >
   >> So, I'd like to review two options.
   >> 1) we uses db_table object class for materialized-views for
   >> a while, until selinux-side become ready. Probably, v9.3 will
   >> use db_table class then switched at v9.4.
   >> 2) we uses db_materialized_view object class from the
   >> begining, but its permission checks are ignored because
   >> installed security policy does not support this class yet.
   >>
   >> My preference is 2), even though we cannot apply label
   >> based permission checks until selinux support it, because
   >> 1) makes troubles when selinux-side become ready to
   >> support new db_materialized_view class. Even though
   >> policy support MV class, working v9.3 will ignore the policy.
   >>
   >> Let me ask selinux folks about this topic also.
   >
   > To make sure I understand, the current patch is consistent with
   > option 1?
   >
   I believe so, even though I didn't take deep and detailed investigation
   yet.
   
   >  It sounds like I have code from a prior version of the
   > patch pretty close to what you describe for option 2, so that can
   > be put back in place if you confirm that as the preferred option.
   >
   As above, I'd like to suggest the option 2.
   Could you once remove the updates related to contrib/sepgsql?
   I'll have a discussion about new materialized_view object class
   on selinux list soon, then I'll submit a patch towards contrib/sepgsql
   according to the consensus here.
   
   > From what you describe, it sounds like the only thing it doesn't
   > have is a new hook for REFRESH, but that doesn't sound like it
   > would take that much to implement.
   >
   I think all we need to give extensions a chance to check permission
   on REFRESH timing is a hook that informs which materialized-view
   shall be refreshed. Probably, OAT_MATERIALIZED_VIEW_RERESH
   event with its oid on object_access_hook is sufficient.
   
   Thanks,
   -- 
   KaiGai Kohei <kaigai@kaigai.gr.jp>
   
   
   

9. Re: sepgsql and materialized views

   Kevin Grittner <kgrittn@ymail.com> — 2013-02-08T20:59:40Z

   Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
   
   > I'll adjust contrib/sepgsql portion to fit materialized-view with
   > matter of existing view.
   
   OK.  In case it is of any use to you as a starting point, attached
   is what I originally had, which seems to be similar to what you
   describe as your preference.  I'll revert everything under
   contrib/sepgsql/ and wait for a patch from you.
   
   If you have something prior to a commit to the community repo, you
   can work against:
   
   https://github.com/kgrittn/postgres/commits/matview
   
   -Kevin

10. Re: sepgsql and materialized views

    Noah Misch <noah@leadboat.com> — 2013-07-05T20:06:11Z

    On Fri, Feb 08, 2013 at 02:51:40PM +0100, Kohei KaiGai wrote:
    > 2013/2/7 Kevin Grittner <kgrittn@ymail.com>:
    > > Kohei KaiGai <kaigai@kaigai.gr.jp> wrote:
    > >
    > >> So, I'd like to review two options.
    > >> 1) we uses db_table object class for materialized-views for
    > >> a while, until selinux-side become ready. Probably, v9.3 will
    > >> use db_table class then switched at v9.4.
    > >> 2) we uses db_materialized_view object class from the
    > >> begining, but its permission checks are ignored because
    > >> installed security policy does not support this class yet.
    > >>
    > >> My preference is 2), even though we cannot apply label
    > >> based permission checks until selinux support it, because
    > >> 1) makes troubles when selinux-side become ready to
    > >> support new db_materialized_view class. Even though
    > >> policy support MV class, working v9.3 will ignore the policy.
    > >>
    > >> Let me ask selinux folks about this topic also.
    > >
    > > To make sure I understand, the current patch is consistent with
    > > option 1?
    > >
    > I believe so, even though I didn't take deep and detailed investigation
    > yet.
    > 
    > >  It sounds like I have code from a prior version of the
    > > patch pretty close to what you describe for option 2, so that can
    > > be put back in place if you confirm that as the preferred option.
    > >
    > As above, I'd like to suggest the option 2.
    > Could you once remove the updates related to contrib/sepgsql?
    > I'll have a discussion about new materialized_view object class
    > on selinux list soon, then I'll submit a patch towards contrib/sepgsql
    > according to the consensus here.
    
    Has this progressed?
    
    Should we consider this a 9.3 release blocker?  sepgsql already has a red box
    warning about its limitations, so adding the limitation that materialized
    views are unrestricted wouldn't be out of the question.
    
    Thanks,
    nm
    
    -- 
    Noah Misch
    EnterpriseDB                                 http://www.enterprisedb.com
    
    
    

11. Re: sepgsql and materialized views

    Tom Lane <tgl@sss.pgh.pa.us> — 2013-07-05T20:40:47Z

    Noah Misch <noah@leadboat.com> writes:
    > On Fri, Feb 08, 2013 at 02:51:40PM +0100, Kohei KaiGai wrote:
    >> I'll have a discussion about new materialized_view object class
    >> on selinux list soon, then I'll submit a patch towards contrib/sepgsql
    >> according to the consensus here.
    
    > Has this progressed?
    
    > Should we consider this a 9.3 release blocker?  sepgsql already has a red box
    > warning about its limitations, so adding the limitation that materialized
    > views are unrestricted wouldn't be out of the question.
    
    Definitely -1 for considering it a release blocker.  If KaiGai-san can
    come up with a fix before we otherwise would release 9.3, that's great,
    but there's no way that sepgsql has a large enough user community to
    justify letting it determine the release schedule.
    
    			regards, tom lane
    
    
    

12. Re: sepgsql and materialized views

    Kohei KaiGai <kaigai@kaigai.gr.jp> — 2013-07-06T05:34:22Z

    Unfortunately, I could not get consensus of design on selinux policy side.
    Even though my opinion is to add individual security class for materialized
    view to implement refresh permission, other people has different opinion.
    So, I don't want it shall be a blocker of v9.3 to avoid waste of time.
    Also, I'll remind selinux community on this issue again, and tries to handle
    in another way from what I proposed before.
    
    Thanks,
    
    2013/7/5 Tom Lane <tgl@sss.pgh.pa.us>:
    > Noah Misch <noah@leadboat.com> writes:
    >> On Fri, Feb 08, 2013 at 02:51:40PM +0100, Kohei KaiGai wrote:
    >>> I'll have a discussion about new materialized_view object class
    >>> on selinux list soon, then I'll submit a patch towards contrib/sepgsql
    >>> according to the consensus here.
    >
    >> Has this progressed?
    >
    >> Should we consider this a 9.3 release blocker?  sepgsql already has a red box
    >> warning about its limitations, so adding the limitation that materialized
    >> views are unrestricted wouldn't be out of the question.
    >
    > Definitely -1 for considering it a release blocker.  If KaiGai-san can
    > come up with a fix before we otherwise would release 9.3, that's great,
    > but there's no way that sepgsql has a large enough user community to
    > justify letting it determine the release schedule.
    >
    >                         regards, tom lane
    
    
    
    -- 
    KaiGai Kohei <kaigai@kaigai.gr.jp>
    
    
    

13. Re: sepgsql and materialized views

    Kevin Grittner <kgrittn@ymail.com> — 2013-07-08T14:19:04Z

    Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Noah Misch <noah@leadboat.com> writes:
    >> On Fri, Feb 08, 2013 at 02:51:40PM +0100, Kohei KaiGai wrote:
    >>> I'll have a discussion about new materialized_view object class
    >>> on selinux list soon, then I'll submit a patch towards
    >>> contrib/sepgsql according to the consensus here.
    >
    >> Has this progressed?
    >>
    >> Should we consider this a 9.3 release blocker?  sepgsql already has a red
    >> box warning about its limitations, so adding the limitation that materialized
    >> views are unrestricted wouldn't be out of the question.
    >
    > Definitely -1 for considering it a release blocker.  If KaiGai-san can
    > come up with a fix before we otherwise would release 9.3, that's great,
    > but there's no way that sepgsql has a large enough user community to
    > justify letting it determine the release schedule.
    
    Agreed.  I posted (many months ago) a proposed version which
    treated them as being subject to the same security labels as
    tables, and another which created new security lables for
    materialized views.  I'm not aware of any third option, but I sure
    don't feel like I'm in a position to determine which is better (or
    whether someone has a third idea), and I don't think we can hold up
    the PostgreSQL release waiting for the security community to
    choose.
    
    --
    Kevin Grittner
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
    

14. Re: sepgsql and materialized views

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2015-03-09T22:40:48Z

    Kohei KaiGai wrote:
    > Unfortunately, I could not get consensus of design on selinux policy side.
    > Even though my opinion is to add individual security class for materialized
    > view to implement refresh permission, other people has different opinion.
    > So, I don't want it shall be a blocker of v9.3 to avoid waste of time.
    > Also, I'll remind selinux community on this issue again, and tries to handle
    > in another way from what I proposed before.
    
    Did we get this fixed?
    
    
    > 2013/7/5 Tom Lane <tgl@sss.pgh.pa.us>:
    > > Noah Misch <noah@leadboat.com> writes:
    > >> On Fri, Feb 08, 2013 at 02:51:40PM +0100, Kohei KaiGai wrote:
    > >>> I'll have a discussion about new materialized_view object class
    > >>> on selinux list soon, then I'll submit a patch towards contrib/sepgsql
    > >>> according to the consensus here.
    > >
    > >> Has this progressed?
    > >
    > >> Should we consider this a 9.3 release blocker?  sepgsql already has a red box
    > >> warning about its limitations, so adding the limitation that materialized
    > >> views are unrestricted wouldn't be out of the question.
    > >
    > > Definitely -1 for considering it a release blocker.  If KaiGai-san can
    > > come up with a fix before we otherwise would release 9.3, that's great,
    > > but there's no way that sepgsql has a large enough user community to
    > > justify letting it determine the release schedule.
    
    
    -- 
    Álvaro Herrera                http://www.2ndQuadrant.com/
    PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
    
    
    

15. Re: sepgsql and materialized views

    Stephen Frost <sfrost@snowman.net> — 2015-03-10T01:16:37Z

    * Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
    > Kohei KaiGai wrote:
    > > Unfortunately, I could not get consensus of design on selinux policy side.
    > > Even though my opinion is to add individual security class for materialized
    > > view to implement refresh permission, other people has different opinion.
    > > So, I don't want it shall be a blocker of v9.3 to avoid waste of time.
    > > Also, I'll remind selinux community on this issue again, and tries to handle
    > > in another way from what I proposed before.
    > 
    > Did we get this fixed?
    
    I don't think so, but it's something I'm interested in and have an
    envrionment where I can work on it.
    
    Will look into it and try to provide an update soon.
    
    Any further thoughts or suggestions would be appreciated.
    
    	Thanks!
    
    		Stephen
    

16. Re: sepgsql and materialized views

    Kouhei Kaigai <kaigai@ak.jp.nec.com> — 2015-03-10T03:51:46Z

    > * Alvaro Herrera (alvherre@2ndquadrant.com) wrote:
    > > Kohei KaiGai wrote:
    > > > Unfortunately, I could not get consensus of design on selinux policy side.
    > > > Even though my opinion is to add individual security class for materialized
    > > > view to implement refresh permission, other people has different opinion.
    > > > So, I don't want it shall be a blocker of v9.3 to avoid waste of time.
    > > > Also, I'll remind selinux community on this issue again, and tries to handle
    > > > in another way from what I proposed before.
    > >
    > > Did we get this fixed?
    > 
    > I don't think so, but it's something I'm interested in and have an
    > envrionment where I can work on it.
    > 
    > Will look into it and try to provide an update soon.
    > 
    > Any further thoughts or suggestions would be appreciated.
    >
    Ah, yes, the issue has been kept unhandled.
    
    May I remind selinux folks again, to add "db_materialized_view" class?
    Or, Stephan, do you have idea to apply equivalent checks on refresh
    operation?
    
    Thanks,
    --
    NEC OSS Promotion Center / PG-Strom Project
    KaiGai Kohei <kaigai@ak.jp.nec.com>
    
    
    

17. Re: sepgsql and materialized views

    Kevin Grittner <kgrittn@ymail.com> — 2015-03-10T05:25:12Z

    Stephen Frost <sfrost@snowman.net> wrote:
    
    > Will look into it and try to provide an update soon.
    >
    > Any further thoughts or suggestions would be appreciated.
    
    My recollection is that there were two ways that were being
    considered, and I posted a patch for each of them, but the security
    community couldn't reach a consensus, so neither was pushed.  Of
    course someone might come up with one or more other ideas, but
    barring that it might just be a matter of finding the right patch
    in the archives and curing any bit rot.
    
    --
    Kevin Grittner
    EDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company