Thread: Prohibit row-security + inheritance in 9.4?

Hi all

I'm having a hard time seeing any reasonable semantics for the
combination of row-security and inheritance in 9.4 that are also
practical to implement.

I'm considering just punting on inheritance for 9.4, adding checks to
prohibit inheritance from being added to a rel with row security and
prohibiting any rel with inheritance from being given a row-security policy.

Here's why:

Detail
----

Earlier discussions seemed to settle on each relation having its own
row-security quals applied independently. So quals on a parent rel did
not cascade down to child rels.

That gives you a consistent view of the data in a child rel when
querying via the parent vs directly, which is good. It's surprising when
you query via the parent and see rows the parent's row-security
qualifier doesn't permit, but that surprising behaviour is consistent
with other surprising things in inheritance, like a primary key on the
parent not constraining rows inserted into children.

The trouble is that this isn't going to work when applying row-security
rules using the security barriers support. Security quals get expanded
before inheritance expansion so that they're copied to all child rels.
Just what you'd expect when querying a relation that's a parent of an
inheritance tree via a view.

It's what you'd expect to happen when querying a parent rel with
row-security, too. Parent quals are applied to children. But that then
gives you an inconsistent view of a rel's contents based on whether you
query it via a parent or directly.

I embarked upon that because of the concern that was expressed here
about the way KaiGai's row-security patch fiddles directly with
remapping attributes during planning; rebuilding row-security on top of
updatable security barrier views was seen as a cleaner approach.

So. I could:

1. Prohibit (in CREATE TABLE ... INHERITS, ALTER TABLE ... INHERITS, and
ALTER TABLE ... SET ROW SECURITY) any parent or child rel from having
row-security policy, i.e. punt it until 9.5;

2. Do another round of security qual expansion that fetches quals from
pg_rowsecurity *after* inheritance expansion, giving us the
each-relation-stands-alone behaviour;

3. Accept the inconsistent view of child rel contents in exchange for
the otherwise sane behaviour of applying a parent's quals to children;
document that if you don't want this, don't grant users direct access to
the child tables;

4. attempt to pull quals from parents when querying a child rel directly.


I'm going to have a go at making (2) happen, but if it doesn't come
together fast, I'll just prohibit the combination of inheritance and
row-security for 9.4.  That won't upset updatable security barrier
views, only actual row-security policy. People who want row-security
over partitioned tables will just have to be patient.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

Craig, all,

* Craig Ringer (craig@2ndquadrant.com) wrote:
> I'm considering just punting on inheritance for 9.4, adding checks to
> prohibit inheritance from being added to a rel with row security and
> prohibiting any rel with inheritance from being given a row-security policy.

I'm alright with punting on it for 9.4, certainly we need to get the
row-security work done soon if we're going to have any chance of having
this in 9.4 (and I have to admit, I suspect we're pushing the envelope
here already wrt getting row-security into 9.4).

> Earlier discussions seemed to settle on each relation having its own
> row-security quals applied independently. So quals on a parent rel did
> not cascade down to child rels.

This strikes me as a bit odd- isn't this against how we handle the GRANT
system when it comes to inheiritance?  That is to say- if you have
access to query the parent, then you'll get rows back from the child and
I believe everyone feels that makes perfect sense.

> That gives you a consistent view of the data in a child rel when
> querying via the parent vs directly, which is good. It's surprising when
> you query via the parent and see rows the parent's row-security
> qualifier doesn't permit, but that surprising behaviour is consistent
> with other surprising things in inheritance, like a primary key on the
> parent not constraining rows inserted into children.

While it agrees with the PK issue, that's a data consistency concern
rather than a security concern, and to contrast this with our existing
security model- if you don't have access to the parent then you can't
query against it; doesn't matter if you have access to the child or not.

> The trouble is that this isn't going to work when applying row-security
> rules using the security barriers support. Security quals get expanded
> before inheritance expansion so that they're copied to all child rels.
> Just what you'd expect when querying a relation that's a parent of an
> inheritance tree via a view.

Right.

> It's what you'd expect to happen when querying a parent rel with
> row-security, too. Parent quals are applied to children. But that then
> gives you an inconsistent view of a rel's contents based on whether you
> query it via a parent or directly.

... Just how our existing GRANT system works.

If you want to constrain the children in the same way as the parent,
then the user can add to the row-security on the children to match the
parent.  If the user wants to have one view for the entire inheiritance
tree then they need to only implement the row-security on the parent and
not grant any access for users on the children (or, if they're paranoid,
add row-security to the children which are essentially deny-all).

If we force everything to behave the same between querying the parent
and querying the children then we cut off various scenarios of
partitioning where users are allowed to query specific children but not
the parent or query the parent to get things they're not able to see
directly from the children.  This matches the "each rel stands alone"
comment below, from my perspective, but seems to result in a different
behavior from what you describe here.  Perhaps that's because "each rel
standing alone", when it comes to inheiritance, looks at the parent as
a simple combination of all the rows from the parent plus the children
and then filtering them based on the row-security of the parent.

> I embarked upon that because of the concern that was expressed here
> about the way KaiGai's row-security patch fiddles directly with
> remapping attributes during planning; rebuilding row-security on top of
> updatable security barrier views was seen as a cleaner approach.

I agree with this approach of adding row-security over top of updatable
security barrier views.

> 1. Prohibit (in CREATE TABLE ... INHERITS, ALTER TABLE ... INHERITS, and
> ALTER TABLE ... SET ROW SECURITY) any parent or child rel from having
> row-security policy, i.e. punt it until 9.5;
>
> 2. Do another round of security qual expansion that fetches quals from
> pg_rowsecurity *after* inheritance expansion, giving us the
> each-relation-stands-alone behaviour;

Again, this doesn't match what I think of as
"each-relation-stands-alone".

> 3. Accept the inconsistent view of child rel contents in exchange for
> the otherwise sane behaviour of applying a parent's quals to children;
> document that if you don't want this, don't grant users direct access to
> the child tables;

Or write identical row-security rules on the children (or deny-all
row-security to force the user to use the parent).

> 4. attempt to pull quals from parents when querying a child rel directly.

That strikes me as borderline insane. ;)
Thanks,
    Stephen

On Thu, Jan 30, 2014 at 2:39 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
> Earlier discussions seemed to settle on each relation having its own
> row-security quals applied independently. So quals on a parent rel did
> not cascade down to child rels.

Do you have a link to that prior discussion?  I don't remember
reaching consensus specifically on that point.  And at any rate, if
the implementation complexity mitigates strongly in the other
direction, that may be a sign that we should consider revising our
opinion on what the right thing to do is.

The problem with just accepting that feature A doesn't work with
feature B and releasing anyway is that, in many cases, nobody ever
gets around to fixing it.  We have some of those warts already, and
I'm not keen to add more.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 01/30/2014 10:41 PM, Stephen Frost wrote:
>> Earlier discussions seemed to settle on each relation having its 
>> own row-security quals applied independently. So quals on a 
>> parent rel did not cascade down to child rels.
> 
> This strikes me as a bit odd- isn't this against how we handle the 
> GRANT system when it comes to inheiritance?  That is to say- if
> you have access to query the parent, then you'll get rows back from
> the child and I believe everyone feels that makes perfect sense.

Thanks for taking the time to look at this.

I agree that it's odd. The trouble is that there's a conflict between
two "makes perfect sense"s here.

I expect to get all rows, including inherited rows, filtered by a
parent's predicate back when you query the parent. Fair enough.

I also expect that when I query a child table, I'll see the same rows
I see when I query it via the parent table. Especially in the common
case of an empty parent table.

One of these has to give, we can't have both.

I'm speaking with an outside party who has an inheritance-based data
model they want to apply row-security to. Hopefully that'll shed some
light on practical implications.

There's another bit of fun too: If you have a policy on a child, and
query the child via the parent, should the child policy be applied?
The immediate thought is "obviously" - but then, often the child and
parent policies are going to be the same, in which case it's a
duplicate filter step. Here security trumps efficiency; I think we
just apply both, and leave proving they're identical and skipping the
child's as an optimization problem for later.

>> It's what you'd expect to happen when querying a parent rel with
>>  row-security, too. Parent quals are applied to children. But
>> that then gives you an inconsistent view of a rel's contents
>> based on whether you query it via a parent or directly.
> 
> ... Just how our existing GRANT system works.

True; it's possible to be able to query the parent, but not its
children, at present.

Treating row-security checks as permission checks, that'd make this
consistent. The difference is that you get a nice error telling you
what's going on currently, not a potentially WTF-y different resultset.

> If you want to constrain the children in the same way as the 
> parent, then the user can add to the row-security on the children 
> to match the parent.

Yes, with the caveat mentioned above that this will cause multiple
nested row-security policies when querying the parent; each child's
policy will get applied, *and* the parent's policy will get applied.
Inefficient and ugly if they're the same.

> If the user wants to have one view for the entire inheiritance
> tree then they need to only implement the row-security on the
> parent and not grant any access for users on the children (or, if
> they're paranoid, add row-security to the children which are
> essentially deny-all).

That works if the children are used for partitioning/inheritance,
where no direct access (or at least no read access) is required. It
doesn't work so well when they're actually being used in a "real"
inheritance model, where they'll have their own additional attributes
that can only be accessed via the child.

In that case the only option is to apply a policy to each child too.
If we apply the parent policy when querying via the parent, we get
multiple nested layers of policy, but it still works.

> If we force everything to behave the same between querying the 
> parent and querying the children then we cut off various scenarios 
> of partitioning where users are allowed to query specific children 
> but not the parent or query the parent to get things they're not 
> able to see directly from the children.

That's a fair concern. Note that the second one won't work if we apply
child policies when querying via the parent though; there'd be no way
to see rows via the parent that you can't see via the child.

>> 4. attempt to pull quals from parents when querying a child rel 
>> directly.
> 
> That strikes me as borderline insane. ;)

I'm glad to hear it, because it'd be inefficient and horrifying to
implement.

- -- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.15 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQEcBAEBAgAGBQJS6u+5AAoJELBXNkqjr+S2QkEH/iXg4qww3KfcKLW/dDHWUL/g
33T/168ZGgeAvgFgTQdrgZmM8bUDsNnCD/GdH4PBmNWMlxwTeHmdANBEsKgDCL7r
Fu4HuF0JFEQMqHPtZSKUIXxW1KYEnWjISd+4YDQqor3aH03OV3z4vFEgAi73truR
kcqOe/xyeuPQDPe/9UTtiyIT2g/sQaeXhNqQx+queKYwjgYTIZgkUs0y46lH4pAK
nvWcWCscPIJ4bFpMr3joJQiFwegRaVIcAac89uZHL5iuMKPzp5lfEfWHUmTreZLu
1gPjRxWTcOhNZoaVpVCBA+Gsqw0255IxWKKD8I2RYPp0bK88t42cCB3aHejAjzo=
=cB2U
-----END PGP SIGNATURE-----

On 01/31/2014 01:18 AM, Robert Haas wrote:
> On Thu, Jan 30, 2014 at 2:39 AM, Craig Ringer <craig@2ndquadrant.com> wrote:
>> Earlier discussions seemed to settle on each relation having its own
>> row-security quals applied independently. So quals on a parent rel did
>> not cascade down to child rels.
> 
> Do you have a link to that prior discussion?

Not to hand; I'm basing that on discussion with KaiGai, and on the
implementation of his RLS patch. The patch goes far out of its way to
ensure that policies on a parent relation aren't applied to children,
only to rows taken directly from the parent.

I read some discussion on the topic when I was first reviewing all the
old threads for this, but didn't see anything that seemed to
conclusively decide on that approach.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

> This strikes me as a bit odd- isn't this against how we handle the
> GRANT system when it comes to inheiritance?  That is to say- if
> you have access to query the parent, then you'll get rows back from
> the child and I believe everyone feels that makes perfect sense.

Thanks for taking the time to look at this.

I agree that it's odd. The trouble is that there's a conflict between
two "makes perfect sense"s here.

I expect to get all rows, including inherited rows, filtered by a
parent's predicate back when you query the parent. Fair enough.

I also expect that when I query a child table, I'll see the same rows
I see when I query it via the parent table. Especially in the common
case of an empty parent table.

One of these has to give, we can't have both.

I'm speaking with an outside party who has an inheritance-based data
model they want to apply row-security to. Hopefully that'll shed some
light on practical implications.

There's another bit of fun too: If you have a policy on a child, and
query the child via the parent, should the child policy be applied?

The immediate thought is "obviously" - but then, often the child and
parent policies are going to be the same, in which case it's a
duplicate filter step. Here security trumps efficiency; I think we
just apply both, and leave proving they're identical and skipping the
child's as an optimization problem for later.

> ... Just how our existing GRANT system works.

True; it's possible to be able to query the parent, but not its
children, at present.

Treating row-security checks as permission checks, that'd make this
consistent. The difference is that you get a nice error telling you
what's going on currently, not a potentially WTF-y different resultset.

> If you want to constrain the children in the same way as the
> parent, then the user can add to the row-security on the children
> to match the parent.

Yes, with the caveat mentioned above that this will cause multiple
nested row-security policies when querying the parent; each child's
policy will get applied, *and* the parent's policy will get applied.
Inefficient and ugly if they're the same.

> If the user wants to have one view for the entire inheiritance
> tree then they need to only implement the row-security on the
> parent and not grant any access for users on the children (or, if
> they're paranoid, add row-security to the children which are
> essentially deny-all).

That works if the children are used for partitioning/inheritance,
where no direct access (or at least no read access) is required. It
doesn't work so well when they're actually being used in a "real"
inheritance model, where they'll have their own additional attributes
that can only be accessed via the child.

In that case the only option is to apply a policy to each child too.
If we apply the parent policy when querying via the parent, we get
multiple nested layers of policy, but it still works.

> If we force everything to behave the same between querying the
> parent and querying the children then we cut off various scenarios
> of partitioning where users are allowed to query specific children
> but not the parent or query the parent to get things they're not
> able to see directly from the children.

That's a fair concern. Note that the second one won't work if we apply
child policies when querying via the parent though; there'd be no way
to see rows via the parent that you can't see via the child.

>> 4. attempt to pull quals from parents when querying a child rel
>> directly.
>
> That strikes me as borderline insane. ;)

I'm glad to hear it, because it'd be inefficient and horrifying to
implement.

On 01/31/2014 09:01 AM, Stephen Frost wrote:
> I don't see where this follows at all- clearly, you already get a subset
> of rows from the child than if you queried the parent because there are
> other children.

Er, what? I don't see what you're saying here.

Currently, when you query the parent, you see rows from other children
(superset). You don't ever _not_ see rows from the child that you would
see when querying the child directly.

> If you are first playing with inheritance in PG then it
> might seem odd for that to be the case. Ditto for what you describe
> where the child returns more rows than the parent, but these things need
> to simply be documented as "this is how it works" for those cases where
> both are reasonable possibilities and we need to pick one. 

I'm increasingly inclined to agree. Everything else is too messy, and
creates inflexible limitations for users.

> Personally, I don't see the suggestion that we filter rows accessed via
> the child based on quals of the parent as making any sense.

Neither do I; that (point 4, original post) was pretty much a way to
make the other approaches look better by comparison ;-)

> I feel the
> same about applying child quals when querying through the parent as we
> don't apply GRANT permissions that way. Using the parent and using the
> child are two different paths by which to access the data and which you
> are using is what drives what you will see.

That's a reasonable way to explain it, and consistent with the
privileges model already used for inheritance.

> Is there a case which can't be implemented if the two are independent as
> I am describing?  There are cases which can NOT be implemented if we
> force the two paths to be handled identically but I feel the approach
> where we keep them independently managed is flexible to allow the other
> cases if people want them. 

The only case prevented is one where access to the child via the parent
shows rows that the parent's row-security qual would hide, because the
child's qual doesn't.

Personally I think that's ugly anyway; I don't want to support that, and
have only been looking at it because it'd solve the consistency issues.

Since the user can achieve this with:

SELECT ...
FROM ONLY parent
UNION ALL
SELECT ...
FROM ONLY child1

I think it's fine to just apply the parent qual to all children.

>     There's another bit of fun too: If you have a policy on a child, and
>     query the child via the parent, should the child policy be applied?
> 
> 
> No!  We do not do that for GRANT and I do not see doing it for row
> security either. 

If we're approaching this as "different entry point, different policy",
that makes sense, and I'm increasingly pesuaded by that view of things.

>     Treating row-security checks as permission checks, that'd make this
>     consistent. The difference is that you get a nice error telling you
>     what's going on currently, not a potentially WTF-y different resultset.
> 
> 
> I understand where you're coming from but this strikes me as a
> documentation/definition issue and not really a cause for concern or
> against POLA. These are complex and important topics that anyone who
> cares about security needs to understand. 

I'm happy with that. It's clear that there isn't going to be any way to
do this that doesn't result in _some_ kind of surprising behaviour, so
it's just a matter of being clear about where the astonishment lies.

So what we're talking about here (conveniently, exactly what's currently
impemented) is to:

Apply the policy of the relation actually named in the query before
inheritance expansion. If the relation has children expanded during
planning, allow the parent policy to be copied to those children. The
children are _not_ checked for their own row-security policies when the
appendrel is created, and any child policies are _not_ applied.

That's consistent with how security barrier views (and views in general)
work, and it means that the policy on a relation is applied consistently
to all rows, including rows from child relations. As we discussed, it's
also consistent with relation-level GRANTs for access to relations. The
trade-off is that it creates inconsistent views of the contents of the
data depending on whether you query via a parent, or query a child
directly, and it means that child policies are ignored when querying via
a parent relation.

Since that's what I've already written, I'm happy with that ;-)

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

* Craig Ringer (craig@2ndquadrant.com) wrote:
> On 01/31/2014 09:01 AM, Stephen Frost wrote:
> > I don't see where this follows at all- clearly, you already get a subset
> > of rows from the child than if you queried the parent because there are
> > other children.
>
> Er, what? I don't see what you're saying here.

You were argueing that people may be confused by the parent and the
child returning different sets of rows.  I was pointing out that this is
already the case.

> Currently, when you query the parent, you see rows from other children
> (superset). You don't ever _not_ see rows from the child that you would
> see when querying the child directly.

Right- but you don't see rows from *other* children when querying the
child either.  I was merely trying to point out that we don't make
children and parents be synonyms of each other.  It's not a very strong
argument when it comes to this discussion, but I thought it helped
illustrate that anyone using inheiritance-as-PG-does-it already has a
fair bit of reading to do to be able to understand what's going on.

We also don't have any mechanism to automatically pick the right child
when you insert into the parent either (of course, that can be done with
triggers but that's not automatic :).  Basically, it requires a
non-trivial amount of effort and understanding to use these features.

> > Is there a case which can't be implemented if the two are independent as
> > I am describing?  There are cases which can NOT be implemented if we
> > force the two paths to be handled identically but I feel the approach
> > where we keep them independently managed is flexible to allow the other
> > cases if people want them.
>
> The only case prevented is one where access to the child via the parent
> shows rows that the parent's row-security qual would hide, because the
> child's qual doesn't.

It makes absolutely zero sense, in my head anyway, to have rows returned
when querying the parent which should NOT be returned based on the quals
of the parent.

> Personally I think that's ugly anyway; I don't want to support that, and
> have only been looking at it because it'd solve the consistency issues.

Good, though I would characterize it as "bug" or "wrong" more than
"ugly". :)

> Since the user can achieve this with:
>
> SELECT ...
> FROM ONLY parent
> UNION ALL
> SELECT ...
> FROM ONLY child1

They could do it with a more complex inheiritance tree also, no?  As in,
with multiple levels or multiple parents where they could redefine the
quals to be used?  Don't think they'd have to resort to views if they
really wanted this (which I tend to doubt many would...).

> I think it's fine to just apply the parent qual to all children.

Agreed. :)

> So what we're talking about here (conveniently, exactly what's currently
> impemented) is to:

I do like that. :)

> Apply the policy of the relation actually named in the query before
> inheritance expansion. If the relation has children expanded during
> planning, allow the parent policy to be copied to those children. The
> children are _not_ checked for their own row-security policies when the
> appendrel is created, and any child policies are _not_ applied.

Right.

> That's consistent with how security barrier views (and views in general)
> work, and it means that the policy on a relation is applied consistently
> to all rows, including rows from child relations. As we discussed, it's
> also consistent with relation-level GRANTs for access to relations. The
> trade-off is that it creates inconsistent views of the contents of the
> data depending on whether you query via a parent, or query a child
> directly, and it means that child policies are ignored when querying via
> a parent relation.

Right, let's make sure the documentation lays this out as clearly as we
can make it and perhaps also draw those parallels to how views are
handled with the GRANT system.

> Since that's what I've already written, I'm happy with that ;-)

Excellent. ;)
Thanks,
    Stephen

* Yeb Havinga (y.t.havinga@mgrid.net) wrote:
> IMHO, there is another way to implement this, other than the
> procedure to override the child-rel-quals with the ones from the
> parent. At DDL time, synchronize quals on the parent with rls quals
> of the childs. Isn't this also what happens with constraints?

No, we're not going to do that.  We don't do it for GRANT and I don't
think it makes sense to do it here.

If we wanted to make them the same then we'd throw out the ability to do
any kind of changes or actions on the child and then we'd have actual
partitioning.  We don't have that though, we have inheiritance.

> Then during expansion of the range table, no code is needed to
> ignore child rls quals and copy parent rels to child rels.

This is what's already implemented and isn't a huge amount of code to
begin with, so I don't see this as being an argument against having the
flexibility.

> Also, the security policy applied would be invariant to the route
> through which the rows were accessed:

You could also get this by simply only allowing access to the parent and
not granting any privileges on the children.

> - directly to the child row: child rls quals and parent quals (by
> propagate at ddl) are applied.
> - through the parent: child rls quals and parent quals applied.

If you want them to be the same then you can implement this yourself
without having PG force it on you.
Thanks,
    Stephen

On 2014-01-31 16:05, Stephen Frost wrote:
> * Yeb Havinga (y.t.havinga@mgrid.net) wrote:
>> IMHO, there is another way to implement this, other than the
>> procedure to override the child-rel-quals with the ones from the
>> parent. At DDL time, synchronize quals on the parent with rls quals
>> of the childs. Isn't this also what happens with constraints?
> No, we're not going to do that.  We don't do it for GRANT and I don't
> think it makes sense to do it here.
This reasoning could go either way. GRANT is on a complete set of rows. 
This is a restriction on the level of individual rows, and in that 
sense, it is more like a row-level CHECK constraint.
> If we wanted to make them the same then we'd throw out the ability to do
> any kind of changes or actions on the child and then we'd have actual
> partitioning.  We don't have that though, we have inheiritance.

I fail to understand this, probably because I do not have a partition 
use case for inheritance, but rather an information model that is more 
ontology like. The more specific childs get down the inheritance tree, 
more columns they get, and their requirements might differ completely in 
nature from their siblings, and make no sense at all as well when 
specified at the level of the parent (or even impossible, since the 
parent does not have all the columns).

>> Then during expansion of the range table, no code is needed to
>> ignore child rls quals and copy parent rels to child rels.
> This is what's already implemented and isn't a huge amount of code to
> begin with, so I don't see this as being an argument against having the
> flexibility.

It would seem to me that any additional logic that can be avoided during 
planning is a good thing. Also, the argument that something is already 
implemented, does not itself make it good to commit.

What do you mean with 'having the flexibility' and why is that good?
>
>> Also, the security policy applied would be invariant to the route
>> through which the rows were accessed:
> You could also get this by simply only allowing access to the parent and
> not granting any privileges on the children.

That might work for partitioning, but not for use cases where childs 
have more columns than parents.
>> - directly to the child row: child rls quals and parent quals (by
>> propagate at ddl) are applied.
>> - through the parent: child rls quals and parent quals applied.
> If you want them to be the same then you can implement this yourself
> without having PG force it on you.

I suggested it as  a solution for a requirement worded upthread as "It 
makes absolutely zero sense, in my head anyway, to have rows returned 
when querying the parent which should NOT be returned based on the quals 
of the parent." without disregarding rls-quals on childs.

regards,
Yeb Havinga

* Yeb Havinga (yebhavinga@gmail.com) wrote:
> This reasoning could go either way. GRANT is on a complete set of
> rows. This is a restriction on the level of individual rows, and in
> that sense, it is more like a row-level CHECK constraint.

Well, we certainly don't force CHECK constraints on children to apply to
the parent.  If you've got a trigger which is inserting into the child,
that's a different story.

> >If we wanted to make them the same then we'd throw out the ability to do
> >any kind of changes or actions on the child and then we'd have actual
> >partitioning.  We don't have that though, we have inheiritance.
>
> I fail to understand this, probably because I do not have a
> partition use case for inheritance, but rather an information model
> that is more ontology like. The more specific childs get down the
> inheritance tree, more columns they get, and their requirements
> might differ completely in nature from their siblings, and make no
> sense at all as well when specified at the level of the parent (or
> even impossible, since the parent does not have all the columns).

My point here is that you're making an argument for reducing what can be
different between a parent and a child relation if you force
permissions, etc.

> >>Then during expansion of the range table, no code is needed to
> >>ignore child rls quals and copy parent rels to child rels.
> >This is what's already implemented and isn't a huge amount of code to
> >begin with, so I don't see this as being an argument against having the
> >flexibility.
>
> It would seem to me that any additional logic that can be avoided
> during planning is a good thing. Also, the argument that something
> is already implemented, does not itself make it good to commit.

We're already avoiding additional logic by *not* considering the child
relation's quals when it's queried by the parent.

> What do you mean with 'having the flexibility' and why is that good?

The flexibility to allow a user to control access to the child
independently from the access for the parent.

> >>Also, the security policy applied would be invariant to the route
> >>through which the rows were accessed:
> >You could also get this by simply only allowing access to the parent and
> >not granting any privileges on the children.
>
> That might work for partitioning, but not for use cases where childs
> have more columns than parents.

You could still put whatever quals you want on the parent and the child
independently to provide whatever security you want.  The true
inheiritance case makes that more clear, imv, not less- the quals that
you want may not make any sense when applied to the parent as the parent
has fewer columns (perhaps it doesn't have the 'sensitive' columns, for
example).

> >>- directly to the child row: child rls quals and parent quals (by
> >>propagate at ddl) are applied.
> >>- through the parent: child rls quals and parent quals applied.
> >If you want them to be the same then you can implement this yourself
> >without having PG force it on you.
>
> I suggested it as  a solution for a requirement worded upthread as
> "It makes absolutely zero sense, in my head anyway, to have rows
> returned when querying the parent which should NOT be returned based
> on the quals of the parent." without disregarding rls-quals on
> childs.

The point was to disregard the rls-quals on the children.  There are
ways we could make what you're suggesting work but trying to do it with
DDL hacks wouldn't be the right answer anyway- consider what happens
when you're setting up different quals on different children or when
someone goes in and removes the quals on the parent directly.
Thanks,
    Stephen

On 2014-01-31 15:10, Stephen Frost wrote:
> * Craig Ringer (craig@2ndquadrant.com) wrote:
>> On 01/31/2014 09:01 AM, Stephen Frost wrote:
>> The only case prevented is one where access to the child via the parent
>> shows rows that the parent's row-security qual would hide, because the
>> child's qual doesn't.
> It makes absolutely zero sense, in my head anyway, to have rows returned
> when querying the parent which should NOT be returned based on the quals
> of the parent.

IMHO, there is another way to implement this, other than the procedure 
to override the child-rel-quals with the ones from the parent. At DDL 
time, synchronize quals on the parent with rls quals of the childs. 
Isn't this also what happens with constraints?

Then during expansion of the range table, no code is needed to ignore 
child rls quals and copy parent rels to child rels.

Also, the security policy applied would be invariant to the route 
through which the rows were accessed:
- directly to the child row: child rls quals and parent quals (by 
propagate at ddl) are applied.
- through the parent: child rls quals and parent quals applied.

regards,
Yeb Havinga

On 01/31/2014 11:24 PM, Yeb Havinga wrote:
> On 2014-01-31 16:05, Stephen Frost wrote:
>> * Yeb Havinga (y.t.havinga@mgrid.net) wrote:
>>> IMHO, there is another way to implement this, other than the
>>> procedure to override the child-rel-quals with the ones from the
>>> parent. At DDL time, synchronize quals on the parent with rls quals
>>> of the childs. Isn't this also what happens with constraints?
>> No, we're not going to do that.  We don't do it for GRANT and I don't
>> think it makes sense to do it here.
> This reasoning could go either way. GRANT is on a complete set of rows.
> This is a restriction on the level of individual rows, and in that
> sense, it is more like a row-level CHECK constraint.
>> If we wanted to make them the same then we'd throw out the ability to do
>> any kind of changes or actions on the child and then we'd have actual
>> partitioning.  We don't have that though, we have inheiritance.
> 
> I fail to understand this, probably because I do not have a partition
> use case for inheritance, but rather an information model that is more
> ontology like. The more specific childs get down the inheritance tree,
> more columns they get, and their requirements might differ completely in
> nature from their siblings, and make no sense at all as well when
> specified at the level of the parent (or even impossible, since the
> parent does not have all the columns).

That's a bit inconsistent with how Pg's inheritance works, though.
Conceptually rows in children *are* part of the parent.

You cannot see the child columns when querying via the parent, so it's
not a problem to constrain the ability to see the extra child columns
with row-security. They can only be accessed when querying the child
directly, where the child's row-security expression will apply.

So you're not exposing information that's specific to the children, and
the inherited common cols are, conceptually *part* of the parent, so
applying the parent's qual makes sense.

>>> Then during expansion of the range table, no code is needed to
>>> ignore child rls quals and copy parent rels to child rels.
>> This is what's already implemented and isn't a huge amount of code to
>> begin with, so I don't see this as being an argument against having the
>> flexibility.
> 
> It would seem to me that any additional logic that can be avoided during
> planning is a good thing. Also, the argument that something is already
> implemented, does not itself make it good to commit.

The implementation with the minimum of required logic and complexity is
"apply the parent's row-security quals to the children, don't check
children for quals".

Any special handling of child rels creates exciting challenges because
inheritance expansion happens _after_ a bunch of the query planner and
optimizer has run. Injecting new expression trees at this point is
messy, especially if you want those expression trees to in turn contain
row-security qualified tables, inheritance, etc.

As previously discussed, applying the parent qual to children ensures
that what's visible when querying a relation that has children is
consistent with its row-security qual.

If the parent qual is applied only on the parent rel directly, not
children, then querying the parent could emit rows not permitted by the
parent's row-security qual. I'm not happy with that, and as Stephen
poined out upthread, it isn't really consistent with how permissions
work with inheritance otherwise.

If instead the parent qual is applied to the parent and all children,
and you *also* add the quals of each child, you get a complex, hard to
debug, hard to optimize mess. You also run back into the problem
mentioned above, that adding quals after inhertiance expansion is messy
and problematic. It's also really ugly in what's going to be the most
common case, where the child quals are the same as the parent quals, as
you'll get nested identical quals.

Despite the challenges with it, I think that's the least insane way to
respect child quals on parents. It has pretty much zero chance of
happening in 9.4, though; the discussed approach of building
row-security on top of updatable security barrier views doesn't play
well with adding inheritance on inheritance-expanded children.

One answer for that would be to keep inheritance as it is for 9.4 (if I
can get the remaining issues sorted out) and in 9.5, if possible, allow
the addition of a row-security qual that, if it appears on a child rel
during inheritance expansion, _is_ expanded. At least, if it proves
necessary, which I'm not entirely convinced of.

> I suggested it as  a solution for a requirement worded upthread as "It
> makes absolutely zero sense, in my head anyway, to have rows returned
> when querying the parent which should NOT be returned based on the quals
> of the parent." without disregarding rls-quals on childs.

I'm not sure I understand what you are saying here.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

On Mon, Feb 3, 2014 at 10:23 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
>>>> Then during expansion of the range table, no code is needed to
>>>> ignore child rls quals and copy parent rels to child rels.
>>> This is what's already implemented and isn't a huge amount of code to
>>> begin with, so I don't see this as being an argument against having the
>>> flexibility.
>>
>> It would seem to me that any additional logic that can be avoided during
>> planning is a good thing. Also, the argument that something is already
>> implemented, does not itself make it good to commit.
>
> The implementation with the minimum of required logic and complexity is
> "apply the parent's row-security quals to the children, don't check
> children for quals".
>
> Any special handling of child rels creates exciting challenges because
> inheritance expansion happens _after_ a bunch of the query planner and
> optimizer has run. Injecting new expression trees at this point is
> messy, especially if you want those expression trees to in turn contain
> row-security qualified tables, inheritance, etc.
>
> As previously discussed, applying the parent qual to children ensures
> that what's visible when querying a relation that has children is
> consistent with its row-security qual.
>
> If the parent qual is applied only on the parent rel directly, not
> children, then querying the parent could emit rows not permitted by the
> parent's row-security qual. I'm not happy with that, and as Stephen
> poined out upthread, it isn't really consistent with how permissions
> work with inheritance otherwise.
>
> If instead the parent qual is applied to the parent and all children,
> and you *also* add the quals of each child, you get a complex, hard to
> debug, hard to optimize mess. You also run back into the problem
> mentioned above, that adding quals after inhertiance expansion is messy
> and problematic. It's also really ugly in what's going to be the most
> common case, where the child quals are the same as the parent quals, as
> you'll get nested identical quals.
>
> Despite the challenges with it, I think that's the least insane way to
> respect child quals on parents. It has pretty much zero chance of
> happening in 9.4, though; the discussed approach of building
> row-security on top of updatable security barrier views doesn't play
> well with adding inheritance on inheritance-expanded children.
>
> One answer for that would be to keep inheritance as it is for 9.4 (if I
> can get the remaining issues sorted out) and in 9.5, if possible, allow
> the addition of a row-security qual that, if it appears on a child rel
> during inheritance expansion, _is_ expanded. At least, if it proves
> necessary, which I'm not entirely convinced of.

Me, neither.  When you first described the scheme you're currently
pursuing, I thought it sounded kind of nuts.  But as I've thought
about it more, it's grown on me.  Today, the way to do row-level
security is:

1. Create a security barrier view over the table.
2. Grant rights to the view instead of the table, and tell people to
go through that.

Now, if you did that, as far as I can see, it would have exactly the
same semantics as what you're proposing to do here, with the sole
exception that you wouldn't need to tell people to access the view
rather the table.  So yeah it's kind of funky but I have a feeling any
rule we come up with here will seem odd in some scenarios, and this
one at least has the virtue of being relatively easy to implement and
consistent with how similar things work today.  I can't knock that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company