From a5ee7e6e16336a016947130df4177c716e0bf1d6 Mon Sep 17 00:00:00 2001 From: Matheus Alcantara Date: Fri, 25 Sep 2026 17:59:44 -0300 Subject: [PATCH] postgres_fdw: Import statistics as the foreign table's owner. ANALYZE called the ImportForeignStatistics callback before switching to the table owner's userid, so the callback ran as the user executing ANALYZE, outside of a security-restricted operation. This had two consequences when postgres_fdw's import_stats option was enabled: - User-defined functions invoked while importing the statistics, such as a domain's CHECK constraint on a foreign table column, were executed with the privileges of the user running ANALYZE. If that user was a superuser, the owner of the foreign table could get code run with superuser privileges. - postgres_fdw accessed the remote server using the user mapping of the user running ANALYZE rather than that of the table's owner, as is done in the sampling case. The imported statistics, such as most common values, could then disclose remote data to the owner of the foreign table that the owner itself has no privilege to read. To fix, switch to the table owner's userid before calling ImportForeignStatistics, lock down security-restricted operations, and restrict search_path, as do_analyze_rel() already does for the sampling case. postgres_fdw's statistics import now uses the owner's user mapping, which makes the identity handling consistent between the sampling and import methods. Also document this in the FDW API docs, and add regression tests for both issues. Author: Noah Misch Co-Authored-by: Matheus Alcantara > Reported-by: Fujii Masao Reported-by: Noah Misch Reviewed-by: Etsuro Fujita Discussion: https://postgr.es/m/CAHGQGwH8+uDTwfohpck9v_9L3nwSWACbmC36JYRyqHd5xQN_OA@mail.gmail.com Backpatch-through: 19 --- .../postgres_fdw/expected/postgres_fdw.out | 49 +++++++++++++++++++ contrib/postgres_fdw/postgres_fdw.c | 9 ++-- contrib/postgres_fdw/sql/postgres_fdw.sql | 44 +++++++++++++++++ doc/src/sgml/fdwhandler.sgml | 13 +++++ src/backend/commands/analyze.c | 30 ++++++++++-- 5 files changed, 136 insertions(+), 9 deletions(-) diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index 739f43af7bb..fbbacbb7734 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -13816,6 +13816,50 @@ WHERE schemaname = 'public' AND tablename = 'dtest_ftable'; ---------+-----------+-----------+-----------+------------+-----+-------------------+----+------------- (0 rows) +-- Statistics import must access the remote server as the foreign table's +-- owner, not as the user running ANALYZE +CREATE ROLE regress_simport_owner NOSUPERUSER; +GRANT USAGE ON FOREIGN SERVER loopback TO regress_simport_owner; +GRANT CREATE ON SCHEMA public TO regress_simport_owner; +CREATE TABLE simport_secret (s text); +REVOKE ALL ON simport_secret FROM PUBLIC; +INSERT INTO simport_secret SELECT 'hunter2' FROM generate_series(1, 10); +ANALYZE simport_secret; +SET ROLE regress_simport_owner; +CREATE FOREIGN TABLE simport_fsecret (s text) + SERVER loopback OPTIONS (table_name 'simport_secret', import_stats 'true'); +RESET ROLE; +ANALYZE simport_fsecret; -- should fail, owner has no user mapping +ERROR: user mapping not found for user "regress_simport_owner", server "loopback" +SELECT most_common_vals FROM pg_stats WHERE tablename = 'simport_fsecret'; + most_common_vals +------------------ +(0 rows) + +-- Functions invoked during statistics import (here a domain's CHECK +-- constraint) must run as the foreign table's owner, with a restricted +-- search_path, as in the sampling case +CREATE USER MAPPING FOR regress_simport_owner SERVER loopback + OPTIONS (password_required 'false'); +CREATE TABLE simport_dtable (c1 int); +INSERT INTO simport_dtable VALUES (1), (1), (2), (2); +ANALYZE simport_dtable; +SET ROLE regress_simport_owner; +CREATE FUNCTION simport_check(int) RETURNS bool LANGUAGE plpgsql AS $$ +BEGIN + RAISE NOTICE 'current_user: %, search_path: %', + current_user, current_setting('search_path'); + RETURN true; +END $$; +CREATE DOMAIN simport_dom AS int CHECK (simport_check(VALUE)); +CREATE FOREIGN TABLE simport_fdtable (c1 simport_dom) + SERVER loopback OPTIONS (table_name 'simport_dtable', import_stats 'true'); +RESET ROLE; +ANALYZE VERBOSE simport_fdtable; -- should work +INFO: importing statistics for foreign table "public.simport_fdtable" +NOTICE: current_user: regress_simport_owner, search_path: pg_catalog, pg_temp +NOTICE: current_user: regress_simport_owner, search_path: pg_catalog, pg_temp +INFO: finished importing statistics for foreign table "public.simport_fdtable" -- cleanup DROP FOREIGN TABLE simport_ftable; DROP FOREIGN TABLE simport_fview; @@ -13825,6 +13869,11 @@ DROP FOREIGN TABLE simport_fpt; DROP TABLE simport_pt; DROP FOREIGN TABLE dtest_ftable; DROP TABLE dtest_table; +DROP TABLE simport_secret; +DROP TABLE simport_dtable; +DROP USER MAPPING FOR regress_simport_owner SERVER loopback; +DROP OWNED BY regress_simport_owner; +DROP ROLE regress_simport_owner; -- =================================================================== -- test for postgres_fdw_get_connections function with check_conn = true -- =================================================================== diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index fbe01fdde85..42a704344b8 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -5832,11 +5832,10 @@ fetch_remote_statistics(Relation relation, } /* - * Get connection to the foreign server. Connection manager will - * establish new connection if necessary. - * - * Note that unlike the sampling case, we only query pg_class and - * pg_stats, so we do the remote access as the current user. + * Get the connection to use. We do the remote access as the table's + * owner. Note that unlike AnalyzeForeignTable(), the core code would + * already have switched us to the table's owner, before we are called + * from ImportForeignStatistics(). */ user = GetUserMapping(GetUserId(), table->serverid); conn = GetConnection(user, false, NULL); diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index f1ca3204382..21558282c35 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -4956,6 +4956,45 @@ SELECT attname, inherited, null_frac, avg_width, n_distinct, FROM pg_stats WHERE schemaname = 'public' AND tablename = 'dtest_ftable'; +-- Statistics import must access the remote server as the foreign table's +-- owner, not as the user running ANALYZE +CREATE ROLE regress_simport_owner NOSUPERUSER; +GRANT USAGE ON FOREIGN SERVER loopback TO regress_simport_owner; +GRANT CREATE ON SCHEMA public TO regress_simport_owner; +CREATE TABLE simport_secret (s text); +REVOKE ALL ON simport_secret FROM PUBLIC; +INSERT INTO simport_secret SELECT 'hunter2' FROM generate_series(1, 10); +ANALYZE simport_secret; +SET ROLE regress_simport_owner; +CREATE FOREIGN TABLE simport_fsecret (s text) + SERVER loopback OPTIONS (table_name 'simport_secret', import_stats 'true'); +RESET ROLE; + +ANALYZE simport_fsecret; -- should fail, owner has no user mapping +SELECT most_common_vals FROM pg_stats WHERE tablename = 'simport_fsecret'; + +-- Functions invoked during statistics import (here a domain's CHECK +-- constraint) must run as the foreign table's owner, with a restricted +-- search_path, as in the sampling case +CREATE USER MAPPING FOR regress_simport_owner SERVER loopback + OPTIONS (password_required 'false'); +CREATE TABLE simport_dtable (c1 int); +INSERT INTO simport_dtable VALUES (1), (1), (2), (2); +ANALYZE simport_dtable; +SET ROLE regress_simport_owner; +CREATE FUNCTION simport_check(int) RETURNS bool LANGUAGE plpgsql AS $$ +BEGIN + RAISE NOTICE 'current_user: %, search_path: %', + current_user, current_setting('search_path'); + RETURN true; +END $$; +CREATE DOMAIN simport_dom AS int CHECK (simport_check(VALUE)); +CREATE FOREIGN TABLE simport_fdtable (c1 simport_dom) + SERVER loopback OPTIONS (table_name 'simport_dtable', import_stats 'true'); +RESET ROLE; + +ANALYZE VERBOSE simport_fdtable; -- should work + -- cleanup DROP FOREIGN TABLE simport_ftable; DROP FOREIGN TABLE simport_fview; @@ -4965,6 +5004,11 @@ DROP FOREIGN TABLE simport_fpt; DROP TABLE simport_pt; DROP FOREIGN TABLE dtest_ftable; DROP TABLE dtest_table; +DROP TABLE simport_secret; +DROP TABLE simport_dtable; +DROP USER MAPPING FOR regress_simport_owner SERVER loopback; +DROP OWNED BY regress_simport_owner; +DROP ROLE regress_simport_owner; -- =================================================================== -- test for postgres_fdw_get_connections function with check_conn = true diff --git a/doc/src/sgml/fdwhandler.sgml b/doc/src/sgml/fdwhandler.sgml index 502441fefcb..77cb4d2c143 100644 --- a/doc/src/sgml/fdwhandler.sgml +++ b/doc/src/sgml/fdwhandler.sgml @@ -1433,6 +1433,19 @@ ImportForeignStatistics(Relation relation, called on the foreign table to generate statistics locally, if supported. + + ImportForeignStatistics is called with the current + user switched to the owner of the foreign table, within a + security-restricted operation, and with + temporarily changed to + pg_catalog, pg_temp, as is done when statistics are + calculated locally. Thus, if the FDW needs to access a remote server, + it should use the user mapping for the current user (that is, the owner + of the foreign table), as returned by GetUserId. + Note that AnalyzeForeignTable is not called in + this environment. + + For reference, the logic for calculating statistics in PostgreSQL is found in diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 4fea106bafd..58370c0a2c5 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -228,10 +228,32 @@ analyze_rel(Oid relid, RangeVar *relation, fdwroutine = GetFdwRoutineForRelation(onerel, false); - if (fdwroutine->ImportForeignStatistics != NULL && - fdwroutine->ImportForeignStatistics(onerel, va_cols, elevel)) - stats_imported = true; - else + if (fdwroutine->ImportForeignStatistics != NULL) + { + Oid save_userid; + int save_sec_context; + int save_nestlevel; + + /* + * Switch to the table owner's userid, as in the sampling method. + * Also lock down security-restricted operations and arrange to + * make GUC variable changes local to this command. + */ + GetUserIdAndSecContext(&save_userid, &save_sec_context); + SetUserIdAndSecContext(onerel->rd_rel->relowner, + save_sec_context | SECURITY_RESTRICTED_OPERATION); + save_nestlevel = NewGUCNestLevel(); + RestrictSearchPath(); + + stats_imported = fdwroutine->ImportForeignStatistics(onerel, + va_cols, + elevel); + + AtEOXact_GUC(false, save_nestlevel); + SetUserIdAndSecContext(save_userid, save_sec_context); + } + + if (!stats_imported) { bool ok = false; -- 2.50.1 (Apple Git-155)