This patch allows the superuser to grant passwordless connection rights in postgres_fdw user mappings.
The patch is authored by my colleague Craig Ringer, with slight bitrot fixed by me. One use case for this is with passphrase-protected client certificates, a patch for which will follow shortly. Here are Craig's remarks on the patch: postgres_fdw denies a non-superuser the ability to establish a connection that doesn't have a password in the connection string, or one that fails to actually use the password in authentication. This is to stop the unprivileged user using OS-level authentication as the postgres server (peer, ident, trust). It also stops unauthorized use of local credentials like .pgpass, a service file, client certificate files, etc. Add the ability for a superuser to create user mappings that override this behaviour by setting the passwordless_ok attribute to true in a user mapping for a non-superuser. The non-superuser gains the ability to use the FDW the mapping applies to even if there's no password in their mapping or in the connection string. This is only safe if the superuser has established that the local server is configured safely. It must be configured not to allow trust/peer/ident/sspi/gssapi auth to allow the OS user the postgres server runs as to log in to postgres as a superuser. Client certificate keys can be used too, if accessible. But the superuser can already GRANT superrole TO normalrole, so it's not any sort of new power. cheers andrew -- Andrew Dunstan https://www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 57ed5f4b90..4739ab0630 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -17,6 +17,7 @@ #include "access/htup_details.h" #include "access/xact.h" #include "catalog/pg_user_mapping.h" +#include "commands/defrem.h" #include "mb/pg_wchar.h" #include "miscadmin.h" #include "pgstat.h" @@ -91,7 +92,7 @@ static bool pgfdw_exec_cleanup_query(PGconn *conn, const char *query, bool ignore_errors); static bool pgfdw_get_cleanup_result(PGconn *conn, TimestampTz endtime, PGresult **result); - +static bool UserMappingPasswordRequired(UserMapping *user); /* * Get a PGconn which can be used to execute queries on the remote PostgreSQL @@ -274,14 +275,16 @@ connect_pg_server(ForeignServer *server, UserMapping *user) /* * Check that non-superuser has used password to establish connection; * otherwise, he's piggybacking on the postgres server's user - * identity. See also dblink_security_check() in contrib/dblink. + * identity. See also dblink_security_check() in contrib/dblink + * and check_conn_params. */ - if (!superuser_arg(user->userid) && !PQconnectionUsedPassword(conn)) + if (!superuser_arg(user->userid) && UserMappingPasswordRequired(user) && + !PQconnectionUsedPassword(conn)) ereport(ERROR, (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED), errmsg("password is required"), errdetail("Non-superuser cannot connect if the server does not request a password."), - errhint("Target server's authentication method must be changed."))); + errhint("Target server's authentication method must be changed or password_required=false set in the user mapping attributes."))); /* Prepare new session for use */ configure_remote_session(conn); @@ -314,12 +317,31 @@ disconnect_pg_server(ConnCacheEntry *entry) } } +/* + * Return true if the password_required is defined and false for this user + * mapping, otherwise false. The mapping has been pre-validated. + */ +static bool +UserMappingPasswordRequired(UserMapping *user) +{ + ListCell *cell; + + foreach(cell, user->options) + { + DefElem *def = (DefElem *) lfirst(cell); + if (strcmp(def->defname, "password_required") == 0) + return defGetBoolean(def); + } + + return true; +} + /* * For non-superusers, insist that the connstr specify a password. This - * prevents a password from being picked up from .pgpass, a service file, - * the environment, etc. We don't want the postgres user's passwords - * to be accessible to non-superusers. (See also dblink_connstr_check in - * contrib/dblink.) + * prevents a password from being picked up from .pgpass, a service file, the + * environment, etc. We don't want the postgres user's passwords, + * certificates, etc to be accessible to non-superusers. (See also + * dblink_connstr_check in contrib/dblink.) */ static void check_conn_params(const char **keywords, const char **values, UserMapping *user) @@ -337,6 +359,10 @@ check_conn_params(const char **keywords, const char **values, UserMapping *user) return; } + /* ok if the superuser explicitly said so at user mapping creation time */ + if (!UserMappingPasswordRequired(user)) + return; + ereport(ERROR, (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED), errmsg("password is required"), diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index f0c842a607..298f652afc 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -8779,5 +8779,99 @@ SELECT b, avg(a), max(a), count(*) FROM pagg_tab GROUP BY b HAVING sum(a) < 700 -> Foreign Scan on fpagg_tab_p3 (15 rows) +-- =================================================================== +-- access rights and superuser +-- =================================================================== +-- Non-superuser cannot create a FDW without a password in the connstr +CREATE ROLE nosuper NOSUPERUSER; +GRANT USAGE ON FOREIGN DATA WRAPPER postgres_fdw TO nosuper; +SET ROLE nosuper; +SHOW is_superuser; + is_superuser +-------------- + off +(1 row) + +-- This will be OK, we can create the FDW +DO $d$ + BEGIN + EXECUTE $$CREATE SERVER loopback_nopw FOREIGN DATA WRAPPER postgres_fdw + OPTIONS (dbname '$$||current_database()||$$', + port '$$||current_setting('port')||$$' + )$$; + END; +$d$; +-- But creation of user mappings for non-superusers should fail +CREATE USER MAPPING FOR public SERVER loopback_nopw; +CREATE USER MAPPING FOR CURRENT_USER SERVER loopback_nopw; +CREATE FOREIGN TABLE ft1_nopw ( + c1 int NOT NULL, + c2 int NOT NULL, + c3 text, + c4 timestamptz, + c5 timestamp, + c6 varchar(10), + c7 char(10) default 'ft1', + c8 user_enum +) SERVER loopback_nopw OPTIONS (schema_name 'public', table_name 'ft1'); +SELECT * FROM ft1_nopw LIMIT 1; +ERROR: password is required +DETAIL: Non-superusers must provide a password in the user mapping. +-- If we add a password to the connstr it'll fail, because we don't allow passwords +-- in connstrs only in user mappings. +DO $d$ + BEGIN + EXECUTE $$ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')$$; + END; +$d$; +ERROR: invalid option "password" +HINT: Valid options in this context are: service, passfile, channel_binding, connect_timeout, dbname, host, hostaddr, port, options, application_name, keepalives, keepalives_idle, keepalives_interval, keepalives_count, tcp_user_timeout, sslmode, sslcompression, sslcert, sslkey, sslrootcert, sslcrl, requirepeer, gssencmode, krbsrvname, target_session_attrs, use_remote_estimate, fdw_startup_cost, fdw_tuple_cost, extensions, updatable, fetch_size +CONTEXT: SQL statement "ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')" +PL/pgSQL function inline_code_block line 3 at EXECUTE +-- If we add a password for our user mapping instead, we should get a different +-- error because the password wasn't actually *used* when we run with trust auth. +-- +-- This won't work with installcheck, but neither will most of the FDW checks. +ALTER USER MAPPING FOR CURRENT_USER SERVER loopback_nopw OPTIONS (ADD password 'dummypw'); +SELECT * FROM ft1_nopw LIMIT 1; +ERROR: password is required +DETAIL: Non-superuser cannot connect if the server does not request a password. +HINT: Target server's authentication method must be changed or password_required=false set in the user mapping attributes. +-- Unpriv user cannot make the mapping passwordless +ALTER USER MAPPING FOR CURRENT_USER SERVER loopback_nopw OPTIONS (ADD password_required 'false'); +ERROR: password_required=false is superuser-only +HINT: User mappings with the password_required option set to false may only be created or modified by the superuser +SELECT * FROM ft1_nopw LIMIT 1; +ERROR: password is required +DETAIL: Non-superuser cannot connect if the server does not request a password. +HINT: Target server's authentication method must be changed or password_required=false set in the user mapping attributes. +RESET ROLE; +-- But the superuser can +ALTER USER MAPPING FOR nosuper SERVER loopback_nopw OPTIONS (ADD password_required 'false'); +SET ROLE nosuper; +-- Should finally work now +SELECT * FROM ft1_nopw LIMIT 1; + c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 +------+----+----+----+----+----+------------+---- + 1111 | 2 | | | | | ft1 | +(1 row) + +-- We're done with the role named after a specific user and need to check the +-- changes to the public mapping. +DROP USER MAPPING FOR CURRENT_USER SERVER loopback_nopw; +-- This will fail again as it'll resolve the user mapping for public, which +-- lacks password_required=false +SELECT * FROM ft1_nopw LIMIT 1; +ERROR: password is required +DETAIL: Non-superusers must provide a password in the user mapping. +RESET ROLE; +-- The user mapping for public is passwordless and lacks the password_required=false +-- mapping option, but will work because the current user is a superuser. +SELECT * FROM ft1_nopw LIMIT 1; + c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8 +------+----+----+----+----+----+------------+---- + 1111 | 2 | | | | | ft1 | +(1 row) + -- Clean-up RESET enable_partitionwise_aggregate; diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c index 7ea68c3ce3..07fc7fa00a 100644 --- a/contrib/postgres_fdw/option.c +++ b/contrib/postgres_fdw/option.c @@ -23,6 +23,7 @@ #include "utils/builtins.h" #include "utils/varlena.h" +#include "miscadmin.h" /* * Describes the valid options for objects that this wrapper uses. @@ -143,6 +144,23 @@ postgres_fdw_validator(PG_FUNCTION_ARGS) errmsg("%s requires a non-negative integer value", def->defname))); } + else if (strcmp(def->defname, "password_required") == 0) + { + bool pw_required = defGetBoolean(def); + + /* + * Only the superuser may set this option on a user mapping, or + * alter a user mapping on which this option is set. We allow a + * user to clear this option if it's set - in fact, we don't have a + * choice since we can't see the old mapping when validating an + * alter. + */ + if (!superuser() && !pw_required) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("password_required=false is superuser-only"), + errhint("User mappings with the password_required option set to false may only be created or modified by the superuser"))); + } } PG_RETURN_VOID(); @@ -177,6 +195,7 @@ InitPgFdwOptions(void) /* fetch_size is available on both server and table */ {"fetch_size", ForeignServerRelationId, false}, {"fetch_size", ForeignTableRelationId, false}, + {"password_required", UserMappingRelationId, false}, {NULL, InvalidOid, false} }; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index 630b803e26..5f50c65566 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -2476,6 +2476,92 @@ SELECT a, count(t1) FROM pagg_tab t1 GROUP BY a HAVING avg(b) < 22 ORDER BY 1; EXPLAIN (COSTS OFF) SELECT b, avg(a), max(a), count(*) FROM pagg_tab GROUP BY b HAVING sum(a) < 700 ORDER BY 1; +-- =================================================================== +-- access rights and superuser +-- =================================================================== + +-- Non-superuser cannot create a FDW without a password in the connstr +CREATE ROLE nosuper NOSUPERUSER; + +GRANT USAGE ON FOREIGN DATA WRAPPER postgres_fdw TO nosuper; + +SET ROLE nosuper; + +SHOW is_superuser; + +-- This will be OK, we can create the FDW +DO $d$ + BEGIN + EXECUTE $$CREATE SERVER loopback_nopw FOREIGN DATA WRAPPER postgres_fdw + OPTIONS (dbname '$$||current_database()||$$', + port '$$||current_setting('port')||$$' + )$$; + END; +$d$; + +-- But creation of user mappings for non-superusers should fail +CREATE USER MAPPING FOR public SERVER loopback_nopw; +CREATE USER MAPPING FOR CURRENT_USER SERVER loopback_nopw; + +CREATE FOREIGN TABLE ft1_nopw ( + c1 int NOT NULL, + c2 int NOT NULL, + c3 text, + c4 timestamptz, + c5 timestamp, + c6 varchar(10), + c7 char(10) default 'ft1', + c8 user_enum +) SERVER loopback_nopw OPTIONS (schema_name 'public', table_name 'ft1'); + +SELECT * FROM ft1_nopw LIMIT 1; + +-- If we add a password to the connstr it'll fail, because we don't allow passwords +-- in connstrs only in user mappings. + +DO $d$ + BEGIN + EXECUTE $$ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw')$$; + END; +$d$; + +-- If we add a password for our user mapping instead, we should get a different +-- error because the password wasn't actually *used* when we run with trust auth. +-- +-- This won't work with installcheck, but neither will most of the FDW checks. + +ALTER USER MAPPING FOR CURRENT_USER SERVER loopback_nopw OPTIONS (ADD password 'dummypw'); + +SELECT * FROM ft1_nopw LIMIT 1; + +-- Unpriv user cannot make the mapping passwordless +ALTER USER MAPPING FOR CURRENT_USER SERVER loopback_nopw OPTIONS (ADD password_required 'false'); + +SELECT * FROM ft1_nopw LIMIT 1; + +RESET ROLE; + +-- But the superuser can +ALTER USER MAPPING FOR nosuper SERVER loopback_nopw OPTIONS (ADD password_required 'false'); + +SET ROLE nosuper; + +-- Should finally work now +SELECT * FROM ft1_nopw LIMIT 1; + +-- We're done with the role named after a specific user and need to check the +-- changes to the public mapping. +DROP USER MAPPING FOR CURRENT_USER SERVER loopback_nopw; + +-- This will fail again as it'll resolve the user mapping for public, which +-- lacks password_required=false +SELECT * FROM ft1_nopw LIMIT 1; + +RESET ROLE; + +-- The user mapping for public is passwordless and lacks the password_required=false +-- mapping option, but will work because the current user is a superuser. +SELECT * FROM ft1_nopw LIMIT 1; -- Clean-up RESET enable_partitionwise_aggregate; diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index 634a7141ef..fb4ceb355a 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -136,6 +136,30 @@ authentication, so always specify the <literal>password</literal> option for user mappings belonging to non-superusers. </para> + <para> + A superuser may override this check on a per-user-mapping basis by setting + the user mapping option <literal>password_required 'false'</literal>, e.g. + <programlisting> + ALTER USER MAPPING FOR some_non_superuser SERVER loopback_nopw + OPTIONS (ADD password_required 'false'); + </programlisting> + To prevent unprivileged users from exploiting the authentication rights + of the unix user the postgres server is running as to escalate to superuser + rights, only the superuser may set this option on a user mapping. + </para> + <para> + Care is required to ensure that this does not allow the mapped + user the ability to connect as superuser to the mapped database per + CVE-2007-3278 and CVE-2007-6601. Don't set + <literal>password_required=false</literal> + on the <literal>public</literal> role. Keep in mind that the mapped + user can potentially use any client certificates, + <filename>.pgpass</filename>, + <filename>.pg_service.conf</filename> etc in the unix home directory of the + system user the postgres server runs as. They can also use any trust + relationship granted by authentication modes like <literal>peer</literal> + or <literal>ident</literal> authentication. + </para> </sect3> <sect3>