Re: [HACKERS] security label support, part.2
2010/8/25 KaiGai Kohei kai...@ak.jp.nec.com: 7. I think we need to write and include in the fine documentation some big picture documentation about enhanced security providers. Of course, we have to decide what we want to say. But the SECURITY LABEL documentation is just kind of hanging out there in space right now; it needs to connect to a broad introduction to the subject. OK, I'll try to describe with appropriate granularity. Do we need an independent section in addition to the introduction of SECURITY LABEL syntax? I think so. I suggest a new chapter called Enhanced Security Providers just after Database Roles and Privileges. OK, Now I'm under describing the new chapter. http://git.postgresql.org/gitweb?p=users/kaigai/sepgsql.git;a=blob;f=doc/src/sgml/esp.sgml;hb=devel/seclabel However, I'm wondering whether the topic about security hooks and some others are appropriate for the III. Server Administration part. Perhaps, it is a good idea a new section at the last of Database Roles and Privileges which introduce a fact that PostgreSQL allows plugins to make access control decision, and a new chapter in the VII. Internals part. How about the idea? Well, I prefer what I suggested, but of course I'm biased. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
7. I think we need to write and include in the fine documentation some big picture documentation about enhanced security providers. Of course, we have to decide what we want to say. But the SECURITY LABEL documentation is just kind of hanging out there in space right now; it needs to connect to a broad introduction to the subject. OK, I'll try to describe with appropriate granularity. Do we need an independent section in addition to the introduction of SECURITY LABEL syntax? I think so. I suggest a new chapter called Enhanced Security Providers just after Database Roles and Privileges. OK, Now I'm under describing the new chapter. http://git.postgresql.org/gitweb?p=users/kaigai/sepgsql.git;a=blob;f=doc/src/sgml/esp.sgml;hb=devel/seclabel However, I'm wondering whether the topic about security hooks and some others are appropriate for the III. Server Administration part. Perhaps, it is a good idea a new section at the last of Database Roles and Privileges which introduce a fact that PostgreSQL allows plugins to make access control decision, and a new chapter in the VII. Internals part. How about the idea? Thanks, -- 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] security label support, part.2
On tis, 2010-08-17 at 20:04 -0400, Stephen Frost wrote: What I'm thinking of is something like a warning if the permissions on the child don't match those of the parent when the relationship is created, or maybe forcibly setting the permissions on the child (with a NOTICE), so it's at least clear what is going on. Or perhaps, set the permissions on the child only if it doesn't have permissions (with the NOTICE), and issue a WARNING if the child already has permissions set. Perhaps also a WARNING if someone changes the permissions on a child after the relationship has been created too, but let it happen in case someone really wants it.. I think there are perfectly good reasons to have different permissions on parent and child tables. I don't see any reason to monkey around with that. -- 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] security label support, part.2
* Peter Eisentraut (pete...@gmx.net) wrote: I think there are perfectly good reasons to have different permissions on parent and child tables. I don't see any reason to monkey around with that. Even though the permissions on the child table aren't invovled at all if queried through the parent..? The parent implicitly adds to the set of privileges which are granted on the child, but that's not clear at all from the permissions visible on the child. That's principally what I'm complaining about here. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
On sön, 2010-08-22 at 15:08 -0400, Stephen Frost wrote: * Peter Eisentraut (pete...@gmx.net) wrote: I think there are perfectly good reasons to have different permissions on parent and child tables. I don't see any reason to monkey around with that. Even though the permissions on the child table aren't invovled at all if queried through the parent..? The parent implicitly adds to the set of privileges which are granted on the child, but that's not clear at all from the permissions visible on the child. That's principally what I'm complaining about here. Perhaps this is a user interface issue then. Maybe the fact that a table is inherited from another one needs to be shown closer to whereever the permissions are listed. -- 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] security label support, part.2
* Peter Eisentraut (pete...@gmx.net) wrote: On sön, 2010-08-22 at 15:08 -0400, Stephen Frost wrote: Even though the permissions on the child table aren't invovled at all if queried through the parent..? The parent implicitly adds to the set of privileges which are granted on the child, but that's not clear at all from the permissions visible on the child. That's principally what I'm complaining about here. Perhaps this is a user interface issue then. Maybe the fact that a table is inherited from another one needs to be shown closer to whereever the permissions are listed. That's a nice idea, except that we've got a pretty well defined API regarding how to determine what the privileges on a table are, and many different UIs which use it. Fixing it in psql (if it needs to be.. iirc, \d or \d+ may already show it) doesn't really address the problem, in my view. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
2010/8/18 KaiGai Kohei kai...@ak.jp.nec.com: It's also worth pointing out that the hook in ExecCheckRTPerms() does not presuppose label-based security. It could be used to implement some other policy altogether, which only strengthens the argument that we can't know how the user of the hook wants to handle these cases. If rte-requiredPerms would not be cleared, the user of the hook will be able to check access rights on the child tables, as they like. How about an idea to add a new flag in RangeTblEntry which shows where the RangeTblEntry came from, instead of clearing requiredPerms? If the flag is true, I think ExecCheckRTEPerms() can simply skip checks on the child tables. Something along those lines might work, although I haven't yet scrutinized the code well enough to have a real clear opinion on what the best way of dealing with this is. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
Robert, * Robert Haas (robertmh...@gmail.com) wrote: If C1, C2, and C3 inherit from P, it's perfectly reasonable to grant permissions to X on C1 and C2, Y on C3, and Z on C1, C2, C3, and P. I don't think we should disallow that. Sure, it's possible to do things that are less sane, but if we put ourselves in the business of removing useful functionality because it might be misused, we'll put ourselves out of business. Having said that, I'm not sure that the same arguments really hold water in the world of label based security. Suppose we have compartmentalized security: P is a table of threats, with C1 containing data on nukes, C2 containing data on terrorists, and C3 containing data on foreign militaries. If we create a label for each of these threat types, we can apply that label to the corresponding table; but what label shall we assign P? Logically, the label for P should be set up in such a fashion that the only people who can read P are those who can read C1, C2, and C3 anyway, but who is to say that such a label exists? Even if KaiGai's intended implementation of SE-PostgreSQL supports construction of such a label, who is to say that EVERY conceivable labeling system will also do so? I don't see why using labels in the second case changes anything. Consider roles. If you only had a role that could see threats, a role that could see nukes, and a role that could see terrorists, but no role that could see all of them, it's the same problem. Additionally, this kind of problem *isn't* typically addressed with the semantics or the structure of inheiritance- it's done with row-level security and is completely orthogonal to the inheiritance issue. Imagine a new table, C4, is added to P and the admin configures it such that only the 'view_c4' role has access to that child table directly. Now, Z can see what's in C4 through P, even though Z doesn't have access to C4. In the old system, if Z's query happened to hit C4, the whole query would fail but at least Z wouldn't see any C4 data. Other queries on P done by Z would be fine, so long as they didn't hit C4. In fact, it seems to me that it might be far more reasonable, in a case like this, to ignore the *parent* label and look only at each *child* label, which to me is an argument that we should set this up so as to allow individual users of this hook to do as they like. I think it'd be more reasonable to do this for inheiritance in general, but the problem is that people use it for partitioning, and there is a claim out there that it's against what the SQL spec says. The folks using inheiritance for partitioning would probably prefer to not have to deal with setting up the permissions on the child tables. I think that's less of an issue now, but I didn't like the previous behavior where certain queries would work and certain queries wouldn't work against the parent table, either. It's also worth pointing out that the hook in ExecCheckRTPerms() does not presuppose label-based security. It could be used to implement some other policy altogether, which only strengthens the argument that we can't know how the user of the hook wants to handle these cases. This comes back around, in my view, to the distinction between really using inheiritance for inheiritance, vs using it for partitioning. If it's used for partitioning (which certainly seems to be the vast majority of the cases I've seen it used) then I think it should really be considered and viewed as a single object to the authentication system. I don't suppose we're going to get rid of inheiritance for inheiritance any time soon though. In the end, I'm thinking that if the external security module wants to enforce a check against all the children of a parent, they could quite possibly handle that already and do it in such a way that it won't break depending on the specific query. To wit, it could query the catalog to determine if the current table is a parent of any children, and if so, go check the labels/permissions/etc on those children. I'd much rather have something where the permissions check either succeeds or fails against the parent, depending on the permissions of the parent and its children, than on what the query is itself and what conditionals are applied to it. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote: If rte-requiredPerms would not be cleared, the user of the hook will be able to check access rights on the child tables, as they like. This would only be the case for those children which are being touched in the current query, which would depend on what conditionals are applied, what the current setting of check_constraints is, and possibly other factors. I do *not* like this approach. How about an idea to add a new flag in RangeTblEntry which shows where the RangeTblEntry came from, instead of clearing requiredPerms? If the flag is true, I think ExecCheckRTEPerms() can simply skip checks on the child tables. How about the external module just checks if the current object being queried has parents, and if so, goes and checks the labels/permissions/etc on those children? That way the query either always fails or never fails for a given caller, rather than sometimes working and sometimes not depending on the query. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
On Wed, Aug 18, 2010 at 8:49 AM, Stephen Frost sfr...@snowman.net wrote: In the end, I'm thinking that if the external security module wants to enforce a check against all the children of a parent, they could quite possibly handle that already and do it in such a way that it won't break depending on the specific query. To wit, it could query the catalog to determine if the current table is a parent of any children, and if so, go check the labels/permissions/etc on those children. I'd much rather have something where the permissions check either succeeds or fails against the parent, depending on the permissions of the parent and its children, than on what the query is itself and what conditionals are applied to it. Interesting idea. Again, I haven't read the code, but seems worth further investigation, at least. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
(2010/08/18 21:52), Stephen Frost wrote: * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: If rte-requiredPerms would not be cleared, the user of the hook will be able to check access rights on the child tables, as they like. This would only be the case for those children which are being touched in the current query, which would depend on what conditionals are applied, what the current setting of check_constraints is, and possibly other factors. I do *not* like this approach. Indeed, the planner might omit scan on the children which are not obviously referenced, but I'm not certain whether its RangeTblEntry would be also removed from the PlannedStmt-rtable, or not. How about an idea to add a new flag in RangeTblEntry which shows where the RangeTblEntry came from, instead of clearing requiredPerms? If the flag is true, I think ExecCheckRTEPerms() can simply skip checks on the child tables. How about the external module just checks if the current object being queried has parents, and if so, goes and checks the labels/permissions/etc on those children? That way the query either always fails or never fails for a given caller, rather than sometimes working and sometimes not depending on the query. Hmm, this idea may be feasible. The RangeTblEntry-inh flag of the parent will give us a hint whether we also should check labels on its children. Thanks, -- 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] security label support, part.2
How about an idea to add a new flag in RangeTblEntry which shows where
the RangeTblEntry came from, instead of clearing requiredPerms?
If the flag is true, I think ExecCheckRTEPerms() can simply skip checks
on the child tables.
How about the external module just checks if the current object being
queried has parents, and if so, goes and checks the
labels/permissions/etc on those children? That way the query either
always fails or never fails for a given caller, rather than sometimes
working and sometimes not depending on the query.
Hmm, this idea may be feasible. The RangeTblEntry-inh flag of the parent
will give us a hint whether we also should check labels on its children.
http://code.google.com/p/sepgsql/source/browse/trunk/sepgsql/relation.c#293
At least, it seems to me this logic works as expected.
postgres=# CREATE TABLE tbl_p (a int, b text);
CREATE TABLE
postgres=# CREATE TABLE tbl_1 (check (a 100)) inherits (tbl_p);
CREATE TABLE
postgres=# CREATE TABLE tbl_2 (check (a = 100 and a 200)) inherits (tbl_p);
CREATE TABLE
postgres=# CREATE TABLE tbl_3 (check (a = 300)) inherits (tbl_p);
CREATE TABLE
postgres=# SECURITY LABEL on TABLE tbl_p IS
'system_u:object_r:sepgsql_table_t:s0';
SECURITY LABEL
postgres=# SECURITY LABEL on COLUMN tbl_p.a IS
'system_u:object_r:sepgsql_table_t:s0';
SECURITY LABEL
postgres=# SECURITY LABEL on COLUMN tbl_p.b IS
'system_u:object_r:sepgsql_table_t:s0';
SECURITY LABEL
postgres=# set sepgsql_debug_audit = on;
SET
postgres=# SELECT a FROM ONLY tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_table name=tbl_p
STATEMENT: SELECT a FROM ONLY tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_column name=tbl_p.a
STATEMENT: SELECT a FROM ONLY tbl_p WHERE a = 150;
a
---
(0 rows)
- ONLY tbl_p was not expanded
postgres=# SELECT a FROM tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_table name=tbl_p
STATEMENT: SELECT a FROM tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_column name=tbl_p.a
STATEMENT: SELECT a FROM tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_table name=tbl_1
STATEMENT: SELECT a FROM tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_column name=tbl_1.a
STATEMENT: SELECT a FROM tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_table name=tbl_2
STATEMENT: SELECT a FROM tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_column name=tbl_2.a
STATEMENT: SELECT a FROM tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_table name=tbl_3
STATEMENT: SELECT a FROM tbl_p WHERE a = 150;
LOG: SELinux: allowed { select }
scontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
tcontext=system_u:object_r:sepgsql_table_t:s0 tclass=db_column name=tbl_3.a
STATEMENT: SELECT a FROM tbl_p WHERE a = 150;
a
---
(0 rows)
- tbl_p was expanded to tbl_1, tbl_2 and tbl_3
postgres=# set sepgsql_debug_audit = off;
SET
postgres=# EXPLAIN SELECT a FROM tbl_p WHERE a = 150;
QUERY PLAN
Result (cost=0.00..50.75 rows=12 width=4)
- Append (cost=0.00..50.75 rows=12 width=4)
- Seq Scan on tbl_p (cost=0.00..25.38 rows=6 width=4)
Filter: (a = 150)
- Seq Scan on tbl_2 tbl_p (cost=0.00..25.38 rows=6 width=4)
Filter: (a = 150)
(6 rows)
- Actually, it does not scan tbl_1 and tbl_3 due to the a = 150.
--
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] security label support, part.2
Stephen Frost sfr...@snowman.net wrote: Let's not build the complication of dealing with inheiritance/ child relations into the external security system when we're in the middle of trying to get rid of that distinction in core PG though. I didn't realize we were trying to do that. I know we're working on making partitioning easier, but there hasn't been a decision to stop supporting other uses of inheritance, has there? -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] security label support, part.2
Stephen Frost sfr...@snowman.net wrote: No.. and I'm not sure we ever would. What we *have* done is removed all permissions checking on child tables when a parent is being queried.. OK, that clarifies things. Thanks. So, essentially that means that you need to set all ancestor levels to something at least as strict as the intersection of all the permissions on lower levels to avoid exposing something through an ancestor which is restricted in a descendant. And you'd better trust the owner of any table you extend, because they can bypass any attempt to restrict access to the table you create which extends theirs. I hope those security implications are well documented. -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] security label support, part.2
* Kevin Grittner (kevin.gritt...@wicourts.gov) wrote: Stephen Frost sfr...@snowman.net wrote: Let's not build the complication of dealing with inheiritance/ child relations into the external security system when we're in the middle of trying to get rid of that distinction in core PG though. I didn't realize we were trying to do that. I know we're working on making partitioning easier, but there hasn't been a decision to stop supporting other uses of inheritance, has there? No.. and I'm not sure we ever would. What we *have* done is removed all permissions checking on child tables when a parent is being queried.. Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
On Tue, Aug 17, 2010 at 1:50 PM, Stephen Frost sfr...@snowman.net wrote: No.. and I'm not sure we ever would. What we *have* done is removed all permissions checking on child tables when a parent is being queried.. Yeah. I'm not totally sure that is sensible for a MAC environment. Heck, it's arguably incorrect (though perhaps quite convenient) in a DAC environment. Anyway, I wonder if it would be sensible to try to adjust the structure of the DAC permissions checks so enhanced security providers can make their own decision about how to handle this case. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
* Robert Haas (robertmh...@gmail.com) wrote: On Tue, Aug 17, 2010 at 1:50 PM, Stephen Frost sfr...@snowman.net wrote: No.. and I'm not sure we ever would. What we *have* done is removed all permissions checking on child tables when a parent is being queried.. Yeah. I'm not totally sure that is sensible for a MAC environment. Heck, it's arguably incorrect (though perhaps quite convenient) in a DAC environment. Anyway, I wonder if it would be sensible to try to adjust the structure of the DAC permissions checks so enhanced security providers can make their own decision about how to handle this case. To be honest, I don't really like the way this is done at all. I'd rather have it such that if and when a table is made a child of another table, it should inherit the permissions of the parent and be kept that way, or it should be completely independent (which is the situation we used to have), or, last resort, we should complain when they don't match. Or we could just not error when we hit a child table that the caller doesn't have access to (but also not return the records). The problem is that we've got different users that want to use inheiritance for very different purposes and we havn't got a way to address all of them. I do worry that we're going to regret making the change to not check permissions on child tables, but at the same time, any query which would have been impacted by that would have failed, so that really begs the question of do people really use/want different permissions on child tables than the parent?. I tend to think 'no', and would rather force them and keep them the same, but maybe that's just me.. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
Robert Haas robertmh...@gmail.com writes: On Tue, Aug 17, 2010 at 1:50 PM, Stephen Frost sfr...@snowman.net wrote: No.. and I'm not sure we ever would. What we *have* done is removed all permissions checking on child tables when a parent is being queried.. Yeah. I'm not totally sure that is sensible for a MAC environment. Heck, it's arguably incorrect (though perhaps quite convenient) in a DAC environment. IIRC, the reason we did it was that we decided the SQL spec requires it. So there's not a lot of point in debating the issue, unless you can convince us we misread the spec. 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] security label support, part.2
(2010/08/18 3:07), Robert Haas wrote: On Tue, Aug 17, 2010 at 1:50 PM, Stephen Frostsfr...@snowman.net wrote: No.. and I'm not sure we ever would. What we *have* done is removed all permissions checking on child tables when a parent is being queried.. Yeah. I'm not totally sure that is sensible for a MAC environment. Heck, it's arguably incorrect (though perhaps quite convenient) in a DAC environment. Anyway, I wonder if it would be sensible to try to adjust the structure of the DAC permissions checks so enhanced security providers can make their own decision about how to handle this case. As long as we handle child tables in consistent way, here is no matter for a MAC environment also. As Stephen mentioned, the question was what is an object. So, I want child tables being either a part of parent table or an independent object from its parent. In the first case, child tables need to have same security properties (ownership, access privileges, security labels) with its parent. In the later case, we need to check permissions on child tables also when we query on the parent, but it is an old perspective now. Thanks, -- 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] security label support, part.2
* Tom Lane (t...@sss.pgh.pa.us) wrote: Robert Haas robertmh...@gmail.com writes: Yeah. I'm not totally sure that is sensible for a MAC environment. Heck, it's arguably incorrect (though perhaps quite convenient) in a DAC environment. IIRC, the reason we did it was that we decided the SQL spec requires it. So there's not a lot of point in debating the issue, unless you can convince us we misread the spec. I've not gone through the spec with regard to this (yet..), but I think we need to consider the whole 'principle of least surprise' here, regardless of what the spec says. For one thing, this isn't how older versions of PG behaved and while I doubt anyone intended to rely on that behavior, it makes me nervous that someone, somewhere, unintentionally relies on it. What I'm thinking of is something like a warning if the permissions on the child don't match those of the parent when the relationship is created, or maybe forcibly setting the permissions on the child (with a NOTICE), so it's at least clear what is going on. Or perhaps, set the permissions on the child only if it doesn't have permissions (with the NOTICE), and issue a WARNING if the child already has permissions set. Perhaps also a WARNING if someone changes the permissions on a child after the relationship has been created too, but let it happen in case someone really wants it.. I dunno. None of the above makes me feel very comfortable from a security perspective because I'm concerned any of the above could too easily be overlooked by someone upgrading. It also doesn't really address the concern that, at some point, a child table could have different permissions than a parent table. If we forcibly set the permissions on the child, or ERROR'd if the permissions weren't either the same or empty on the child, and then ERROR'd if someone tried to change the child's permissions later, I'd be happier. I don't really want to force people doing routine partition additions to have to set the permissions on the child before adding it, but I also don't want to have to figure out are these two sets of permissions identical, since that's not really trivial to determine. We do have default permissions now though, so maybe requiring the permissions be the same from the get-go is the right idea. Of course, if we change the permissions on the child when the inheiritance relationship is created, we'll need to update those perms every time the parents perms are changed. Just my 2c. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
(2010/08/18 9:04), Stephen Frost wrote: * Tom Lane (t...@sss.pgh.pa.us) wrote: Robert Haasrobertmh...@gmail.com writes: Yeah. I'm not totally sure that is sensible for a MAC environment. Heck, it's arguably incorrect (though perhaps quite convenient) in a DAC environment. IIRC, the reason we did it was that we decided the SQL spec requires it. So there's not a lot of point in debating the issue, unless you can convince us we misread the spec. I've not gone through the spec with regard to this (yet..), but I think we need to consider the whole 'principle of least surprise' here, regardless of what the spec says. For one thing, this isn't how older versions of PG behaved and while I doubt anyone intended to rely on that behavior, it makes me nervous that someone, somewhere, unintentionally relies on it. I believed that table inheritance is a unique feature in PostgreSQL. Does the SQL spec really mention about whether a child table is an independent table object, or not? Or, are you talking about the behavior that parent's permission also controls accesses on child tables? If so, all of us already agreed. What I'm thinking of is something like a warning if the permissions on the child don't match those of the parent when the relationship is created, or maybe forcibly setting the permissions on the child (with a NOTICE), so it's at least clear what is going on. Or perhaps, set the permissions on the child only if it doesn't have permissions (with the NOTICE), and issue a WARNING if the child already has permissions set. Perhaps also a WARNING if someone changes the permissions on a child after the relationship has been created too, but let it happen in case someone really wants it.. I dunno. None of the above makes me feel very comfortable from a security perspective because I'm concerned any of the above could too easily be overlooked by someone upgrading. It also doesn't really address the concern that, at some point, a child table could have different permissions than a parent table. If we forcibly set the permissions on the child, or ERROR'd if the permissions weren't either the same or empty on the child, and then ERROR'd if someone tried to change the child's permissions later, I'd be happier. I also think ERROR should be raised when user tries to assign different security properties on child tables from its parent. At least, I think it should be configurable using a guc variable. If WARNING/NOTICE, we can easily break consistency of the permissions... I don't really want to force people doing routine partition additions to have to set the permissions on the child before adding it, but I also don't want to have to figure out are these two sets of permissions identical, since that's not really trivial to determine. We do have default permissions now though, so maybe requiring the permissions be the same from the get-go is the right idea. Of course, if we change the permissions on the child when the inheiritance relationship is created, we'll need to update those perms every time the parents perms are changed. I also think it is a good idea to copy permissions from the parent when we try to define an inheritance relationship. It obviously reduces user's routine task on defining many of child tables. It seems to me a case when we provide a NOTICE to users, if permissions of child table is overwritten. Thanks, -- 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] security label support, part.2
KaiGai, * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: I believed that table inheritance is a unique feature in PostgreSQL. It's actually not.. Does the SQL spec really mention about whether a child table is an independent table object, or not? The SQL spec does discuss 'subtables' and inheiritance. It also does describe some information under 'Access Rules' regarding these sub-tables (check the 'table definition' clause). I've been looking at them and trying to make some sense out of what I see. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
2010/8/17 KaiGai Kohei kai...@ak.jp.nec.com: I dunno. None of the above makes me feel very comfortable from a security perspective because I'm concerned any of the above could too easily be overlooked by someone upgrading. It also doesn't really address the concern that, at some point, a child table could have different permissions than a parent table. If we forcibly set the permissions on the child, or ERROR'd if the permissions weren't either the same or empty on the child, and then ERROR'd if someone tried to change the child's permissions later, I'd be happier. I also think ERROR should be raised when user tries to assign different security properties on child tables from its parent. At least, I think it should be configurable using a guc variable. If C1, C2, and C3 inherit from P, it's perfectly reasonable to grant permissions to X on C1 and C2, Y on C3, and Z on C1, C2, C3, and P. I don't think we should disallow that. Sure, it's possible to do things that are less sane, but if we put ourselves in the business of removing useful functionality because it might be misused, we'll put ourselves out of business. Having said that, I'm not sure that the same arguments really hold water in the world of label based security. Suppose we have compartmentalized security: P is a table of threats, with C1 containing data on nukes, C2 containing data on terrorists, and C3 containing data on foreign militaries. If we create a label for each of these threat types, we can apply that label to the corresponding table; but what label shall we assign P? Logically, the label for P should be set up in such a fashion that the only people who can read P are those who can read C1, C2, and C3 anyway, but who is to say that such a label exists? Even if KaiGai's intended implementation of SE-PostgreSQL supports construction of such a label, who is to say that EVERY conceivable labeling system will also do so? In fact, it seems to me that it might be far more reasonable, in a case like this, to ignore the *parent* label and look only at each *child* label, which to me is an argument that we should set this up so as to allow individual users of this hook to do as they like. It's also worth pointing out that the hook in ExecCheckRTPerms() does not presuppose label-based security. It could be used to implement some other policy altogether, which only strengthens the argument that we can't know how the user of the hook wants to handle these cases. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
(2010/08/18 12:02), Robert Haas wrote: 2010/8/17 KaiGai Koheikai...@ak.jp.nec.com: I dunno. None of the above makes me feel very comfortable from a security perspective because I'm concerned any of the above could too easily be overlooked by someone upgrading. It also doesn't really address the concern that, at some point, a child table could have different permissions than a parent table. If we forcibly set the permissions on the child, or ERROR'd if the permissions weren't either the same or empty on the child, and then ERROR'd if someone tried to change the child's permissions later, I'd be happier. I also think ERROR should be raised when user tries to assign different security properties on child tables from its parent. At least, I think it should be configurable using a guc variable. If C1, C2, and C3 inherit from P, it's perfectly reasonable to grant permissions to X on C1 and C2, Y on C3, and Z on C1, C2, C3, and P. I don't think we should disallow that. Sure, it's possible to do things that are less sane, but if we put ourselves in the business of removing useful functionality because it might be misused, we'll put ourselves out of business. Hmm. If C1, C2 and C3 are defined to store information for different categories, but shares common data structure, indeed, it is useful. Having said that, I'm not sure that the same arguments really hold water in the world of label based security. Suppose we have compartmentalized security: P is a table of threats, with C1 containing data on nukes, C2 containing data on terrorists, and C3 containing data on foreign militaries. If we create a label for each of these threat types, we can apply that label to the corresponding table; but what label shall we assign P? Logically, the label for P should be set up in such a fashion that the only people who can read P are those who can read C1, C2, and C3 anyway, but who is to say that such a label exists? Right. If access privileges on P implicitly allow accesses on children, the P must have a label that only allows people who can access both of the children. However, in SELinux at least, here is no guarantee that we can always find out such a label in the security policy installed. :( Even if KaiGai's intended implementation of SE-PostgreSQL supports construction of such a label, who is to say that EVERY conceivable labeling system will also do so? In fact, it seems to me that it might be far more reasonable, in a case like this, to ignore the *parent* label and look only at each *child* label, which to me is an argument that we should set this up so as to allow individual users of this hook to do as they like. It will be helpful. If we can check each children's label, no need to restrict an identical security label within a certain inheritance hierarchy. Of course, other security module may check permissions in different basic, but it seems to me characteristics. It's also worth pointing out that the hook in ExecCheckRTPerms() does not presuppose label-based security. It could be used to implement some other policy altogether, which only strengthens the argument that we can't know how the user of the hook wants to handle these cases. If rte-requiredPerms would not be cleared, the user of the hook will be able to check access rights on the child tables, as they like. How about an idea to add a new flag in RangeTblEntry which shows where the RangeTblEntry came from, instead of clearing requiredPerms? If the flag is true, I think ExecCheckRTEPerms() can simply skip checks on the child tables. Thanks, -- 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] security label support, part.2
KaiGai, * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: The purpose of this restriction is to ensure an access control decision using parent's label being also consistent on child tables. Robert and I understand the concern that you have. The answer, at least for now, is that we don't agree with you. PG doesn't consider child tables to be independent objects when they're being accessed through the parent. As such, they don't have their own permissions checking. If we control accesses on child tables using child's label, no need to restrict an identical label within an entire inheritance hierarchy. But it needs to provide the original rte-requiredPerms of child tables. Now it is cleared at expand_inherited_rtentry(), so we have no way to control accesses on child tables using child's label. :( If you want to argue that we should care about the childs permissions, or do something different with regard to inheritance, then you need to make that argument for all of PG, not just try to do what you think is right in the security definer framework. From viewpoint of MAC, both of the following SQLs should be denied, when accesses on parent_tbl is allowed, but child_tbl is denied. KaiGai, this is not a MAC vs. DAC difference. This is a question of what is an object and if a child table is really an independent object from a parent table. In PG, we've decided they're not. We should probably do more to make that clearer in PG, rather than have different parts of the system treat them differently. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
Stephen Frost sfr...@snowman.net wrote: PG doesn't consider child tables to be independent objects when they're being accessed through the parent. As such, they don't have their own permissions checking. I've been thinking about this from the perspective of possible eventual use by the Wisconsin Courts, and want to throw out my perspective on long-term direction here, without venturing any opinion on the immediate issue. It wouldn't be totally insane for the courts to some day use inheritance to deal with court cases. All court cases have much in common and have the same basic structure, but specific case types need to store some additional information. If we did that, we would want different permissions on different case types -- for example, juvenile cases are not open to the public as many case types are. We would also need the ability to revoke public permissions on specific rows, as judges can seal cases or various pieces of information on a case (like the address of a stalker victim). The point being, we would want a structure something like (picking a few of our case types): Case \_ ChargeableCase \_ FelonyCase \_ MisdemeanorCase \_ CivilForfeitureCase \_ JuvenileCase \_ NonchargableCase \_ CivilCase \_ SmallClaimsCase \_ ProbateCase \_ MentalCommitmentCase Just because most of these case types are a matter of public record and subject to open records law disclosure requests (which we largely avoid by putting what we can on the web site), juvenile and mental commitment cases are confidential; unless you need to handle something related to such a case to support its progress through the courts, you're not supposed to see anything beyond such sketchy information as the existence of the case number, a filing date, and a caption where names are replaced by initials (e.g., In the interest of E.B.) -- and even that information is held back from the web site because of possible data mining attacks. Many of the features KaiGai has discussed would fit nicely with court requirements -- and might even be prerequisites for considering moving security to the database level. Mandating identical security for all tables in a hierarchy would be a problem. We'd want to be able to `grant select on Case to public` and then `revoke select on JuvenileCase, MentalCommitmentCase from public` and have those cases disappear from selects against the ancestor levels unless the user has the appropriate permission. Or less convenient, but still feasible, would be to grant nothing at the ancestor levels, and grant what is appropriate at each child level and have that affect the results of a query against the ancestor. Of course, if one was really careful, this could all be done by adding views with appropriate permissions and blocking access to the underlying ancestor tables, but that seems like a lot more work and rather more error prone. -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] security label support, part.2
* Kevin Grittner (kevin.gritt...@wicourts.gov) wrote: Many of the features KaiGai has discussed would fit nicely with court requirements -- and might even be prerequisites for considering moving security to the database level. Mandating identical security for all tables in a hierarchy would be a problem. What you're describing isn't how inheiritance used to work in PG anyway, so it's not really like we've made things worse. What used to happen is that if your query against the parent table happened to hit a table you didn't have access to, it'd fail outright with a permissions error, not just skip over the things you didn't have access to. That certainly wasn't ideal. I think what you're really looking for is RLS (Row-Level Security), which I think we would want to implement independently of the inheiritance system (though it'd have to work with it, of course). That's certainly something that I think would be great to have in PG and would ideally be something which would address both of your sometimes everything is public except rows which look like X and all of these types are non-public situations. I don't believe it's something that could be addressed *only* by inheiritance though, in any case. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
(2010/08/16 22:14), Stephen Frost wrote: KaiGai, * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: The purpose of this restriction is to ensure an access control decision using parent's label being also consistent on child tables. Robert and I understand the concern that you have. The answer, at least for now, is that we don't agree with you. PG doesn't consider child tables to be independent objects when they're being accessed through the parent. As such, they don't have their own permissions checking. If we control accesses on child tables using child's label, no need to restrict an identical label within an entire inheritance hierarchy. But it needs to provide the original rte-requiredPerms of child tables. Now it is cleared at expand_inherited_rtentry(), so we have no way to control accesses on child tables using child's label. :( If you want to argue that we should care about the childs permissions, or do something different with regard to inheritance, then you need to make that argument for all of PG, not just try to do what you think is right in the security definer framework. From viewpoint of MAC, both of the following SQLs should be denied, when accesses on parent_tbl is allowed, but child_tbl is denied. KaiGai, this is not a MAC vs. DAC difference. This is a question of what is an object and if a child table is really an independent object from a parent table. In PG, we've decided they're not. We should probably do more to make that clearer in PG, rather than have different parts of the system treat them differently. Ahh, yes, the question is what is an object, not a MAC vs DAC. Indeed, PG does not try to handle child table as an independent object from a parent table. However, if so, it seems to me strange that we can assign individual ownership and access privileges on child tables. If we stand on the perspective that child tables are a part of the parent table, isn't it necessary to keep same ownership and access privileges between parent and children? It seems to me the current implementation is in the halfway from the perspective of child tables as independent object to the perspective of child tables as a part of parent table. If PG can keep consistency of ownership and access privileges between parent and children, it is quite natural we keep consistency of labels, isn't it? Thanks, -- 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] security label support, part.2
* KaiGai Kohei (kai...@ak.jp.nec.com) wrote: Ahh, yes, the question is what is an object, not a MAC vs DAC. Indeed, PG does not try to handle child table as an independent object from a parent table. However, if so, it seems to me strange that we can assign individual ownership and access privileges on child tables. I tend to agree. Perhaps we should bring up, in an independent thread, the question of if that really makes sense or if we should do something to prevent it (or at least issue a warning when we detect it). If we stand on the perspective that child tables are a part of the parent table, isn't it necessary to keep same ownership and access privileges between parent and children? It seems to me the current implementation is in the halfway from the perspective of child tables as independent object to the perspective of child tables as a part of parent table. I tend to agree- PG isn't doing this as cleanly as it should. If PG can keep consistency of ownership and access privileges between parent and children, it is quite natural we keep consistency of labels, isn't it? Yes, but that's something that should be dealt with in PG and not as part of an external security infrastructure. For example, PG could just force that the same label is applied to a child table when it's made a child of a particular parent, perhaps with a check to the external security system to see if there's a problem changing whatever label is on the child table before it's changed to be that of the parent, but once it's a child of the parent (if that operation was permitted and was successful), it no longer has its own 'identity'. Let's not build the complication of dealing with inheiritance/child relations into the external security system when we're in the middle of trying to get rid of that distinction in core PG though. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
(2010/08/17 9:51), Stephen Frost wrote: * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: Ahh, yes, the question is what is an object, not a MAC vs DAC. Indeed, PG does not try to handle child table as an independent object from a parent table. However, if so, it seems to me strange that we can assign individual ownership and access privileges on child tables. I tend to agree. Perhaps we should bring up, in an independent thread, the question of if that really makes sense or if we should do something to prevent it (or at least issue a warning when we detect it). If we stand on the perspective that child tables are a part of the parent table, isn't it necessary to keep same ownership and access privileges between parent and children? It seems to me the current implementation is in the halfway from the perspective of child tables as independent object to the perspective of child tables as a part of parent table. I tend to agree- PG isn't doing this as cleanly as it should. If PG can keep consistency of ownership and access privileges between parent and children, it is quite natural we keep consistency of labels, isn't it? Yes, but that's something that should be dealt with in PG and not as part of an external security infrastructure. For example, PG could just force that the same label is applied to a child table when it's made a child of a particular parent, perhaps with a check to the external security system to see if there's a problem changing whatever label is on the child table before it's changed to be that of the parent, but once it's a child of the parent (if that operation was permitted and was successful), it no longer has its own 'identity'. Let's not build the complication of dealing with inheiritance/child relations into the external security system when we're in the middle of trying to get rid of that distinction in core PG though. I also agree the idea that PG core handles the recursion of inheritance hierarchy and enforce same label between them. The reason why I handled it within the module is the core does not enforce same labels. OK, I'll rid 'expected_parents' argument from the security hook on relabeling. Right now, it is incomplete, but should be fixed up in the future. In addition, I'll also post a design proposal to keep consistency of ownership and access privileges between parent and children. Please also wait for a while. Thanks, -- 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] security label support, part.2
Stephen Frost sfr...@snowman.net writes: * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: Indeed, PG does not try to handle child table as an independent object from a parent table. However, if so, it seems to me strange that we can assign individual ownership and access privileges on child tables. I tend to agree. Perhaps we should bring up, in an independent thread, the question of if that really makes sense or if we should do something to prevent it (or at least issue a warning when we detect it). The reason there is still some value in setting permissions state on a child table is that that controls what happens when you address the child table directly, rather than implicitly by querying its parent. 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] security label support, part.2
(2010/08/17 11:58), Tom Lane wrote: Stephen Frostsfr...@snowman.net writes: * KaiGai Kohei (kai...@ak.jp.nec.com) wrote: Indeed, PG does not try to handle child table as an independent object from a parent table. However, if so, it seems to me strange that we can assign individual ownership and access privileges on child tables. I tend to agree. Perhaps we should bring up, in an independent thread, the question of if that really makes sense or if we should do something to prevent it (or at least issue a warning when we detect it). The reason there is still some value in setting permissions state on a child table is that that controls what happens when you address the child table directly, rather than implicitly by querying its parent. However, isn't it strange if we stand on the perspective that child table is a part of parent object? It means an object have multiple properties depending on the context. If we want to allow someone to reference a part of the table (= child table), I think VIEW is more appropriate and flexible tool. Thanks, -- 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] security label support, part.2
On tis, 2010-08-17 at 13:00 +0900, KaiGai Kohei wrote: However, isn't it strange if we stand on the perspective that child table is a part of parent object? It means an object have multiple properties depending on the context. Such is the nature of inheritance, isn't it? -- 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] security label support, part.2
(2010/08/17 13:28), Peter Eisentraut wrote: On tis, 2010-08-17 at 13:00 +0900, KaiGai Kohei wrote: However, isn't it strange if we stand on the perspective that child table is a part of parent object? It means an object have multiple properties depending on the context. Such is the nature of inheritance, isn't it? Yep, it will return different set of user data depending on the table queried, when we reference either parent or child table. But it seems to me too stretch interpretation to apply this behavior on metadata of the tables also. Thanks, -- 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] security label support, part.2
(2010/08/15 9:55), Robert Haas wrote: 2010/8/14 KaiGai Koheikai...@kaigai.gr.jp: (2010/08/15 9:16), Stephen Frost wrote: * KaiGai Kohei (kai...@kaigai.gr.jp) wrote: Yep, rte-requiredPerms of inherited relations are cleared on the expand_inherited_rtentry() since the v9.0, so we cannot know what kind of accesses are required on the individual child relations. This is really a PG issue and decision, in my view. We're moving more and more towards a decision that inherited relations are really just the same relation but broken up per tables (ala true partitioning). As such, PG has chosen to view them as the same wrt permissions checking. I don't think we should make a different decision for security labels. If you don't want people who have access to the parent to have access to the children, then you shouldn't be making them children. No, what I want to do is people have identical access rights on both of the parent and children. If they have always same label, SE-PgSQL always makes same access control decision. This behavior is suitable to the standpoint that inherited relations are really just the same relation of the parent. For this purpose, I want to enforce a unique label on a certain inheritance tree. This seems like a bad idea to me, too. I think it's arguable whether access to the children should be controlled by the parent's label or the child's label, but enforcing that the entire inheritance hierarchy is identically labeled seems like a pointless restriction. As Stephen points out, it's also wildly inconsistent with the way we currently handle it. There's also the problem that the hooks we're talking about here are inadequate to support such a restriction anyway. You'd need some kind of a hook in ALTER TABLE ... [NO] INHERIT, at a minimum. As has been mentioned many times before in reviewing many generations of SE-PostgreSQL patches, we're not going to get into the business of re-engineering our security architecture just because you would have designed it differently. Inventing something that's randomly different will not only make the code ugly and hard to maintain; it will also be confusing and difficult to manage for end-users. Indeed, our existing mechanism allows to assign individual privileges on child tables, even if it is in a certain inheritance hierarchy. The purpose of this restriction is to ensure an access control decision using parent's label being also consistent on child tables. If we control accesses on child tables using child's label, no need to restrict an identical label within an entire inheritance hierarchy. But it needs to provide the original rte-requiredPerms of child tables. Now it is cleared at expand_inherited_rtentry(), so we have no way to control accesses on child tables using child's label. :( From viewpoint of MAC, both of the following SQLs should be denied, when accesses on parent_tbl is allowed, but child_tbl is denied. 1) SELECT * FROM parent_tbl; 2) SELECT * FROM child_tbl; Thanks, -- 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] security label support, part.2
(2010/08/10 8:39), Robert Haas wrote: 2010/8/9kai...@kaigai.gr.jp: 2. Some of this code refers to local security labels. I'm not sure what's local about them - aren't they just security labels? On a related note, I don't like the trivial wrappers you have here, with DeleteSecurityLabel around DeleteLocalSecLabel, SetSecurityLabel around SetLocalSecLabel, etc. Just collapse these into a single set of functions. In the feature, I plan to assign security labels on the shared database objects such as pg_database. The local is a contradistinction towards these shared objects. Oh, I see. I don't think that's entirely clear: and in any event it seems a bit premature, since we're not at that point yet. Let's just get rid of this stuff for now as I suggested. OK. We can add this supportanytime we need it. 5. Why do we think that the relabel hook needs to be passed the number of expected parents? We need to prevent relabeling on inherited relations/columns from the multiple origin, like ALTER RENAME TO. It requires to pass the expected parents into the provider, or to check it in the caller. Please explain further. I don't understand. Yep, rte-requiredPerms of inherited relations are cleared on the expand_inherited_rtentry() since the v9.0, so we cannot know what kind of accesses are required on the individual child relations. It needs the inherited relations/columns being labeled with same security label of their parent, because SE-PgSQL always makes same access control decision on same security labels. Thus, we want to check whether the relabeling operation breaks the uniqueness of security label within a certain inheritance tree, or not. Here is the logic to check relabeling on relation/column. http://code.google.com/p/sepgsql/source/browse/trunk/sepgsql/hooks.c#254 It checks two things. 1) The given relation/column must be the origin of inheritance tree when expected_parents = 0. 2) The given relation/column must not belong to multiple inheritance tree. So, the hook need to provide the expected_parents for each relations/columns. 6. What are we doing about the assignment of initial security labels? I had initially thought that perhaps new objects would just start out unlabeled, and the user would be responsible for labeling them as needed. But maybe we can do better. Perhaps we should extend the security provider hook API with a function that gets called when a labellable object gets created, and let each loaded security provider return any label it would like attached. Even if we don't do that now, esp_relabel_hook_entry needs to be renamed to something more generic; we will certainly want to add more fields to that structure later. Starting with unlabeled is wrong, because it does not distinguish an object created by security sensitive users and insensitive users, for example. It is similar to relation's relowner is InvalidOid. I plan the security provider hook on the creation time works two things. 1. It checks user's privilege to create this object. 2. It returns security labels to be assigned. Then, the caller assigns these returned labels on the new object, if one or more valid labels are returned. OK, let's give that a try and see how it looks. I don't think that's in this version of the patch, right? Yes, this version of the patch didn't support labeling on creation time of database objects. It shall be added in separated patch. 7. I think we need to write and include in the fine documentation some big picture documentation about enhanced security providers. Of course, we have to decide what we want to say. But the SECURITY LABEL documentation is just kind of hanging out there in space right now; it needs to connect to a broad introduction to the subject. OK, I'll try to describe with appropriate granularity. Do we need an independent section in addition to the introduction of SECURITY LABEL syntax? I think so. I suggest a new chapter called Enhanced Security Providers just after Database Roles and Privileges. OK, 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] security label support, part.2
* KaiGai Kohei (kai...@kaigai.gr.jp) wrote: Yep, rte-requiredPerms of inherited relations are cleared on the expand_inherited_rtentry() since the v9.0, so we cannot know what kind of accesses are required on the individual child relations. This is really a PG issue and decision, in my view. We're moving more and more towards a decision that inherited relations are really just the same relation but broken up per tables (ala true partitioning). As such, PG has chosen to view them as the same wrt permissions checking. I don't think we should make a different decision for security labels. If you don't want people who have access to the parent to have access to the children, then you shouldn't be making them children. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
(2010/08/15 9:16), Stephen Frost wrote: * KaiGai Kohei (kai...@kaigai.gr.jp) wrote: Yep, rte-requiredPerms of inherited relations are cleared on the expand_inherited_rtentry() since the v9.0, so we cannot know what kind of accesses are required on the individual child relations. This is really a PG issue and decision, in my view. We're moving more and more towards a decision that inherited relations are really just the same relation but broken up per tables (ala true partitioning). As such, PG has chosen to view them as the same wrt permissions checking. I don't think we should make a different decision for security labels. If you don't want people who have access to the parent to have access to the children, then you shouldn't be making them children. No, what I want to do is people have identical access rights on both of the parent and children. If they have always same label, SE-PgSQL always makes same access control decision. This behavior is suitable to the standpoint that inherited relations are really just the same relation of the parent. For this purpose, I want to enforce a unique label on a certain inheritance tree. 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] security label support, part.2
2010/8/14 KaiGai Kohei kai...@kaigai.gr.jp: (2010/08/15 9:16), Stephen Frost wrote: * KaiGai Kohei (kai...@kaigai.gr.jp) wrote: Yep, rte-requiredPerms of inherited relations are cleared on the expand_inherited_rtentry() since the v9.0, so we cannot know what kind of accesses are required on the individual child relations. This is really a PG issue and decision, in my view. We're moving more and more towards a decision that inherited relations are really just the same relation but broken up per tables (ala true partitioning). As such, PG has chosen to view them as the same wrt permissions checking. I don't think we should make a different decision for security labels. If you don't want people who have access to the parent to have access to the children, then you shouldn't be making them children. No, what I want to do is people have identical access rights on both of the parent and children. If they have always same label, SE-PgSQL always makes same access control decision. This behavior is suitable to the standpoint that inherited relations are really just the same relation of the parent. For this purpose, I want to enforce a unique label on a certain inheritance tree. This seems like a bad idea to me, too. I think it's arguable whether access to the children should be controlled by the parent's label or the child's label, but enforcing that the entire inheritance hierarchy is identically labeled seems like a pointless restriction. As Stephen points out, it's also wildly inconsistent with the way we currently handle it. There's also the problem that the hooks we're talking about here are inadequate to support such a restriction anyway. You'd need some kind of a hook in ALTER TABLE ... [NO] INHERIT, at a minimum. As has been mentioned many times before in reviewing many generations of SE-PostgreSQL patches, we're not going to get into the business of re-engineering our security architecture just because you would have designed it differently. Inventing something that's randomly different will not only make the code ugly and hard to maintain; it will also be confusing and difficult to manage for end-users. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
2010/7/26 KaiGai Kohei kai...@ak.jp.nec.com: The attached patches are revised ones, as follows. I think this is pretty good, and I'm generally in favor of committing it. Some concerns: 1. Since nobody has violently objected to the comment.c refactoring patch I recently proposed, I'm hopeful that can go in. And if that's the case, then I'd prefer to see that committed first, and then rework this to use that code. That would eliminate some code here, and it would also make it much easier to support labels on other types of objects. 2. Some of this code refers to local security labels. I'm not sure what's local about them - aren't they just security labels? On a related note, I don't like the trivial wrappers you have here, with DeleteSecurityLabel around DeleteLocalSecLabel, SetSecurityLabel around SetLocalSecLabel, etc. Just collapse these into a single set of functions. 3. Is it really appropriate for ExecRelationSecLabel() to have an Exec prefix? I don't think so. 4. Please get rid of the nkeys++ stuff in DeleteLocalSecLabel() and just use fixed offsets as we do everywhere else. 5. Why do we think that the relabel hook needs to be passed the number of expected parents? 6. What are we doing about the assignment of initial security labels? I had initially thought that perhaps new objects would just start out unlabeled, and the user would be responsible for labeling them as needed. But maybe we can do better. Perhaps we should extend the security provider hook API with a function that gets called when a labellable object gets created, and let each loaded security provider return any label it would like attached. Even if we don't do that now, esp_relabel_hook_entry needs to be renamed to something more generic; we will certainly want to add more fields to that structure later. 7. I think we need to write and include in the fine documentation some big picture documentation about enhanced security providers. Of course, we have to decide what we want to say. But the SECURITY LABEL documentation is just kind of hanging out there in space right now; it needs to connect to a broad introduction to the subject. 8. Generally, the English in both the comments and documentation needs work. However, we can address that problem when we're closer to commit. I am going to mark this Returned with Feedback because I don't believe it's realistic to get the comment code committed in the next week, rework this patch, and then get this patch committed also. However, I'm feeling pretty good about this effort and I think we're making good progress toward getting this done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
Thanks for your reviewing. On Mon, 9 Aug 2010 16:02:12 -0400 Robert Haas robertmh...@gmail.com wrote: 2010/7/26 KaiGai Kohei kai...@ak.jp.nec.com: The attached patches are revised ones, as follows. I think this is pretty good, and I'm generally in favor of committing it. Some concerns: 1. Since nobody has violently objected to the comment.c refactoring patch I recently proposed, I'm hopeful that can go in. And if that's the case, then I'd prefer to see that committed first, and then rework this to use that code. That would eliminate some code here, and it would also make it much easier to support labels on other types of objects. It seems to me fair enough. This parse_object.c can also provide a facility to resolve the name of object to be labeled. 2. Some of this code refers to local security labels. I'm not sure what's local about them - aren't they just security labels? On a related note, I don't like the trivial wrappers you have here, with DeleteSecurityLabel around DeleteLocalSecLabel, SetSecurityLabel around SetLocalSecLabel, etc. Just collapse these into a single set of functions. In the feature, I plan to assign security labels on the shared database objects such as pg_database. The local is a contradistinction towards these shared objects. 3. Is it really appropriate for ExecRelationSecLabel() to have an Exec prefix? I don't think so. I don't have any preferences about 4. Please get rid of the nkeys++ stuff in DeleteLocalSecLabel() and just use fixed offsets as we do everywhere else. OK, I'll fix it. 5. Why do we think that the relabel hook needs to be passed the number of expected parents? We need to prevent relabeling on inherited relations/columns from the multiple origin, like ALTER RENAME TO. It requires to pass the expected parents into the provider, or to check it in the caller. 6. What are we doing about the assignment of initial security labels? I had initially thought that perhaps new objects would just start out unlabeled, and the user would be responsible for labeling them as needed. But maybe we can do better. Perhaps we should extend the security provider hook API with a function that gets called when a labellable object gets created, and let each loaded security provider return any label it would like attached. Even if we don't do that now, esp_relabel_hook_entry needs to be renamed to something more generic; we will certainly want to add more fields to that structure later. Starting with unlabeled is wrong, because it does not distinguish an object created by security sensitive users and insensitive users, for example. It is similar to relation's relowner is InvalidOid. I plan the security provider hook on the creation time works two things. 1. It checks user's privilege to create this object. 2. It returns security labels to be assigned. Then, the caller assigns these returned labels on the new object, if one or more valid labels are returned. 7. I think we need to write and include in the fine documentation some big picture documentation about enhanced security providers. Of course, we have to decide what we want to say. But the SECURITY LABEL documentation is just kind of hanging out there in space right now; it needs to connect to a broad introduction to the subject. OK, I'll try to describe with appropriate granularity. Do we need an independent section in addition to the introduction of SECURITY LABEL syntax? 8. Generally, the English in both the comments and documentation needs work. However, we can address that problem when we're closer to commit. OK BTW, I'll go on the area where internet unconnectable during next two days. Perhaps, my reply will run late. 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] security label support, part.2
2010/8/9 kai...@kaigai.gr.jp: 2. Some of this code refers to local security labels. I'm not sure what's local about them - aren't they just security labels? On a related note, I don't like the trivial wrappers you have here, with DeleteSecurityLabel around DeleteLocalSecLabel, SetSecurityLabel around SetLocalSecLabel, etc. Just collapse these into a single set of functions. In the feature, I plan to assign security labels on the shared database objects such as pg_database. The local is a contradistinction towards these shared objects. Oh, I see. I don't think that's entirely clear: and in any event it seems a bit premature, since we're not at that point yet. Let's just get rid of this stuff for now as I suggested. 5. Why do we think that the relabel hook needs to be passed the number of expected parents? We need to prevent relabeling on inherited relations/columns from the multiple origin, like ALTER RENAME TO. It requires to pass the expected parents into the provider, or to check it in the caller. Please explain further. I don't understand. 6. What are we doing about the assignment of initial security labels? I had initially thought that perhaps new objects would just start out unlabeled, and the user would be responsible for labeling them as needed. But maybe we can do better. Perhaps we should extend the security provider hook API with a function that gets called when a labellable object gets created, and let each loaded security provider return any label it would like attached. Even if we don't do that now, esp_relabel_hook_entry needs to be renamed to something more generic; we will certainly want to add more fields to that structure later. Starting with unlabeled is wrong, because it does not distinguish an object created by security sensitive users and insensitive users, for example. It is similar to relation's relowner is InvalidOid. I plan the security provider hook on the creation time works two things. 1. It checks user's privilege to create this object. 2. It returns security labels to be assigned. Then, the caller assigns these returned labels on the new object, if one or more valid labels are returned. OK, let's give that a try and see how it looks. I don't think that's in this version of the patch, right? 7. I think we need to write and include in the fine documentation some big picture documentation about enhanced security providers. Of course, we have to decide what we want to say. But the SECURITY LABEL documentation is just kind of hanging out there in space right now; it needs to connect to a broad introduction to the subject. OK, I'll try to describe with appropriate granularity. Do we need an independent section in addition to the introduction of SECURITY LABEL syntax? I think so. I suggest a new chapter called Enhanced Security Providers just after Database Roles and Privileges. BTW, I'll go on the area where internet unconnectable during next two days. Perhaps, my reply will run late. No problem. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
The attached patches are revised ones, as follows.
* A new SECURITY LABEL statement replaced the previous ALTER TABLE statement
with SECURITY LABEL TO option. It has the following syntax.
SECURITY LABEL [ FOR provider ] ON object class object name IS
'label';
E.g) SECURITY LABEL ON TABLE t1 IS 'system_u:object_r:sepgsql_table_t:s0';
* It supports multiple security providers to assign its security label on
a database object. The pg_seclabel catalog was modified as follows:
CATALOG(pg_seclabel,3037) BKI_WITHOUT_OIDS
{
Oid reloid; /* OID of table containing the object */
Oid objoid; /* OID of the object itself */
int4subid; /* column number, or 0 if not used */
+ texttag;/* identifier of external security provider */
textlabel; /* security label of the object */
} FormData_pg_seclabel;
The new 'tag' field identifies which security provider manages this
security label. For example, SE-PostgreSQL uses selinux for its
identifier.
* The security hook to check relabeling become to be registered using
register_object_relabel_hook() which takes a tag of ESP module and
a function pointer to the security hook.
ExecSecLabelStmt() picks up an appropriate security hook, then it
shall be invoked even if multiple modules are loaded.
* Add _copySecLabelStmt() on nodes/copyfuncs.c and _equalSecLabelStmt()
on nodes/equalfuncs.c, because I forgot to add them, although new
parsenode (SecLabelStmt) was added.
* Add descriptions about pg_seclabel catalog and SECURITY LABEL statement
on the documentation.
Thanks,
(2010/07/23 22:36), Robert Haas wrote:
On Fri, Jul 23, 2010 at 8:59 AM, KaiGai Koheikai...@kaigai.gr.jp wrote:
(2010/07/23 20:44), Robert Haas wrote:
2010/7/23 KaiGai Koheikai...@ak.jp.nec.com:
Hmm. How about if there's just one provider loaded, you can omit it,
but if you fail to specify it and there's1 loaded, we just throw an
error saying you didn't specify whose label it is.
Perhaps, we need to return the caller a state whether one provider
checked
the given label at least, or not.
Return to the caller? This is an SQL command. You either get an
error, or you don't.
Ahh, I was talked about relationship between the core PG code and ESP
module.
It means the security hook returns a state which informs the core PG code
whether one provider checked the given label, then the core PG code can
decide whether it raise an actual error to users, or not.
In other words, I'd like to suggest the security hook which returns a tag
of ESP module, as follows:
const char *
check_object_relabel_hook(const ObjectAddress *object,
const char *provider,
const char *seclabel);
I don't think that's a very good design. What I had in mind was a
simple API for security providers to register themselves (including
their names), and then the core code will only call the relevant
security provider. I did try to explain this in point #3 of my
original review.
--
KaiGai Kohei kai...@ak.jp.nec.com
pgsql-v9.1-security-label-2.v2.patch
Description: application/octect-stream
pgsql-v9.1-security-label-1.v2.patch
Description: application/octect-stream
--
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] security label support, part.2
2010/7/23 KaiGai Kohei kai...@ak.jp.nec.com: Hmm. How about if there's just one provider loaded, you can omit it, but if you fail to specify it and there's1 loaded, we just throw an error saying you didn't specify whose label it is. Perhaps, we need to return the caller a state whether one provider checked the given label at least, or not. Return to the caller? This is an SQL command. You either get an error, or you don't. If it was omitted, all the providers try to check the given label, but it has mutually different format, so one of providers will raise an error at least. Yeah, but it won't be a very clear error, and what if you have, say, a provider that allows arbitrary strings as labels? Since this is a security feature, I think it's a pretty bad idea to allow the user to do anything that might be ambiguous. It means we have to specify the provider when two or more providers are loaded, but not necessary when just one provider. But that should be fine. Loading multiple providers should, as you say, be fairly rare. I think it is 100% clear that row-level security will require completely separate infrastructure, and therefore I'm not even a tiny bit worried about this. :-) Hmm. Are you saying we may degrade the feature when we switch to the completely separate infrastructure? Is it preferable?? Uh... no, not really. I'm saying that I don't think we're backing ourselves into a corner. What makes you think we are? Sorry, meaning of the last question was unclear for me Is it a idiom? I don't understand why we wouldn't be able to support multiple providers for row-level security. Why do you think that's a problem? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
* Robert Haas (robertmh...@gmail.com) wrote: I don't understand why we wouldn't be able to support multiple providers for row-level security. Why do you think that's a problem? My guess would be that he's concerned about only having space in the tuple header for 1 label. I see two answers- only allow 1 provider for a given relation (doesn't strike me as a horrible limitation), or handle labels as extra columns where you could have more than one. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] security label support, part.2
On Fri, Jul 23, 2010 at 8:32 AM, Stephen Frost sfr...@snowman.net wrote: * Robert Haas (robertmh...@gmail.com) wrote: I don't understand why we wouldn't be able to support multiple providers for row-level security. Why do you think that's a problem? My guess would be that he's concerned about only having space in the tuple header for 1 label. I see two answers- only allow 1 provider for a given relation (doesn't strike me as a horrible limitation), or handle labels as extra columns where you could have more than one. I think we've been pretty clear in previous discussions that any row-level security implementation should be a general one, and SE-Linux or whatever can integrate with that to do what it needs to do. So I'm pretty sure we'll be using regular columns rather than cramming anything into the tuple header. There are pretty substantial performance benefits to such an implementation, as well. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
(2010/07/23 20:44), Robert Haas wrote: 2010/7/23 KaiGai Koheikai...@ak.jp.nec.com: Hmm. How about if there's just one provider loaded, you can omit it, but if you fail to specify it and there's1 loaded, we just throw an error saying you didn't specify whose label it is. Perhaps, we need to return the caller a state whether one provider checked the given label at least, or not. Return to the caller? This is an SQL command. You either get an error, or you don't. Ahh, I was talked about relationship between the core PG code and ESP module. It means the security hook returns a state which informs the core PG code whether one provider checked the given label, then the core PG code can decide whether it raise an actual error to users, or not. In other words, I'd like to suggest the security hook which returns a tag of ESP module, as follows: const char * check_object_relabel_hook(const ObjectAddress *object, const char *provider, const char *seclabel); The second argument reflects FOR provider clause. It informs ESP modules what provider is specified. If omitted, it will be NULL. Then, ESP module which checked the given security label must return its tag. Maybe, selinux, if SE-PostgreSQL. Or, NULL will be returned if nobody checked it. If NULL or incorrect tag is returned, the core PG code can know the given seclabel is not checked/validated, then it will raise an error to users. Elsewhere, the validated label will be stored with the returned tag. It enables to recognize what label is validated by SELinux, and what label is not. If it was omitted, all the providers try to check the given label, but it has mutually different format, so one of providers will raise an error at least. Yeah, but it won't be a very clear error, and what if you have, say, a provider that allows arbitrary strings as labels? Since this is a security feature, I think it's a pretty bad idea to allow the user to do anything that might be ambiguous. It is provider's job to validate the given security label. So, if we install such a security module which accept arbitrary strings as label, the core PG code also need to believe the ESP module. But the arbitrary label will be tagged with something other than selinux, so it does not confuse other module, according to the above idea. It means we have to specify the provider when two or more providers are loaded, but not necessary when just one provider. But that should be fine. Loading multiple providers should, as you say, be fairly rare. I think it is 100% clear that row-level security will require completely separate infrastructure, and therefore I'm not even a tiny bit worried about this. :-) Hmm. Are you saying we may degrade the feature when we switch to the completely separate infrastructure? Is it preferable?? Uh... no, not really. I'm saying that I don't think we're backing ourselves into a corner. What makes you think we are? Sorry, meaning of the last question was unclear for me Is it a idiom? I don't understand why we wouldn't be able to support multiple providers for row-level security. Why do you think that's a problem? I don't have any clear reason why we wouldn't be able to support multiple labels on user tuples, but it is intangible anxiety, because I have not implemented it as a working example yet. (So, I never think it is impossible.) 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] security label support, part.2
On Fri, Jul 23, 2010 at 8:59 AM, KaiGai Kohei kai...@kaigai.gr.jp wrote: (2010/07/23 20:44), Robert Haas wrote: 2010/7/23 KaiGai Koheikai...@ak.jp.nec.com: Hmm. How about if there's just one provider loaded, you can omit it, but if you fail to specify it and there's1 loaded, we just throw an error saying you didn't specify whose label it is. Perhaps, we need to return the caller a state whether one provider checked the given label at least, or not. Return to the caller? This is an SQL command. You either get an error, or you don't. Ahh, I was talked about relationship between the core PG code and ESP module. It means the security hook returns a state which informs the core PG code whether one provider checked the given label, then the core PG code can decide whether it raise an actual error to users, or not. In other words, I'd like to suggest the security hook which returns a tag of ESP module, as follows: const char * check_object_relabel_hook(const ObjectAddress *object, const char *provider, const char *seclabel); I don't think that's a very good design. What I had in mind was a simple API for security providers to register themselves (including their names), and then the core code will only call the relevant security provider. I did try to explain this in point #3 of my original review. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
2010/7/14 KaiGai Kohei kai...@ak.jp.nec.com: The attached patch is a part of efforts to support security label on database objects. This is similar to what I had in mind as a design for this feature, but I have some gripes: 1. I am inclined to suggest the syntax SECURITY LABEL ON ... IS ..., following COMMENT ON (it's also somewhat similar to the GRANT syntax). While the hook in ExecCheckRTPerms() will only allow meaningful permissions checks on the use of relations, there will presumably be ongoing demand to add additional hooks to cover other types of objects, and I'd rather add label support all at once rather than bit-by-bit. I also think that the SECURITY LABEL syntax is more future-proof; we don't need to worry about conflicts in other parts of the grammar. 2. Similarly, the design of the hook in secabel.h is way too short-sighted and won't scale to any other object type. We don't need or want one hook per object type here. Use an ObjectAddress. 3. I am firmly of the view that we want to allow multiple security providers. I think the way this should work here is that we should keep a list somewhere of security providers known to the system, which loadable modules should add themselves to. Each such security provider should be represented by a struct containing, at least, a name and a function that gets called on relabel. The labels should be stored in the catalog. That way there is never any possibility of one security provider getting a label that was originally applied by some other security provider. If we were storing these labels in pg_class or pg_attribute or similar, the storage cost for this might be worth worrying about, but as this is a separate catalog, it's not. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
Thanks for your reviewing. (2010/07/23 0:54), Robert Haas wrote: 2010/7/14 KaiGai Koheikai...@ak.jp.nec.com: The attached patch is a part of efforts to support security label on database objects. This is similar to what I had in mind as a design for this feature, but I have some gripes: 1. I am inclined to suggest the syntax SECURITY LABEL ON ... IS ..., following COMMENT ON (it's also somewhat similar to the GRANT syntax). While the hook in ExecCheckRTPerms() will only allow meaningful permissions checks on the use of relations, there will presumably be ongoing demand to add additional hooks to cover other types of objects, and I'd rather add label support all at once rather than bit-by-bit. I also think that the SECURITY LABEL syntax is more future-proof; we don't need to worry about conflicts in other parts of the grammar. Hmm. Indeed, we cannot deny the possibility to conflict with other part in the future, if we use ALTER xxx statement here. But, right now, we have no statement that starts in noun, rather than verb. The comment is a noun, but comment on is a phrasal-verb, isn't it? How about RELABEL object TO label, instead? The relabel is a transitive-verb, we don't need ON between RELABEL and object. And, it tries to change a property of the object, so it seems to me TO is more appropriate than IS. 2. Similarly, the design of the hook in secabel.h is way too short-sighted and won't scale to any other object type. We don't need or want one hook per object type here. Use an ObjectAddress. I think the relation type is an exceptional object class, because of the recursion due to the table inheritances. For other object classes, I also think one security hook which takes ObjectAddress as an argument is enough to implement. So, I expect we need two hooks on relabeling eventually. (One for relation, one for other object classes) 3. I am firmly of the view that we want to allow multiple security providers. I think the way this should work here is that we should keep a list somewhere of security providers known to the system, which loadable modules should add themselves to. Each such security provider should be represented by a struct containing, at least, a name and a function that gets called on relabel. The labels should be stored in the catalog. That way there is never any possibility of one security provider getting a label that was originally applied by some other security provider. If we were storing these labels in pg_class or pg_attribute or similar, the storage cost for this might be worth worrying about, but as this is a separate catalog, it's not. What I'm worrying about is that we cannot estimate amount of works when we expand the concept to row-level security. We will need to revise the implementation, if individual user tuples have its security label in the future version. If we don't support multiple labels right now, it will not be feature degradation, even if it will be hard to implement multiple label support for each user tuples. :( I don't deny worth of multiple security providers concurrently, however, I doubt whether it should be supported from the beginning, or not. It seems to me it is not too late after we can find out the way to label individual user tuples. Thanks, -- 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] security label support, part.2
2010/7/22 KaiGai Kohei kai...@ak.jp.nec.com: Thanks for your reviewing. 1. I am inclined to suggest the syntax SECURITY LABEL ON ... IS ..., following COMMENT ON (it's also somewhat similar to the GRANT syntax). While the hook in ExecCheckRTPerms() will only allow meaningful permissions checks on the use of relations, there will presumably be ongoing demand to add additional hooks to cover other types of objects, and I'd rather add label support all at once rather than bit-by-bit. I also think that the SECURITY LABEL syntax is more future-proof; we don't need to worry about conflicts in other parts of the grammar. Hmm. Indeed, we cannot deny the possibility to conflict with other part in the future, if we use ALTER xxx statement here. But, right now, we have no statement that starts in noun, rather than verb. The comment is a noun, but comment on is a phrasal-verb, isn't it? How about RELABEL object TO label, instead? Well, I like SECURITY LABEL better because it's more clear about what kind of label we're talking about, but if there's consensus on some other option it's OK with me. Actually, we need to work the security provider name in there too, I think, so perhaps SECURITY LABEL FOR provider ON object IS labeltext. I realize it's slightly odd grammatically, but it's no worse than the COMMENT syntax. 2. Similarly, the design of the hook in secabel.h is way too short-sighted and won't scale to any other object type. We don't need or want one hook per object type here. Use an ObjectAddress. I think the relation type is an exceptional object class, because of the recursion due to the table inheritances. For other object classes, I also think one security hook which takes ObjectAddress as an argument is enough to implement. So, I expect we need two hooks on relabeling eventually. (One for relation, one for other object classes) Please explain in more detail. 3. I am firmly of the view that we want to allow multiple security providers. I think the way this should work here is that we should keep a list somewhere of security providers known to the system, which loadable modules should add themselves to. Each such security provider should be represented by a struct containing, at least, a name and a function that gets called on relabel. The labels should be stored in the catalog. That way there is never any possibility of one security provider getting a label that was originally applied by some other security provider. If we were storing these labels in pg_class or pg_attribute or similar, the storage cost for this might be worth worrying about, but as this is a separate catalog, it's not. What I'm worrying about is that we cannot estimate amount of works when we expand the concept to row-level security. We will need to revise the implementation, if individual user tuples have its security label in the future version. If we don't support multiple labels right now, it will not be feature degradation, even if it will be hard to implement multiple label support for each user tuples. :( I think it is 100% clear that row-level security will require completely separate infrastructure, and therefore I'm not even a tiny bit worried about this. :-) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
(2010/07/23 10:05), Robert Haas wrote: 2010/7/22 KaiGai Koheikai...@ak.jp.nec.com: Thanks for your reviewing. 1. I am inclined to suggest the syntax SECURITY LABEL ON ... IS ..., following COMMENT ON (it's also somewhat similar to the GRANT syntax). While the hook in ExecCheckRTPerms() will only allow meaningful permissions checks on the use of relations, there will presumably be ongoing demand to add additional hooks to cover other types of objects, and I'd rather add label support all at once rather than bit-by-bit. I also think that the SECURITY LABEL syntax is more future-proof; we don't need to worry about conflicts in other parts of the grammar. Hmm. Indeed, we cannot deny the possibility to conflict with other part in the future, if we use ALTER xxx statement here. But, right now, we have no statement that starts in noun, rather than verb. The comment is a noun, but comment on is a phrasal-verb, isn't it? How about RELABELobject TOlabel, instead? Well, I like SECURITY LABEL better because it's more clear about what kind of label we're talking about, but if there's consensus on some other option it's OK with me. Actually, we need to work the security provider name in there too, I think, so perhaps SECURITY LABEL FOR provider ON object IS labeltext. I realize it's slightly odd grammatically, but it's no worse than the COMMENT syntax. The FOR provider clause should be optional. I expect most use cases installs only one security provider, rather than multiple. If no explicit provider is specified, all the security providers check the given security label. If two or more providers are here, of course, either of them will raise an error, because they have different label formats. It is right. Anyway, I'd like to implement according to the idea. SECURITY LABEL [FOR provider] ON object IS label; 2. Similarly, the design of the hook in secabel.h is way too short-sighted and won't scale to any other object type. We don't need or want one hook per object type here. Use an ObjectAddress. I think the relation type is an exceptional object class, because of the recursion due to the table inheritances. For other object classes, I also think one security hook which takes ObjectAddress as an argument is enough to implement. So, I expect we need two hooks on relabeling eventually. (One for relation, one for other object classes) Please explain in more detail. For relations, one SECURITY LABEL statement may relabel multiple tables when it has child tables, if ONLY clause was not given. So, we need to obtain oids to be relabeled using find_all_inheritors(), and need to ask providers whether it allows, or not. But, obviously, it is specific for relations. For other object class, the target object to be relabeled is identified by object in SECURITY LABEL statement. It will be parsed by the upcoming parse_object.c feature, then it solves the object name to ObjectAddress. So, we can apply access controls after setting up the ObjectAddress with common hooks for object classes except for relations, like: void check_object_relabel(ObjectAddress object, const char *new_label); 3. I am firmly of the view that we want to allow multiple security providers. I think the way this should work here is that we should keep a list somewhere of security providers known to the system, which loadable modules should add themselves to. Each such security provider should be represented by a struct containing, at least, a name and a function that gets called on relabel. The labels should be stored in the catalog. That way there is never any possibility of one security provider getting a label that was originally applied by some other security provider. If we were storing these labels in pg_class or pg_attribute or similar, the storage cost for this might be worth worrying about, but as this is a separate catalog, it's not. What I'm worrying about is that we cannot estimate amount of works when we expand the concept to row-level security. We will need to revise the implementation, if individual user tuples have its security label in the future version. If we don't support multiple labels right now, it will not be feature degradation, even if it will be hard to implement multiple label support for each user tuples. :( I think it is 100% clear that row-level security will require completely separate infrastructure, and therefore I'm not even a tiny bit worried about this. :-) Hmm. Are you saying we may degrade the feature when we switch to the completely separate infrastructure? Is it preferable?? Thanks, -- 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] security label support, part.2
2010/7/22 KaiGai Kohei kai...@ak.jp.nec.com: Well, I like SECURITY LABEL better because it's more clear about what kind of label we're talking about, but if there's consensus on some other option it's OK with me. Actually, we need to work the security provider name in there too, I think, so perhaps SECURITY LABEL FOR provider ON object IS labeltext. I realize it's slightly odd grammatically, but it's no worse than the COMMENT syntax. The FOR provider clause should be optional. I expect most use cases installs only one security provider, rather than multiple. If no explicit provider is specified, all the security providers check the given security label. If two or more providers are here, of course, either of them will raise an error, because they have different label formats. It is right. Hmm. How about if there's just one provider loaded, you can omit it, but if you fail to specify it and there's 1 loaded, we just throw an error saying you didn't specify whose label it is. So, I expect we need two hooks on relabeling eventually. (One for relation, one for other object classes) Please explain in more detail. For relations, one SECURITY LABEL statement may relabel multiple tables when it has child tables, if ONLY clause was not given. So, we need to obtain oids to be relabeled using find_all_inheritors(), and need to ask providers whether it allows, or not. But, obviously, it is specific for relations. For other object class, the target object to be relabeled is identified by object in SECURITY LABEL statement. It will be parsed by the upcoming parse_object.c feature, then it solves the object name to ObjectAddress. So, we can apply access controls after setting up the ObjectAddress with common hooks for object classes except for relations, like: void check_object_relabel(ObjectAddress object, const char *new_label); So just construct an ObjectAddress for each relation and call the check function once for each. I think it is 100% clear that row-level security will require completely separate infrastructure, and therefore I'm not even a tiny bit worried about this. :-) Hmm. Are you saying we may degrade the feature when we switch to the completely separate infrastructure? Is it preferable?? Uh... no, not really. I'm saying that I don't think we're backing ourselves into a corner. What makes you think we are? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] security label support, part.2
(2010/07/23 12:56), Robert Haas wrote: 2010/7/22 KaiGai Koheikai...@ak.jp.nec.com: Well, I like SECURITY LABEL better because it's more clear about what kind of label we're talking about, but if there's consensus on some other option it's OK with me. Actually, we need to work the security provider name in there too, I think, so perhaps SECURITY LABEL FOR provider ON object IS labeltext. I realize it's slightly odd grammatically, but it's no worse than the COMMENT syntax. The FORprovider clause should be optional. I expect most use cases installs only one security provider, rather than multiple. If no explicitprovider is specified, all the security providers check the given security label. If two or more providers are here, of course, either of them will raise an error, because they have different label formats. It is right. Hmm. How about if there's just one provider loaded, you can omit it, but if you fail to specify it and there's1 loaded, we just throw an error saying you didn't specify whose label it is. Perhaps, we need to return the caller a state whether one provider checked the given label at least, or not. If invalid provider was specified so nobody checked it, nobody returns the caller a state of checked, then it raises an error to notice invalid security provider. If valid provider was specified, only specified provider checks the given label, and returns the caller a state of it was checked by . If it was omitted, all the providers try to check the given label, but it has mutually different format, so one of providers will raise an error at least. It means we have to specify the provider when two or more providers are loaded, but not necessary when just one provider. So, I expect we need two hooks on relabeling eventually. (One for relation, one for other object classes) Please explain in more detail. For relations, one SECURITY LABEL statement may relabel multiple tables when it has child tables, if ONLY clause was not given. So, we need to obtain oids to be relabeled using find_all_inheritors(), and need to ask providers whether it allows, or not. But, obviously, it is specific for relations. For other object class, the target object to be relabeled is identified byobject in SECURITY LABEL statement. It will be parsed by the upcoming parse_object.c feature, then it solves the object name to ObjectAddress. So, we can apply access controls after setting up the ObjectAddress with common hooks for object classes except for relations, like: void check_object_relabel(ObjectAddress object, const char *new_label); So just construct an ObjectAddress for each relation and call the check function once for each. OK, I'll revise it. I think it is 100% clear that row-level security will require completely separate infrastructure, and therefore I'm not even a tiny bit worried about this. :-) Hmm. Are you saying we may degrade the feature when we switch to the completely separate infrastructure? Is it preferable?? Uh... no, not really. I'm saying that I don't think we're backing ourselves into a corner. What makes you think we are? Sorry, meaning of the last question was unclear for me Is it a idiom? Thanks, -- 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
