Re: [HACKERS] sepgsql and materialized views
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 t...@sss.pgh.pa.us: Noah Misch n...@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 Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
Stephen Frost sfr...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
* Alvaro Herrera (alvhe...@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 kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
* Alvaro Herrera (alvhe...@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 signature.asc Description: Digital signature
Re: [HACKERS] sepgsql and materialized views
Tom Lane t...@sss.pgh.pa.us wrote: Noah Misch n...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
On Fri, Feb 08, 2013 at 02:51:40PM +0100, Kohei KaiGai wrote: 2013/2/7 Kevin Grittner kgri...@ymail.com: Kohei KaiGai kai...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
Noah Misch n...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
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 t...@sss.pgh.pa.us: Noah Misch n...@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 kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
2013/2/7 Kevin Grittner kgri...@ymail.com: Kohei KaiGai kai...@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 kai...@kaigai.gr.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
Kohei KaiGai kai...@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
-Kevindiff --git a/contrib/sepgsql/dml.c b/contrib/sepgsql/dml.c
index c3ef2b7..4db5883 100644
--- a/contrib/sepgsql/dml.c
+++ b/contrib/sepgsql/dml.c
@@ -191,6 +191,7 @@ check_relation_privileges(Oid relOid,
switch (relkind)
{
case RELKIND_RELATION:
+ case RELKIND_MATVIEW:
result = sepgsql_avc_check_perms(object,
SEPG_CLASS_DB_TABLE,
required,
@@ -226,7 +227,7 @@ check_relation_privileges(Oid relOid,
/*
* Only columns owned by relations shall be checked
*/
- if (relkind != RELKIND_RELATION)
+ if (relkind != RELKIND_RELATION relkind != RELKIND_MATVIEW)
return true;
/*
diff --git a/contrib/sepgsql/label.c b/contrib/sepgsql/label.c
index a5bdde3..7ebf525 100644
--- a/contrib/sepgsql/label.c
+++ b/contrib/sepgsql/label.c
@@ -764,6 +764,8 @@ exec_object_restorecon(struct selabel_handle * sehnd, Oid catalogId)
objtype = SELABEL_DB_SEQUENCE;
else if (relForm-relkind == RELKIND_VIEW)
objtype = SELABEL_DB_VIEW;
+else if (relForm-relkind == RELKIND_MATVIEW)
+ objtype = SELABEL_DB_MATVIEW;
else
continue; /* no need to assign security label */
@@ -782,7 +784,8 @@ exec_object_restorecon(struct selabel_handle * sehnd, Oid catalogId)
case AttributeRelationId:
attForm = (Form_pg_attribute) GETSTRUCT(tuple);
-if (get_rel_relkind(attForm-attrelid) != RELKIND_RELATION)
+if (get_rel_relkind(attForm-attrelid) != RELKIND_RELATION
+ get_rel_relkind(attForm-attrelid) != RELKIND_MATVIEW)
continue; /* no need to assign security label */
objtype = SELABEL_DB_COLUMN;
diff --git a/contrib/sepgsql/relation.c b/contrib/sepgsql/relation.c
index a277fab..feaecfd 100644
--- a/contrib/sepgsql/relation.c
+++ b/contrib/sepgsql/relation.c
@@ -54,8 +54,8 @@ sepgsql_attribute_post_create(Oid relOid, AttrNumber attnum)
Form_pg_attribute attForm;
/*
- * Only attributes within regular relation have individual security
- * labels.
+ * Only attributes within regular relation or materialized view have
+ * individual security labels.
*/
if (get_rel_relkind(relOid) != RELKIND_RELATION)
return;
@@ -159,7 +159,8 @@ sepgsql_attribute_relabel(Oid relOid, AttrNumber attnum,
ObjectAddress object;
char *audit_name;
- if (get_rel_relkind(relOid) != RELKIND_RELATION)
+ if (get_rel_relkind(relOid) != RELKIND_RELATION
+ get_rel_relkind(relOid) != RELKIND_MATVIEW)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg(cannot set security label on non-regular columns)));
@@ -263,6 +264,10 @@ sepgsql_relation_post_create(Oid relOid)
tclass = SEPG_CLASS_DB_VIEW;
tclass_text = view;
break;
+ case RELKIND_MATVIEW:
+ tclass = SEPG_CLASS_DB_MATVIEW;
+ tclass_text = materialized view; /* TODO: matview? */
+ break;
case RELKIND_INDEX:
/* deal with indexes specially; no need for tclass */
sepgsql_index_modify(relOid);
@@ -301,10 +306,11 @@ sepgsql_relation_post_create(Oid relOid)
SetSecurityLabel(object, SEPGSQL_LABEL_TAG, rcontext);
/*
- * We also assigns a default security label on columns of the new regular
- * tables.
+ * We also assign a default security label on columns of new regular
+ * tables and materialized views.
*/
- if (classForm-relkind == RELKIND_RELATION)
+ if (classForm-relkind == RELKIND_RELATION ||
+ classForm-relkind == RELKIND_MATVIEW)
{
Relation arel;
ScanKeyData akey;
@@ -386,6 +392,9 @@ sepgsql_relation_drop(Oid relOid)
case RELKIND_VIEW:
tclass = SEPG_CLASS_DB_VIEW;
break;
+ case RELKIND_MATVIEW:
+ tclass = SEPG_CLASS_DB_MATVIEW;
+ break;
case RELKIND_INDEX:
/* ignore indexes on toast tables */
if (get_rel_namespace(relOid) == PG_TOAST_NAMESPACE)
@@ -420,7 +429,7 @@ sepgsql_relation_drop(Oid relOid)
}
/*
- * check db_table/sequence/view:{drop} permission
+ * check db_table/sequence/view/matview:{drop} permission
*/
object.classId = RelationRelationId;
object.objectId = relOid;
@@ -436,6 +445,8 @@ sepgsql_relation_drop(Oid relOid)
/*
* check db_column:{drop} permission
+ *
+ * TODO: Anything to do here for materialized views?
*/
if (relkind == RELKIND_RELATION)
{
@@ -489,11 +500,13 @@ sepgsql_relation_relabel(Oid relOid, const char *seclabel)
tclass = SEPG_CLASS_DB_SEQUENCE;
else if (relkind == RELKIND_VIEW)
tclass = SEPG_CLASS_DB_VIEW;
+ else if (relkind == RELKIND_MATVIEW)
+ tclass = SEPG_CLASS_DB_MATVIEW;
else
ereport(ERROR,
Re: [HACKERS] sepgsql and materialized views
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 kgri...@ymail.com: Kohei KaiGai kai...@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
Re: [HACKERS] sepgsql and materialized views
Kohei KaiGai kai...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
2013/2/3 Kevin Grittner kgri...@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 kai...@kaigai.gr.jp
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
Kohei KaiGai kai...@kaigai.gr.jp wrote:
2013/2/3 Kevin Grittner kgri...@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 Im 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
2013/2/4 Kevin Grittner kgri...@ymail.com: Kohei KaiGai kai...@kaigai.gr.jp wrote: 2013/2/3 Kevin Grittner kgri...@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 Im 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
Re: [HACKERS] sepgsql and materialized views
Kohei KaiGai kai...@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
