On 07/22/2015 02:17 PM, Dean Rasheed wrote:
> On 21 July 2015 at 04:53, Michael Paquier <michael.paqu...@gmail.com> wrote:
>> On Tue, Jul 14, 2015 at 4:01 AM, Stephen Frost <sfr...@snowman.net> wrote:
>>> We need to be careful to avoid the slippery slope of trying to prevent
>>> all covert channels, which has been extensively discussed previously.
>
> I think this is more serious than the covert channel leaks discussed
> before, since most_common_vals explicitly reveals values from the
> table, making it an overt leak, albeit of a small portion of the
> table's values.
>
>> Looking at that I am not seeing any straight-forward way to resolve
>> this issue except by hardening pg_stats by having an additional filter
>> of this type so as a non-owner of a relation cannot see the stats of
>> this table directly when RLS is enabled:
>> c.relrowsecurity = false OR c.relowner = current_user::regrole::oid
>> Attached is a patch doing that (/me now hides, expecting to receive
>> laser shots because of the use of current_user on a system view).
>> Thoughts?
>
> Hmm, I think it probably ought to do more, based on whether or not RLS
> is being bypassed or in force-mode -- see the first few checks in
> get_row_security_policies(). Perhaps a new SQL-callable function
> exposing those checks and calling check_enable_rls(). It's probably
> still worth including the "c.relrowsecurity = false" check in SQL to
> save calling the function for the majority of tables that don't have
> RLS.
Please see the attached patch and let me know what you think. I believe
the only thing lacking is documentation for the two new user visible
functions. Comments?
Joe
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index e82a53a..c0bd6fa 100644
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
*************** CREATE VIEW pg_indexes AS
*** 150,156 ****
LEFT JOIN pg_tablespace T ON (T.oid = I.reltablespace)
WHERE C.relkind IN ('r', 'm') AND I.relkind = 'i';
! CREATE VIEW pg_stats AS
SELECT
nspname AS schemaname,
relname AS tablename,
--- 150,156 ----
LEFT JOIN pg_tablespace T ON (T.oid = I.reltablespace)
WHERE C.relkind IN ('r', 'm') AND I.relkind = 'i';
! CREATE VIEW pg_stats WITH (security_barrier) AS
SELECT
nspname AS schemaname,
relname AS tablename,
*************** CREATE VIEW pg_stats AS
*** 211,217 ****
FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
! WHERE NOT attisdropped AND has_column_privilege(c.oid, a.attnum, 'select');
REVOKE ALL on pg_statistic FROM public;
--- 211,219 ----
FROM pg_statistic s JOIN pg_class c ON (c.oid = s.starelid)
JOIN pg_attribute a ON (c.oid = attrelid AND attnum = s.staattnum)
LEFT JOIN pg_namespace n ON (n.oid = c.relnamespace)
! WHERE NOT attisdropped
! AND has_column_privilege(c.oid, a.attnum, 'select')
! AND (c.relrowsecurity = false OR NOT row_security_active(c.oid));
REVOKE ALL on pg_statistic FROM public;
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c
index 3ca168b..094ac33 100644
*** a/src/backend/utils/adt/acl.c
--- b/src/backend/utils/adt/acl.c
*************** static AclMode convert_priv_string(text
*** 92,98 ****
static AclMode convert_any_priv_string(text *priv_type_text,
const priv_map *privileges);
- static Oid convert_table_name(text *tablename);
static AclMode convert_table_priv_string(text *priv_type_text);
static AclMode convert_sequence_priv_string(text *priv_type_text);
static AttrNumber convert_column_name(Oid tableoid, text *column);
--- 92,97 ----
*************** has_table_privilege_id_id(PG_FUNCTION_AR
*** 1998,2004 ****
/*
* Given a table name expressed as a string, look it up and return Oid
*/
! static Oid
convert_table_name(text *tablename)
{
RangeVar *relrv;
--- 1997,2003 ----
/*
* Given a table name expressed as a string, look it up and return Oid
*/
! Oid
convert_table_name(text *tablename)
{
RangeVar *relrv;
diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c
index 44cb374..cd1a521 100644
*** a/src/backend/utils/misc/rls.c
--- b/src/backend/utils/misc/rls.c
***************
*** 19,24 ****
--- 19,25 ----
#include "catalog/pg_class.h"
#include "miscadmin.h"
#include "utils/acl.h"
+ #include "utils/builtins.h"
#include "utils/elog.h"
#include "utils/rls.h"
#include "utils/syscache.h"
*************** check_enable_rls(Oid relid, Oid checkAsU
*** 111,113 ****
--- 112,141 ----
/* RLS should be fully enabled for this relation. */
return RLS_ENABLED;
}
+
+ /*
+ * row_security_active variants
+ *
+ * check_enable_rls wrapped as a SQL callable function except
+ * RLS_NONE_ENV and RLS_NONE are the same for this purpose.
+ */
+ Datum
+ row_security_active_name(PG_FUNCTION_ARGS)
+ {
+ text *tablename = PG_GETARG_TEXT_P(0);
+ Oid tableoid = convert_table_name(tablename);
+
+ if (check_enable_rls(tableoid, GetUserId(), true) == RLS_ENABLED)
+ return true;
+ else
+ return false;
+ }
+
+ Datum
+ row_security_active_id(PG_FUNCTION_ARGS)
+ {
+ if (check_enable_rls(PG_GETARG_OID(0), GetUserId(), true) == RLS_ENABLED)
+ return true;
+ else
+ return false;
+ }
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 8f6685f..5fe1930 100644
*** a/src/include/catalog/catversion.h
--- b/src/include/catalog/catversion.h
***************
*** 53,58 ****
*/
/* yyyymmddN */
! #define CATALOG_VERSION_NO 201507171
#endif
--- 53,58 ----
*/
/* yyyymmddN */
! #define CATALOG_VERSION_NO 201507250
#endif
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index 1d68ad7..91b3b24 100644
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
*************** DESCR("tsm_bernoulli_reset(internal)");
*** 5348,5353 ****
--- 5348,5359 ----
DATA(insert OID = 3346 ( tsm_bernoulli_cost PGNSP PGUID 12 1 0 0 0 f f f f t f v 7 0 2278 "2281 2281 2281 2281 2281 2281 2281" _null_ _null_ _null_ _null_ _null_ tsm_bernoulli_cost _null_ _null_ _null_ ));
DESCR("tsm_bernoulli_cost(internal)");
+ /* rls */
+ DATA(insert OID = 3298 ( row_security_active PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 16 "25" _null_ _null_ _null_ _null_ _null_ row_security_active_name _null_ _null_ _null_ ));
+ DESCR("row security for current context active on table by table name");
+ DATA(insert OID = 3299 ( row_security_active PGNSP PGUID 12 1 0 0 0 f f f f t f s 1 0 16 "26" _null_ _null_ _null_ _null_ _null_ row_security_active_id _null_ _null_ _null_ ));
+ DESCR("row security for current context active on table by table oid");
+
/*
* Symbolic values for provolatile column: these indicate whether the result
* of a function is dependent *only* on the values of its explicit arguments,
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index fcb0bf0..5d80de3 100644
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
***************
*** 22,27 ****
--- 22,28 ----
*/
/* acl.c */
+ extern Oid convert_table_name(text *tablename);
extern Datum has_any_column_privilege_name_name(PG_FUNCTION_ARGS);
extern Datum has_any_column_privilege_name_id(PG_FUNCTION_ARGS);
extern Datum has_any_column_privilege_id_name(PG_FUNCTION_ARGS);
*************** extern Datum set_config_by_name(PG_FUNCT
*** 1119,1124 ****
--- 1120,1129 ----
extern Datum show_all_settings(PG_FUNCTION_ARGS);
extern Datum show_all_file_settings(PG_FUNCTION_ARGS);
+ /* rls.c */
+ extern Datum row_security_active_name(PG_FUNCTION_ARGS);
+ extern Datum row_security_active_id(PG_FUNCTION_ARGS);
+
/* lockfuncs.c */
extern Datum pg_lock_status(PG_FUNCTION_ARGS);
extern Datum pg_advisory_lock_int8(PG_FUNCTION_ARGS);
diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out
index 414299a..b17c637 100644
*** a/src/test/regress/expected/rowsecurity.out
--- b/src/test/regress/expected/rowsecurity.out
*************** SELECT * FROM current_check;
*** 2837,2846 ****
COMMIT;
--
-- Collation support
--
BEGIN;
! SET row_security = force;
CREATE TABLE coll_t (c) AS VALUES ('bar'::text);
CREATE POLICY coll_p ON coll_t USING (c < ('foo'::text COLLATE "C"));
ALTER TABLE coll_t ENABLE ROW LEVEL SECURITY;
--- 2837,2880 ----
COMMIT;
--
+ -- check pg_stats view filtering
+ --
+ SET row_security TO ON;
+ SET SESSION AUTHORIZATION rls_regress_user0;
+ ANALYZE current_check;
+ -- Stats visible
+ SELECT row_security_active('current_check');
+ row_security_active
+ ---------------------
+ f
+ (1 row)
+
+ SELECT most_common_vals FROM pg_stats where tablename = 'current_check';
+ most_common_vals
+ ---------------------
+
+
+ {rls_regress_user1}
+ (3 rows)
+
+ SET SESSION AUTHORIZATION rls_regress_user1;
+ -- Stats not visible
+ SELECT row_security_active('current_check');
+ row_security_active
+ ---------------------
+ t
+ (1 row)
+
+ SELECT most_common_vals FROM pg_stats where tablename = 'current_check';
+ most_common_vals
+ ------------------
+ (0 rows)
+
+ --
-- Collation support
--
BEGIN;
! SET row_security TO FORCE;
CREATE TABLE coll_t (c) AS VALUES ('bar'::text);
CREATE POLICY coll_p ON coll_t USING (c < ('foo'::text COLLATE "C"));
ALTER TABLE coll_t ENABLE ROW LEVEL SECURITY;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index cd53375..2fc05a5 100644
*** a/src/test/regress/expected/rules.out
--- b/src/test/regress/expected/rules.out
*************** pg_stats| SELECT n.nspname AS schemaname
*** 2061,2067 ****
JOIN pg_class c ON ((c.oid = s.starelid)))
JOIN pg_attribute a ON (((c.oid = a.attrelid) AND (a.attnum = s.staattnum))))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
! WHERE ((NOT a.attisdropped) AND has_column_privilege(c.oid, a.attnum, 'select'::text));
pg_tables| SELECT n.nspname AS schemaname,
c.relname AS tablename,
pg_get_userbyid(c.relowner) AS tableowner,
--- 2061,2067 ----
JOIN pg_class c ON ((c.oid = s.starelid)))
JOIN pg_attribute a ON (((c.oid = a.attrelid) AND (a.attnum = s.staattnum))))
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
! WHERE ((NOT a.attisdropped) AND has_column_privilege(c.oid, a.attnum, 'select'::text) AND ((c.relrowsecurity = false) OR (NOT row_security_active(c.oid))));
pg_tables| SELECT n.nspname AS schemaname,
c.relname AS tablename,
pg_get_userbyid(c.relowner) AS tableowner,
diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql
index 039070b..7c377ca 100644
*** a/src/test/regress/sql/rowsecurity.sql
--- b/src/test/regress/sql/rowsecurity.sql
*************** SELECT * FROM current_check;
*** 1137,1146 ****
COMMIT;
--
-- Collation support
--
BEGIN;
! SET row_security = force;
CREATE TABLE coll_t (c) AS VALUES ('bar'::text);
CREATE POLICY coll_p ON coll_t USING (c < ('foo'::text COLLATE "C"));
ALTER TABLE coll_t ENABLE ROW LEVEL SECURITY;
--- 1137,1161 ----
COMMIT;
--
+ -- check pg_stats view filtering
+ --
+ SET row_security TO ON;
+ SET SESSION AUTHORIZATION rls_regress_user0;
+ ANALYZE current_check;
+ -- Stats visible
+ SELECT row_security_active('current_check');
+ SELECT most_common_vals FROM pg_stats where tablename = 'current_check';
+
+ SET SESSION AUTHORIZATION rls_regress_user1;
+ -- Stats not visible
+ SELECT row_security_active('current_check');
+ SELECT most_common_vals FROM pg_stats where tablename = 'current_check';
+
+ --
-- Collation support
--
BEGIN;
! SET row_security TO FORCE;
CREATE TABLE coll_t (c) AS VALUES ('bar'::text);
CREATE POLICY coll_p ON coll_t USING (c < ('foo'::text COLLATE "C"));
ALTER TABLE coll_t ENABLE ROW LEVEL SECURITY;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
