diff --git a/contrib/ltree/ltree_op.c b/contrib/ltree/ltree_op.c index df61c63..39451a9 100644 --- a/contrib/ltree/ltree_op.c +++ b/contrib/ltree/ltree_op.c @@ -599,13 +599,16 @@ ltreeparentsel(PG_FUNCTION_ARGS) { /* Variable is being compared to a known non-null constant */ Datum constval = ((Const *) other)->constvalue; + Oid contcode; FmgrInfo contproc; double mcvsum; double mcvsel; double nullfrac; int hist_size; - fmgr_info(get_opcode(operator), &contproc); + contcode = get_opcode(operator); + pg_proc_trustcheck(contcode); + fmgr_info(contcode, &contproc); /* * Is the constant "<@" to any of the column's most common values? diff --git a/doc/src/sgml/ref/create_role.sgml b/doc/src/sgml/ref/create_role.sgml index db84273..41d1524 100644 --- a/doc/src/sgml/ref/create_role.sgml +++ b/doc/src/sgml/ref/create_role.sgml @@ -43,6 +43,7 @@ CREATE ROLE name [ [ WITH ] uid + Description diff --git a/src/backend/access/common/scankey.c b/src/backend/access/common/scankey.c index 781516c..60ddfcc 100644 --- a/src/backend/access/common/scankey.c +++ b/src/backend/access/common/scankey.c @@ -20,9 +20,11 @@ /* * ScanKeyEntryInitialize - * Initializes a scan key entry given all the field values. - * The target procedure is specified by OID (but can be invalid - * if SK_SEARCHNULL or SK_SEARCHNOTNULL is set). + * Initializes a scan key entry given all the field values. The target + * procedure is specified by OID (but can be invalid if SK_SEARCHNULL or + * SK_SEARCHNOTNULL is set). No access or trust check is done, so the + * procedure had better be innocuous. This is normally used for operator + * class members, which are subject to superuser approval. * * Note: CurrentMemoryContext at call should be as long-lived as the ScanKey * itself, because that's what will be used for any subsidiary info attached @@ -62,7 +64,8 @@ ScanKeyEntryInitialize(ScanKey entry, * * This is the recommended version for hardwired lookups in system catalogs. * It cannot handle NULL arguments, unary operators, or nondefault operators, - * but we need none of those features for most hardwired lookups. + * but we need none of those features for most hardwired lookups. Like other + * versions, it performs no security checks. * * We set collation to DEFAULT_COLLATION_OID always. This is appropriate * for textual columns in system catalogs, and it will be ignored for diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c index eade540..cbf9cea 100644 --- a/src/backend/access/index/indexam.c +++ b/src/backend/access/index/indexam.c @@ -93,6 +93,8 @@ * doesn't prevent the actual rebuild because we don't use RELATION_CHECKS * when calling the index AM's ambuild routine, and there is no reason for * ambuild to call its subsidiary routines through this file. + * + * We use neither access nor trust checks. * ---------------------------------------------------------------- */ #define RELATION_CHECKS \ diff --git a/src/backend/access/transam/README.parallel b/src/backend/access/transam/README.parallel index 85e5840..c333504 100644 --- a/src/backend/access/transam/README.parallel +++ b/src/backend/access/transam/README.parallel @@ -116,11 +116,10 @@ worker. This includes: - The active snapshot, which might be different from the transaction snapshot. - - The currently active user ID and security context. Note that this is - the fourth user ID we restore: the initial step of binding to the correct - database also involves restoring the authenticated user ID. When GUC - values are restored, this incidentally sets SessionUserId and OuterUserId - to the correct values. This final step restores CurrentUserId. + - The TransientUserStack. Note that we restored three user IDs already: the + initial step of binding to the correct database also involves restoring + the authenticated user ID. When GUC values are restored, this + incidentally sets SessionUserId and OuterUser to the correct values. - State related to pending REINDEX operations, which prevents access to an index that is currently being rebuilt. diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c index b9a9ae5..e2855a0 100644 --- a/src/backend/access/transam/parallel.c +++ b/src/backend/access/transam/parallel.c @@ -70,9 +70,10 @@ #define PARALLEL_KEY_TRANSACTION_STATE UINT64CONST(0xFFFFFFFFFFFF0008) #define PARALLEL_KEY_ENTRYPOINT UINT64CONST(0xFFFFFFFFFFFF0009) #define PARALLEL_KEY_SESSION_DSM UINT64CONST(0xFFFFFFFFFFFF000A) -#define PARALLEL_KEY_REINDEX_STATE UINT64CONST(0xFFFFFFFFFFFF000B) -#define PARALLEL_KEY_RELMAPPER_STATE UINT64CONST(0xFFFFFFFFFFFF000C) -#define PARALLEL_KEY_ENUMBLACKLIST UINT64CONST(0xFFFFFFFFFFFF000D) +#define PARALLEL_KEY_USER UINT64CONST(0xFFFFFFFFFFFF000B) +#define PARALLEL_KEY_REINDEX_STATE UINT64CONST(0xFFFFFFFFFFFF000C) +#define PARALLEL_KEY_RELMAPPER_STATE UINT64CONST(0xFFFFFFFFFFFF000D) +#define PARALLEL_KEY_ENUMBLACKLIST UINT64CONST(0xFFFFFFFFFFFF000E) /* Fixed-size parallel state. */ typedef struct FixedParallelState @@ -80,11 +81,9 @@ typedef struct FixedParallelState /* Fixed-size state that workers must restore. */ Oid database_id; Oid authenticated_user_id; - Oid current_user_id; Oid outer_user_id; Oid temp_namespace_id; Oid temp_toast_namespace_id; - int sec_context; bool is_superuser; PGPROC *parallel_master_pgproc; pid_t parallel_master_pid; @@ -210,6 +209,7 @@ InitializeParallelDSM(ParallelContext *pcxt) Size tsnaplen = 0; Size asnaplen = 0; Size tstatelen = 0; + Size userlen = 0; Size reindexlen = 0; Size relmapperlen = 0; Size enumblacklistlen = 0; @@ -262,6 +262,8 @@ InitializeParallelDSM(ParallelContext *pcxt) tstatelen = EstimateTransactionStateSpace(); shm_toc_estimate_chunk(&pcxt->estimator, tstatelen); shm_toc_estimate_chunk(&pcxt->estimator, sizeof(dsm_handle)); + userlen = EstimateTransientUserStackSpace(); + shm_toc_estimate_chunk(&pcxt->estimator, userlen); reindexlen = EstimateReindexStateSpace(); shm_toc_estimate_chunk(&pcxt->estimator, reindexlen); relmapperlen = EstimateRelationMapSpace(); @@ -319,7 +321,6 @@ InitializeParallelDSM(ParallelContext *pcxt) fps->authenticated_user_id = GetAuthenticatedUserId(); fps->outer_user_id = GetCurrentRoleId(); fps->is_superuser = session_auth_is_superuser; - GetUserIdAndSecContext(&fps->current_user_id, &fps->sec_context); GetTempNamespaceState(&fps->temp_namespace_id, &fps->temp_toast_namespace_id); fps->parallel_master_pgproc = MyProc; @@ -340,6 +341,7 @@ InitializeParallelDSM(ParallelContext *pcxt) char *tsnapspace; char *asnapspace; char *tstatespace; + char *userspace; char *reindexspace; char *relmapperspace; char *error_queue_space; @@ -384,6 +386,11 @@ InitializeParallelDSM(ParallelContext *pcxt) SerializeTransactionState(tstatelen, tstatespace); shm_toc_insert(pcxt->toc, PARALLEL_KEY_TRANSACTION_STATE, tstatespace); + /* Serialize transient user stack. */ + userspace = shm_toc_allocate(pcxt->toc, userlen); + SerializeTransientUserStack(userlen, userspace); + shm_toc_insert(pcxt->toc, PARALLEL_KEY_USER, userspace); + /* Serialize reindex state. */ reindexspace = shm_toc_allocate(pcxt->toc, reindexlen); SerializeReindexState(reindexlen, reindexspace); @@ -1228,6 +1235,7 @@ ParallelWorkerMain(Datum main_arg) char *tsnapspace; char *asnapspace; char *tstatespace; + char *userspace; char *reindexspace; char *relmapperspace; char *enumblacklistspace; @@ -1402,8 +1410,9 @@ ParallelWorkerMain(Datum main_arg) */ SetCurrentRoleId(fps->outer_user_id, fps->is_superuser); - /* Restore user ID and security context. */ - SetUserIdAndSecContext(fps->current_user_id, fps->sec_context); + /* Restore transient user stack. */ + userspace = shm_toc_lookup(toc, PARALLEL_KEY_USER, false); + RestoreTransientUserStack(userspace); /* Restore temp-namespace state to ensure search path matches leader's. */ SetTempNamespaceState(fps->temp_namespace_id, diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c index d967400..db37912 100644 --- a/src/backend/access/transam/xact.c +++ b/src/backend/access/transam/xact.c @@ -183,8 +183,7 @@ typedef struct TransactionStateData TransactionId *childXids; /* subcommitted child XIDs, in XID order */ int nChildXids; /* # of subcommitted child XIDs */ int maxChildXids; /* allocated size of childXids[] */ - Oid prevUser; /* previous CurrentUserId setting */ - int prevSecContext; /* previous SecurityRestrictionContext */ + TransientUser prevUser; /* cookie for previous user state */ bool prevXactReadOnly; /* entry-time xact r/o state */ bool startedInRecovery; /* did we start in recovery? */ bool didLogXid; /* has xid been included in WAL record? */ @@ -1836,10 +1835,9 @@ StartTransaction(void) * Once the current user ID and the security context flags are fetched, * both will be properly reset even if transaction startup fails. */ - GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext); - - /* SecurityRestrictionContext should never be set outside a transaction */ - Assert(s->prevSecContext == 0); + s->prevUser = GetTransientUser(); + Assert(!InLocalUserIdChange()); + Assert(!InSecurityRestrictedOperation()); /* * Make sure we've reset xact state variables @@ -2538,14 +2536,13 @@ AbortTransaction(void) /* * Reset user ID which might have been changed transiently. We need this * to clean up in case control escaped out of a SECURITY DEFINER function - * or other local change of CurrentUserId; therefore, the prior value of - * SecurityRestrictionContext also needs to be restored. + * or other local change of user ID. * * (Note: it is not necessary to restore session authorization or role * settings here because those can only be changed via GUC, and GUC will * take care of rolling them back if need be.) */ - SetUserIdAndSecContext(s->prevUser, s->prevSecContext); + RestoreTransientUser(s->prevUser); /* If in parallel mode, clean up workers and exit parallel mode. */ if (IsInParallelMode()) @@ -4755,7 +4752,7 @@ AbortSubTransaction(void) * Reset user ID which might have been changed transiently. (See notes in * AbortTransaction.) */ - SetUserIdAndSecContext(s->prevUser, s->prevSecContext); + RestoreTransientUser(s->prevUser); /* Exit from parallel mode, if necessary. */ if (IsInParallelMode()) @@ -4904,7 +4901,7 @@ PushTransaction(void) s->savepointLevel = p->savepointLevel; s->state = TRANS_DEFAULT; s->blockState = TBLOCK_SUBBEGIN; - GetUserIdAndSecContext(&s->prevUser, &s->prevSecContext); + s->prevUser = GetTransientUser(); s->prevXactReadOnly = XactReadOnly; s->parallelModeLevel = 0; diff --git a/src/backend/catalog/Makefile b/src/backend/catalog/Makefile index 0865240..6330e58 100644 --- a/src/backend/catalog/Makefile +++ b/src/backend/catalog/Makefile @@ -38,7 +38,8 @@ CATALOG_HEADERS := \ pg_statistic.h pg_rewrite.h pg_trigger.h pg_event_trigger.h pg_description.h \ pg_cast.h pg_enum.h pg_namespace.h pg_conversion.h pg_depend.h \ pg_database.h pg_db_role_setting.h pg_tablespace.h pg_pltemplate.h \ - pg_authid.h pg_auth_members.h pg_shdepend.h pg_shdescription.h \ + pg_authid.h pg_auth_members.h pg_auth_trust.h \ + pg_shdepend.h pg_shdescription.h \ pg_ts_config.h pg_ts_config_map.h pg_ts_dict.h \ pg_ts_parser.h pg_ts_template.h pg_extension.h \ pg_foreign_data_wrapper.h pg_foreign_server.h pg_user_mapping.h \ @@ -59,7 +60,8 @@ POSTGRES_BKI_SRCS := $(addprefix $(top_srcdir)/src/include/catalog/,\ # The .dat files we need can just be listed alphabetically. POSTGRES_BKI_DATA = $(addprefix $(top_srcdir)/src/include/catalog/,\ - pg_aggregate.dat pg_am.dat pg_amop.dat pg_amproc.dat pg_authid.dat \ + pg_aggregate.dat pg_am.dat pg_amop.dat pg_amproc.dat \ + pg_authid.dat pg_auth_trust.dat \ pg_cast.dat pg_class.dat pg_collation.dat \ pg_database.dat pg_language.dat \ pg_namespace.dat pg_opclass.dat pg_operator.dat pg_opfamily.dat \ diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 1dd70bb..cd70ce5 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -4649,7 +4649,8 @@ pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode) } /* - * Exported routine for checking a user's access privileges to a function + * Exported routine for checking a user's access privileges to a function. + * See also pg_proc_execcheck(). */ AclResult pg_proc_aclcheck(Oid proc_oid, Oid roleid, AclMode mode) @@ -4661,6 +4662,213 @@ pg_proc_aclcheck(Oid proc_oid, Oid roleid, AclMode mode) } /* + * Perform a "trust check", a kind of mirror image of an ACL check, against a + * particular object owner. ALTER ROLE TRUST creates a trust relationship, + * analogous to an ACL created by GRANT, from one user to another. If Alice + * has granted trust to Bob, trust checks with Alice as the current user and + * Bob as the object owner will pass. A trust relationship can name PUBLIC as + * one of the roles, and this serves as a wildcard. + * + * This mechanism arose as a response to the difficulty of avoiding accidental + * calls to functions controlled by potentially-hostile users. One could not + * write to a table or select from a view without implicitly permitting the + * table or view owner to cause arbitrary code to run as oneself. Hostile + * users can use function overloading and the search path to capture function + * calls. For example, you may write length(varcharcol) expecting to call + * length(text), but a hostile user's public.length(varchar) function would be + * chosen. The successful attacker can effectively hijack your database role. + * By establishing a conservative set of trust relationships, accidental calls + * to functions of untrusted users will end harmlessly with an error. + * + * Trust checks apply to functions (pg_proc_trustcheck_extended()) and + * operators (pg_oper_trustcheck_extended()). The value of checking functions + * is clear enough; they can run arbitrary commands. Operators are less + * threatening; trust is still checked for the underlying function, so + * tricking a user into calling an untrusted operator does not allow arbitrary + * code execution. However, an attacker could silently compromise query + * results by creating an =(text,text) operator that actually calls textne(). + * + * The choice to add operators and no other object type to the remit of trust + * checks is a judgement call rather than recognition of a bright line; one + * could argue for applying trust checks to objects like types, text search + * configurations, and even tables. In each case, an attacker who can lure a + * user into referencing the substitute object can falsify the user's query + * results. The decision arises from practical concerns: + * 1. The syntax for unambiguously-qualifying an operator is cumbersome. + * 2. Only functions and operators are subject to overloading; overloading + * makes it easier to reference the wrong object accidentally. + * 3. A typical query references more operators than any other object type, + * so simplifying the need for diligence brings a high relative payoff. + * 4. Types, probably the next most credible candidate, are difficult to + * inject unnoticed without also injecting an associated function. + * + * When SECURITY DEFINER functions and similar constructs are on the stack, we + * check users in addition to the current user for trust. See comments at + * GetSecurityApplicableRoles(). We actually could check only the current + * user for operators, but it's not worth complicating the policy. + * + * Returns the first applicable call stack user not accepting this owner or + * InvalidOid if this owner is approved. + * + * XXX This can be called in performance-sensitive contexts; consider adding a + * cache of 1-4 owners that have passed the trust check, invalidating that + * cache when the list of applicable roles changes. + */ +static Oid +owner_trustcheck(Oid owner) +{ + Oid *roles; + int nroles; + int i; + Oid ret = InvalidOid; + + if ( + /* Does PUBLIC trust owner? Common for BOOTSTRAP_SUPERUSERID. */ + SearchSysCacheExists2(AUTHTRUSTGRANTORTRUSTEE, + ObjectIdGetDatum(InvalidOid), + ObjectIdGetDatum(owner)) || + /* Does PUBLIC trust PUBLIC? Not recommended but true by default. */ + SearchSysCacheExists2(AUTHTRUSTGRANTORTRUSTEE, + ObjectIdGetDatum(InvalidOid), + ObjectIdGetDatum(InvalidOid))) + { + return InvalidOid; + } + + /* Find all roles having a stake in code that runs at this moment. */ + GetSecurityApplicableRoles(&roles, &nroles); + + for (i = 0; i < nroles; ++i) + { + if ( + /* A role implicitly trusts itself. */ + roles[i] != owner && + /* Does caller trust owner? */ + !SearchSysCacheExists2(AUTHTRUSTGRANTORTRUSTEE, + ObjectIdGetDatum(roles[i]), + ObjectIdGetDatum(owner)) && + /* Does caller trust PUBLIC? Not recommended, so check last. */ + !SearchSysCacheExists2(AUTHTRUSTGRANTORTRUSTEE, + ObjectIdGetDatum(roles[i]), + ObjectIdGetDatum(InvalidOid))) + { + ret = roles[i]; + break; + } + } + + pfree(roles); + return ret; +} + +/* + * Exported routine for checking trust relationships of an operator's owner. + * See comments at owner_trustcheck() for the semantics of trust. + * + * Operator trust can be a bit looser than function trust, because the + * consequence of skipping a check can include wrong query results but not + * arbitrary code execution. So, for example, one may skip checks before + * evaluating an operator in the planner so long as the executor will do them. + */ +bool +pg_oper_trustcheck_extended(Oid oper_oid, bool errorOK) +{ + HeapTuple tuple; + Oid owner; + Oid vetoer; + + tuple = SearchSysCache1(OPEROID, ObjectIdGetDatum(oper_oid)); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("operator with OID %u does not exist", oper_oid))); + owner = ((Form_pg_operator) GETSTRUCT(tuple))->oprowner; + ReleaseSysCache(tuple); + + vetoer = owner_trustcheck(owner); + if (!OidIsValid(vetoer)) + return true; + else if (errorOK) + return false; + + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("user %s does not trust operator %s", + GetUserNameFromId(vetoer, false), + get_opname(oper_oid)))); +} + +/* + * Exported routine for checking trust relationships of a function's owner. + * See comments at owner_trustcheck() for the semantics of trust and at + * pg_proc_execcheck() regarding when to use each of these variants. + */ +bool +pg_proc_trustcheck_extended(Oid proc_oid, bool errorOK) +{ + HeapTuple tuple; + Oid owner; + Oid vetoer; + + tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(proc_oid)); + if (!HeapTupleIsValid(tuple)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_FUNCTION), + errmsg("function with OID %u does not exist", proc_oid))); + owner = ((Form_pg_proc) GETSTRUCT(tuple))->proowner; + ReleaseSysCache(tuple); + + vetoer = owner_trustcheck(owner); + if (!OidIsValid(vetoer)) + return true; + else if (errorOK) + return false; + + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("user %s does not trust function %s", + GetUserNameFromId(vetoer, false), + get_func_name(proc_oid)))); +} + +/* + * Exported routine to make standard permissions checks applicable to running + * a function. Verifies that the current role has permission to execute the + * function and that the function owner has the necessary trust relationships. + * + * DDL that binds a function to another object without executing the function, + * such as CREATE TRIGGER, should not perform a trust check; instead, call + * pg_proc_aclcheck() directly. This allows a superuser to restore a database + * dump without trusting all trigger function owners. On the flip side, + * objects like triggers and range types that check ACLs at creation time and + * skip them at runtime must still check trust at runtime; use + * pg_proc_trustcheck() directly. + * + * Functions named when creating a base type (input, output, receive, send, + * typmod_in, typmod_out, analyze) require no trust checks or ACL checks; only + * superusers can create new base types, and the creator is expected to use + * functions free from security considerations that would entail either check. + * Likewise for the functions associated with index access methods, operator + * families, languages, foreign data wrappers, text search templates, and text + * search parsers. Selectivity functions also need no trust check, because + * only a superuser can define one by virtue of the arguments of type + * internal. However, all of these exempt functions are responsible for + * performing trust checks on _functions they execute_ that are not themselves + * exempt; this affects selectivity functions especially. + */ +void +pg_proc_execcheck(Oid proc_oid) +{ + AclResult aclresult; + + aclresult = pg_proc_aclcheck(proc_oid, GetUserId(), ACL_EXECUTE); + if (aclresult != ACLCHECK_OK) + aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(proc_oid)); + + pg_proc_trustcheck(proc_oid); +} + +/* * Exported routine for checking a user's access privileges to a language */ AclResult diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 5a160bf..173e314 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -29,6 +29,7 @@ #include "catalog/indexing.h" #include "catalog/namespace.h" #include "catalog/pg_auth_members.h" +#include "catalog/pg_auth_trust.h" #include "catalog/pg_authid.h" #include "catalog/pg_database.h" #include "catalog/pg_namespace.h" @@ -226,6 +227,7 @@ IsSharedRelation(Oid relationId) /* These are the shared catalogs (look for BKI_SHARED_RELATION) */ if (relationId == AuthIdRelationId || relationId == AuthMemRelationId || + relationId == AuthTrustRelationId || relationId == DatabaseRelationId || relationId == PLTemplateRelationId || relationId == SharedDescriptionRelationId || @@ -241,6 +243,8 @@ IsSharedRelation(Oid relationId) relationId == AuthIdOidIndexId || relationId == AuthMemRoleMemIndexId || relationId == AuthMemMemRoleIndexId || + relationId == AuthTrustGrantorTrusteeIndexId || + relationId == AuthTrustTrusteeGrantorIndexId || relationId == DatabaseNameIndexId || relationId == DatabaseOidIndexId || relationId == PLTemplateNameIndexId || diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 8709e8c..eefe269 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -2241,8 +2241,6 @@ index_build(Relation heapRelation, bool parallel) { IndexBuildResult *stats; - Oid save_userid; - int save_sec_context; int save_nestlevel; /* @@ -2284,9 +2282,7 @@ index_build(Relation heapRelation, * as that user. Also lock down security-restricted operations and * arrange to make GUC variable changes local to this command. */ - GetUserIdAndSecContext(&save_userid, &save_sec_context); - SetUserIdAndSecContext(heapRelation->rd_rel->relowner, - save_sec_context | SECURITY_RESTRICTED_OPERATION); + PushTransientUser(heapRelation->rd_rel->relowner, true, false); save_nestlevel = NewGUCNestLevel(); /* @@ -2390,11 +2386,9 @@ index_build(Relation heapRelation, if (indexInfo->ii_ExclusionOps != NULL) IndexCheckExclusion(heapRelation, indexRelation, indexInfo); - /* Roll back any GUC changes executed by index functions */ + /* Roll back GUC changes by index functions; revert user ID */ AtEOXact_GUC(false, save_nestlevel); - - /* Restore userid and security context */ - SetUserIdAndSecContext(save_userid, save_sec_context); + PopTransientUser(); } @@ -3127,8 +3121,6 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot) IndexInfo *indexInfo; IndexVacuumInfo ivinfo; v_i_state state; - Oid save_userid; - int save_sec_context; int save_nestlevel; /* Open and lock the parent heap relation */ @@ -3151,9 +3143,7 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot) * as that user. Also lock down security-restricted operations and * arrange to make GUC variable changes local to this command. */ - GetUserIdAndSecContext(&save_userid, &save_sec_context); - SetUserIdAndSecContext(heapRelation->rd_rel->relowner, - save_sec_context | SECURITY_RESTRICTED_OPERATION); + PushTransientUser(heapRelation->rd_rel->relowner, true, false); save_nestlevel = NewGUCNestLevel(); /* @@ -3200,11 +3190,9 @@ validate_index(Oid heapId, Oid indexId, Snapshot snapshot) "validate_index found %.0f heap tuples, %.0f index tuples; inserted %.0f missing tuples", state.htups, state.itups, state.tups_inserted); - /* Roll back any GUC changes executed by index functions */ + /* Roll back GUC changes by index functions; revert user ID */ AtEOXact_GUC(false, save_nestlevel); - - /* Restore userid and security context */ - SetUserIdAndSecContext(save_userid, save_sec_context); + PopTransientUser(); /* Close rels, but keep locks */ index_close(indexRelation, NoLock); diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c index 4b12e9f..280c89f 100644 --- a/src/backend/catalog/pg_aggregate.c +++ b/src/backend/catalog/pg_aggregate.c @@ -772,7 +772,9 @@ AggregateCreate(const char *aggName, /* * lookup_agg_function - * common code for finding aggregate support functions + * Subroutine of AggregateCreate for finding a support function, validating + * it, and performing an ACL check. Skip the trust check, because CREATE + * AGGREGATE won't execute the function. * * fnName: possibly-schema-qualified function name * nargs, input_types: expected function argument types diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index b8445dc..9d89946 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -311,8 +311,6 @@ do_analyze_rel(Relation onerel, int options, VacuumParams *params, PGRUsage ru0; TimestampTz starttime = 0; MemoryContext caller_context; - Oid save_userid; - int save_sec_context; int save_nestlevel; if (inh) @@ -340,9 +338,7 @@ do_analyze_rel(Relation onerel, int options, VacuumParams *params, * as that user. 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); + PushTransientUser(onerel->rd_rel->relowner, true, false); save_nestlevel = NewGUCNestLevel(); /* measure elapsed time iff autovacuum logging requires it */ @@ -669,11 +665,9 @@ do_analyze_rel(Relation onerel, int options, VacuumParams *params, pg_rusage_show(&ru0)))); } - /* Roll back any GUC changes executed by index functions */ + /* Roll back GUC changes by index functions; revert user ID */ AtEOXact_GUC(false, save_nestlevel); - - /* Restore userid and security context */ - SetUserIdAndSecContext(save_userid, save_sec_context); + PopTransientUser(); /* Restore current context and release memory */ MemoryContextSwitchTo(caller_context); @@ -1993,7 +1987,10 @@ compute_distinct_stats(VacAttrStatsP stats, firstcount1 = track_cnt; for (j = 0; j < track_cnt; j++) { - /* We always use the default collation for statistics */ + /* + * We always use the default collation for statistics. This is + * always an operator class member, so no trust check. + */ if (DatumGetBool(FunctionCall2Coll(&f_cmpeq, DEFAULT_COLLATION_OID, value, track[j].value))) diff --git a/src/backend/commands/conversioncmds.c b/src/backend/commands/conversioncmds.c index e36fc23..a9e257b 100644 --- a/src/backend/commands/conversioncmds.c +++ b/src/backend/commands/conversioncmds.c @@ -96,7 +96,8 @@ CreateConversionCommand(CreateConversionStmt *stmt) * Check that the conversion function is suitable for the requested source * and target encodings. We do that by calling the function with an empty * string; the conversion function should throw an error if it can't - * perform the requested conversion. + * perform the requested conversion. Only superusers can define these + * functions, so skip the trust check. */ OidFunctionCall5(funcoid, Int32GetDatum(from_encoding), diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index d01b258..a5cea20 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -229,8 +229,6 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString, IntoClause *into = stmt->into; bool is_matview = (into->viewQuery != NULL); DestReceiver *dest; - Oid save_userid = InvalidOid; - int save_sec_context = 0; int save_nestlevel = 0; ObjectAddress address; List *rewritten; @@ -286,9 +284,7 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString, */ if (is_matview) { - GetUserIdAndSecContext(&save_userid, &save_sec_context); - SetUserIdAndSecContext(save_userid, - save_sec_context | SECURITY_RESTRICTED_OPERATION); + PushTransientUser(GetUserId(), true, false); save_nestlevel = NewGUCNestLevel(); } @@ -370,11 +366,9 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString, if (is_matview) { - /* Roll back any GUC changes */ + /* Roll back GUC changes by index functions; revert user ID */ AtEOXact_GUC(false, save_nestlevel); - - /* Restore userid and security context */ - SetUserIdAndSecContext(save_userid, save_sec_context); + PopTransientUser(); } return address; diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index ebece4d..8ecfa2d 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -2238,6 +2238,7 @@ ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver aclresult = pg_proc_aclcheck(fexpr->funcid, GetUserId(), ACL_EXECUTE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_PROCEDURE, get_func_name(fexpr->funcid)); + pg_proc_trustcheck(fexpr->funcid); /* Prep the context object we'll pass to the procedure */ callcontext = makeNode(CallContext); diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index a171eba..0c64603 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -64,8 +64,7 @@ static void transientrel_destroy(DestReceiver *self); static uint64 refresh_matview_datafill(DestReceiver *dest, Query *query, const char *queryString); static char *make_temptable_name_n(char *tempname, int n); -static void refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, - int save_sec_context); +static void refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner); static void refresh_by_heap_swap(Oid matviewOid, Oid OIDNewHeap, char relpersistence); static bool is_usable_unique_index(Relation indexRel); static void OpenMatViewIncrementalMaintenance(void); @@ -148,8 +147,6 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, bool concurrent; LOCKMODE lockmode; char relpersistence; - Oid save_userid; - int save_sec_context; int save_nestlevel; ObjectAddress address; @@ -273,9 +270,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, * Don't lock it down too tight to create a temporary table just yet. We * will switch modes when we are about to execute user code. */ - GetUserIdAndSecContext(&save_userid, &save_sec_context); - SetUserIdAndSecContext(relowner, - save_sec_context | SECURITY_LOCAL_USERID_CHANGE); + PushTransientUser(relowner, false, false); save_nestlevel = NewGUCNestLevel(); /* Concurrent refresh builds new data in temp tablespace, and does diff. */ @@ -303,8 +298,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, /* * Now lock down security-restricted operations. */ - SetUserIdAndSecContext(relowner, - save_sec_context | SECURITY_RESTRICTED_OPERATION); + PushTransientUser(relowner, true, false); /* Generate the data, if wanted. */ if (!stmt->skipData) @@ -317,8 +311,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, PG_TRY(); { - refresh_by_match_merge(matviewOid, OIDNewHeap, relowner, - save_sec_context); + refresh_by_match_merge(matviewOid, OIDNewHeap, relowner); } PG_CATCH(); { @@ -345,11 +338,10 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString, heap_close(matviewRel, NoLock); - /* Roll back any GUC changes */ + /* Roll back GUC changes by index functions; revert user ID */ AtEOXact_GUC(false, save_nestlevel); - - /* Restore userid and security context */ - SetUserIdAndSecContext(save_userid, save_sec_context); + PopTransientUser(); + PopTransientUser(); ObjectAddressSet(address, RelationRelationId, matviewOid); @@ -577,8 +569,7 @@ make_temptable_name_n(char *tempname, int n) * this command. */ static void -refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, - int save_sec_context) +refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner) { StringInfoData querybuf; Relation matviewRel; @@ -648,8 +639,7 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, SPI_getvalue(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1)))); } - SetUserIdAndSecContext(relowner, - save_sec_context | SECURITY_LOCAL_USERID_CHANGE); + PopTransientUser(); /* exit security-restricted operation */ /* Start building the query for creating the diff table. */ resetStringInfo(&querybuf); @@ -787,8 +777,8 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, if (SPI_exec(querybuf.data, 0) != SPI_OK_UTILITY) elog(ERROR, "SPI_exec failed: %s", querybuf.data); - SetUserIdAndSecContext(relowner, - save_sec_context | SECURITY_RESTRICTED_OPERATION); + /* reenter security-restricted operation */ + PushTransientUser(relowner, true, false); /* * We have no further use for data from the "full-data" temp table, but we diff --git a/src/backend/commands/schemacmds.c b/src/backend/commands/schemacmds.c index f0ebe2d1..e3f61db 100644 --- a/src/backend/commands/schemacmds.c +++ b/src/backend/commands/schemacmds.c @@ -56,13 +56,12 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString, OverrideSearchPath *overridePath; List *parsetree_list; ListCell *parsetree_item; + Oid issuer_uid; Oid owner_uid; - Oid saved_uid; - int save_sec_context; AclResult aclresult; ObjectAddress address; - GetUserIdAndSecContext(&saved_uid, &save_sec_context); + issuer_uid = GetUserId(); /* * Who is supposed to own the new schema? @@ -70,7 +69,7 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString, if (stmt->authrole) owner_uid = get_rolespec_oid(stmt->authrole, false); else - owner_uid = saved_uid; + owner_uid = issuer_uid; /* fill schema name with the user name if not specified */ if (!schemaName) @@ -92,12 +91,12 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString, * The latter provision guards against "giveaway" attacks. Note that a * superuser will always have both of these privileges a fortiori. */ - aclresult = pg_database_aclcheck(MyDatabaseId, saved_uid, ACL_CREATE); + aclresult = pg_database_aclcheck(MyDatabaseId, issuer_uid, ACL_CREATE); if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_DATABASE, get_database_name(MyDatabaseId)); - check_is_member_of_role(saved_uid, owner_uid); + check_is_member_of_role(issuer_uid, owner_uid); /* Additional check to protect reserved schema names */ if (!allowSystemTableMods && IsReservedName(schemaName)) @@ -131,9 +130,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString, * (The setting will be restored at the end of this routine, or in case of * error, transaction abort will clean things up.) */ - if (saved_uid != owner_uid) - SetUserIdAndSecContext(owner_uid, - save_sec_context | SECURITY_LOCAL_USERID_CHANGE); + if (issuer_uid != owner_uid) + PushTransientUser(owner_uid, false, false); /* Create the schema's namespace */ namespaceId = NamespaceCreate(schemaName, owner_uid, false); @@ -205,8 +203,8 @@ CreateSchemaCommand(CreateSchemaStmt *stmt, const char *queryString, /* Reset search path to normal state */ PopOverrideSearchPath(); - /* Reset current user and security context */ - SetUserIdAndSecContext(saved_uid, save_sec_context); + if (issuer_uid != owner_uid) + PopTransientUser(); return namespaceId; } diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 843ed48..453a6bb 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -11002,7 +11002,7 @@ AlterTableMoveAll(AlterTableMoveAllStmt *stmt) HeapTuple tuple; Oid orig_tablespaceoid; Oid new_tablespaceoid; - List *role_oids = roleSpecsToIds(stmt->roles); + List *role_oids = roleSpecsToIds(stmt->roles, false); /* Ensure we were not asked to move something we can't */ if (stmt->objtype != OBJECT_TABLE && stmt->objtype != OBJECT_INDEX && diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index bcdd86c..310194a 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -2379,10 +2379,13 @@ ExecCallTriggerFunc(TriggerData *trigdata, /* * We cache fmgr lookup info, to avoid making the lookup again on each - * call. + * call. This is also a convenient moment to check function trust. */ if (finfo->fn_oid == InvalidOid) + { + pg_proc_trustcheck(trigdata->tg_trigger->tgfoid); fmgr_info(trigdata->tg_trigger->tgfoid, finfo); + } Assert(finfo->fn_oid == trigdata->tg_trigger->tgfoid); diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 12afa18..564a1d6 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -22,6 +22,7 @@ #include "catalog/indexing.h" #include "catalog/objectaccess.h" #include "catalog/pg_auth_members.h" +#include "catalog/pg_auth_trust.h" #include "catalog/pg_authid.h" #include "catalog/pg_database.h" #include "catalog/pg_db_role_setting.h" @@ -55,6 +56,12 @@ static void AddRoleMems(const char *rolename, Oid roleid, static void DelRoleMems(const char *rolename, Oid roleid, List *memberSpecs, List *memberIds, bool admin_opt); +static void StoreRoleTrust(const RoleSpec *role, Oid roleid, + List *addSpecs, List *delSpecs); +static void AddRoleTrust(const RoleSpec *role, Oid roleid, + List *memberNames, List *memberIds); +static void DelRoleTrust(const RoleSpec *role, Oid roleid, + List *memberNames, List *memberIds); /* Check if current user has createrole privileges */ @@ -77,6 +84,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) Datum new_record[Natts_pg_authid]; bool new_record_nulls[Natts_pg_authid]; Oid roleid; + RoleSpec *spec; ListCell *item; ListCell *option; char *password = NULL; /* user password */ @@ -94,6 +102,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) char *validUntil = NULL; /* time the login is valid until */ Datum validUntil_datum; /* same, as timestamptz Datum */ bool validUntil_null; + List *trust = NIL; + List *notrust = NIL; DefElem *dpassword = NULL; DefElem *dissuper = NULL; DefElem *dinherit = NULL; @@ -106,6 +116,8 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) DefElem *drolemembers = NULL; DefElem *dadminmembers = NULL; DefElem *dvalidUntil = NULL; + DefElem *dtrust = NULL; + DefElem *dnotrust = NULL; DefElem *dbypassRLS = NULL; /* The defaults can vary depending on the original statement type */ @@ -239,6 +251,26 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) parser_errposition(pstate, defel->location))); dvalidUntil = defel; } + else if (strcmp(defel->defname, "trust") == 0) + { + if (dtrust) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting or redundant options"))); + dtrust = defel; + } + else if (strcmp(defel->defname, "notrust") == 0) + { + /* + * XXX This will only ever issue warnings about the absence of + * entries to remove; should we forbid it entirely? + */ + if (dnotrust) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting or redundant options"))); + dnotrust = defel; + } else if (strcmp(defel->defname, "bypassrls") == 0) { if (dbypassRLS) @@ -283,6 +315,10 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) adminmembers = (List *) dadminmembers->arg; if (dvalidUntil) validUntil = strVal(dvalidUntil->arg); + if (dtrust) + trust = (List *) dtrust->arg; + if (dnotrust) + notrust = (List *) dnotrust->arg; if (dbypassRLS) bypassrls = intVal(dbypassRLS->arg) != 0; @@ -452,13 +488,17 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) */ CatalogTupleInsert(pg_authid_rel, tuple); - /* - * Advance command counter so we can see new record; else tests in - * AddRoleMems may fail. - */ - if (addroleto || adminmembers || rolemembers) + /* Next steps may examine the pg_authid row. */ + if (trust || notrust || addroleto || adminmembers || rolemembers) CommandCounterIncrement(); + /* pg_auth_trust */ + spec = makeNode(RoleSpec); + spec->roletype = ROLESPEC_CSTRING; + spec->rolename = stmt->role; + spec->location = -1; + StoreRoleTrust(spec, roleid, trust, notrust); + /* * Add the new role to the specified existing roles. */ @@ -483,10 +523,10 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) * option, rolemembers don't. */ AddRoleMems(stmt->role, roleid, - adminmembers, roleSpecsToIds(adminmembers), + adminmembers, roleSpecsToIds(adminmembers, false), GetUserId(), true); AddRoleMems(stmt->role, roleid, - rolemembers, roleSpecsToIds(rolemembers), + rolemembers, roleSpecsToIds(rolemembers, false), GetUserId(), false); /* Post creation hook for new role */ @@ -533,6 +573,8 @@ AlterRole(AlterRoleStmt *stmt) char *validUntil = NULL; /* time the login is valid until */ Datum validUntil_datum; /* same, as timestamptz Datum */ bool validUntil_null; + List *trust = NIL; + List *notrust = NIL; int bypassrls = -1; DefElem *dpassword = NULL; DefElem *dissuper = NULL; @@ -544,6 +586,8 @@ AlterRole(AlterRoleStmt *stmt) DefElem *dconnlimit = NULL; DefElem *drolemembers = NULL; DefElem *dvalidUntil = NULL; + DefElem *dtrust = NULL; + DefElem *dnotrust = NULL; DefElem *dbypassRLS = NULL; Oid roleid; @@ -636,6 +680,22 @@ AlterRole(AlterRoleStmt *stmt) errmsg("conflicting or redundant options"))); dvalidUntil = defel; } + else if (strcmp(defel->defname, "trust") == 0) + { + if (dtrust) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting or redundant options"))); + dtrust = defel; + } + else if (strcmp(defel->defname, "notrust") == 0) + { + if (dnotrust) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting or redundant options"))); + dnotrust = defel; + } else if (strcmp(defel->defname, "bypassrls") == 0) { if (dbypassRLS) @@ -647,6 +707,13 @@ AlterRole(AlterRoleStmt *stmt) else elog(ERROR, "option \"%s\" not recognized", defel->defname); + + if (stmt->role->roletype == ROLESPEC_PUBLIC && + strcmp(defel->defname, "trust") != 0 && + strcmp(defel->defname, "notrust") != 0) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("only [NO] TRUST allowed for PUBLIC"))); } if (dpassword && dpassword->arg) @@ -675,10 +742,24 @@ AlterRole(AlterRoleStmt *stmt) rolemembers = (List *) drolemembers->arg; if (dvalidUntil) validUntil = strVal(dvalidUntil->arg); + if (dtrust) + trust = (List *) dtrust->arg; + if (dnotrust) + notrust = (List *) dnotrust->arg; if (dbypassRLS) bypassrls = intVal(dbypassRLS->arg); /* + * If changing PUBLIC, modify pg_auth_trust and we're done. Since there's + * no pg_authid row, don't fire OAT_POST_ALTER. + */ + if (stmt->role->roletype == ROLESPEC_PUBLIC) + { + StoreRoleTrust(stmt->role, InvalidOid, trust, notrust); + return 0; + } + + /* * Scan the pg_authid relation to be certain the user exists. */ pg_authid_rel = heap_open(AuthIdRelationId, RowExclusiveLock); @@ -690,8 +771,15 @@ AlterRole(AlterRoleStmt *stmt) roleid = authform->oid; /* - * To mess with a superuser you gotta be superuser; else you need - * createrole, or just want to change your own password + * Lock the role; among other things, this prevents a concurrent session + * from dropping it while we add then-orphaned pg_auth_trust roles. + */ + shdepLockAndCheckObject(AuthIdRelationId, roleid); + + /* + * To mess with a superuser or replication user, you gotta be superuser. + * StoreRoleTrust() applies its own permissions checks. Any role may + * change its own password. For other settings, you need createrole. */ if (authform->rolsuper || issuper >= 0) { @@ -724,8 +812,8 @@ AlterRole(AlterRoleStmt *stmt) !dconnlimit && !rolemembers && !validUntil && - dpassword && - roleid == GetUserId())) + ((dpassword && roleid == GetUserId()) || + trust || notrust))) ereport(ERROR, (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), errmsg("permission denied"))); @@ -866,16 +954,19 @@ AlterRole(AlterRoleStmt *stmt) * Advance command counter so we can see new record; else tests in * AddRoleMems may fail. */ - if (rolemembers) + if (trust || notrust || rolemembers) CommandCounterIncrement(); + /* pg_auth_trust */ + StoreRoleTrust(stmt->role, roleid, trust, notrust); + if (stmt->action == +1) /* add members to role */ AddRoleMems(rolename, roleid, - rolemembers, roleSpecsToIds(rolemembers), + rolemembers, roleSpecsToIds(rolemembers, false), GetUserId(), false); else if (stmt->action == -1) /* drop members from role */ DelRoleMems(rolename, roleid, - rolemembers, roleSpecsToIds(rolemembers), + rolemembers, roleSpecsToIds(rolemembers, false), false); /* @@ -971,11 +1062,30 @@ AlterRoleSet(AlterRoleSetStmt *stmt) /* * DROP ROLE */ +static void +DeleteRoleCatalogRows(Relation rel, Oid index, AttrNumber att, Oid key) +{ + ScanKeyData scankey; + SysScanDesc sscan; + HeapTuple tuple; + + ScanKeyInit(&scankey, att, BTEqualStrategyNumber, F_OIDEQ, + ObjectIdGetDatum(key)); + + sscan = systable_beginscan(rel, index, true, NULL, 1, &scankey); + + while (HeapTupleIsValid(tuple = systable_getnext(sscan))) + CatalogTupleDelete(rel, &tuple->t_self); + + systable_endscan(sscan); +} + void DropRole(DropRoleStmt *stmt) { Relation pg_authid_rel, - pg_auth_members_rel; + pg_auth_members_rel, + pg_auth_trust_rel; ListCell *item; if (!have_createrole_privilege()) @@ -989,18 +1099,16 @@ DropRole(DropRoleStmt *stmt) */ pg_authid_rel = heap_open(AuthIdRelationId, RowExclusiveLock); pg_auth_members_rel = heap_open(AuthMemRelationId, RowExclusiveLock); + pg_auth_trust_rel = heap_open(AuthTrustRelationId, RowExclusiveLock); foreach(item, stmt->roles) { RoleSpec *rolspec = lfirst(item); char *role; - HeapTuple tuple, - tmp_tuple; + HeapTuple tuple; Form_pg_authid roleform; - ScanKeyData scankey; char *detail; char *detail_log; - SysScanDesc sscan; Oid roleid; if (rolspec->roletype != ROLESPEC_CSTRING) @@ -1086,35 +1194,26 @@ DropRole(DropRoleStmt *stmt) * * XXX what about grantor entries? Maybe we should do one heap scan. */ - ScanKeyInit(&scankey, - Anum_pg_auth_members_roleid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(roleid)); - - sscan = systable_beginscan(pg_auth_members_rel, AuthMemRoleMemIndexId, - true, NULL, 1, &scankey); - - while (HeapTupleIsValid(tmp_tuple = systable_getnext(sscan))) - { - CatalogTupleDelete(pg_auth_members_rel, &tmp_tuple->t_self); - } - - systable_endscan(sscan); + DeleteRoleCatalogRows(pg_auth_members_rel, + AuthMemRoleMemIndexId, + Anum_pg_auth_members_roleid, + roleid); + DeleteRoleCatalogRows(pg_auth_members_rel, + AuthMemMemRoleIndexId, + Anum_pg_auth_members_member, + roleid); - ScanKeyInit(&scankey, - Anum_pg_auth_members_member, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(roleid)); - - sscan = systable_beginscan(pg_auth_members_rel, AuthMemMemRoleIndexId, - true, NULL, 1, &scankey); - - while (HeapTupleIsValid(tmp_tuple = systable_getnext(sscan))) - { - CatalogTupleDelete(pg_auth_members_rel, &tmp_tuple->t_self); - } - - systable_endscan(sscan); + /* + * Remove role from pg_auth_trust table (as grantor and trustee). + */ + DeleteRoleCatalogRows(pg_auth_trust_rel, + AuthTrustGrantorTrusteeIndexId, + Anum_pg_auth_trust_grantor, + roleid); + DeleteRoleCatalogRows(pg_auth_trust_rel, + AuthTrustTrusteeGrantorIndexId, + Anum_pg_auth_trust_trustee, + roleid); /* * Remove any comments or security labels on this role. @@ -1142,6 +1241,7 @@ DropRole(DropRoleStmt *stmt) /* * Now we can clean up; but keep locks until commit. */ + heap_close(pg_auth_trust_rel, NoLock); heap_close(pg_auth_members_rel, NoLock); heap_close(pg_authid_rel, NoLock); } @@ -1293,7 +1393,7 @@ GrantRole(GrantRoleStmt *stmt) else grantor = GetUserId(); - grantee_ids = roleSpecsToIds(stmt->grantee_roles); + grantee_ids = roleSpecsToIds(stmt->grantee_roles, false); /* AccessShareLock is enough since we aren't modifying pg_authid */ pg_authid_rel = heap_open(AuthIdRelationId, AccessShareLock); @@ -1342,7 +1442,7 @@ GrantRole(GrantRoleStmt *stmt) void DropOwnedObjects(DropOwnedStmt *stmt) { - List *role_ids = roleSpecsToIds(stmt->roles); + List *role_ids = roleSpecsToIds(stmt->roles, false); ListCell *cell; /* Check privileges */ @@ -1368,7 +1468,7 @@ DropOwnedObjects(DropOwnedStmt *stmt) void ReassignOwnedObjects(ReassignOwnedStmt *stmt) { - List *role_ids = roleSpecsToIds(stmt->roles); + List *role_ids = roleSpecsToIds(stmt->roles, false); ListCell *cell; Oid newrole; @@ -1399,11 +1499,11 @@ ReassignOwnedObjects(ReassignOwnedStmt *stmt) * roleSpecsToIds * * Given a list of RoleSpecs, generate a list of role OIDs in the same order. - * - * ROLESPEC_PUBLIC is not allowed. + * If public_ok, ROLESPEC_PUBLIC translates to InvalidOid; otherwise, the + * presence of ROLESPEC_PUBLIC is an error. */ List * -roleSpecsToIds(List *memberNames) +roleSpecsToIds(List *memberNames, bool public_ok) { List *result = NIL; ListCell *l; @@ -1413,7 +1513,10 @@ roleSpecsToIds(List *memberNames) RoleSpec *rolespec = lfirst_node(RoleSpec, l); Oid roleid; - roleid = get_rolespec_oid(rolespec, false); + if (public_ok && rolespec->roletype == ROLESPEC_PUBLIC) + roleid = InvalidOid; + else + roleid = get_rolespec_oid(rolespec, false); result = lappend_oid(result, roleid); } return result; @@ -1673,3 +1776,181 @@ DelRoleMems(const char *rolename, Oid roleid, */ heap_close(pg_authmem_rel, NoLock); } + + +/* + * StoreRoleTrust -- apply [NO] TRUST clauses + * + * role: RoleSpec of role to add to (used only for error messages) + * roleid: OID of role to add to (InvalidOid means PUBLIC) + * addSpecs: list of RoleSpec of roles to add + * delSpecs: list of RoleSpec of roles to remove + */ +static void +StoreRoleTrust(const RoleSpec *role, Oid roleid, + List *addSpecs, List *delSpecs) +{ + List *addIds, + *delIds; + + if (!addSpecs && !delSpecs) + return; /* nothing to do */ + + /* Reject request to both add and remove the same role. */ + addIds = roleSpecsToIds(addSpecs, true); + delIds = roleSpecsToIds(delSpecs, true); + if (list_intersection_oid(addIds, delIds) != NIL) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting or redundant options"))); + + /* + * Check permissions: must have createrole or admin option on the role to + * be changed. Must be superuser if the target is PUBLIC or a superuser. + */ + if (superuser_arg(roleid)) + { + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to alter superusers"))); + } + else if (role->roletype == ROLESPEC_PUBLIC) + { + Assert(roleid == InvalidOid); + if (!superuser()) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must be superuser to alter trust of PUBLIC"))); + } + else + { + if (!have_createrole_privilege() && + !is_admin_of_role(GetUserId(), roleid)) + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("must have admin option on role \"%s\"", + get_rolespec_name(role)))); + } + + if (addSpecs) + AddRoleTrust(role, roleid, addSpecs, addIds); + if (delSpecs) + DelRoleTrust(role, roleid, delSpecs, delIds); +} + +/* + * AddRoleTrust -- Record a role as trusting some other roles + * + * role: RoleSpec of role to add to (used only for error messages) + * roleid: OID of role to add to (InvalidOid means PUBLIC) + * memberSpecs: list of RoleSpec of roles to add (used only for error messages) + * memberIds: OIDs of roles to add (InvalidOid means PUBLIC) + */ +static void +AddRoleTrust(const RoleSpec *role, Oid roleid, + List *memberSpecs, List *memberIds) +{ + Relation pg_auth_trust_rel; + TupleDesc pg_auth_trust_dsc; + ListCell *specitem; + ListCell *iditem; + + Assert(list_length(memberSpecs) == list_length(memberIds)); + + pg_auth_trust_rel = heap_open(AuthTrustRelationId, RowExclusiveLock); + pg_auth_trust_dsc = RelationGetDescr(pg_auth_trust_rel); + + forboth(specitem, memberSpecs, iditem, memberIds) + { + RoleSpec *memberRole = lfirst(specitem); + Oid memberid = lfirst_oid(iditem); + HeapTuple tuple; + Datum new_record[Natts_pg_auth_trust]; + bool new_record_nulls[Natts_pg_auth_trust]; + + /* Unlike role membership, loops in role trust are fine. */ + + /* + * Lock prevents a dangling pg_auth_trust row when someone is dropping + * the target member concurrently. + */ + if (OidIsValid(memberid)) + shdepLockAndCheckObject(AuthIdRelationId, memberid); + + /* Warn if entry for this role/member already exists. */ + if (SearchSysCacheExists2(AUTHTRUSTGRANTORTRUSTEE, + ObjectIdGetDatum(roleid), + ObjectIdGetDatum(memberid))) + { + ereport(NOTICE, + (errmsg("role \"%s\" already trusts role \"%s\"", + get_rolespec_name(role), + get_rolespec_name(memberRole)))); + continue; + } + + /* Insert a tuple */ + new_record[Anum_pg_auth_trust_grantor - 1] = ObjectIdGetDatum(roleid); + new_record[Anum_pg_auth_trust_trustee - 1] = ObjectIdGetDatum(memberid); + MemSet(new_record_nulls, false, sizeof(new_record_nulls)); + + tuple = heap_form_tuple(pg_auth_trust_dsc, + new_record, new_record_nulls); + CatalogTupleInsert(pg_auth_trust_rel, tuple); + + /* CCI after each change, in case there are duplicates in list */ + CommandCounterIncrement(); + } + + heap_close(pg_auth_trust_rel, NoLock); +} + +/* + * DelRoleTrust -- Remove members from a role's trusted list + * + * role: RoleSpec of role to del from (used only for error messages) + * roleid: OID of role to del from + * memberSpecs: list of RoleSpec of roles to del (used only for error messages) + * memberIds: OIDs of roles to del + */ +static void +DelRoleTrust(const RoleSpec *role, Oid roleid, + List *memberSpecs, List *memberIds) +{ + Relation pg_auth_trust_rel; + ListCell *specitem; + ListCell *iditem; + + Assert(list_length(memberSpecs) == list_length(memberIds)); + + pg_auth_trust_rel = heap_open(AuthTrustRelationId, RowExclusiveLock); + + forboth(specitem, memberSpecs, iditem, memberIds) + { + RoleSpec *memberRole = lfirst(specitem); + Oid memberid = lfirst_oid(iditem); + HeapTuple tuple; + + /* Warn if no corresponding entry exists. */ + tuple = SearchSysCache2(AUTHTRUSTGRANTORTRUSTEE, + ObjectIdGetDatum(roleid), + ObjectIdGetDatum(memberid)); + if (!HeapTupleIsValid(tuple)) + { + ereport(WARNING, + (errmsg("role \"%s\" does not trust role \"%s\"", + get_rolespec_name(role), + get_rolespec_name(memberRole)))); + continue; + } + + CatalogTupleDelete(pg_auth_trust_rel, &tuple->t_self); + ReleaseSysCache(tuple); + + /* CCI after each change, in case there are duplicates in list */ + CommandCounterIncrement(); + } + + heap_close(pg_auth_trust_rel, NoLock); +} diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 25b3b03..0fb9019 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -1527,8 +1527,6 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params) Relation onerel; LockRelId onerelid; Oid toast_relid; - Oid save_userid; - int save_sec_context; int save_nestlevel; Assert(params != NULL); @@ -1688,9 +1686,7 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params) * arrange to make GUC variable changes local to this command. (This is * unnecessary, but harmless, for lazy VACUUM.) */ - GetUserIdAndSecContext(&save_userid, &save_sec_context); - SetUserIdAndSecContext(onerel->rd_rel->relowner, - save_sec_context | SECURITY_RESTRICTED_OPERATION); + PushTransientUser(onerel->rd_rel->relowner, true, false); save_nestlevel = NewGUCNestLevel(); /* @@ -1713,11 +1709,9 @@ vacuum_rel(Oid relid, RangeVar *relation, int options, VacuumParams *params) else lazy_vacuum_rel(onerel, options, params, vac_strategy); - /* Roll back any GUC changes executed by index functions */ + /* Roll back GUC changes by index functions; revert user ID */ AtEOXact_GUC(false, save_nestlevel); - - /* Restore userid and security context */ - SetUserIdAndSecContext(save_userid, save_sec_context); + PopTransientUser(); /* all done with this class, but hold lock until commit */ if (onerel) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index d9087ca..1a242e5 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -891,6 +891,7 @@ ExecInitExprRec(Expr *node, ExprState *state, { OpExpr *op = (OpExpr *) node; + pg_oper_trustcheck_extended(op->opno, false); ExecInitFunc(&scratch, node, op->args, op->opfuncid, op->inputcollid, state); @@ -902,6 +903,7 @@ ExecInitExprRec(Expr *node, ExprState *state, { DistinctExpr *op = (DistinctExpr *) node; + pg_oper_trustcheck_extended(op->opno, false); ExecInitFunc(&scratch, node, op->args, op->opfuncid, op->inputcollid, state); @@ -924,6 +926,7 @@ ExecInitExprRec(Expr *node, ExprState *state, { NullIfExpr *op = (NullIfExpr *) node; + pg_oper_trustcheck_extended(op->opno, false); ExecInitFunc(&scratch, node, op->args, op->opfuncid, op->inputcollid, state); @@ -949,19 +952,14 @@ ExecInitExprRec(Expr *node, ExprState *state, Expr *arrayarg; FmgrInfo *finfo; FunctionCallInfo fcinfo; - AclResult aclresult; Assert(list_length(opexpr->args) == 2); scalararg = (Expr *) linitial(opexpr->args); arrayarg = (Expr *) lsecond(opexpr->args); /* Check permission to call function */ - aclresult = pg_proc_aclcheck(opexpr->opfuncid, - GetUserId(), - ACL_EXECUTE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_FUNCTION, - get_func_name(opexpr->opfuncid)); + pg_oper_trustcheck_extended(opexpr->opno, false); + pg_proc_execcheck(opexpr->opfuncid); InvokeFunctionExecuteHook(opexpr->opfuncid); /* Set up the primary fmgr lookup information */ @@ -2159,16 +2157,13 @@ ExecInitFunc(ExprEvalStep *scratch, Expr *node, List *args, Oid funcid, Oid inputcollid, ExprState *state) { int nargs = list_length(args); - AclResult aclresult; FmgrInfo *flinfo; FunctionCallInfo fcinfo; int argno; ListCell *lc; /* Check permission to call function */ - aclresult = pg_proc_aclcheck(funcid, GetUserId(), ACL_EXECUTE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(funcid)); + pg_proc_execcheck(funcid); InvokeFunctionExecuteHook(funcid); /* @@ -3378,13 +3373,9 @@ ExecBuildGroupingEqual(TupleDesc ldesc, TupleDesc rdesc, Oid foid = eqfunctions[natt]; FmgrInfo *finfo; FunctionCallInfo fcinfo; - AclResult aclresult; /* Check permission to call function */ - aclresult = pg_proc_aclcheck(foid, GetUserId(), ACL_EXECUTE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(foid)); - + pg_proc_execcheck(foid); InvokeFunctionExecuteHook(foid); /* Set up the primary fmgr lookup information */ diff --git a/src/backend/executor/execSRF.c b/src/backend/executor/execSRF.c index bf73f05..0ff5293 100644 --- a/src/backend/executor/execSRF.c +++ b/src/backend/executor/execSRF.c @@ -677,12 +677,8 @@ init_sexpr(Oid foid, Oid input_collation, Expr *node, SetExprState *sexpr, PlanState *parent, MemoryContext sexprCxt, bool allowSRF, bool needDescForSRF) { - AclResult aclresult; - /* Check permission to call function */ - aclresult = pg_proc_aclcheck(foid, GetUserId(), ACL_EXECUTE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(foid)); + pg_proc_execcheck(foid); InvokeFunctionExecuteHook(foid); /* diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index daf56cd..dc5bb48 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -2569,6 +2569,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_AGGREGATE, get_func_name(aggref->aggfnoid)); + pg_proc_trustcheck(aggref->aggfnoid); InvokeFunctionExecuteHook(aggref->aggfnoid); /* planner recorded transition state type in the Aggref itself */ @@ -2641,7 +2642,10 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) } } - /* Check that aggregate owner has permission to call component fns */ + /* + * Check that aggregate owner has permission to call component + * functions and that the necessary trust relationships exist. + */ { HeapTuple procTuple; Oid aggOwner; @@ -2659,6 +2663,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(transfn_oid)); + pg_proc_trustcheck(transfn_oid); InvokeFunctionExecuteHook(transfn_oid); if (OidIsValid(finalfn_oid)) { @@ -2667,6 +2672,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(finalfn_oid)); + pg_proc_trustcheck(finalfn_oid); InvokeFunctionExecuteHook(finalfn_oid); } if (OidIsValid(serialfn_oid)) @@ -2676,6 +2682,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(serialfn_oid)); + pg_proc_trustcheck(serialfn_oid); InvokeFunctionExecuteHook(serialfn_oid); } if (OidIsValid(deserialfn_oid)) @@ -2685,6 +2692,7 @@ ExecInitAgg(Agg *node, EState *estate, int eflags) if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(deserialfn_oid)); + pg_proc_trustcheck(deserialfn_oid); InvokeFunctionExecuteHook(deserialfn_oid); } } diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 298e370..1537ac6 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -2404,7 +2404,6 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) WindowFuncExprState *wfuncstate = (WindowFuncExprState *) lfirst(l); WindowFunc *wfunc = wfuncstate->wfunc; WindowStatePerFunc perfuncstate; - AclResult aclresult; int i; if (wfunc->winref != node->winref) /* planner screwed up? */ @@ -2432,11 +2431,7 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) wfuncstate->wfuncno = wfuncno; /* Check permission to call window function */ - aclresult = pg_proc_aclcheck(wfunc->winfnoid, GetUserId(), - ACL_EXECUTE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_FUNCTION, - get_func_name(wfunc->winfnoid)); + pg_proc_execcheck(wfunc->winfnoid); InvokeFunctionExecuteHook(wfunc->winfnoid); /* Fill in the perfuncstate data */ @@ -2715,6 +2710,7 @@ initialize_peragg(WindowAggState *winstate, WindowFunc *wfunc, if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(transfn_oid)); + pg_proc_trustcheck(transfn_oid); InvokeFunctionExecuteHook(transfn_oid); if (OidIsValid(invtransfn_oid)) @@ -2724,6 +2720,7 @@ initialize_peragg(WindowAggState *winstate, WindowFunc *wfunc, if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(invtransfn_oid)); + pg_proc_trustcheck(invtransfn_oid); InvokeFunctionExecuteHook(invtransfn_oid); } @@ -2734,6 +2731,7 @@ initialize_peragg(WindowAggState *winstate, WindowFunc *wfunc, if (aclresult != ACLCHECK_OK) aclcheck_error(aclresult, OBJECT_FUNCTION, get_func_name(finalfn_oid)); + pg_proc_trustcheck(finalfn_oid); InvokeFunctionExecuteHook(finalfn_oid); } } diff --git a/src/backend/nodes/list.c b/src/backend/nodes/list.c index 55fd4c3..61ad7e6 100644 --- a/src/backend/nodes/list.c +++ b/src/backend/nodes/list.c @@ -846,6 +846,32 @@ list_intersection_int(const List *list1, const List *list2) } /* + * This variant of list_intersection() operates upon lists of OIDs. + */ +List * +list_intersection_oid(const List *list1, const List *list2) +{ + List *result; + const ListCell *cell; + + if (list1 == NIL || list2 == NIL) + return NIL; + + Assert(IsOidList(list1)); + Assert(IsOidList(list2)); + + result = NIL; + foreach(cell, list1) + { + if (list_member_oid(list2, lfirst_oid(cell))) + result = lappend_oid(result, lfirst_oid(cell)); + } + + check_list_invariants(result); + return result; +} + +/* * Return a list that contains all the cells in list1 that are not in * list2. The returned list is freshly allocated via palloc(), but the * cells themselves point to the same objects as the cells of the diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index 8df3693..b18cc39 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -2767,7 +2767,12 @@ eval_const_expressions_mutator(Node *node, true, true, context); - if (simple) /* successfully simplified it */ + + /* + * If we don't trust the operator owner, forget the + * simplification and expect an error later. + */ + if (simple && pg_oper_trustcheck_extended(expr->opno, true)) return (Node *) simple; /* @@ -4164,6 +4169,18 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod, *args_p = args; } + /* + * Don't try to simplify an untrusted function; evaluate_function() would + * throw an error, and we'd rather fail later. inline_function() must not + * be attempted, because we could no longer check trust. XXX Should we + * likewise preview the ACL check? + */ + if (!pg_proc_trustcheck_extended(funcid, true)) + { + ReleaseSysCache(func_tuple); + return NULL; + } + /* Now attempt simplification of the function call proper. */ newexpr = evaluate_function(funcid, result_type, result_typmod, @@ -4191,6 +4208,12 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod, fexpr.args = args; fexpr.location = -1; + /* + * Only superusers can associate a transform function, so skip + * security checks. If the original function has security relevance, + * its transform function must either perform required checks or + * return a node tree that still elicits the right runtime checks. + */ newexpr = (Expr *) DatumGetPointer(OidFunctionCall1(func_form->protransform, PointerGetDatum(&fexpr))); @@ -4607,6 +4630,7 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid, /* Check permission to call function (fail later, if not) */ if (pg_proc_aclcheck(funcid, GetUserId(), ACL_EXECUTE) != ACLCHECK_OK) return NULL; + /* simplify_function() already checked trust. */ /* Check whether a plugin wants to hook function entry/exit */ if (FmgrHookIsNeeded(funcid)) @@ -5104,7 +5128,8 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte) return NULL; /* Check permission to call function (fail later, if not) */ - if (pg_proc_aclcheck(func_oid, GetUserId(), ACL_EXECUTE) != ACLCHECK_OK) + if (pg_proc_aclcheck(func_oid, GetUserId(), ACL_EXECUTE) != ACLCHECK_OK || + !pg_proc_trustcheck_extended(func_oid, true)) return NULL; /* Check whether a plugin wants to hook function entry/exit */ diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 2c2208f..23e3290 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -682,7 +682,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); TABLE TABLES TABLESAMPLE TABLESPACE TEMP TEMPLATE TEMPORARY TEXT_P THEN TIES TIME TIMESTAMP TO TRAILING TRANSACTION TRANSFORM TREAT TRIGGER TRIM TRUE_P - TRUNCATE TRUSTED TYPE_P TYPES_P + TRUNCATE TRUST TRUSTED TYPE_P TYPES_P UNBOUNDED UNCOMMITTED UNENCRYPTED UNION UNIQUE UNKNOWN UNLISTEN UNLOGGED UNTIL UPDATE USER USING @@ -1038,6 +1038,14 @@ AlterOptRoleElem: { $$ = makeDefElem("validUntil", (Node *)makeString($3), @1); } + | TRUST role_list + { + $$ = makeDefElem("trust", (Node *)$2, @1); + } + | NO TRUST role_list + { + $$ = makeDefElem("notrust", (Node *)$3, @1); + } /* Supported but not documented for roles, for use by ALTER GROUP. */ | USER role_list { @@ -15225,6 +15233,7 @@ unreserved_keyword: | TRANSFORM | TRIGGER | TRUNCATE + | TRUST | TRUSTED | TYPE_P | TYPES_P diff --git a/src/backend/tcop/fastpath.c b/src/backend/tcop/fastpath.c index d16ba5e..4b0b96c 100644 --- a/src/backend/tcop/fastpath.c +++ b/src/backend/tcop/fastpath.c @@ -319,10 +319,12 @@ HandleFunctionRequest(StringInfo msgBuf) get_namespace_name(fip->namespace)); InvokeNamespaceSearchHook(fip->namespace, true); - aclresult = pg_proc_aclcheck(fid, GetUserId(), ACL_EXECUTE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_FUNCTION, - get_func_name(fid)); + /* + * A trust check is arguably superfluous,, seeing the user specified the + * function by OID. Doesn't seem worth a special case in the trust rules, + * though. + */ + pg_proc_execcheck(fid); InvokeFunctionExecuteHook(fid); /* diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.c index 30cf3d0..82bab7c 100644 --- a/src/backend/utils/adt/acl.c +++ b/src/backend/utils/adt/acl.c @@ -5310,6 +5310,10 @@ get_rolespec_name(const RoleSpec *role) Form_pg_authid authForm; char *rolename; + if (role->roletype == ROLESPEC_PUBLIC) + return pstrdup("public"); + Assert(role->roletype == ROLESPEC_CSTRING); + tp = get_rolespec_tuple(role); authForm = (Form_pg_authid) GETSTRUCT(tp); rolename = pstrdup(NameStr(authForm->rolname)); diff --git a/src/backend/utils/adt/rangetypes.c b/src/backend/utils/adt/rangetypes.c index 5a5d0a0..cc7c626 100644 --- a/src/backend/utils/adt/rangetypes.c +++ b/src/backend/utils/adt/rangetypes.c @@ -34,6 +34,7 @@ #include "lib/stringinfo.h" #include "libpq/pqformat.h" #include "miscadmin.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/date.h" #include "utils/int8.h" @@ -1799,8 +1800,11 @@ make_range(TypeCacheEntry *typcache, RangeBound *lower, RangeBound *upper, /* no need to call canonical on empty ranges ... */ if (OidIsValid(typcache->rng_canonical_finfo.fn_oid) && !RangeIsEmpty(range)) + { + pg_proc_trustcheck(typcache->rng_canonical_finfo.fn_oid); range = DatumGetRangeTypeP(FunctionCall1(&typcache->rng_canonical_finfo, RangeTypePGetDatum(range))); + } return range; } diff --git a/src/backend/utils/adt/rangetypes_gist.c b/src/backend/utils/adt/rangetypes_gist.c index 450479e..87248f4 100644 --- a/src/backend/utils/adt/rangetypes_gist.c +++ b/src/backend/utils/adt/rangetypes_gist.c @@ -16,6 +16,7 @@ #include "access/gist.h" #include "access/stratnum.h" +#include "utils/acl.h" #include "utils/float.h" #include "utils/fmgrprotos.h" #include "utils/datum.h" @@ -1522,6 +1523,7 @@ call_subtype_diff(TypeCacheEntry *typcache, Datum val1, Datum val2) { float8 value; + pg_proc_trustcheck(typcache->rng_subdiff_finfo.fn_oid); value = DatumGetFloat8(FunctionCall2Coll(&typcache->rng_subdiff_finfo, typcache->rng_collation, val1, val2)); diff --git a/src/backend/utils/adt/rangetypes_selfuncs.c b/src/backend/utils/adt/rangetypes_selfuncs.c index 32c4933..7337a50 100644 --- a/src/backend/utils/adt/rangetypes_selfuncs.c +++ b/src/backend/utils/adt/rangetypes_selfuncs.c @@ -23,6 +23,7 @@ #include "catalog/pg_operator.h" #include "catalog/pg_statistic.h" #include "catalog/pg_type.h" +#include "utils/acl.h" #include "utils/float.h" #include "utils/fmgrprotos.h" #include "utils/lsyscache.h" @@ -695,6 +696,8 @@ get_position(TypeCacheEntry *typcache, RangeBound *value, RangeBound *hist1, if (!has_subdiff) return 0.5; + pg_proc_trustcheck(typcache->rng_subdiff_finfo.fn_oid); + /* Calculate relative position using subdiff function. */ bin_width = DatumGetFloat8(FunctionCall2Coll( &typcache->rng_subdiff_finfo, @@ -806,11 +809,14 @@ get_distance(TypeCacheEntry *typcache, RangeBound *bound1, RangeBound *bound2) * value of 1.0 if no subdiff is available. */ if (has_subdiff) + { + pg_proc_trustcheck(typcache->rng_subdiff_finfo.fn_oid); return DatumGetFloat8(FunctionCall2Coll(&typcache->rng_subdiff_finfo, typcache->rng_collation, bound2->val, bound1->val)); + } else return 1.0; } diff --git a/src/backend/utils/adt/rangetypes_typanalyze.c b/src/backend/utils/adt/rangetypes_typanalyze.c index 9c50e4c..b344825 100644 --- a/src/backend/utils/adt/rangetypes_typanalyze.c +++ b/src/backend/utils/adt/rangetypes_typanalyze.c @@ -26,6 +26,7 @@ #include "catalog/pg_operator.h" #include "commands/vacuum.h" +#include "utils/acl.h" #include "utils/float.h" #include "utils/fmgrprotos.h" #include "utils/lsyscache.h" @@ -165,6 +166,7 @@ compute_range_stats(VacAttrStats *stats, AnalyzeAttrFetchFunc fetchfunc, * For an ordinary range, use subdiff function between upper * and lower bound values. */ + pg_proc_trustcheck(typcache->rng_subdiff_finfo.fn_oid); length = DatumGetFloat8(FunctionCall2Coll( &typcache->rng_subdiff_finfo, typcache->rng_collation, diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index cdda860..f43e5a9 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -2385,8 +2385,6 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes, { SPIPlanPtr qplan; Relation query_rel; - Oid save_userid; - int save_sec_context; /* * Use the query type code to determine whether the query is run against @@ -2398,10 +2396,7 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes, query_rel = fk_rel; /* Switch to proper UID to perform check as */ - GetUserIdAndSecContext(&save_userid, &save_sec_context); - SetUserIdAndSecContext(RelationGetForm(query_rel)->relowner, - save_sec_context | SECURITY_LOCAL_USERID_CHANGE | - SECURITY_NOFORCE_RLS); + PushTransientUser(RelationGetForm(query_rel)->relowner, false, true); /* Create the plan */ qplan = SPI_prepare(querystr, nargs, argtypes); @@ -2409,8 +2404,7 @@ ri_PlanCheck(const char *querystr, int nargs, Oid *argtypes, if (qplan == NULL) elog(ERROR, "SPI_prepare returned %s for %s", SPI_result_code_string(SPI_result), querystr); - /* Restore UID and security context */ - SetUserIdAndSecContext(save_userid, save_sec_context); + PopTransientUser(); /* Save the plan if requested */ if (cache_plan) @@ -2439,8 +2433,6 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo, Snapshot crosscheck_snapshot; int limit; int spi_result; - Oid save_userid; - int save_sec_context; Datum vals[RI_MAX_NUMKEYS * 2]; char nulls[RI_MAX_NUMKEYS * 2]; @@ -2519,10 +2511,7 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo, limit = (expect_OK == SPI_OK_SELECT) ? 1 : 0; /* Switch to proper UID to perform check as */ - GetUserIdAndSecContext(&save_userid, &save_sec_context); - SetUserIdAndSecContext(RelationGetForm(query_rel)->relowner, - save_sec_context | SECURITY_LOCAL_USERID_CHANGE | - SECURITY_NOFORCE_RLS); + PushTransientUser(RelationGetForm(query_rel)->relowner, false, true); /* Finally we can run the query. */ spi_result = SPI_execute_snapshot(qplan, @@ -2530,8 +2519,7 @@ ri_PerformCheck(const RI_ConstraintInfo *riinfo, test_snapshot, crosscheck_snapshot, false, false, limit); - /* Restore UID and security context */ - SetUserIdAndSecContext(save_userid, save_sec_context); + PopTransientUser(); /* Check result */ if (spi_result < 0) @@ -2964,6 +2952,12 @@ ri_AttributesEqual(Oid eq_opr, Oid typeid, /* Do we need to cast the values? */ if (OidIsValid(entry->cast_func_finfo.fn_oid)) { + /* + * Superusers govern operator classes, but creating a cast is not so + * restricted. Protect against trojan cast functions. + */ + pg_proc_trustcheck(entry->cast_func_finfo.fn_oid); + oldvalue = FunctionCall3(&entry->cast_func_finfo, oldvalue, Int32GetDatum(-1), /* typmod */ diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index ffca0fe..b5605c0 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -92,7 +92,14 @@ * should ignore the actual operator collation and use DEFAULT_COLLATION_OID. * We expect that the error induced by doing this is usually not large enough * to justify complicating matters. - *---------- + * + * Only superusers can define selectivity functions, so there's no need for a + * trust check before calling one. Since unprivileged users can attach core + * selectivity functions to any operator, selectivity functions that call the + * associated operator do perform a trust check on the operator's underlying + * function. They do not check trust on the operator itself; the executor + * will check, and any misguidance here would merely deceive the planner. + * ---------- */ #include "postgres.h" @@ -363,6 +370,7 @@ var_eq_const(VariableStatData *vardata, Oid operator, { FmgrInfo eqproc; + pg_proc_trustcheck(opfuncoid); fmgr_info(opfuncoid, &eqproc); for (i = 0; i < sslot.nvalues; i++) @@ -573,6 +581,7 @@ scalarineqsel(PlannerInfo *root, Oid operator, bool isgt, bool iseq, VariableStatData *vardata, Datum constval, Oid consttype) { Form_pg_statistic stats; + Oid opcode; FmgrInfo opproc; double mcv_selec, hist_selec, @@ -586,7 +595,9 @@ scalarineqsel(PlannerInfo *root, Oid operator, bool isgt, bool iseq, } stats = (Form_pg_statistic) GETSTRUCT(vardata->statsTuple); - fmgr_info(get_opcode(operator), &opproc); + opcode = get_opcode(operator); + pg_proc_trustcheck(opcode); + fmgr_info(opcode, &opproc); /* * If we have most-common-values info, add up the fractions of the MCV @@ -664,6 +675,7 @@ mcv_selectivity(VariableStatData *vardata, FmgrInfo *opproc, { for (i = 0; i < sslot.nvalues; i++) { + /* Caller is responsible for trust check. */ if (varonleft ? DatumGetBool(FunctionCall2Coll(opproc, DEFAULT_COLLATION_OID, @@ -742,6 +754,7 @@ histogram_selectivity(VariableStatData *vardata, FmgrInfo *opproc, for (i = n_skip; i < sslot.nvalues - n_skip; i++) { + /* Caller is responsible for trust check. */ if (varonleft ? DatumGetBool(FunctionCall2Coll(opproc, DEFAULT_COLLATION_OID, @@ -872,6 +885,7 @@ ineq_histogram_selectivity(PlannerInfo *root, NULL, &sslot.values[probe]); + /* Caller is responsible for trust check. */ ltcmp = DatumGetBool(FunctionCall2Coll(opproc, DEFAULT_COLLATION_OID, sslot.values[probe], @@ -1387,12 +1401,15 @@ patternsel(PG_FUNCTION_ARGS, Pattern_Type ptype, bool negate) */ Selectivity selec; int hist_size; + Oid opcode; FmgrInfo opproc; double mcv_selec, sumcommon; /* Try to use the histogram entries to get selectivity */ - fmgr_info(get_opcode(operator), &opproc); + opcode = get_opcode(operator); + pg_proc_trustcheck(opcode); + fmgr_info(opcode, &opproc); selec = histogram_selectivity(&vardata, &opproc, constval, true, 10, 1, &hist_size); @@ -2478,6 +2495,7 @@ eqjoinsel_inner(Oid opfuncoid, int i, nmatches; + pg_proc_trustcheck(opfuncoid); fmgr_info(opfuncoid, &eqproc); hasmatch1 = (bool *) palloc0(sslot1->nvalues * sizeof(bool)); hasmatch2 = (bool *) palloc0(sslot2->nvalues * sizeof(bool)); @@ -2691,6 +2709,7 @@ eqjoinsel_semi(Oid opfuncoid, */ clamped_nvalues2 = Min(sslot2->nvalues, nd2); + pg_proc_trustcheck(opfuncoid); fmgr_info(opfuncoid, &eqproc); hasmatch1 = (bool *) palloc0(sslot1->nvalues * sizeof(bool)); hasmatch2 = (bool *) palloc0(clamped_nvalues2 * sizeof(bool)); @@ -5396,6 +5415,8 @@ get_variable_range(PlannerInfo *root, VariableStatData *vardata, Oid sortop, bool tmax_is_mcv = false; FmgrInfo opproc; + /* Could skip the trust check: currently always an opclass member. */ + pg_proc_trustcheck(opfuncoid); fmgr_info(opfuncoid, &opproc); for (i = 0; i < sslot.nvalues; i++) @@ -6021,6 +6042,7 @@ prefix_selectivity(PlannerInfo *root, VariableStatData *vardata, BTGreaterEqualStrategyNumber); if (cmpopr == InvalidOid) elog(ERROR, "no >= operator for opfamily %u", opfamily); + /* Skip trust check since it's obviously an opclass member. */ fmgr_info(get_opcode(cmpopr), &opproc); prefixsel = ineq_histogram_selectivity(root, vardata, @@ -6427,6 +6449,7 @@ make_greater_string(const Const *str_const, FmgrInfo *ltproc, Oid collation) else workstr_const = string_to_const(workstr, datatype); + /* opclass function; no trust check */ if (DatumGetBool(FunctionCall2Coll(ltproc, collation, cmpstr, diff --git a/src/backend/utils/cache/syscache.c b/src/backend/utils/cache/syscache.c index c26808a..3087fd4 100644 --- a/src/backend/utils/cache/syscache.c +++ b/src/backend/utils/cache/syscache.c @@ -28,6 +28,7 @@ #include "catalog/pg_amop.h" #include "catalog/pg_amproc.h" #include "catalog/pg_auth_members.h" +#include "catalog/pg_auth_trust.h" #include "catalog/pg_authid.h" #include "catalog/pg_cast.h" #include "catalog/pg_collation.h" @@ -231,6 +232,17 @@ static const struct cachedesc cacheinfo[] = { }, 8 }, + {AuthTrustRelationId, /* AUTHTRUSTGRANTORTRUSTEE */ + AuthTrustGrantorTrusteeIndexId, + 2, + { + Anum_pg_auth_trust_grantor, + Anum_pg_auth_trust_trustee, + 0, + 0 + }, + 32 + }, {AuthIdRelationId, /* AUTHNAME */ AuthIdRolnameIndexId, 1, diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c index 73ff48c..0fda601 100644 --- a/src/backend/utils/fmgr/fmgr.c +++ b/src/backend/utils/fmgr/fmgr.c @@ -662,8 +662,6 @@ fmgr_security_definer(PG_FUNCTION_ARGS) Datum result; struct fmgr_security_definer_cache *volatile fcache; FmgrInfo *save_flinfo; - Oid save_userid; - int save_sec_context; volatile int save_nestlevel; PgStat_FunctionCallUsage fcusage; @@ -708,16 +706,13 @@ fmgr_security_definer(PG_FUNCTION_ARGS) else fcache = fcinfo->flinfo->fn_extra; - /* GetUserIdAndSecContext is cheap enough that no harm in a wasted call */ - GetUserIdAndSecContext(&save_userid, &save_sec_context); if (fcache->proconfig) /* Need a new GUC nesting level */ save_nestlevel = NewGUCNestLevel(); else save_nestlevel = 0; /* keep compiler quiet */ if (OidIsValid(fcache->userid)) - SetUserIdAndSecContext(fcache->userid, - save_sec_context | SECURITY_LOCAL_USERID_CHANGE); + PushTransientUser(fcache->userid, false, false); if (fcache->proconfig) { @@ -770,7 +765,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS) if (fcache->proconfig) AtEOXact_GUC(true, save_nestlevel); if (OidIsValid(fcache->userid)) - SetUserIdAndSecContext(save_userid, save_sec_context); + PopTransientUser(); if (fmgr_hook) (*fmgr_hook) (FHET_END, &fcache->flinfo, &fcache->arg); @@ -2223,9 +2218,7 @@ CheckFunctionValidatorAccess(Oid validatorOid, Oid functionOid) * execute it, there should be no possible side-effect of * compiling/validation that execution can't have. */ - aclresult = pg_proc_aclcheck(functionOid, GetUserId(), ACL_EXECUTE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_FUNCTION, NameStr(procStruct->proname)); + pg_proc_execcheck(functionOid); ReleaseSysCache(procTup); ReleaseSysCache(langTup); diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c index 3d10aa5..d94819c 100644 --- a/src/backend/utils/init/miscinit.c +++ b/src/backend/utils/init/miscinit.c @@ -234,36 +234,48 @@ ChangeToDataDir(void) * changed by SET SESSION AUTHORIZATION (if AuthenticatedUserIsSuperuser). * This is the ID reported by the SESSION_USER SQL function. * - * OuterUserId is the current user ID in effect at the "outer level" (outside - * any transaction or function). This is initially the same as SessionUserId, - * but can be changed by SET ROLE to any role that SessionUserId is a - * member of. (XXX rename to something like CurrentRoleId?) + * OuterUser is the current user in effect at the "outer level" (outside any + * transaction or function). This initially has SessionUserId, but SET ROLE + * can change that to any role that SessionUserId is a member of. (XXX rename + * to something like CurrentRole?) * - * CurrentUserId is the current effective user ID; this is the one to use - * for all normal permissions-checking purposes. At outer level this will - * be the same as OuterUserId, but it changes during calls to SECURITY - * DEFINER functions, as well as locally in some specialized commands. + * CurrentUser is the effective user; this is the one to use for all normal + * permissions-checking purposes. At outer level this will be the same as + * OuterUser, but it changes during calls to SECURITY DEFINER functions, as + * well as locally in some specialized commands. * - * SecurityRestrictionContext holds flags indicating reason(s) for changing - * CurrentUserId. In some cases we need to lock down operations that are - * not directly controlled by privilege settings, and this provides a - * convenient way to do it. + * + * Operations that change only CurrentUser do so by adding entries to the + * TransientUserStack. OuterUser points to the bottom of that stack, and + * CurrentUser points to the top. The stack also records transitions into + * security-restricted and "FORCE ROW LEVEL SECURITY"-ignoring operations. * ---------------------------------------------------------------- */ static Oid AuthenticatedUserId = InvalidOid; static Oid SessionUserId = InvalidOid; -static Oid OuterUserId = InvalidOid; -static Oid CurrentUserId = InvalidOid; /* We also have to remember the superuser state of some of these levels */ static bool AuthenticatedUserIsSuperuser = false; static bool SessionUserIsSuperuser = false; -static int SecurityRestrictionContext = 0; - /* We also remember if a SET ROLE is currently active */ static bool SetRoleIsActive = false; +/* The stack of transient user ID changes. */ +struct TransientUserEntry +{ + Oid userid; + unsigned security_restriction_depth; + bool noforce_rls; + struct TransientUserStack *prev; +}; +static struct TransientUserEntry *TransientUserStack; +static int TransientUserStack_cur; +static int TransientUserStack_max; + +#define OuterUser (&TransientUserStack[0]) +#define CurrentUser (&TransientUserStack[TransientUserStack_cur]) + /* * Initialize the basic environment for a postmaster child * @@ -373,14 +385,12 @@ SwitchBackToLocalLatch(void) /* * GetUserId - get the current effective user ID. - * - * Note: there's no SetUserId() anymore; use SetUserIdAndSecContext(). */ Oid GetUserId(void) { - AssertState(OidIsValid(CurrentUserId)); - return CurrentUserId; + AssertState(OidIsValid(CurrentUser->userid)); + return CurrentUser->userid; } @@ -390,20 +400,17 @@ GetUserId(void) Oid GetOuterUserId(void) { - AssertState(OidIsValid(OuterUserId)); - return OuterUserId; + AssertState(OidIsValid(OuterUser->userid)); + return OuterUser->userid; } static void SetOuterUserId(Oid userid) { - AssertState(SecurityRestrictionContext == 0); + AssertState(CurrentUser == OuterUser); AssertArg(OidIsValid(userid)); - OuterUserId = userid; - - /* We force the effective user ID to match, too */ - CurrentUserId = userid; + OuterUser->userid = userid; } @@ -421,15 +428,14 @@ GetSessionUserId(void) static void SetSessionUserId(Oid userid, bool is_superuser) { - AssertState(SecurityRestrictionContext == 0); + AssertState(CurrentUser == OuterUser); AssertArg(OidIsValid(userid)); SessionUserId = userid; SessionUserIsSuperuser = is_superuser; SetRoleIsActive = false; /* We force the effective user IDs to match, too */ - OuterUserId = userid; - CurrentUserId = userid; + OuterUser->userid = userid; } /* @@ -444,74 +450,206 @@ GetAuthenticatedUserId(void) /* - * GetUserIdAndSecContext/SetUserIdAndSecContext - get/set the current user ID - * and the SecurityRestrictionContext flags. + * PushTransientUser/PopTransientUser - perform a temporary user ID change * - * Currently there are three valid bits in SecurityRestrictionContext: + * (Sub)transaction abort will undo this change; otherwise, callers are + * responsible for pairing a pop with each push. SET ROLE and SET SESSION + * AUTHORIZATION are forbidden until the last pop. * - * SECURITY_LOCAL_USERID_CHANGE indicates that we are inside an operation - * that is temporarily changing CurrentUserId via these functions. This is - * needed to indicate that the actual value of CurrentUserId is not in sync - * with guc.c's internal state, so SET ROLE has to be disallowed. + * Specifying security_restricted=true signals the start of an operation that + * does not wish to trust called user-defined functions at all. This prevents + * not only SET ROLE, but various other changes of session state that normally + * is unprotected but might possibly be used to subvert the calling session + * later. An example is replacing an existing prepared statement with new + * code, which will then be executed with the outer session's permissions when + * the prepared statement is next used. Since these restrictions are fairly + * draconian, we apply them only in contexts where the called functions are + * really supposed to be side-effect-free anyway, such as + * VACUUM/ANALYZE/REINDEX. GUC changes are still permitted; the caller had + * better push a GUC nesting level, too. * - * SECURITY_RESTRICTED_OPERATION indicates that we are inside an operation - * that does not wish to trust called user-defined functions at all. This - * bit prevents not only SET ROLE, but various other changes of session state - * that normally is unprotected but might possibly be used to subvert the - * calling session later. An example is replacing an existing prepared - * statement with new code, which will then be executed with the outer - * session's permissions when the prepared statement is next used. Since - * these restrictions are fairly draconian, we apply them only in contexts - * where the called functions are really supposed to be side-effect-free - * anyway, such as VACUUM/ANALYZE/REINDEX. - * - * SECURITY_NOFORCE_RLS indicates that we are inside an operation which should - * ignore the FORCE ROW LEVEL SECURITY per-table indication. This is used to - * ensure that FORCE RLS does not mistakenly break referential integrity - * checks. Note that this is intentionally only checked when running as the - * owner of the table (which should always be the case for referential - * integrity checks). - * - * Unlike GetUserId, GetUserIdAndSecContext does *not* Assert that the current - * value of CurrentUserId is valid; nor does SetUserIdAndSecContext require - * the new value to be valid. In fact, these routines had better not - * ever throw any kind of error. This is because they are used by - * StartTransaction and AbortTransaction to save/restore the settings, - * and during the first transaction within a backend, the value to be saved - * and perhaps restored is indeed invalid. We have to be able to get - * through AbortTransaction without asserting in case InitPostgres fails. + * Specifying noforce_rls=true indicates that we are inside an operation which + * should ignore the FORCE ROW LEVEL SECURITY per-table indication. This is + * used to ensure that FORCE RLS does not mistakenly break referential + * integrity checks. Note that this is intentionally only checked when + * running as the owner of the table (which should always be the case for + * referential integrity checks). */ void -GetUserIdAndSecContext(Oid *userid, int *sec_context) +PushTransientUser(Oid userid, + bool security_restricted, bool noforce_rls) { - *userid = CurrentUserId; - *sec_context = SecurityRestrictionContext; + struct TransientUserEntry *prev; + + if (TransientUserStack_cur + 1 == TransientUserStack_max) + { + int new_max = 2 * TransientUserStack_max; + + TransientUserStack = repalloc(TransientUserStack, + new_max * sizeof(*TransientUserStack)); + TransientUserStack_max = new_max; + } + + prev = CurrentUser; + TransientUserStack_cur++; + CurrentUser->userid = userid; + CurrentUser->security_restriction_depth = prev->security_restriction_depth; + if (security_restricted) + CurrentUser->security_restriction_depth++; + CurrentUser->noforce_rls = prev->noforce_rls || noforce_rls; } void -SetUserIdAndSecContext(Oid userid, int sec_context) +PopTransientUser(void) { - CurrentUserId = userid; - SecurityRestrictionContext = sec_context; + Assert(CurrentUser > OuterUser); + TransientUserStack_cur--; } /* - * InLocalUserIdChange - are we inside a local change of CurrentUserId? + * GetTransientUser/RestoreTransientUser + * + * xact.c uses these functions to clean up transient user ID changes at + * (sub)transaction abort; they probably have no other use. The TransientUser + * value should be considered opaque outside of this file; it is the stack + * offset of CurrentUser. + * + * Note that AtEOXact_GUC() takes responsibility for reverting changes to the + * session and outer user IDs. */ -bool -InLocalUserIdChange(void) +TransientUser +GetTransientUser(void) +{ + return TransientUserStack_cur; +} + +void +RestoreTransientUser(TransientUser u) { - return (SecurityRestrictionContext & SECURITY_LOCAL_USERID_CHANGE) != 0; + Assert(u >= 0 && u <= TransientUserStack_cur); + TransientUserStack_cur = u; } + +Size +EstimateTransientUserStackSpace(void) +{ + return mul_size(1 + 1 + TransientUserStack_cur, + sizeof(*TransientUserStack)); +} + +void +SerializeTransientUserStack(Size maxsize, char *start_address) +{ + struct TransientUserEntry *state = + (struct TransientUserEntry *) start_address; + + Assert(maxsize >= EstimateTransientUserStackSpace()); + + /* First entry stores the offset and capacity. */ + state[0].userid = TransientUserStack_cur; + state[0].security_restriction_depth = TransientUserStack_max; + state[0].noforce_rls = false; + + memcpy(state + 1, TransientUserStack, + (TransientUserStack_cur + 1) * sizeof(*TransientUserStack)); +} + +void +RestoreTransientUserStack(char *start_address) +{ + struct TransientUserEntry *state = + (struct TransientUserEntry *) start_address; + int new_cur, new_max; + + Assert(TransientUserStack_cur == 0); + + new_cur = state[0].userid; + new_max = state[0].security_restriction_depth; + TransientUserStack = repalloc(TransientUserStack, + new_max * sizeof(*TransientUserStack)); + TransientUserStack_max = new_max; + + TransientUserStack_cur = new_cur; + memcpy(TransientUserStack, state + 1, + (TransientUserStack_cur + 1) * sizeof(*TransientUserStack)); +} + + /* - * InSecurityRestrictedOperation - are we inside a security-restricted command? + * GetSecurityApplicableRoles - list roles that can veto code execution + * + * Of course, the current user is always concerned with the code that runs; + * such code uses its privileges. Less obviously, roles that will be restored + * after popping a transient user change also have an interest. Code that + * runs now can change GUCs, create temporary tables, and mutate the session + * in other ways; in doing so, it may entice code running in those higher + * stack frames to misbehave. However, when a transient user change is done + * with the security_restricted flag, that change establishes a security + * partition; essential session-mutating actions are either forbidden therein + * or (GUCs) reverted on exit. Therefore, roles on the stack outside the + * current security restriction depth need not be concerned with the code + * permitted to run. Recognizing this exception is important; it allows, for + * example, superusers to run ANALYZE on user tables despite not trusting the + * functions used in index expressions on those tables. + * + * The session user and authenticated user are not considered directly; + * immediately after SET ROLE, only the new outer user qualifies. SET ROLE is + * even less of a security partition than a SECURITY DEFINER call. Not only + * can the new role mutate the session, it can issue RESET ROLE to regain the + * original credentials. Issuing SET ROLE does not cap the potential damage + * from subsequently running arbitrary code, making it more a tool for users + * to run specifically-chosen commands with a different effective user ID. In + * that light, it is marginally more useful to adopt the target role's trust + * relationships, suspending existing ones. Similar arguments apply to SET + * SESSION AUTHORIZATION. + * + * The prepared list is sorted, free from duplicates, and palloc'd. */ -bool -InSecurityRestrictedOperation(void) +void +GetSecurityApplicableRoles(Oid **roles, int *nroles) { - return (SecurityRestrictionContext & SECURITY_RESTRICTED_OPERATION) != 0; + struct TransientUserEntry *entry; + unsigned depth = CurrentUser->security_restriction_depth; + + /* Allocate worst-case space. */ + *roles = palloc((TransientUserStack_cur + 1) * sizeof(**roles)); + + /* + * Deduplicate and insertion-sort. Typical stacks will be short, and + * typical unique lists even shorter. + */ + *nroles = 0; + for (entry = CurrentUser; entry >= OuterUser; --entry) + { + int i; + + if (entry->security_restriction_depth != depth) + break; /* cross into a different partition */ + + for (i = 0; i < *nroles; i++) + { + if (entry->userid == (*roles)[i]) + goto next_stack_entry; + else if (entry->userid < (*roles)[i]) + { + int j; + + for (j = *nroles; j > i; j--) + (*roles)[j] = (*roles)[j - 1]; + + break; + } + } + + (*roles)[i] = entry->userid; + (*nroles)++; + +next_stack_entry:; + } + + Assert(*nroles > 0); } /* @@ -520,37 +658,26 @@ InSecurityRestrictedOperation(void) bool InNoForceRLSOperation(void) { - return (SecurityRestrictionContext & SECURITY_NOFORCE_RLS) != 0; + return CurrentUser->noforce_rls;; } /* - * These are obsolete versions of Get/SetUserIdAndSecContext that are - * only provided for bug-compatibility with some rather dubious code in - * pljava. We allow the userid to be set, but only when not inside a - * security restriction context. + * InLocalUserIdChange - are we inside a local change of CurrentUser? */ -void -GetUserIdAndContext(Oid *userid, bool *sec_def_context) +bool +InLocalUserIdChange(void) { - *userid = CurrentUserId; - *sec_def_context = InLocalUserIdChange(); + return CurrentUser != OuterUser; } -void -SetUserIdAndContext(Oid userid, bool sec_def_context) +/* + * InSecurityRestrictedOperation - are we inside a security-restricted command? + */ +bool +InSecurityRestrictedOperation(void) { - /* We throw the same error SET ROLE would. */ - if (InSecurityRestrictedOperation()) - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("cannot set parameter \"%s\" within security-restricted operation", - "role"))); - CurrentUserId = userid; - if (sec_def_context) - SecurityRestrictionContext |= SECURITY_LOCAL_USERID_CHANGE; - else - SecurityRestrictionContext &= ~SECURITY_LOCAL_USERID_CHANGE; + return CurrentUser->security_restriction_depth > 0; } @@ -573,6 +700,23 @@ has_rolreplication(Oid roleid) } /* + * InitUserStack - call once, before any other user ID state function + */ +void +InitUserStack(void) +{ + TransientUserStack_max = 8; + TransientUserStack = + MemoryContextAlloc(TopMemoryContext, + TransientUserStack_max * sizeof(*OuterUser)); + + TransientUserStack_cur = 0; + OuterUser->userid = InvalidOid; + OuterUser->security_restriction_depth = 0; + OuterUser->noforce_rls = false; +} + +/* * Initialize user identity during normal backend startup */ void @@ -622,7 +766,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid) AuthenticatedUserId = roleid; AuthenticatedUserIsSuperuser = rform->rolsuper; - /* This sets OuterUserId/CurrentUserId too */ + /* This updates OuterUser/CurrentUser too */ SetSessionUserId(roleid, AuthenticatedUserIsSuperuser); /* Also mark our PGPROC entry with the authenticated user id */ @@ -739,7 +883,7 @@ Oid GetCurrentRoleId(void) { if (SetRoleIsActive) - return OuterUserId; + return OuterUser->userid; else return InvalidOid; } diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index b636b1e..1ca0f21 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -703,6 +703,12 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, */ before_shmem_exit(ShutdownPostgres, 0); + /* + * Make the user stack physically-available, another prerequisite for + * starting a transaction. No valid user OIDs on record yet. + */ + InitUserStack(); + /* The autovacuum launcher is done here */ if (IsAutoVacuumLauncherProcess()) { diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c index 5176626..3634359 100644 --- a/src/bin/pg_dump/pg_dumpall.c +++ b/src/bin/pg_dump/pg_dumpall.c @@ -556,6 +556,8 @@ main(int argc, char *argv[]) dumpRoleMembership(conn); else dumpGroups(conn); + + /* FIXME pg_auth_trust */ } /* Dump tablespaces */ diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index ee88e1c..d70796c 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -806,6 +806,8 @@ exec_command_d(PsqlScanState scan_state, bool active_branch, const char *cmd) if (pattern2) free(pattern2); } + else if (cmd[2] == 't') + success = listRoleTrustSettings(pattern); else status = PSQL_CMD_UNKNOWN; break; diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 0a181b0..195497b 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -3484,6 +3484,68 @@ listDbRoleSettings(const char *pattern, const char *pattern2) return true; } +/* + * \drt + */ +bool +listRoleTrustSettings(const char *pattern) +{ + PQExpBufferData buf; + PGresult *res; + printQueryOpt myopt = pset.popt; + + initPQExpBuffer(&buf); + + if (pset.sversion >= 90300) + { + printfPQExpBuffer(&buf, "WITH roles AS\n" + "(SELECT oid, rolname FROM pg_roles " + "UNION ALL VALUES (0, 'public'))\n" + "SELECT " + "gr.rolname AS \"%s\", tr.rolname AS \"%s\"\n" + "FROM pg_auth_trust\n" + "LEFT JOIN roles gr ON gr.oid = grantor\n" + "LEFT JOIN roles tr ON tr.oid = trustee\n", + gettext_noop("Granting Role"), + gettext_noop("Trusted Role")); + processSQLNamePattern(pset.db, &buf, pattern, false, false, + NULL, "gr.rolname", NULL, NULL); + appendPQExpBufferStr(&buf, "ORDER BY 1, 2;"); + } + else + { + fprintf(pset.queryFout, + _("No role trust support in this server version.\n")); + return false; + } + + res = PSQLexec(buf.data); + if (!res) + return false; + + if (PQntuples(res) == 0 && !pset.quiet) + { + if (pattern) + fprintf(pset.queryFout, + _("No matching role trust relationships found.\n")); + else + fprintf(pset.queryFout, + _("No role trust relationships found.\n")); + } + else + { + myopt.nullPrint = NULL; + myopt.title = _("List of role trust relationships"); + myopt.translate_header = true; + + printQuery(res, &myopt, pset.queryFout, false, pset.logfile); + } + + PQclear(res); + resetPQExpBuffer(&buf); + return true; +} + /* * listTables() diff --git a/src/bin/psql/describe.h b/src/bin/psql/describe.h index a4cc5ef..0f58c64 100644 --- a/src/bin/psql/describe.h +++ b/src/bin/psql/describe.h @@ -33,6 +33,9 @@ extern bool describeRoles(const char *pattern, bool verbose, bool showSystem); /* \drds */ extern bool listDbRoleSettings(const char *pattern1, const char *pattern2); +/* \drt */ +extern bool listRoleTrustSettings(const char *pattern); + /* \z (or \dp) */ extern bool permissionsList(const char *pattern); diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c index 586aebd..ab6307e 100644 --- a/src/bin/psql/help.c +++ b/src/bin/psql/help.c @@ -250,6 +250,7 @@ slashUsage(unsigned short int pager) fprintf(output, _(" \\dO[S+] [PATTERN] list collations\n")); fprintf(output, _(" \\dp [PATTERN] list table, view, and sequence access privileges\n")); fprintf(output, _(" \\drds [PATRN1 [PATRN2]] list per-database role settings\n")); + fprintf(output, _(" \\drt [PATTERN] list role trust relationships\n")); fprintf(output, _(" \\dRp[+] [PATTERN] list replication publications\n")); fprintf(output, _(" \\dRs[+] [PATTERN] list replication subscriptions\n")); fprintf(output, _(" \\ds[S+] [PATTERN] list sequences\n")); diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c index 9dbd555..3ad96ab 100644 --- a/src/bin/psql/tab-complete.c +++ b/src/bin/psql/tab-complete.c @@ -1333,7 +1333,7 @@ psql_completion(const char *text, int start, int end) "\\des", "\\det", "\\deu", "\\dew", "\\dE", "\\df", "\\dF", "\\dFd", "\\dFp", "\\dFt", "\\dg", "\\di", "\\dl", "\\dL", "\\dm", "\\dn", "\\do", "\\dO", "\\dp", - "\\drds", "\\dRs", "\\dRp", "\\ds", "\\dS", + "\\drds", "\\drt", "\\dRs", "\\dRp", "\\ds", "\\dS", "\\dt", "\\dT", "\\dv", "\\du", "\\dx", "\\dy", "\\e", "\\echo", "\\ef", "\\elif", "\\else", "\\encoding", "\\endif", "\\errverbose", "\\ev", diff --git a/src/include/catalog/indexing.h b/src/include/catalog/indexing.h index 2359b4c..e955378 100644 --- a/src/include/catalog/indexing.h +++ b/src/include/catalog/indexing.h @@ -102,6 +102,11 @@ DECLARE_UNIQUE_INDEX(pg_auth_members_role_member_index, 2694, on pg_auth_members DECLARE_UNIQUE_INDEX(pg_auth_members_member_role_index, 2695, on pg_auth_members using btree(member oid_ops, roleid oid_ops)); #define AuthMemMemRoleIndexId 2695 +DECLARE_UNIQUE_INDEX(pg_auth_trust_grantor_trustee_index, 2233, on pg_auth_trust using btree(grantor oid_ops, trustee oid_ops)); +#define AuthTrustGrantorTrusteeIndexId 2233 +DECLARE_UNIQUE_INDEX(pg_auth_trust_trustee_grantor_index, 2234, on pg_auth_trust using btree(trustee oid_ops, grantor oid_ops)); +#define AuthTrustTrusteeGrantorIndexId 2234 + DECLARE_UNIQUE_INDEX(pg_cast_oid_index, 2660, on pg_cast using btree(oid oid_ops)); #define CastOidIndexId 2660 DECLARE_UNIQUE_INDEX(pg_cast_source_target_index, 2661, on pg_cast using btree(castsource oid_ops, casttarget oid_ops)); diff --git a/src/include/catalog/pg_auth_trust.dat b/src/include/catalog/pg_auth_trust.dat new file mode 100644 index 0000000..d16f3aa --- /dev/null +++ b/src/include/catalog/pg_auth_trust.dat @@ -0,0 +1,18 @@ +#---------------------------------------------------------------------- +# +# pg_auth_trust.dat +# Initial contents of the pg_auth_trust system catalog. +# +# Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group +# Portions Copyright (c) 1994, Regents of the University of California +# +# src/include/catalog/pg_auth_trust.dat +# +#---------------------------------------------------------------------- + +[ +# For backward compatibility, PUBLIC trusts PUBLIC. +{ grantor => 0, trustee => 0 }, +# Redundant with that, PUBLIC trusts BOOTSTRAP_SUPERUSERID. +{ grantor => 0, trustee => 10 }, +] diff --git a/src/include/catalog/pg_auth_trust.h b/src/include/catalog/pg_auth_trust.h new file mode 100644 index 0000000..0271682a --- /dev/null +++ b/src/include/catalog/pg_auth_trust.h @@ -0,0 +1,43 @@ +/*------------------------------------------------------------------------- + * + * pg_auth_trust.h + * definition of the "authorization identifier trust" system catalog + * (pg_auth_trust) + * + * + * Portions Copyright (c) 1996-2018, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/include/catalog/pg_auth_trust.h + * + * NOTES + * The Catalog.pm module reads this file and derives schema + * information. + * + *------------------------------------------------------------------------- + */ +#ifndef PG_AUTH_TRUST_H +#define PG_AUTH_TRUST_H + +#include "catalog/genbki.h" +#include "catalog/pg_auth_trust_d.h" + +/* ---------------- + * pg_auth_trust definition. cpp turns this into + * typedef struct FormData_pg_auth_trust + * ---------------- + */ +CATALOG(pg_auth_trust,1364,AuthTrustRelationId) BKI_SHARED_RELATION +{ + Oid grantor; /* trusting role; InvalidOid is a wildcard */ + Oid trustee; /* trusted role; InvalidOid is a wildcard */ +} FormData_pg_auth_trust; + +/* ---------------- + * Form_pg_auth_trust corresponds to a pointer to a tuple with + * the format of pg_auth_trust relation. + * ---------------- + */ +typedef FormData_pg_auth_trust *Form_pg_auth_trust; + +#endif /* PG_AUTH_TRUST_H */ diff --git a/src/include/commands/user.h b/src/include/commands/user.h index 028e0dd..cb083dc 100644 --- a/src/include/commands/user.h +++ b/src/include/commands/user.h @@ -32,6 +32,6 @@ extern void GrantRole(GrantRoleStmt *stmt); extern ObjectAddress RenameRole(const char *oldname, const char *newname); extern void DropOwnedObjects(DropOwnedStmt *stmt); extern void ReassignOwnedObjects(ReassignOwnedStmt *stmt); -extern List *roleSpecsToIds(List *memberNames); +extern List *roleSpecsToIds(List *memberNames, bool public_ok); #endif /* USER_H */ diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index d6b32c0..2a6b774 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -295,11 +295,6 @@ extern int trace_recovery(int trace_level); * POSTGRES directory path definitions. * *****************************************************************************/ -/* flags to be OR'd to form sec_context */ -#define SECURITY_LOCAL_USERID_CHANGE 0x0001 -#define SECURITY_RESTRICTED_OPERATION 0x0002 -#define SECURITY_NOFORCE_RLS 0x0004 - extern char *DatabasePath; /* now in utils/init/miscinit.c */ @@ -308,23 +303,32 @@ extern void InitStandaloneProcess(const char *argv0); extern void SetDatabasePath(const char *path); +/* opaque cookie */ +typedef unsigned TransientUser; + extern char *GetUserNameFromId(Oid roleid, bool noerr); extern Oid GetUserId(void); extern Oid GetOuterUserId(void); extern Oid GetSessionUserId(void); +extern void PushTransientUser(Oid userid, + bool security_restricted, bool noforce_rls); +extern void PopTransientUser(void); +extern TransientUser GetTransientUser(void); +extern void RestoreTransientUser(TransientUser u); +extern Size EstimateTransientUserStackSpace(void); +extern void SerializeTransientUserStack(Size maxsize, char *start_address); +extern void RestoreTransientUserStack(char *start_address); extern Oid GetAuthenticatedUserId(void); -extern void GetUserIdAndSecContext(Oid *userid, int *sec_context); -extern void SetUserIdAndSecContext(Oid userid, int sec_context); extern bool InLocalUserIdChange(void); extern bool InSecurityRestrictedOperation(void); extern bool InNoForceRLSOperation(void); -extern void GetUserIdAndContext(Oid *userid, bool *sec_def_context); -extern void SetUserIdAndContext(Oid userid, bool sec_def_context); +extern void InitUserStack(void); extern void InitializeSessionUserId(const char *rolename, Oid useroid); extern void InitializeSessionUserIdStandalone(void); extern void SetSessionAuthorization(Oid userid, bool is_superuser); extern Oid GetCurrentRoleId(void); extern void SetCurrentRoleId(Oid roleid, bool is_superuser); +extern void GetSecurityApplicableRoles(Oid **roles, int *nroles); extern void checkDataDir(void); extern void SetDataDir(const char *dir); diff --git a/src/include/nodes/pg_list.h b/src/include/nodes/pg_list.h index e6cd2cd..1910fd1 100644 --- a/src/include/nodes/pg_list.h +++ b/src/include/nodes/pg_list.h @@ -245,8 +245,9 @@ extern List *list_union_oid(const List *list1, const List *list2); extern List *list_intersection(const List *list1, const List *list2); extern List *list_intersection_int(const List *list1, const List *list2); +extern List *list_intersection_oid(const List *list1, const List *list2); -/* currently, there's no need for list_intersection_ptr etc */ +/* currently, there's no need for list_intersection_ptr */ extern List *list_difference(const List *list1, const List *list2); extern List *list_difference_ptr(const List *list1, const List *list2); diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h index 23db401..c8eca3a 100644 --- a/src/include/parser/kwlist.h +++ b/src/include/parser/kwlist.h @@ -412,6 +412,7 @@ PG_KEYWORD("trigger", TRIGGER, UNRESERVED_KEYWORD) PG_KEYWORD("trim", TRIM, COL_NAME_KEYWORD) PG_KEYWORD("true", TRUE_P, RESERVED_KEYWORD) PG_KEYWORD("truncate", TRUNCATE, UNRESERVED_KEYWORD) +PG_KEYWORD("trust", TRUST, UNRESERVED_KEYWORD) PG_KEYWORD("trusted", TRUSTED, UNRESERVED_KEYWORD) PG_KEYWORD("type", TYPE_P, UNRESERVED_KEYWORD) PG_KEYWORD("types", TYPES_P, UNRESERVED_KEYWORD) diff --git a/src/include/utils/acl.h b/src/include/utils/acl.h index 72936ee..a1bae03 100644 --- a/src/include/utils/acl.h +++ b/src/include/utils/acl.h @@ -262,7 +262,12 @@ extern AclResult pg_attribute_aclcheck_all(Oid table_oid, Oid roleid, AclMode mode, AclMaskHow how); extern AclResult pg_class_aclcheck(Oid table_oid, Oid roleid, AclMode mode); extern AclResult pg_database_aclcheck(Oid db_oid, Oid roleid, AclMode mode); +extern bool pg_oper_trustcheck_extended(Oid oper_oid, bool errorOK); extern AclResult pg_proc_aclcheck(Oid proc_oid, Oid roleid, AclMode mode); +extern bool pg_proc_trustcheck_extended(Oid proc_oid, bool errorOK); +#define pg_proc_trustcheck(proc_oid) \ + pg_proc_trustcheck_extended((proc_oid), false) +extern void pg_proc_execcheck(Oid proc_oid); extern AclResult pg_language_aclcheck(Oid lang_oid, Oid roleid, AclMode mode); extern AclResult pg_largeobject_aclcheck_snapshot(Oid lang_oid, Oid roleid, AclMode mode, Snapshot snapshot); diff --git a/src/include/utils/syscache.h b/src/include/utils/syscache.h index 6f290c7..20dae44 100644 --- a/src/include/utils/syscache.h +++ b/src/include/utils/syscache.h @@ -41,6 +41,7 @@ enum SysCacheIdentifier ATTNUM, AUTHMEMMEMROLE, AUTHMEMROLEMEM, + AUTHTRUSTGRANTORTRUSTEE, AUTHNAME, AUTHOID, CASTSOURCETARGET, diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c index 3b1454f..afe2945 100644 --- a/src/pl/tcl/pltcl.c +++ b/src/pl/tcl/pltcl.c @@ -619,9 +619,7 @@ call_pltcl_start_proc(Oid prolang, bool pltrusted) procOid = LookupFuncName(namelist, 0, fargtypes, false); /* Current user must have permission to call function */ - aclresult = pg_proc_aclcheck(procOid, GetUserId(), ACL_EXECUTE); - if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, OBJECT_FUNCTION, start_proc); + pg_proc_execcheck(procOid); /* Get the function's pg_proc entry */ procTup = SearchSysCache1(PROCOID, ObjectIdGetDatum(procOid)); diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 83b3196..f7c9b07 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -20,10 +20,10 @@ SELECT lo_unlink(oid) FROM pg_largeobject_metadata WHERE oid >= 1000 AND oid < 3 RESET client_min_messages; -- test proper begins here CREATE USER regress_priv_user1; -CREATE USER regress_priv_user2; -CREATE USER regress_priv_user3; -CREATE USER regress_priv_user4; -CREATE USER regress_priv_user5; +CREATE USER regress_priv_user2 TRUST regress_priv_user1; +CREATE USER regress_priv_user3 TRUST regress_priv_user1; +CREATE USER regress_priv_user4 TRUST regress_priv_user1; +CREATE USER regress_priv_user5 TRUST regress_priv_user1; CREATE USER regress_priv_user5; -- duplicate ERROR: role "regress_priv_user5" already exists CREATE GROUP regress_priv_group1; @@ -1909,6 +1909,94 @@ revoke select on dep_priv_test from regress_priv_user4 cascade; set session role regress_priv_user1; drop table dep_priv_test; +-- Function Trust +-- Think of regress_priv_user3 as the lead malefactor, regress_priv_user2 as +-- the accomplice, and regress_priv_user1 as the innocent party. +RESET ROLE; +-- Change PUBLIC's function trust in a transaction we never COMMIT. Other +-- sessions will not see the change, and a failed "make installcheck" will not +-- leave a durable change. +BEGIN; +-- Suppress WARNING when the installcheck cluster is already so-configured. +SET client_min_messages TO 'error'; +ALTER USER public NO TRUST public; +RESET client_min_messages; +-- Prepare objects. +CREATE FUNCTION regress_priv_user3_f() RETURNS int SECURITY DEFINER IMMUTABLE + LANGUAGE plpgsql AS $$ +BEGIN + RAISE NOTICE 'malefactor has control'; + RETURN 1; +END +$$; +ALTER FUNCTION regress_priv_user3_f() OWNER TO regress_priv_user3; +CREATE FUNCTION regress_priv_user2_f() RETURNS int SECURITY DEFINER IMMUTABLE + LANGUAGE sql AS 'SELECT regress_priv_user3_f()'; +ALTER FUNCTION regress_priv_user2_f() OWNER TO regress_priv_user2; +CREATE TABLE trojan (c int); +INSERT INTO trojan SELECT 1 FROM generate_series(1, 20); +INSERT INTO trojan SELECT 2 FROM generate_series(1, 60); +ALTER TABLE trojan OWNER TO regress_priv_user1; +-- One SECURITY DEFINER level. +SET SESSION AUTHORIZATION regress_priv_user2; +ALTER USER regress_priv_user2 TRUST regress_priv_user3; +SAVEPOINT q; +ALTER USER regress_priv_user1 TRUST regress_priv_user3; -- fails: no admin option +ERROR: must have admin option on role "regress_priv_user1" +ROLLBACK TO q; +SELECT regress_priv_user2_f(); -- works +NOTICE: malefactor has control + regress_priv_user2_f +---------------------- + 1 +(1 row) + +-- Two SECURITY DEFINER levels; bottom level lacks trust. +SET SESSION AUTHORIZATION regress_priv_user1; +ALTER USER regress_priv_user1 TRUST regress_priv_user2; +SAVEPOINT q; +SELECT regress_priv_user2_f(); -- fails +ERROR: user regress_priv_user1 does not trust function regress_priv_user3_f +CONTEXT: SQL function "regress_priv_user2_f" statement 1 +ROLLBACK TO q; +ALTER USER regress_priv_user1 TRUST regress_priv_user3; +SELECT regress_priv_user2_f(); -- now works +NOTICE: malefactor has control + regress_priv_user2_f +---------------------- + 1 +(1 row) + +-- Two SECURITY DEFINER levels; top level lacks trust. +SAVEPOINT q; +SET SESSION AUTHORIZATION regress_priv_user2; +ALTER USER regress_priv_user2 NO TRUST regress_priv_user3; +SET SESSION AUTHORIZATION regress_priv_user1; +SELECT regress_priv_user2_f(); -- now fails again +ERROR: user regress_priv_user2 does not trust function regress_priv_user3_f +CONTEXT: SQL function "regress_priv_user2_f" statement 1 +ROLLBACK TO q; -- revert TRUST change +-- Laxness allowed by use of a security-restricted context. +RESET SESSION AUTHORIZATION; +ALTER ROLE regress_priv_user4 SUPERUSER; +SET ROLE regress_priv_user4; +CREATE INDEX ON trojan (c) WHERE regress_priv_user2_f() > 0; +NOTICE: malefactor has control +SAVEPOINT q; +INSERT INTO trojan VALUES (1); -- fails +ERROR: user regress_priv_user4 does not trust function regress_priv_user2_f +ROLLBACK TO q; +ANALYZE trojan; +NOTICE: malefactor has control +ALTER USER regress_priv_user1 NO TRUST regress_priv_user3; +SAVEPOINT q; +ANALYZE trojan; -- fails: owner ceased trusting function +ERROR: user regress_priv_user1 does not trust function regress_priv_user3_f +CONTEXT: SQL function "regress_priv_user2_f" statement 1 +ROLLBACK TO q; +RESET ROLE; +ALTER ROLE regress_priv_user4 NOSUPERUSER; +ROLLBACK; -- clean up \c drop sequence x_seq; diff --git a/src/test/regress/expected/rolenames.out b/src/test/regress/expected/rolenames.out index 68dacb7..7d6dba7 100644 --- a/src/test/regress/expected/rolenames.out +++ b/src/test/regress/expected/rolenames.out @@ -200,9 +200,9 @@ LINE 1: ALTER ROLE ALL WITH REPLICATION; ALTER ROLE SESSION_ROLE WITH NOREPLICATION; -- error ERROR: role "session_role" does not exist ALTER ROLE PUBLIC WITH NOREPLICATION; -- error -ERROR: role "public" does not exist +ERROR: only [NO] TRUST allowed for PUBLIC ALTER ROLE "public" WITH NOREPLICATION; -- error -ERROR: role "public" does not exist +ERROR: only [NO] TRUST allowed for PUBLIC ALTER ROLE NONE WITH NOREPLICATION; -- error ERROR: role name "none" is reserved LINE 1: ALTER ROLE NONE WITH NOREPLICATION; @@ -316,9 +316,9 @@ LINE 1: ALTER USER ALL WITH REPLICATION; ALTER USER SESSION_ROLE WITH NOREPLICATION; -- error ERROR: role "session_role" does not exist ALTER USER PUBLIC WITH NOREPLICATION; -- error -ERROR: role "public" does not exist +ERROR: only [NO] TRUST allowed for PUBLIC ALTER USER "public" WITH NOREPLICATION; -- error -ERROR: role "public" does not exist +ERROR: only [NO] TRUST allowed for PUBLIC ALTER USER NONE WITH NOREPLICATION; -- error ERROR: role name "none" is reserved LINE 1: ALTER USER NONE WITH NOREPLICATION; diff --git a/src/test/regress/expected/sanity_check.out b/src/test/regress/expected/sanity_check.out index c77060d..c77f017 100644 --- a/src/test/regress/expected/sanity_check.out +++ b/src/test/regress/expected/sanity_check.out @@ -108,6 +108,7 @@ pg_amproc|t pg_attrdef|t pg_attribute|t pg_auth_members|t +pg_auth_trust|t pg_authid|t pg_cast|t pg_class|t diff --git a/src/test/regress/expected/select_views.out b/src/test/regress/expected/select_views.out index bf003ad..9c359e7 100644 --- a/src/test/regress/expected/select_views.out +++ b/src/test/regress/expected/select_views.out @@ -1250,7 +1250,9 @@ SELECT * FROM toyemp WHERE name = 'sharon'; -- -- Test for Leaky view scenario -- -CREATE ROLE regress_alice; +DO $$BEGIN + EXECUTE 'CREATE ROLE regress_alice TRUST ' || quote_ident(current_user); +END$$; CREATE FUNCTION f_leak (text) RETURNS bool LANGUAGE 'plpgsql' COST 0.0000001 AS 'BEGIN RAISE NOTICE ''f_leak => %'', $1; RETURN true; END'; diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index ac2c3df..b9fcc87 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -24,10 +24,10 @@ RESET client_min_messages; -- test proper begins here CREATE USER regress_priv_user1; -CREATE USER regress_priv_user2; -CREATE USER regress_priv_user3; -CREATE USER regress_priv_user4; -CREATE USER regress_priv_user5; +CREATE USER regress_priv_user2 TRUST regress_priv_user1; +CREATE USER regress_priv_user3 TRUST regress_priv_user1; +CREATE USER regress_priv_user4 TRUST regress_priv_user1; +CREATE USER regress_priv_user5 TRUST regress_priv_user1; CREATE USER regress_priv_user5; -- duplicate CREATE GROUP regress_priv_group1; @@ -1130,6 +1130,84 @@ set session role regress_priv_user1; drop table dep_priv_test; +-- Function Trust + +-- Think of regress_priv_user3 as the lead malefactor, regress_priv_user2 as +-- the accomplice, and regress_priv_user1 as the innocent party. + +RESET ROLE; + +-- Change PUBLIC's function trust in a transaction we never COMMIT. Other +-- sessions will not see the change, and a failed "make installcheck" will not +-- leave a durable change. +BEGIN; + +-- Suppress WARNING when the installcheck cluster is already so-configured. +SET client_min_messages TO 'error'; +ALTER USER public NO TRUST public; +RESET client_min_messages; + +-- Prepare objects. +CREATE FUNCTION regress_priv_user3_f() RETURNS int SECURITY DEFINER IMMUTABLE + LANGUAGE plpgsql AS $$ +BEGIN + RAISE NOTICE 'malefactor has control'; + RETURN 1; +END +$$; +ALTER FUNCTION regress_priv_user3_f() OWNER TO regress_priv_user3; +CREATE FUNCTION regress_priv_user2_f() RETURNS int SECURITY DEFINER IMMUTABLE + LANGUAGE sql AS 'SELECT regress_priv_user3_f()'; +ALTER FUNCTION regress_priv_user2_f() OWNER TO regress_priv_user2; +CREATE TABLE trojan (c int); +INSERT INTO trojan SELECT 1 FROM generate_series(1, 20); +INSERT INTO trojan SELECT 2 FROM generate_series(1, 60); +ALTER TABLE trojan OWNER TO regress_priv_user1; + +-- One SECURITY DEFINER level. +SET SESSION AUTHORIZATION regress_priv_user2; +ALTER USER regress_priv_user2 TRUST regress_priv_user3; +SAVEPOINT q; +ALTER USER regress_priv_user1 TRUST regress_priv_user3; -- fails: no admin option +ROLLBACK TO q; +SELECT regress_priv_user2_f(); -- works + +-- Two SECURITY DEFINER levels; bottom level lacks trust. +SET SESSION AUTHORIZATION regress_priv_user1; +ALTER USER regress_priv_user1 TRUST regress_priv_user2; +SAVEPOINT q; +SELECT regress_priv_user2_f(); -- fails +ROLLBACK TO q; +ALTER USER regress_priv_user1 TRUST regress_priv_user3; +SELECT regress_priv_user2_f(); -- now works + +-- Two SECURITY DEFINER levels; top level lacks trust. +SAVEPOINT q; +SET SESSION AUTHORIZATION regress_priv_user2; +ALTER USER regress_priv_user2 NO TRUST regress_priv_user3; +SET SESSION AUTHORIZATION regress_priv_user1; +SELECT regress_priv_user2_f(); -- now fails again +ROLLBACK TO q; -- revert TRUST change + +-- Laxness allowed by use of a security-restricted context. +RESET SESSION AUTHORIZATION; +ALTER ROLE regress_priv_user4 SUPERUSER; +SET ROLE regress_priv_user4; +CREATE INDEX ON trojan (c) WHERE regress_priv_user2_f() > 0; +SAVEPOINT q; +INSERT INTO trojan VALUES (1); -- fails +ROLLBACK TO q; +ANALYZE trojan; +ALTER USER regress_priv_user1 NO TRUST regress_priv_user3; +SAVEPOINT q; +ANALYZE trojan; -- fails: owner ceased trusting function +ROLLBACK TO q; +RESET ROLE; +ALTER ROLE regress_priv_user4 NOSUPERUSER; + +ROLLBACK; + + -- clean up \c diff --git a/src/test/regress/sql/select_views.sql b/src/test/regress/sql/select_views.sql index e742f13..1caa50a 100644 --- a/src/test/regress/sql/select_views.sql +++ b/src/test/regress/sql/select_views.sql @@ -12,7 +12,9 @@ SELECT * FROM toyemp WHERE name = 'sharon'; -- -- Test for Leaky view scenario -- -CREATE ROLE regress_alice; +DO $$BEGIN + EXECUTE 'CREATE ROLE regress_alice TRUST ' || quote_ident(current_user); +END$$; CREATE FUNCTION f_leak (text) RETURNS bool LANGUAGE 'plpgsql' COST 0.0000001