diff options
author | Stephen Frost <sfrost@snowman.net> | 2023-04-08 07:21:35 -0400 |
---|---|---|
committer | Stephen Frost <sfrost@snowman.net> | 2023-04-08 07:21:35 -0400 |
commit | 3d03b24c350ab060bb223623bdff38835bd7afd0 (patch) | |
tree | 26137687e4b234c47de0140295baaed9928cc968 /contrib | |
parent | db4f21e4a34b1d5a3f7123e28e77f575d1a971ea (diff) | |
download | postgresql-3d03b24c350ab060bb223623bdff38835bd7afd0.tar.gz postgresql-3d03b24c350ab060bb223623bdff38835bd7afd0.zip |
Revert "Add support for Kerberos credential delegation"
This reverts commit 3d4fa227bce4294ce1cc214b4a9d3b7caa3f0454.
Per discussion and buildfarm, this depends on APIs that seem to not
be available on at least one platform (NetBSD). Should be certainly
possible to rework to be optional on that platform if necessary but bit
late for that at this point.
Discussion: https://postgr.es/m/3286097.1680922218@sss.pgh.pa.us
Diffstat (limited to 'contrib')
-rw-r--r-- | contrib/dblink/dblink.c | 127 | ||||
-rw-r--r-- | contrib/dblink/expected/dblink.out | 4 | ||||
-rw-r--r-- | contrib/postgres_fdw/connection.c | 72 | ||||
-rw-r--r-- | contrib/postgres_fdw/expected/postgres_fdw.out | 19 | ||||
-rw-r--r-- | contrib/postgres_fdw/option.c | 6 | ||||
-rw-r--r-- | contrib/postgres_fdw/sql/postgres_fdw.sql | 3 |
6 files changed, 73 insertions, 158 deletions
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c index 55f75eff361..78a8bcee6e3 100644 --- a/contrib/dblink/dblink.c +++ b/contrib/dblink/dblink.c @@ -48,7 +48,6 @@ #include "funcapi.h" #include "lib/stringinfo.h" #include "libpq-fe.h" -#include "libpq/libpq-be.h" #include "libpq/libpq-be-fe-helpers.h" #include "mb/pg_wchar.h" #include "miscadmin.h" @@ -112,8 +111,7 @@ static HeapTuple get_tuple_of_interest(Relation rel, int *pkattnums, int pknumat static Relation get_rel_from_relname(text *relname_text, LOCKMODE lockmode, AclMode aclmode); static char *generate_relation_name(Relation rel); static void dblink_connstr_check(const char *connstr); -static bool dblink_connstr_has_pw(const char *connstr); -static void dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr); +static void dblink_security_check(PGconn *conn, remoteConn *rconn); static void dblink_res_error(PGconn *conn, const char *conname, PGresult *res, bool fail, const char *fmt,...) pg_attribute_printf(5, 6); static char *get_connect_string(const char *servername); @@ -215,7 +213,7 @@ dblink_get_conn(char *conname_or_str, errmsg("could not establish connection"), errdetail_internal("%s", msg))); } - dblink_security_check(conn, rconn, connstr); + dblink_security_check(conn, rconn); if (PQclientEncoding(conn) != GetDatabaseEncoding()) PQsetClientEncoding(conn, GetDatabaseEncodingName()); freeconn = true; @@ -309,7 +307,7 @@ dblink_connect(PG_FUNCTION_ARGS) } /* check password actually used if not superuser */ - dblink_security_check(conn, rconn, connstr); + dblink_security_check(conn, rconn); /* attempt to set client encoding to match server encoding, if needed */ if (PQclientEncoding(conn) != GetDatabaseEncoding()) @@ -2586,99 +2584,64 @@ deleteConnection(const char *name) errmsg("undefined connection name"))); } -/* - * We need to make sure that the connection made used credentials - * which were provided by the user, so check what credentials were - * used to connect and then make sure that they came from the user. - */ static void -dblink_security_check(PGconn *conn, remoteConn *rconn, const char *connstr) +dblink_security_check(PGconn *conn, remoteConn *rconn) { - /* Superuser bypasses security check */ - if (superuser()) - return; - - /* If password was used to connect, make sure it was one provided */ - if (PQconnectionUsedPassword(conn) && dblink_connstr_has_pw(connstr)) - return; - -#ifdef ENABLE_GSS - /* If GSSAPI creds used to connect, make sure it was one delegated */ - if (PQconnectionUsedGSSAPI(conn) && be_gssapi_get_deleg(MyProcPort)) - return; -#endif - - /* Otherwise, fail out */ - libpqsrv_disconnect(conn); - if (rconn) - pfree(rconn); + if (!superuser()) + { + if (!PQconnectionUsedPassword(conn)) + { + libpqsrv_disconnect(conn); + if (rconn) + pfree(rconn); - ereport(ERROR, - (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED), - errmsg("password or GSSAPI delegated credentials required"), - errdetail("Non-superusers may only connect using credentials they provide, eg: password in connection string or delegated GSSAPI credentials"), - errhint("Ensure provided credentials match target server's authentication method."))); + 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."))); + } + } } /* - * Function to check if the connection string includes an explicit - * password, needed to ensure that non-superuser password-based auth - * is using a provided password and not one picked up from the - * environment. + * 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. */ -static bool -dblink_connstr_has_pw(const char *connstr) +static void +dblink_connstr_check(const char *connstr) { - PQconninfoOption *options; - PQconninfoOption *option; - bool connstr_gives_password = false; - - options = PQconninfoParse(connstr, NULL); - if (options) + if (!superuser()) { - for (option = options; option->keyword != NULL; option++) + PQconninfoOption *options; + PQconninfoOption *option; + bool connstr_gives_password = false; + + options = PQconninfoParse(connstr, NULL); + if (options) { - if (strcmp(option->keyword, "password") == 0) + for (option = options; option->keyword != NULL; option++) { - if (option->val != NULL && option->val[0] != '\0') + if (strcmp(option->keyword, "password") == 0) { - connstr_gives_password = true; - break; + if (option->val != NULL && option->val[0] != '\0') + { + connstr_gives_password = true; + break; + } } } + PQconninfoFree(options); } - PQconninfoFree(options); - } - - return connstr_gives_password; -} -/* - * For non-superusers, insist that the connstr specify a password, except - * if GSSAPI credentials have been delegated (and we check that they are used - * for the connection in dblink_security_check later). This prevents a - * password or GSSAPI credentials from being picked up from .pgpass, a - * service file, the environment, etc. We don't want the postgres user's - * passwords or Kerberos credentials to be accessible to non-superusers. - */ -static void -dblink_connstr_check(const char *connstr) -{ - if (superuser()) - return; - - if (dblink_connstr_has_pw(connstr)) - return; - -#ifdef ENABLE_GSS - if (be_gssapi_get_deleg(MyProcPort)) - return; -#endif - - ereport(ERROR, - (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED), - errmsg("password or GSSAPI delegated credentials required"), - errdetail("Non-superusers must provide a password in the connection string or send delegated GSSAPI credentials."))); + if (!connstr_gives_password) + ereport(ERROR, + (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED), + errmsg("password is required"), + errdetail("Non-superusers must provide a password in the connection string."))); + } } /* diff --git a/contrib/dblink/expected/dblink.out b/contrib/dblink/expected/dblink.out index 7809f58d96b..0f5050b4093 100644 --- a/contrib/dblink/expected/dblink.out +++ b/contrib/dblink/expected/dblink.out @@ -903,8 +903,8 @@ GRANT EXECUTE ON FUNCTION dblink_connect_u(text, text) TO regress_dblink_user; SET SESSION AUTHORIZATION regress_dblink_user; -- should fail SELECT dblink_connect('myconn', 'fdtest'); -ERROR: password or GSSAPI delegated credentials required -DETAIL: Non-superusers must provide a password in the connection string or send delegated GSSAPI credentials. +ERROR: password is required +DETAIL: Non-superusers must provide a password in the connection string. -- should succeed SELECT dblink_connect_u('myconn', 'fdtest'); dblink_connect_u diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c index 75d93d6eadf..2969351e9a9 100644 --- a/contrib/postgres_fdw/connection.c +++ b/contrib/postgres_fdw/connection.c @@ -17,7 +17,6 @@ #include "catalog/pg_user_mapping.h" #include "commands/defrem.h" #include "funcapi.h" -#include "libpq/libpq-be.h" #include "libpq/libpq-be-fe-helpers.h" #include "mb/pg_wchar.h" #include "miscadmin.h" @@ -150,8 +149,6 @@ static void pgfdw_finish_pre_subcommit_cleanup(List *pending_entries, static void pgfdw_finish_abort_cleanup(List *pending_entries, List *cancel_requested, bool toplevel); -static void pgfdw_security_check(const char **keywords, const char **values, - UserMapping *user, PGconn *conn); static bool UserMappingPasswordRequired(UserMapping *user); static bool disconnect_cached_connections(Oid serverid); @@ -388,47 +385,6 @@ make_new_connection(ConnCacheEntry *entry, UserMapping *user) } /* - * Check that non-superuser has used password or delegated credentials - * to establish connection; otherwise, he's piggybacking on the - * postgres server's user identity. See also dblink_security_check() - * in contrib/dblink and check_conn_params. - */ -static void -pgfdw_security_check(const char **keywords, const char **values, UserMapping *user, PGconn *conn) -{ - /* Superusers bypass the check */ - if (superuser_arg(user->userid)) - return; - -#ifdef ENABLE_GSS - /* Connected via GSSAPI with delegated credentials- all good. */ - if (PQconnectionUsedGSSAPI(conn) && be_gssapi_get_deleg(MyProcPort)) - return; -#endif - - /* Ok if superuser set PW required false. */ - if (!UserMappingPasswordRequired(user)) - return; - - /* Connected via PW, with PW required true, and provided non-empty PW. */ - if (PQconnectionUsedPassword(conn)) - { - /* ok if params contain a non-empty password */ - for (int i = 0; keywords[i] != NULL; i++) - { - if (strcmp(keywords[i], "password") == 0 && values[i][0] != '\0') - return; - } - } - - ereport(ERROR, - (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED), - errmsg("password or GSSAPI delegated credentials required"), - errdetail("Non-superuser cannot connect if the server does not request a password or use GSSAPI with delegated credentials."), - errhint("Target server's authentication method must be changed or password_required=false set in the user mapping attributes."))); -} - -/* * Connect to remote server using specified server and user mapping properties. */ static PGconn * @@ -539,8 +495,19 @@ connect_pg_server(ForeignServer *server, UserMapping *user) server->servername), errdetail_internal("%s", pchomp(PQerrorMessage(conn))))); - /* Perform post-connection security checks */ - pgfdw_security_check(keywords, values, user, conn); + /* + * 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 and + * check_conn_params. + */ + 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 or password_required=false set in the user mapping attributes."))); /* Prepare new session for use */ configure_remote_session(conn); @@ -594,8 +561,7 @@ UserMappingPasswordRequired(UserMapping *user) } /* - * For non-superusers, insist that the connstr specify a password or that the - * user provided their own GSSAPI delegated credentials. This + * 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, * certificates, etc to be accessible to non-superusers. (See also @@ -610,12 +576,6 @@ check_conn_params(const char **keywords, const char **values, UserMapping *user) if (superuser_arg(user->userid)) return; -#ifdef ENABLE_GSS - /* ok if the user provided their own delegated credentials */ - if (be_gssapi_get_deleg(MyProcPort)) - return; -#endif - /* ok if params contain a non-empty password */ for (i = 0; keywords[i] != NULL; i++) { @@ -629,8 +589,8 @@ check_conn_params(const char **keywords, const char **values, UserMapping *user) ereport(ERROR, (errcode(ERRCODE_S_R_E_PROHIBITED_SQL_STATEMENT_ATTEMPTED), - errmsg("password or GSSAPI delegated credentials required"), - errdetail("Non-superusers must delegate GSSAPI credentials or provide a password in the user mapping."))); + errmsg("password is required"), + errdetail("Non-superusers must provide a password in the user mapping."))); } /* diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out index fd5752bd5bf..8f6a04f71b6 100644 --- a/contrib/postgres_fdw/expected/postgres_fdw.out +++ b/contrib/postgres_fdw/expected/postgres_fdw.out @@ -171,8 +171,7 @@ ALTER SERVER testserver1 OPTIONS ( sslcrl 'value', --requirepeer 'value', krbsrvname 'value', - gsslib 'value', - gssdeleg 'value' + gsslib 'value' --replication 'value' ); -- Error, invalid list syntax @@ -9841,8 +9840,8 @@ CREATE FOREIGN TABLE pg_temp.ft1_nopw ( c8 user_enum ) SERVER loopback_nopw OPTIONS (schema_name 'public', table_name 'ft1'); SELECT 1 FROM ft1_nopw LIMIT 1; -ERROR: password or GSSAPI delegated credentials required -DETAIL: Non-superusers must delegate GSSAPI credentials or provide a password in the user mapping. +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. ALTER SERVER loopback_nopw OPTIONS (ADD password 'dummypw'); @@ -9854,16 +9853,16 @@ HINT: Perhaps you meant the option "passfile". -- 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 1 FROM ft1_nopw LIMIT 1; -ERROR: password or GSSAPI delegated credentials required -DETAIL: Non-superuser cannot connect if the server does not request a password or use GSSAPI with delegated credentials. +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 1 FROM ft1_nopw LIMIT 1; -ERROR: password or GSSAPI delegated credentials required -DETAIL: Non-superuser cannot connect if the server does not request a password or use GSSAPI with delegated credentials. +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 @@ -9891,8 +9890,8 @@ 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 1 FROM ft1_nopw LIMIT 1; -ERROR: password or GSSAPI delegated credentials required -DETAIL: Non-superusers must delegate GSSAPI credentials or provide a password in the user mapping. +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. diff --git a/contrib/postgres_fdw/option.c b/contrib/postgres_fdw/option.c index fe40d50c6dd..4229d2048c3 100644 --- a/contrib/postgres_fdw/option.c +++ b/contrib/postgres_fdw/option.c @@ -288,12 +288,6 @@ InitPgFdwOptions(void) {"sslcert", UserMappingRelationId, true}, {"sslkey", UserMappingRelationId, true}, - /* - * gssdeleg is also a libpq option but should be allowed in a user - * mapping context too - */ - {"gssdeleg", UserMappingRelationId, true}, - {NULL, InvalidOid, false} }; diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql index c05046f8676..5bd69339dfc 100644 --- a/contrib/postgres_fdw/sql/postgres_fdw.sql +++ b/contrib/postgres_fdw/sql/postgres_fdw.sql @@ -185,8 +185,7 @@ ALTER SERVER testserver1 OPTIONS ( sslcrl 'value', --requirepeer 'value', krbsrvname 'value', - gsslib 'value', - gssdeleg 'value' + gsslib 'value' --replication 'value' ); |