Currently, it is pretty easy to subvert the restrictions imposed
by row-level security and security_barrier views. All you have to
to is use EXPLAIN (ANALYZE) and see how many rows were filtered
out by the RLS policy or the view condition.
This is not considered a security bug (I asked), but I still think
it should be fixed.
My idea is to forbid EXPLAIN (ANALYZE) for ordinary users whenever
a statement uses either of these features. But restricting it to
superusers would be too restrictive (with a superuser, you can never
observe RLS, since superusers are exempt) and it would also be
dangerous (you shouldn't perform DML on untrusted tables as superuser).
So I thought we could restrict the use of EXPLAIN (ANALYZE) in these
situations to the members of a predefined role. That could be a new
predefined role, but I think it might as well be "pg_read_all_stats",
since that role allows you to view sensitive data like the MCV in
pg_statistic, and EXPLAIN (ANALYZE) can be seen as provideing executor
statistics.
Attached is a POC patch that implements that (documentation and
regression tests are still missing) to form a basis for a discussion.
There are a few things I would like feedback about:
- is it OK to use "pg_read_all_stats" for that?
- should the check be moved to standard_ExplainOneQuery()?
Yours,
Laurenz Albe
From f31ee5919a9d30f51ff9d54adc7397cb98dfa370 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.a...@cybertec.at>
Date: Mon, 6 May 2024 12:43:02 +0200
Subject: [PATCH v1] 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.
---
src/backend/commands/explain.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
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,
--
2.45.0
