Re: Multi-tenancy with RLS - Mailing list pgsql-hackers

On 09/01/2015 11:25 PM, Haribabu Kommi wrote:
> If any user is granted any permissions on that object then that user
> can view it's meta data of that object from the catalog tables.
> To check the permissions of the user on the object, instead of
> checking each and every available option, I just added a new
> privilege check option called "any". If user have any permissions on
> the object, the corresponding permission check function returns
> true. Patch attached for the same.
>
> Any thoughts/comments?

Thanks for working on this! Overall I like the concept and know of use
cases where it is critical and should be supported. Some comments:

1.) "... WITH ROW LEVEL SECURITY ...": name is not really accurate. This
feature does not, for example automatically add row security to all
tables in the database. It is really a specific set of RLS policies on
system tables to accomplish one very specific goal -- restrict metadata
access to those objects on which the user has some access. So maybe
something like one of these? ... WITH METADATA SECURITY ... ... WITH CATALOG SECURITY ... ... WITH METADATA RESTRICT
...... WITH CATALOG RESTRICT ... 

2.) Related to #1, I wonder if we should even provide a set of policies,
or should we just allow RLS on system tables and let people create their
own policies to suit their own needs. Maybe that could be dangerous, but
so are a lot of things we allow superusers do.

3.) In RelationBuildRowSecurity:

8<--------------
+     /* */
+     if (!criticalRelcachesBuilt || !criticalSharedRelcachesBuilt ||
relation->rd_id == PolicyRelationId)
+         return;
8<--------------

Bypassing RelationBuildRowSecurity() for relation->rd_id ==
PolicyRelationId causes a segfault for me and prevents me from even
logging into the database. Additionally I would want row security to
apply to pg_policy as well. Maybe not the best approach, but I solved
the infinite recursion issue like this:

8<--------------
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c
index 45326a3..49db767 100644
*** a/src/backend/commands/policy.c
--- b/src/backend/commands/policy.c
*************** static char parse_policy_command(const c
*** 52,57 ****
--- 105,116 ---- static Datum *policy_role_list_to_array(List *roles, int *num_roles);
 /*
+  * used to prevent infinite recursion when building
+  * row security for pg_policy itself
+  */
+ int rdepth = 0;
+
+ /*  * Callback to RangeVarGetRelidExtended().  *  * Checks the following:
*************** RelationBuildRowSecurity(Relation relati
*** 197,202 ****
--- 257,273 ----       MemoryContext oldcxt = CurrentMemoryContext;       RowSecurityDesc *volatile rsdesc = NULL;

+       /* */
+       if (!criticalRelcachesBuilt || !criticalSharedRelcachesBuilt)
+               return;
+       if (relation->rd_id == PolicyRelationId && rdepth > 0)
+       {
+               rdepth = 0;
+               return;
+       }
+       if (relation->rd_id == PolicyRelationId)
+               rdepth = 1;
+       /*        * Create a memory context to hold everything associated with this        * relation's row security
policy. This makes it easy to clean 
up during
*************** RelationBuildRowSecurity(Relation relati
*** 366,377 ****
--- 437,450 ----               /* Delete rscxt, first making sure it isn't active */
MemoryContextSwitchTo(oldcxt);              MemoryContextDelete(rscxt); 
+               rdepth = 0;               PG_RE_THROW();       }       PG_END_TRY();
       /* Success --- attach the policy descriptor to the relcache entry */       relation->rd_rsdesc = rsdesc;
+       rdepth = 0; }
 /*
8<--------------

4.) Currently, policies are always installed in the current database
rather than the target database. Didn't try to figure out how to fix:
8<--------------
postgres=# create database test with ROW LEVEL SECURITY = true;
CREATE DATABASE
postgres=# select count(1) from pg_policy;count
-------   30
(1 row)

postgres=# \c test
You are now connected to database "test" as user "postgres".
test=# select count(1) from pg_policy;count
-------    0
(1 row)
8<--------------

5.) I'm not thrilled with all the additional included headers added to
policy.c for this, but I don't offhand see a better way either.

6.) I would like to see this work in conjunction with sepgsql (and/or
other security label providers) as well. That could be accomplished
either as mentioned in #2 above (let me create custom policies to suit
my needs), or by providing security label hooks somewhere that they
affect the output of the has_<obj>_privilege*() functions.

HTH,

Joe

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