Multi-tenancy with RLS

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
63 messages Options
1234
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Haribabu Kommi-2
On Wed, Dec 30, 2015 at 11:28 AM, Haribabu Kommi
<[hidden email]> wrote:

> On Thu, Dec 17, 2015 at 12:46 PM, Haribabu Kommi
> <[hidden email]> wrote:
>> Rebased patch is attached as it is having an OID conflict with the
>> latest set of changes
>> in the master branch.
>
> Here I attached new series of patches with a slightly different approach.
> Instead of creating the policies on the system catalog tables whenever
> the catalog security command is executed, just enable row level security
> on the system catalog tables. During the relation build, in
> RelationBuildRowSecurity function, if it is a system relation, frame the
> policy using the policy query which we earlier used to create by parsing it.
>
> With the above approach, in case of any problems in the policy, to use
> the corrected policy, user just needs to replace the binaries. whereas in
> earlier approach, either pg_upgrade or disabling and enabling of catalog
> security is required.
>
> Currently it is changed only for shared system catalog tables and also the
> way of enabling catalog security on shared system catalog tables is through
> initdb only. This also can be changed later. I will do similar changes for
> remaining catalog tables.
>
> Any comments on the approach?
Instead of creating policies during the "alter database" command for database
catalog tables, generating at relation building is leading to an
infinite recursion
loop because of transformExpr call for the qual. Any ideas to handle the same?

Here I attached updated patches to HEAD.

Regards,
Hari Babu
Fujitsu Australia


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

4_database_catalog_tenancy_v4.patch (127K) Download Attachment
3_shared_catalog_tenancy_v3.patch (27K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Amit Langote-2
On 2016/01/04 14:43, Haribabu Kommi wrote:

>>
>> Here I attached new series of patches with a slightly different approach.
>> Instead of creating the policies on the system catalog tables whenever
>> the catalog security command is executed, just enable row level security
>> on the system catalog tables. During the relation build, in
>> RelationBuildRowSecurity function, if it is a system relation, frame the
>> policy using the policy query which we earlier used to create by parsing it.
>>
>> With the above approach, in case of any problems in the policy, to use
>> the corrected policy, user just needs to replace the binaries. whereas in
>> earlier approach, either pg_upgrade or disabling and enabling of catalog
>> security is required.
>>
>> Currently it is changed only for shared system catalog tables and also the
>> way of enabling catalog security on shared system catalog tables is through
>> initdb only. This also can be changed later. I will do similar changes for
>> remaining catalog tables.
>>
>> Any comments on the approach?
>
> Instead of creating policies during the "alter database" command for database
> catalog tables, generating at relation building is leading to an
> infinite recursion
> loop because of transformExpr call for the qual. Any ideas to handle the same?

I tried your latest patch to see what may have caused the infinite
recursion. The recursion occurs during backend startup itself, right?

ISTM, doing transformWhereClause during RelationCacheInitializePhase3()
would not work. Things like operators, functions within the policy qual
require namespace lookup which down the line would call
RelationBuildRowSecurity for pg_namespace build and so on thus causing the
infinite recursion. Perhaps, it would have to be done in a separate phase
after the phase 3 but I'm not sure.

I wonder why do the policy quals need to be built from scratch every time
in RelationBuildRowSecurity for system tables (shared or otherwise)? Why
not just read from the pg_policy catalog like regular relations if ALTER
DATABASE CATALOG SECURITY TRUE already created those entries? Maybe I
missing something though.

Thanks,
Amit




--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Haribabu Kommi-2
On Mon, Jan 4, 2016 at 8:34 PM, Amit Langote
<[hidden email]> wrote:

> On 2016/01/04 14:43, Haribabu Kommi wrote:
>>>
>>> Here I attached new series of patches with a slightly different approach.
>>> Instead of creating the policies on the system catalog tables whenever
>>> the catalog security command is executed, just enable row level security
>>> on the system catalog tables. During the relation build, in
>>> RelationBuildRowSecurity function, if it is a system relation, frame the
>>> policy using the policy query which we earlier used to create by parsing it.
>>>
>>> With the above approach, in case of any problems in the policy, to use
>>> the corrected policy, user just needs to replace the binaries. whereas in
>>> earlier approach, either pg_upgrade or disabling and enabling of catalog
>>> security is required.
>>>
>>> Currently it is changed only for shared system catalog tables and also the
>>> way of enabling catalog security on shared system catalog tables is through
>>> initdb only. This also can be changed later. I will do similar changes for
>>> remaining catalog tables.
>>>
>>> Any comments on the approach?
>>
>> Instead of creating policies during the "alter database" command for database
>> catalog tables, generating at relation building is leading to an
>> infinite recursion
>> loop because of transformExpr call for the qual. Any ideas to handle the same?
>
> I tried your latest patch to see what may have caused the infinite
> recursion. The recursion occurs during backend startup itself, right?
>
> ISTM, doing transformWhereClause during RelationCacheInitializePhase3()
> would not work. Things like operators, functions within the policy qual
> require namespace lookup which down the line would call
> RelationBuildRowSecurity for pg_namespace build and so on thus causing the
> infinite recursion. Perhaps, it would have to be done in a separate phase
> after the phase 3 but I'm not sure.

Thanks for the test. Yes, the issue happens at backend startup itself.
I will give a try by separating the initialization of security
policies after init phase 3.

> I wonder why do the policy quals need to be built from scratch every time
> in RelationBuildRowSecurity for system tables (shared or otherwise)? Why
> not just read from the pg_policy catalog like regular relations if ALTER
> DATABASE CATALOG SECURITY TRUE already created those entries? Maybe I
> missing something though.

Yes, creating policies at start and using them every time when
relation is building works
until there is no problem is found in the policies. The row level
security policies on catalog
tables are created automatically when user enables catalog security,
so user don't have
any control on these policies.

In case if we found any problem in these policies, later if we want to
correct them, for
shared system catalog tables policies user needs to do a pg_upgrade to
correct them.
And for the other catalog tables, user needs to disable and enable the
catalog security
on all databases.

Instead of the above, if we built the policy at run time always for
catalog tables, user
can just replace binaries with latest works. Currently it is working
fine for shared system
catalog tables. I will give a try by separating
RelationBuildRowSecurity from init phase 3.

Regards,
Hari Babu
Fujitsu Australia


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Haribabu Kommi-2
On Mon, Jan 4, 2016 at 10:43 PM, Haribabu Kommi
<[hidden email]> wrote:

> On Mon, Jan 4, 2016 at 8:34 PM, Amit Langote
> <[hidden email]> wrote:
>>
>> I tried your latest patch to see what may have caused the infinite
>> recursion. The recursion occurs during backend startup itself, right?
>>
>> ISTM, doing transformWhereClause during RelationCacheInitializePhase3()
>> would not work. Things like operators, functions within the policy qual
>> require namespace lookup which down the line would call
>> RelationBuildRowSecurity for pg_namespace build and so on thus causing the
>> infinite recursion. Perhaps, it would have to be done in a separate phase
>> after the phase 3 but I'm not sure.
>
> Thanks for the test. Yes, the issue happens at backend startup itself.
> I will give a try by separating the initialization of security
> policies after init phase 3.
Here I attached updated patches with the fix of infinite recursion in
RelationBuildRowSecurity function by checking with a variable that
whether the build row security is already in progress for a system
relation or not. If it is already in progress for a relation, then it doesn't
build the row security description for this relation.

Regards,
Hari Babu
Fujitsu Australia


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

4_database_catalog_tenancy_v5.patch (127K) Download Attachment
3_shared_catalog_tenancy_v4.patch (28K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Amit Langote-2
On 2016/01/06 10:17, Haribabu Kommi wrote:

> On Mon, Jan 4, 2016 at 10:43 PM, Haribabu Kommi
>>
>> Thanks for the test. Yes, the issue happens at backend startup itself.
>> I will give a try by separating the initialization of security
>> policies after init phase 3.
>
> Here I attached updated patches with the fix of infinite recursion in
> RelationBuildRowSecurity function by checking with a variable that
> whether the build row security is already in progress for a system
> relation or not. If it is already in progress for a relation, then it doesn't
> build the row security description for this relation.

Thanks for updating the patch.

Patch 4_database_catalog_tenancy_v5 fails to apply:

patching file src/backend/commands/policy.c
Hunk #3 succeeded at 112 with fuzz 2 (offset 3 lines).
Hunk #4 succeeded at 269 with fuzz 1 (offset 13 lines).
Hunk #5 succeeded at 298 (offset 13 lines).
Hunk #6 succeeded at 365 (offset 12 lines).
Hunk #7 FAILED at 466.
Hunk #8 succeeded at 577 (offset 22 lines).
Hunk #9 succeeded at 607 with fuzz 2 (offset 22 lines).
Hunk #10 succeeded at 633 with fuzz 2 (offset 22 lines).
Hunk #11 FAILED at 801.
Hunk #12 FAILED at 813.
3 out of 12 hunks FAILED -- saving rejects to file
src/backend/commands/policy.c.rej

Thanks,
Amit





--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Haribabu Kommi-2
On Wed, Jan 6, 2016 at 1:43 PM, Amit Langote
<[hidden email]> wrote:

> On 2016/01/06 10:17, Haribabu Kommi wrote:
>> On Mon, Jan 4, 2016 at 10:43 PM, Haribabu Kommi
>>>
>>> Thanks for the test. Yes, the issue happens at backend startup itself.
>>> I will give a try by separating the initialization of security
>>> policies after init phase 3.
>>
>> Here I attached updated patches with the fix of infinite recursion in
>> RelationBuildRowSecurity function by checking with a variable that
>> whether the build row security is already in progress for a system
>> relation or not. If it is already in progress for a relation, then it doesn't
>> build the row security description for this relation.
>
> Thanks for updating the patch.
>
> Patch 4_database_catalog_tenancy_v5 fails to apply:
>
> patching file src/backend/commands/policy.c
> Hunk #3 succeeded at 112 with fuzz 2 (offset 3 lines).
> Hunk #4 succeeded at 269 with fuzz 1 (offset 13 lines).
> Hunk #5 succeeded at 298 (offset 13 lines).
> Hunk #6 succeeded at 365 (offset 12 lines).
> Hunk #7 FAILED at 466.
> Hunk #8 succeeded at 577 (offset 22 lines).
> Hunk #9 succeeded at 607 with fuzz 2 (offset 22 lines).
> Hunk #10 succeeded at 633 with fuzz 2 (offset 22 lines).
> Hunk #11 FAILED at 801.
> Hunk #12 FAILED at 813.
> 3 out of 12 hunks FAILED -- saving rejects to file
> src/backend/commands/policy.c.rej
May be you missed to apply the 3_shared_catalog_tenancy_v4 path,
because 4_database_catalog_tenancy_v5 patch depends on it.

Here I attached all the patches for your convenience, I am able to
apply all patches in the order without any problem.

Regards,
Hari Babu
Fujitsu Australia


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

4_database_catalog_tenancy_v5.patch (127K) Download Attachment
1_any_privilege_option_v3.patch (7K) Download Attachment
2_view_security_definer_v3.patch (17K) Download Attachment
3_shared_catalog_tenancy_v4.patch (28K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Robert Haas
On Tue, Jan 5, 2016 at 11:07 PM, Haribabu Kommi
<[hidden email]> wrote:
> May be you missed to apply the 3_shared_catalog_tenancy_v4 path,
> because 4_database_catalog_tenancy_v5 patch depends on it.
>
> Here I attached all the patches for your convenience, I am able to
> apply all patches in the order without any problem.

Is any committer thinking about taking a serious look at this patch series?

I ask because (1) it seems like it could be nice to have but (2) it
frightens me terribly.  We are generally very sparing about assuming
that "stuff" (triggers, partial indexes, etc.) that works for user
tables can also be made to work for system tables.  I haven't thought
deeply about what might go wrong in this particular case, but it
strikes me that if Haribabu Kommi is building something that is doomed
for some reason, it would be good to figure that out before he spends
any more time on it than he already has.

Apart from the issue of whether this is doomed for some architectural
reason, it is not entirely clear to me that there's any consensus that
we want this.  I don't think that I understand the issues here well
enough to proffer an opinion of my own just yet... but I'd like to
hear what other people think.

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


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Stephen Frost
Robert,

* Robert Haas ([hidden email]) wrote:
> On Tue, Jan 5, 2016 at 11:07 PM, Haribabu Kommi
> <[hidden email]> wrote:
> > May be you missed to apply the 3_shared_catalog_tenancy_v4 path,
> > because 4_database_catalog_tenancy_v5 patch depends on it.
> >
> > Here I attached all the patches for your convenience, I am able to
> > apply all patches in the order without any problem.
>
> Is any committer thinking about taking a serious look at this patch series?

Joe took a look at it earlier and I'm definitely interested in it.
Furhter, he and I have chatted about it a few times.

Reference here to comments from Joe:
http://www.postgresql.org/message-id/55F1FB2E.8020101@...

> I ask because (1) it seems like it could be nice to have but (2) it
> frightens me terribly.  We are generally very sparing about assuming
> that "stuff" (triggers, partial indexes, etc.) that works for user
> tables can also be made to work for system tables.  I haven't thought
> deeply about what might go wrong in this particular case, but it
> strikes me that if Haribabu Kommi is building something that is doomed
> for some reason, it would be good to figure that out before he spends
> any more time on it than he already has.

I'm not aware of any particular reason for it to be doomed out of the
gate.  That isn't to say that we won't find an issue with it or that
I've given it an in depth look (I've not), but the concept seems
reasonable enough and I can't think of any off-hand reasons why it
won't work.

I will note that there's a couple different patches involved here and
they really deserve their own review and consideration.

> Apart from the issue of whether this is doomed for some architectural
> reason, it is not entirely clear to me that there's any consensus that
> we want this.  I don't think that I understand the issues here well
> enough to proffer an opinion of my own just yet... but I'd like to
> hear what other people think.

I'm certainly of the opinion that we want this or something similar.

The big caveat kicking around in my head is if we want to have our own
set of defined policies or if we want to give flexibility to the
administrator to define their own policies.  In particular, I'm
wondering about things like:

CREATE POLICY namespace_limit ON pg_namespace TO company1 USING
  (substring(nspname,1,8) = 'company1_');

Which is a bit different, as I understand it, from what Haribadu has
been proposing and quite a bit more complicated, as we'd then have to
make the internal lookups respect the policy (so things like CREATE
SCHEMA would have to check if you're allowed to actually create that
schema, which would be based on the policy...).

I don't know if we want to try and support that level of flexibility
or if we'd like to simply go based on the concept of 'you only get to
see what you have access to', which I'm thinking will allow us to avoid
changing the existing functions as they are already doing permissions
checks.

My general thinking here is that we could support one set of provided
policies via these patches and then, if there is sufficient interest,
generalize how policies on system catalogs are handled and eventually
allow users to redefine the system catalog policies.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Amit Langote-2
In reply to this post by Haribabu Kommi-2
On 2016/01/06 13:07, Haribabu Kommi wrote:
> On Wed, Jan 6, 2016 at 1:43 PM, Amit Langote
>>
>> Patch 4_database_catalog_tenancy_v5 fails to apply:
>
> May be you missed to apply the 3_shared_catalog_tenancy_v4 path,
> because 4_database_catalog_tenancy_v5 patch depends on it.

Oops, I even missed patches 1 and 2 at all.

>
> Here I attached all the patches for your convenience, I am able to
> apply all patches in the order without any problem.

Okay, thanks!

I applied all the patches. I have a basic question. Sorry though if I've
entirely missed the point (and/or scope) of your proposal. I wonder if
something like the following should not have failed with the patch:

postgres=# CREATE POLICY class_policy ON pg_class TO PUBLIC USING
(relowner = current_user);
ERROR:  permission denied: "pg_class" is a system catalog

Is there no support yet for user-defined catalog policies?

Regards,
Amit




--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Haribabu Kommi-2
On Thu, Jan 7, 2016 at 4:31 PM, Amit Langote
<[hidden email]> wrote:

>
> I applied all the patches. I have a basic question. Sorry though if I've
> entirely missed the point (and/or scope) of your proposal. I wonder if
> something like the following should not have failed with the patch:
>
> postgres=# CREATE POLICY class_policy ON pg_class TO PUBLIC USING
> (relowner = current_user);
> ERROR:  permission denied: "pg_class" is a system catalog
>
> Is there no support yet for user-defined catalog policies?
Currently the patches don't have the support of allowing user to
create policies on catalog tables. The policies similar like you
specified are prepared for all eligible catalog tables and those
will be used when the user enables the catalog security.

Presently, default policies are used to provide proper multi-tenancy
behavior. May be we can add the support of user to update the
existing policies and add new policies on the catalog tables
without dropping the creation of default polices, as these are
required for supporting multi-tenancy by default without any
user policies.

Example usage:

postgres=# create role test_user1;
CREATE ROLE
postgres=# create role test_user2;
CREATE ROLE
postgres=# alter database postgres with catalog security true;
ALTER DATABASE
postgres=# set session authorization test_user1;
SET
postgres=> create table tbl1(f1 int);
CREATE TABLE
postgres=> set session authorization test_user2;
SET
postgres=> create table tbl2(f2 int);
CREATE TABLE
postgres=> \d
         List of relations
 Schema | Name | Type  |   Owner
--------+------+-------+------------
 public | tbl2 | table | test_user2
(1 row)

postgres=> select attrelid, attname from pg_attribute where attname
like 'f%' and attrelid > 16345;
 attrelid | attname
----------+---------
    16389 | f2
(1 row)

postgres=> set session authorization test_user1;
SET
postgres=> \d
         List of relations
 Schema | Name | Type  |   Owner
--------+------+-------+------------
 public | tbl1 | table | test_user1
(1 row)

postgres=> select attrelid, attname from pg_attribute where attname
like 'f%' and attrelid > 16345;
 attrelid | attname
----------+---------
    16386 | f1
(1 row)

Without multi-tenancy patches, both users can see two tables
and columns that are created.

Turning off catalog security is not working in earlier patches,
because of a using a wrong tuple. updated patch is attached.

Regards,
Hari Babu
Fujitsu Australia


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

4_database_catalog_tenancy_v6.patch (127K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Haribabu Kommi-2
In reply to this post by Stephen Frost
On Thu, Jan 7, 2016 at 2:29 PM, Stephen Frost <[hidden email]> wrote:

> Robert,
>
> * Robert Haas ([hidden email]) wrote:
>
>> Apart from the issue of whether this is doomed for some architectural
>> reason, it is not entirely clear to me that there's any consensus that
>> we want this.  I don't think that I understand the issues here well
>> enough to proffer an opinion of my own just yet... but I'd like to
>> hear what other people think.
>
> I'm certainly of the opinion that we want this or something similar.
>
> The big caveat kicking around in my head is if we want to have our own
> set of defined policies or if we want to give flexibility to the
> administrator to define their own policies.  In particular, I'm
> wondering about things like:
>
> CREATE POLICY namespace_limit ON pg_namespace TO company1 USING
>   (substring(nspname,1,8) = 'company1_');
>
> Which is a bit different, as I understand it, from what Haribadu has
> been proposing and quite a bit more complicated, as we'd then have to
> make the internal lookups respect the policy (so things like CREATE
> SCHEMA would have to check if you're allowed to actually create that
> schema, which would be based on the policy...).

I feel we may needed both our own set of policies and also providing
the user to create/alter/drop the catalog policies. This way we can
support both simple and complex scenarios. With default policies
an user can setup multi-tenancy easily. With the help of edit option,
user can tune the policies according to their scenarios.

The one problem with either approach as i am thinking, currently with
our own set of policies, the objects entries that are present on the
catalog tables are visible to the users, those are having any kind of
privileges on those objects. In case if a user tries to create an object
that is already present in the catalog relation will produce an error, but
user cannot view that object because of permissions problem.

To avoid such problem, administrator has to add policies such as
"namespace_prefix" needs to be added to all catalog tables.

Regards,
Hari Babu
Fujitsu Australia


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Amit Langote-2
In reply to this post by Haribabu Kommi-2
On 2016/01/07 15:25, Haribabu Kommi wrote:

> On Thu, Jan 7, 2016 at 4:31 PM, Amit Langote
> <[hidden email]> wrote:
>>
>> I applied all the patches. I have a basic question. Sorry though if I've
>> entirely missed the point (and/or scope) of your proposal. I wonder if
>> something like the following should not have failed with the patch:
>>
>> postgres=# CREATE POLICY class_policy ON pg_class TO PUBLIC USING
>> (relowner = current_user);
>> ERROR:  permission denied: "pg_class" is a system catalog
>>
>> Is there no support yet for user-defined catalog policies?
>
> Currently the patches don't have the support of allowing user to
> create policies on catalog tables. The policies similar like you
> specified are prepared for all eligible catalog tables and those
> will be used when the user enables the catalog security.
>
> Presently, default policies are used to provide proper multi-tenancy
> behavior. May be we can add the support of user to update the
> existing policies and add new policies on the catalog tables
> without dropping the creation of default polices, as these are
> required for supporting multi-tenancy by default without any
> user policies.

Okay. Thanks for explaining.

> Example usage:
>
> postgres=# create role test_user1;
> CREATE ROLE
> postgres=# create role test_user2;
> CREATE ROLE
> postgres=# alter database postgres with catalog security true;
> ALTER DATABASE
> postgres=# set session authorization test_user1;
> SET
> postgres=> create table tbl1(f1 int);
> CREATE TABLE
> postgres=> set session authorization test_user2;
> SET
> postgres=> create table tbl2(f2 int);
> CREATE TABLE
> postgres=> \d
>          List of relations
>  Schema | Name | Type  |   Owner
> --------+------+-------+------------
>  public | tbl2 | table | test_user2
> (1 row)

Okay, so the patch's system-defined policy seems enough to achieve this
much isolation.

By the way, if test_user2 had created a table with the same name, it would
produce the following error:

ERROR:  relation "tbl1" already exists

So, certain information (relname in this case) is bound to be leaked here
because syscache look-ups don't abide by catalog RLS. But I guess hoping
that it would may be being overly paranoid and if pursued at all, an
entirely separate project.

Thanks,
Amit




--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Joe Conway
In reply to this post by Robert Haas
On 01/06/2016 12:15 PM, Robert Haas wrote:

> On Tue, Jan 5, 2016 at 11:07 PM, Haribabu Kommi
> <[hidden email]> wrote:
>> May be you missed to apply the 3_shared_catalog_tenancy_v4 path,
>> because 4_database_catalog_tenancy_v5 patch depends on it.
>>
>> Here I attached all the patches for your convenience, I am able to
>> apply all patches in the order without any problem.
>
> Is any committer thinking about taking a serious look at this patch series?
>
> I ask because (1) it seems like it could be nice to have but (2) it
> frightens me terribly.  We are generally very sparing about assuming
> that "stuff" (triggers, partial indexes, etc.) that works for user
> tables can also be made to work for system tables.  I haven't thought
> deeply about what might go wrong in this particular case, but it
> strikes me that if Haribabu Kommi is building something that is doomed
> for some reason, it would be good to figure that out before he spends
> any more time on it than he already has.
As Stephen mentioned, yes, I am very interested in at least some aspects
of this patch. The ability to apply RLS to system tables could be useful
to solve a number of problems we don't have a good story for today,
multi-tenancy only being one of them.

> Apart from the issue of whether this is doomed for some architectural
> reason, it is not entirely clear to me that there's any consensus that
> we want this.  I don't think that I understand the issues here well
> enough to proffer an opinion of my own just yet... but I'd like to
> hear what other people think.

As said above, I definitely see a need for something like this if not
this specifically.

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development


signature.asc (853 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Tom Lane-2
Joe Conway <[hidden email]> writes:

> On 01/06/2016 12:15 PM, Robert Haas wrote:
>> Is any committer thinking about taking a serious look at this patch series?
>>
>> I ask because (1) it seems like it could be nice to have but (2) it
>> frightens me terribly.  We are generally very sparing about assuming
>> that "stuff" (triggers, partial indexes, etc.) that works for user
>> tables can also be made to work for system tables.  I haven't thought
>> deeply about what might go wrong in this particular case, but it
>> strikes me that if Haribabu Kommi is building something that is doomed
>> for some reason, it would be good to figure that out before he spends
>> any more time on it than he already has.

> As Stephen mentioned, yes, I am very interested in at least some aspects
> of this patch. The ability to apply RLS to system tables could be useful
> to solve a number of problems we don't have a good story for today,
> multi-tenancy only being one of them.

FWIW, it seems offhand like we might not have that much trouble with
applying RLS to system catalogs as long as it's understood that RLS
only has anything to do with SQL queries issued against the catalogs.

If we imagine that RLS should somehow filter a backend's own operations on
the catalogs, then I agree with Robert that the entire thing is deeply
scary and probably incapable of being made to work robustly.

However, by "not that much trouble" I only mean getting an implementation
that works and doesn't create more security problems than it fixes.
Usability is still likely to be a huge problem.  In particular it seems
likely that any attempt to actually put RLS policies on the catalogs would
completely destroy the ability to run pg_dump except as a BYPASSRLS role.
That would be an unpleasant consequence.

I've not read the patch set, so maybe it contains some ideas about how
to mitigate the usability issues, in which case never mind ...

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Stephen Frost
* Tom Lane ([hidden email]) wrote:
> Joe Conway <[hidden email]> writes:
> > As Stephen mentioned, yes, I am very interested in at least some aspects
> > of this patch. The ability to apply RLS to system tables could be useful
> > to solve a number of problems we don't have a good story for today,
> > multi-tenancy only being one of them.
>
> FWIW, it seems offhand like we might not have that much trouble with
> applying RLS to system catalogs as long as it's understood that RLS
> only has anything to do with SQL queries issued against the catalogs.

Right, that's what this patch set is about.

> If we imagine that RLS should somehow filter a backend's own operations on
> the catalogs, then I agree with Robert that the entire thing is deeply
> scary and probably incapable of being made to work robustly.

Personally, I like the idea of the capability, but I also agree that
it'd be a great deal more challenging to do and would require a lot of
pretty invasive and scary changes.  Hence, my thinking was that we'd
define our own set of policies which mimic what we already do through
the permissions system (thus, only impacting SQL queries against the
catalog and not anything about how the backend accesses the catalogs).

I'm on the fence about if we'd allow those policies to be modified by
users or not.

> However, by "not that much trouble" I only mean getting an implementation
> that works and doesn't create more security problems than it fixes.
> Usability is still likely to be a huge problem.  In particular it seems
> likely that any attempt to actually put RLS policies on the catalogs would
> completely destroy the ability to run pg_dump except as a BYPASSRLS role.
> That would be an unpleasant consequence.

I don't follow how this would destroy the ability to run pg_dump.
Ideally, we'd have a result where a user could run pg_dump without
having to apply any filters of their own and they'd get a dump of all
objects they're allowed to see.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Tom Lane-2
Stephen Frost <[hidden email]> writes:
> * Tom Lane ([hidden email]) wrote:
>> However, by "not that much trouble" I only mean getting an implementation
>> that works and doesn't create more security problems than it fixes.
>> Usability is still likely to be a huge problem.  In particular it seems
>> likely that any attempt to actually put RLS policies on the catalogs would
>> completely destroy the ability to run pg_dump except as a BYPASSRLS role.
>> That would be an unpleasant consequence.

> I don't follow how this would destroy the ability to run pg_dump.
> Ideally, we'd have a result where a user could run pg_dump without
> having to apply any filters of their own and they'd get a dump of all
> objects they're allowed to see.

You mean, other than the fact that pg_dump sets row_security = off
to ensure that what it's seeing *isn't* filtered.

The bigger picture here is that I do not think that you can just
arbitrarily exclude non-owned objects from its view and still expect to
get a valid dump; that will break dependency chains for example, possibly
leading to stuff getting output in an order that doesn't restore.

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Stephen Frost
* Tom Lane ([hidden email]) wrote:

> Stephen Frost <[hidden email]> writes:
> > * Tom Lane ([hidden email]) wrote:
> >> However, by "not that much trouble" I only mean getting an implementation
> >> that works and doesn't create more security problems than it fixes.
> >> Usability is still likely to be a huge problem.  In particular it seems
> >> likely that any attempt to actually put RLS policies on the catalogs would
> >> completely destroy the ability to run pg_dump except as a BYPASSRLS role.
> >> That would be an unpleasant consequence.
>
> > I don't follow how this would destroy the ability to run pg_dump.
> > Ideally, we'd have a result where a user could run pg_dump without
> > having to apply any filters of their own and they'd get a dump of all
> > objects they're allowed to see.
>
> You mean, other than the fact that pg_dump sets row_security = off
> to ensure that what it's seeing *isn't* filtered.
There's a specific option to turn it back on already though.  This
wouldn't change that.

> The bigger picture here is that I do not think that you can just
> arbitrarily exclude non-owned objects from its view and still expect to
> get a valid dump; that will break dependency chains for example, possibly
> leading to stuff getting output in an order that doesn't restore.

We already have that issue when users select to dump out specific
schemas, I don't see this as being any different.  The idea behind
multi-tenancy is, generally speaking, you don't see or have any
references or dependencies with what other people have.  In those cases,
there won't be any dependencies to objects that you can't see.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Tom Lane-2
Stephen Frost <[hidden email]> writes:
> * Tom Lane ([hidden email]) wrote:
>> Stephen Frost <[hidden email]> writes:
>>> I don't follow how this would destroy the ability to run pg_dump.
>>> Ideally, we'd have a result where a user could run pg_dump without
>>> having to apply any filters of their own and they'd get a dump of all
>>> objects they're allowed to see.

>> You mean, other than the fact that pg_dump sets row_security = off
>> to ensure that what it's seeing *isn't* filtered.

> There's a specific option to turn it back on already though.

Whereupon you'd have no certainty that what you got represented a
complete dump of your own data.

                        regards, tom lane


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Stephen Frost
* Tom Lane ([hidden email]) wrote:

> Stephen Frost <[hidden email]> writes:
> > * Tom Lane ([hidden email]) wrote:
> >> Stephen Frost <[hidden email]> writes:
> >>> I don't follow how this would destroy the ability to run pg_dump.
> >>> Ideally, we'd have a result where a user could run pg_dump without
> >>> having to apply any filters of their own and they'd get a dump of all
> >>> objects they're allowed to see.
>
> >> You mean, other than the fact that pg_dump sets row_security = off
> >> to ensure that what it's seeing *isn't* filtered.
>
> > There's a specific option to turn it back on already though.
>
> Whereupon you'd have no certainty that what you got represented a
> complete dump of your own data.
It would be a dump of what you're allowed to see, rather than an error
saying you couldn't dump something you couldn't see, which is the
alternative we're talking about here.  Even if you've got a dependency
to something-or-other, if you don't have access to it, then you're
going to get an error.

In practice, you have to make sure to remember to include all of your
schemas when you pg_dump, and don't get it wrong or you'll get an error
(you don't have access to some schema referenced) or a subset of what
you intended (you forgot to include one you meant to).  That is not a
better user experience than being able to say "dump out everything I've
got access to."

In many, many use-cases that's exactly what you want.  pg_dump is more
than just a whole-database backup tool, and when it's used as a
whole-database backup tool, you'll need to make sure it has BYPASSRLS or
is a superuser or you could end up getting errors.  I don't see any
issue with that.

If the policies are incorrect then that'd be a problem, but I'm
certainly hopeful that we'd be able to get that right.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Multi-tenancy with RLS

Alvaro Herrera-9
I've closed this as returned-with-feedback.

--
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
1234
Previous Thread Next Thread
Loading...