On Mon, 2024-05-06 at 16:46 +0200, Laurenz Albe wrote: > Attached is a POC patch that implements that (documentation and > regression tests are still missing) to form a basis for a discussion.
... and here is a complete patch with regression tests and documentation. Yours, Laurenz Albe
From 201c23d0716af7451375608644a4cf2a002744df Mon Sep 17 00:00:00 2001 From: Laurenz Albe <laurenz.a...@cybertec.at> Date: Wed, 15 May 2024 17:09:48 +0200 Subject: [PATCH v2] Restrict EXPLAIN (ANALYZE) on security relevant statements Using EXPLAIN (ANALYZE), it is easy to work around restrictions imposed by row-level security or security barrier views. This is not considered a security bug, but we ought to do better. Restricting the use of EXPLAIN (ANALYZE) to superusers in such cases would be too much, and superusers bypass row-level security, so that would leave no way to debug such statements. Consequently, restrict EXPLAIN (ANALYZE) on statements that involve security_barrier views or row-level security to members of the predefined role pg_read_all_stats. Discussion: https://postgr.es/m/3a60be45e7a89f50d166dba49553950d6b8a97f5.camel%40cybertec.at --- doc/src/sgml/ddl.sgml | 4 +++ doc/src/sgml/ref/explain.sgml | 7 ++++- doc/src/sgml/rules.sgml | 13 +++++++-- doc/src/sgml/user-manag.sgml | 4 ++- src/backend/commands/explain.c | 33 ++++++++++++++++++++++ src/test/regress/expected/rowsecurity.out | 17 +++++++++++ src/test/regress/expected/select_views.out | 18 ++++++++++++ src/test/regress/sql/rowsecurity.sql | 19 +++++++++++++ src/test/regress/sql/select_views.sql | 20 +++++++++++++ 9 files changed, 131 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 026bfff70f..74d05300b7 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2560,6 +2560,10 @@ GRANT SELECT (col1), UPDATE (col1) ON mytable TO miriam_rw; no rows are visible or can be modified. Operations that apply to the whole table, such as <command>TRUNCATE</command> and <literal>REFERENCES</literal>, are not subject to row security. + Note that only members of the predefined role <literal>pg_read_all_stats</literal> + may run <command>EXPLAIN (ANALYZE)</command> for a statement that is subject + to row security, because the execution statistics can be used to reveal the + existence of rows the user is not allowed to see. </para> <para> diff --git a/doc/src/sgml/ref/explain.sgml b/doc/src/sgml/ref/explain.sgml index db9d3a8549..6586983467 100644 --- a/doc/src/sgml/ref/explain.sgml +++ b/doc/src/sgml/ref/explain.sgml @@ -84,7 +84,12 @@ EXPLAIN [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] <rep the display, including the total elapsed time expended within each plan node (in milliseconds) and the total number of rows it actually returned. This is useful for seeing whether the planner's estimates - are close to reality. + are close to reality. Since the execution statistics of a statement + allow the user to infer if rows were excluded by a row-level security + policy or a condition in a <literal>security_barrier</literal> view, + only members of the predefined role <literal>pg_read_all_stats</literal> + can use <command>EXPLAIN ANALYZE</command> with statements that use these + features. </para> <important> diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml index 7a928bd7b9..f2c0911643 100644 --- a/doc/src/sgml/rules.sgml +++ b/doc/src/sgml/rules.sgml @@ -2167,13 +2167,22 @@ CREATE VIEW phone_number WITH (security_barrier) AS view's row filters. </para> +<para> + To prevent attackers from using <command>EXPLAIN (ANALYZE)</command> to + get information about data excluded by the definition of a view with the + <literal>security_barrier</literal> option, only superusers and members + of the predefined role <literal>pg_read_all_stats</literal> are allowed + to use the <literal>ANALYZE</literal> option of <command>EXPLAIN</command> + on a statement that involves such a view. +</para> + <para> It is important to understand that even a view created with the <literal>security_barrier</literal> option is intended to be secure only in the limited sense that the contents of the invisible tuples will not be passed to possibly-insecure functions. The user may well have other means - of making inferences about the unseen data; for example, they can see the - query plan using <command>EXPLAIN</command>, or measure the run time of + of making inferences about the unseen data; for example, they can + measure the run time of queries against the view. A malicious attacker might be able to infer something about the amount of unseen data, or even gain some information about the data distribution or most common values (since these things may diff --git a/doc/src/sgml/user-manag.sgml b/doc/src/sgml/user-manag.sgml index 07a16247d7..a2109304cd 100644 --- a/doc/src/sgml/user-manag.sgml +++ b/doc/src/sgml/user-manag.sgml @@ -639,7 +639,9 @@ DROP ROLE doomed_role; <row> <entry>pg_read_all_stats</entry> <entry>Read all pg_stat_* views and use various statistics related extensions, - even those normally visible only to superusers.</entry> + even those normally visible only to superusers. + Use <command>EXPLAIN (ANALYZE)</command> on statements that involve row-level + security or <literal>security_barrier</literal> views.</entry> </row> <row> <entry>pg_stat_scan_tables</entry> diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index c0c73aa3c9..85782e614e 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -14,6 +14,7 @@ #include "postgres.h" #include "access/xact.h" +#include "catalog/pg_authid_d.h" #include "catalog/pg_type.h" #include "commands/createas.h" #include "commands/defrem.h" @@ -21,6 +22,7 @@ #include "foreign/fdwapi.h" #include "jit/jit.h" #include "libpq/pqformat.h" +#include "miscadmin.h" #include "nodes/extensible.h" #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" @@ -29,6 +31,7 @@ #include "rewrite/rewriteHandler.h" #include "storage/bufmgr.h" #include "tcop/tcopprot.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/guc_tables.h" #include "utils/json.h" @@ -431,6 +434,36 @@ ExplainOneQuery(Query *query, int cursorOptions, return; } + /* + * Since EXPLAIN (ANALYZE) shows data like the number of rows removed by a + * filter, it can be used to work around security restrictions that hide + * table data from the user, such as security barrier views and row-level + * security. Only members of pg_read_all_stats and superusers can see such + * statistics. The check is here rather than in standard_ExplainOneQuery + * to keep plugins from inadvertently subverting security. + */ + if (es->analyze && + !has_privs_of_role(GetUserId(), ROLE_PG_READ_ALL_STATS)) + { + ListCell *rtable; + + foreach(rtable, query->rtable) + { + RangeTblEntry *rte = lfirst(rtable); + if (rte->security_barrier) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied to use EXPLAIN (ANALYZE) with security_barrier views"), + errdetail("Only members of pg_read_all_stats may see statement execution statistics."))); + } + + if (query->hasRowSecurity) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("permission denied to use EXPLAIN (ANALYZE) with row-level security"), + errdetail("Only members of pg_read_all_stats may see statement execution statistics."))); + } + /* if an advisor plugin is present, let it manage things */ if (ExplainOneQuery_hook) (*ExplainOneQuery_hook) (query, cursorOptions, into, es, diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 527cf7e7bf..cbc7d3d0f0 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -288,6 +288,23 @@ EXPLAIN (COSTS OFF) SELECT * FROM document NATURAL JOIN category WHERE f_leak(dt Filter: ((dlevel <= (InitPlan 1).col1) AND f_leak(dtitle)) (9 rows) +-- EXPLAIN (ANALYZE) is only allowed for members of pg_read_all_stats +EXPLAIN (ANALYZE) SELECT * FROM document WHERE f_leak(dtitle); -- error +ERROR: permission denied to use EXPLAIN (ANALYZE) with row-level security +DETAIL: Only members of pg_read_all_stats may see statement execution statistics. +RESET SESSION AUTHORIZATION; +GRANT pg_read_all_stats TO regress_rls_carol; +SET SESSION AUTHORIZATION regress_rls_carol; +DO +$$DECLARE + t text; +BEGIN + SET LOCAL client_min_messages = error; + -- should cause no error + EXECUTE 'EXPLAIN (ANALYZE) SELECT * FROM document WHERE f_leak(dtitle)' INTO t; +END;$$; +RESET SESSION AUTHORIZATION; +REVOKE pg_read_all_stats FROM regress_rls_carol; -- viewpoint from regress_rls_dave SET SESSION AUTHORIZATION regress_rls_dave; SELECT * FROM document WHERE f_leak(dtitle) ORDER BY did; diff --git a/src/test/regress/expected/select_views.out b/src/test/regress/expected/select_views.out index 1aeed8452b..0734c5ecae 100644 --- a/src/test/regress/expected/select_views.out +++ b/src/test/regress/expected/select_views.out @@ -1348,6 +1348,24 @@ EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure WHERE f_leak(passwd); Filter: (name = CURRENT_USER) (4 rows) +-- EXPLAIN (ANALYZE) should only be allowed to members of pg_read_all_stats +EXPLAIN (ANALYZE) SELECT * FROM my_property_secure WHERE f_leak(passwd); -- error +ERROR: permission denied to use EXPLAIN (ANALYZE) with security_barrier views +DETAIL: Only members of pg_read_all_stats may see statement execution statistics. +RESET SESSION AUTHORIZATION; +GRANT pg_read_all_stats TO regress_alice; +SET SESSION AUTHORIZATION regress_alice; +DO +$$DECLARE + t text; +BEGIN + -- should cause no error + EXECUTE 'EXPLAIN (ANALYZE) SELECT * FROM my_property_secure WHERE f_leak(passwd)' INTO t; +END;$$; +NOTICE: f_leak => passwd123 +RESET SESSION AUTHORIZATION; +REVOKE pg_read_all_stats FROM regress_alice; +SET SESSION AUTHORIZATION regress_alice; -- -- scenario: qualifiers can be pushed down if they contain leaky functions, -- provided they aren't passed data from inside the view. diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 1d5ed0a647..9ef933710d 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -133,6 +133,25 @@ SELECT * FROM document TABLESAMPLE BERNOULLI(50) REPEATABLE(0) EXPLAIN (COSTS OFF) SELECT * FROM document WHERE f_leak(dtitle); EXPLAIN (COSTS OFF) SELECT * FROM document NATURAL JOIN category WHERE f_leak(dtitle); +-- EXPLAIN (ANALYZE) is only allowed for members of pg_read_all_stats +EXPLAIN (ANALYZE) SELECT * FROM document WHERE f_leak(dtitle); -- error + +RESET SESSION AUTHORIZATION; +GRANT pg_read_all_stats TO regress_rls_carol; +SET SESSION AUTHORIZATION regress_rls_carol; + +DO +$$DECLARE + t text; +BEGIN + SET LOCAL client_min_messages = error; + -- should cause no error + EXECUTE 'EXPLAIN (ANALYZE) SELECT * FROM document WHERE f_leak(dtitle)' INTO t; +END;$$; + +RESET SESSION AUTHORIZATION; +REVOKE pg_read_all_stats FROM regress_rls_carol; + -- viewpoint from regress_rls_dave SET SESSION AUTHORIZATION regress_rls_dave; SELECT * FROM document WHERE f_leak(dtitle) ORDER BY did; diff --git a/src/test/regress/sql/select_views.sql b/src/test/regress/sql/select_views.sql index e742f13699..4db7244f54 100644 --- a/src/test/regress/sql/select_views.sql +++ b/src/test/regress/sql/select_views.sql @@ -95,6 +95,26 @@ EXPLAIN (COSTS OFF) SELECT * FROM my_property_normal WHERE f_leak(passwd); SELECT * FROM my_property_secure WHERE f_leak(passwd); EXPLAIN (COSTS OFF) SELECT * FROM my_property_secure WHERE f_leak(passwd); +-- EXPLAIN (ANALYZE) should only be allowed to members of pg_read_all_stats + +EXPLAIN (ANALYZE) SELECT * FROM my_property_secure WHERE f_leak(passwd); -- error + +RESET SESSION AUTHORIZATION; +GRANT pg_read_all_stats TO regress_alice; +SET SESSION AUTHORIZATION regress_alice; + +DO +$$DECLARE + t text; +BEGIN + -- should cause no error + EXECUTE 'EXPLAIN (ANALYZE) SELECT * FROM my_property_secure WHERE f_leak(passwd)' INTO t; +END;$$; + +RESET SESSION AUTHORIZATION; +REVOKE pg_read_all_stats FROM regress_alice; +SET SESSION AUTHORIZATION regress_alice; + -- -- scenario: qualifiers can be pushed down if they contain leaky functions, -- provided they aren't passed data from inside the view. -- 2.45.0
