Re: sepgsql and materialized views

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

2013/2/3 Kevin Grittner <kgrittn(at)ymail(dot)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(at)kaigai(dot)gr(dot)jp>

Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
> 2013/2/3 Kevin Grittner <kgrittn(at)ymail(dot)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

2013/2/4 Kevin Grittner <kgrittn(at)ymail(dot)com>:
> Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp> wrote:
>> 2013/2/3 Kevin Grittner <kgrittn(at)ymail(dot)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(at)kaigai(dot)gr(dot)jp>

Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)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

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(at)ymail(dot)com>:
> Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)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(at)kaigai(dot)gr(dot)jp>

Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)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

2013/2/7 Kevin Grittner <kgrittn(at)ymail(dot)com>:
> Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)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(at)kaigai(dot)gr(dot)jp>

Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)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

On Fri, Feb 08, 2013 at 02:51:40PM +0100, Kohei KaiGai wrote:
> 2013/2/7 Kevin Grittner <kgrittn(at)ymail(dot)com>:
> > Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)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

Noah Misch <noah(at)leadboat(dot)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

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(at)sss(dot)pgh(dot)pa(dot)us>:
> Noah Misch <noah(at)leadboat(dot)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(at)kaigai(dot)gr(dot)jp>

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Noah Misch <noah(at)leadboat(dot)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

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(at)sss(dot)pgh(dot)pa(dot)us>:
> > Noah Misch <noah(at)leadboat(dot)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

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)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

> * Alvaro Herrera (alvherre(at)2ndquadrant(dot)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(at)ak(dot)jp(dot)nec(dot)com>

Stephen Frost <sfrost(at)snowman(dot)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