On Tuesday, September 3, 2019 12:39:51 PM CEST Kuntal Ghosh wrote:
> Hello Pierre,
Hello Kuntal
>
> > When using a functional index on a table, we realized that the permission
> > check done in pg_stats was incorrect and thus preventing valid access to
> > the statistics from users.
> >
> > The attached patch fixes this by introducing a second path in privilege
> > check in pg_stats view.
>
> The patch doesn't apply on the latest HEAD [1].
All my apologies for that. I submitted this patch some time ago but forgot to
add it to the commit fest. Attached to this mail is a rebased version.
> IIUC, the patch introduces an additional privilege check for the
> underlying objects involved in the expression/functional index. If the
> user has 'select' privileges on all of the columns/objects included in
> the expression/functional index, then it should be visible in pg_stats
> view. I've applied the patch manually and tested the feature. It works
> as expected.
Indeed, you understood correctly. I have not digged around to find out the
origin of the current situation, but it does not look like an intentional
behaviour, more like a small oversight.
> > I have not written a regression test yet, mainly because I'm not 100%
> > certain where to write it. Given some hints, I would happily add it to
> > this patch.
> Yeah, it'll be good to have some regression tests for the same. I'm
> also not sure which regression file best suites for these tests.
Thank you very much for your review
Pierre
>From d1d99104f8f157257a30cb9bf9cfea1cc467b1a7 Mon Sep 17 00:00:00 2001
From: Pierre Ducroquet <p.p...@pinaraf.info>
Date: Sat, 6 Apr 2019 13:22:29 +0200
Subject: [PATCH] Use a different permission check path for indexes and
relations in pg_stats
pg_statistic contains information about both table attributes and functional indexes,
but the permission check done in pg_stats was only valid for table attributes.
This patch thus implements a seperate permission check for functional indexes, that
verifies the access for each attribute contained in the index.
---
src/backend/catalog/system_views.sql | 10 +++++++++-
src/test/regress/expected/rules.out | 4 +++-
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index ea4c85e395..07bdfc12ac 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -248,7 +248,15 @@ CREATE VIEW pg_stats WITH (security_barrier) AS
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.relkind = 'r' AND has_column_privilege(c.oid, a.attnum, 'select'::text)
+ OR
+ (c.relkind = 'i' AND NOT(EXISTS(
+ SELECT 1 FROM pg_depend
+ WHERE pg_depend.objid = c.oid
+ AND pg_depend.refobjsubid > 0
+ AND NOT has_column_privilege(pg_depend.refobjid, pg_depend.refobjsubid::smallint, 'select'::text)))
+ )
+ )
AND (c.relrowsecurity = false OR NOT row_security_active(c.oid));
REVOKE ALL on pg_statistic FROM public;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 210e9cd146..9f5679314a 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2283,7 +2283,9 @@ pg_stats| SELECT n.nspname AS schemaname,
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))));
+ WHERE ((NOT a.attisdropped) AND (((c.relkind = 'r'::"char") AND has_column_privilege(c.oid, a.attnum, 'select'::text)) OR ((c.relkind = 'i'::"char") AND (NOT (EXISTS ( SELECT 1
+ FROM pg_depend
+ WHERE ((pg_depend.objid = c.oid) AND (pg_depend.refobjsubid > 0) AND (NOT has_column_privilege(pg_depend.refobjid, (pg_depend.refobjsubid)::smallint, 'select'::text)))))))) AND ((c.relrowsecurity = false) OR (NOT row_security_active(c.oid))));
pg_stats_ext| SELECT cn.nspname AS schemaname,
c.relname AS tablename,
sn.nspname AS statistics_schemaname,
--
2.23.0
