On Sun, Aug 12, 2018 at 10:40:30PM -0400, Robert Haas wrote: > On Wed, Aug 8, 2018 at 1:15 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > > If we had, say, a catalog that provided the desired list of trusted roles > > for every role, then we could imagine implementing that context change > > automatically. Likewise, stuff like autovacuum or REINDEX would want > > to run with the table owner's list of trusted roles, but the GUC approach > > doesn't really provide enough infrastructure to know what to do there. > > Yeah, I think these are all good points. It seems natural for trust > to be a property of a role, for just the reasons that you mention. > However, there does also seem to be a use case for varying it > temporarily on a per-session or per-function basis, and I'm not > exactly sure how to cater to those needs.
Yep. A GUC is great for src/bin sessions, since many of those applications consistently want tight trust settings. > I wonder if Noah would like to rebase and post his version also. Sure, attached (last merge at 757c518). The owner_trustcheck() header comment is a good starting point for reading it. Tom Lane later asserted that it's okay to perform the function trust check in fmgr_info_cxt_security() instead of doing it in most fmgr_info() callers and some *FunctionCall* callers. While I suspected he was right, I have not made that change in this rebase. (I also haven't audited every fmgr_info() call added after -v1.) When I shared -v1 with the security team, I included the following submission notes: === Here's an implementation. Design decisions: 1. A role trusts itself implicitly 2. Default pg_auth_trust entry for "public TRUST public" This effectively disables the new restrictions; concerned administrators will test their applications with it removed, then remove it. 3. Default pg_auth_trust entry for "public TRUST <bootstrap superuser>" Almost everyone will leave this untouched. 4. Changing trust for a role requires ADMIN OPTION on the role. Trust is the next step down from granting role membership. ("ALTER ROLE public TRUST ..." does require superuser.) 5. Non-inheritance of trust Trust is like a reverse permission; I find it helpful to think of "ALTER ROLE foo TRUST bar" as "GRANT may_supply_functions_to_foo TO bar". It would be reasonable to have that trust relationship be effective for INHERITS members of bar; after all, they could always "ALTER FUNCTION ... OWNER TO bar". For now, trust relationships are not subject to inheritance. This keeps things simpler to understand and poses no major downsides. For similar reasons, I have not made a role trust its own members implicitly. 6. Non-transitivity of trust If I trust only alice, alice trusts only bob, and I call alice's function that calls bob's function, should the call succeed? This is a tough one. In favor of "yes", this choice would allow alice to refactor her function without needing her callers to revise their trust settings. That is, she could switch from bob's function to carol's function, and her callers would never notice. My trust in alice is probably misplaced if she chooses her friends badly. In favor of "no", making the trust relationship transitive seems to mix two otherwise-separate concepts: alice's willingness to run code as herself and alice's willingness to certify third-party functions to her friends. In particular, current defects in the system are at odds with conflating those concepts. Everyone should trust the bootstrap superuser, but it currently owns functions like integer_pl_date that are not careful in what they call. That closed the issue for me; trust is not transitive by default. Even so, I think there would be value in a facility for requesting trust transitivity in specific situations. 7. (Lack of) negative trust entries There's no way to opt out of a PUBLIC trust relationship; until a superuser removes the "public TRUST public" entry, no user can achieve anything with ALTER USER [NO] TRUST. I considered adding the concept of a negative trust relationship to remedy this problem; it could also be used to remove the implicit self-trust for testing purposes. I have refrained; I don't know whether the benefits would pay for the extra user-facing complexity. 8. Applicable roles during trust checks We had examined whether the check could be looser for SECURITY DEFINER functions, since those can't perform arbitrary actions as the caller. I described some challenges then, but a deeper look turned up more: a SECURITY DEFINER function can do things like "SET search_path = ..." that affect the caller. Consequently, I concluded that all roles on the active call stack should have a stake in whether a particular function owner is permitted. Each trust check walks the call stack and checks every role represented there; if any lacks the trust relationships to approve the newly-contemplated function call, the call is rejected. The stack traversal may and does stop at the edge of a security-restricted operation; since those restrictions must prevent important session changes, the stack entries active before the operation started need not be concerned with the code running therein. A CREATE FUNCTION option specifying voluntary use of a security-restricted context would provide the "facility for requesting trust transitivity" mentioned earlier. To make walking the role stack possible, I redid the API for changing the current user ID. SetUserIdAndSecContext() gives way to PushTransientUser() and PopTransientUser(); xact.c has its own little API for unwinding this stack at (sub)transaction abort. This patch just removes the old APIs, but that probably wouldn't cut it for the back branches; I do have a design sketch for reintroducing backward-compatible versions. 9. Trust checks on other object types Despite trust checks on functions, hostile users can make mischief with something like "CREATE OPERATOR * (PROCEDURE = int4pl, LEFTARG = int4, RIGHTARG = int4)". Therefore, I also caused trust checks to be enforced on operators themselves. As I opine in the comments at owner_trustcheck(), where to stop is more a judgement call than a science. The following work remains TODO: - Documentation for the new concepts, syntax and system catalog - pg_dumpall support - Testing the performance impact and perhaps adding a cache - Backward-compatible versions of removed miscinit.c functions
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 <replaceable class="parameter">name</replaceable> [ [ WITH ] <replac | SYSID <replaceable class="parameter">uid</replaceable> </synopsis> </refsynopsisdiv> + <!-- FIXME document TRUST here, trust concepts, and pg_auth_trust catalog --> <refsect1> <title>Description</title> 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