diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c index ddc088f781..358830c47f 100644 --- a/src/backend/access/index/genam.c +++ b/src/backend/access/index/genam.c @@ -25,11 +25,14 @@ #include "lib/stringinfo.h" #include "miscadmin.h" #include "storage/bufmgr.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/lsyscache.h" #include "utils/rel.h" +#include "utils/rls.h" #include "utils/ruleutils.h" #include "utils/snapmgr.h" +#include "utils/syscache.h" #include "utils/tqual.h" @@ -155,6 +158,11 @@ IndexScanEnd(IndexScanDesc scan) * form "(key_name, ...)=(key_value, ...)". This is currently used * for building unique-constraint and exclusion-constraint error messages. * + * Note that if the user does not have permissions to view all of the + * columns involved then a NULL is returned. Returning a partial key seems + * unlikely to be useful and we have no way to know which of the columns the + * user provided (unlike in ExecBuildSlotValueDescription). + * * The passed-in values/nulls arrays are the "raw" input to the index AM, * e.g. results of FormIndexDatum --- this is not necessarily what is stored * in the index, but it's what the user perceives to be stored. @@ -164,13 +172,72 @@ BuildIndexValueDescription(Relation indexRelation, Datum *values, bool *isnull) { StringInfoData buf; + Form_pg_index idxrec; + HeapTuple ht_idx; int natts = indexRelation->rd_rel->relnatts; int i; + int keyno; + Oid indexrelid = RelationGetRelid(indexRelation); + Oid indrelid; + AclResult aclresult; + + /* + * Check permissions- if the user does not have access to view all of the + * key columns then return NULL to avoid leaking data. + * + * First check if RLS is enabled for the relation. If so, return NULL + * to avoid leaking data. + * + * Next we need to check table-level SELECT access and then, if + * there is no access there, check column-level permissions. + */ + + /* + * Fetch the pg_index tuple by the Oid of the index + */ + ht_idx = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexrelid)); + if (!HeapTupleIsValid(ht_idx)) + elog(ERROR, "cache lookup failed for index %u", indexrelid); + idxrec = (Form_pg_index) GETSTRUCT(ht_idx); + + indrelid = idxrec->indrelid; + Assert(indexrelid == idxrec->indexrelid); + + /* RLS check- if RLS is enabled then we don't return anything. */ + if (check_enable_rls(indrelid, GetUserId(), true) == RLS_ENABLED) + { + ReleaseSysCache(ht_idx); + return NULL; + } + + /* Table-level SELECT is enough, if the user has it */ + aclresult = pg_class_aclcheck(indrelid, GetUserId(), ACL_SELECT); + if (aclresult != ACLCHECK_OK) + { + /* + * No table-level access, so step through the columns in the + * index and make sure the user has SELECT rights on all of them. + */ + for (keyno = 0; keyno < idxrec->indnatts; keyno++) + { + AttrNumber attnum = idxrec->indkey.values[keyno]; + + aclresult = pg_attribute_aclcheck(indrelid, attnum, GetUserId(), + ACL_SELECT); + + if (aclresult != ACLCHECK_OK) + { + /* No access, so clean up and return */ + ReleaseSysCache(ht_idx); + return NULL; + } + } + } + ReleaseSysCache(ht_idx); initStringInfo(&buf); appendStringInfo(&buf, "(%s)=(", - pg_get_indexdef_columns(RelationGetRelid(indexRelation), - true)); + pg_get_indexdef_columns(indexrelid, true)); for (i = 0; i < natts; i++) { diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c index 7db8a9613d..932c6f78e3 100644 --- a/src/backend/access/nbtree/nbtinsert.c +++ b/src/backend/access/nbtree/nbtinsert.c @@ -389,16 +389,20 @@ _bt_check_unique(Relation rel, IndexTuple itup, Relation heapRel, { Datum values[INDEX_MAX_KEYS]; bool isnull[INDEX_MAX_KEYS]; + char *key_desc; index_deform_tuple(itup, RelationGetDescr(rel), values, isnull); + + key_desc = BuildIndexValueDescription(rel, values, + isnull); + ereport(ERROR, (errcode(ERRCODE_UNIQUE_VIOLATION), errmsg("duplicate key value violates unique constraint \"%s\"", RelationGetRelationName(rel)), - errdetail("Key %s already exists.", - BuildIndexValueDescription(rel, - values, isnull)), + key_desc ? errdetail("Key %s already exists.", + key_desc) : 0, errtableconstraint(heapRel, RelationGetRelationName(rel)))); } diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 0e604b7525..72abd770f7 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -40,7 +40,6 @@ #include "parser/parse_relation.h" #include "nodes/makefuncs.h" #include "rewrite/rewriteHandler.h" -#include "rewrite/rowsecurity.h" #include "storage/fd.h" #include "tcop/tcopprot.h" #include "utils/acl.h" @@ -49,6 +48,7 @@ #include "utils/memutils.h" #include "utils/portal.h" #include "utils/rel.h" +#include "utils/rls.h" #include "utils/snapmgr.h" @@ -163,6 +163,7 @@ typedef struct CopyStateData int *defmap; /* array of default att numbers */ ExprState **defexprs; /* array of default att expressions */ bool volatile_defexprs; /* is any of defexprs volatile? */ + List *range_table; /* * These variables are used to reduce overhead in textual COPY FROM. @@ -789,6 +790,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed) Relation rel; Oid relid; Node *query = NULL; + RangeTblEntry *rte; /* Disallow COPY to/from file or program except to superusers. */ if (!pipe && !superuser()) @@ -811,7 +813,6 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed) { TupleDesc tupDesc; AclMode required_access = (is_from ? ACL_INSERT : ACL_SELECT); - RangeTblEntry *rte; List *attnums; ListCell *cur; @@ -857,7 +858,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed) * If RLS is not enabled for this, then just fall through to the * normal non-filtering relation handling. */ - if (check_enable_rls(rte->relid, InvalidOid) == RLS_ENABLED) + if (check_enable_rls(rte->relid, InvalidOid, false) == RLS_ENABLED) { SelectStmt *select; ColumnRef *cr; @@ -920,6 +921,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed) cstate = BeginCopyFrom(rel, stmt->filename, stmt->is_program, stmt->attlist, stmt->options); + cstate->range_table = list_make1(rte); *processed = CopyFrom(cstate); /* copy from file to database */ EndCopyFrom(cstate); } @@ -928,6 +930,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed) cstate = BeginCopyTo(rel, query, queryString, relid, stmt->filename, stmt->is_program, stmt->attlist, stmt->options); + cstate->range_table = list_make1(rte); *processed = DoCopyTo(cstate); /* copy from database to file */ EndCopyTo(cstate); } @@ -2277,6 +2280,7 @@ CopyFrom(CopyState cstate) estate->es_result_relations = resultRelInfo; estate->es_num_result_relations = 1; estate->es_result_relation_info = resultRelInfo; + estate->es_range_table = cstate->range_table; /* Set up a tuple slot too */ myslot = ExecInitExtraTupleSlot(estate); diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index abc0fe8e58..c961429a0f 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -38,12 +38,12 @@ #include "miscadmin.h" #include "parser/parse_clause.h" #include "rewrite/rewriteHandler.h" -#include "rewrite/rowsecurity.h" #include "storage/smgr.h" #include "tcop/tcopprot.h" #include "utils/builtins.h" #include "utils/lsyscache.h" #include "utils/rel.h" +#include "utils/rls.h" #include "utils/snapmgr.h" @@ -446,7 +446,7 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) * be enabled here. We don't actually support that currently, so throw * our own ereport(ERROR) if that happens. */ - if (check_enable_rls(intoRelationId, InvalidOid) == RLS_ENABLED) + if (check_enable_rls(intoRelationId, InvalidOid, false) == RLS_ENABLED) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), (errmsg("policies not yet implemented for this command")))); diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c index 74415b8ba0..92d9032328 100644 --- a/src/backend/commands/matview.c +++ b/src/backend/commands/matview.c @@ -596,6 +596,13 @@ refresh_by_match_merge(Oid matviewOid, Oid tempOid, Oid relowner, elog(ERROR, "SPI_exec failed: %s", querybuf.data); if (SPI_processed > 0) { + /* + * Note that this ereport() is returning data to the user. Generally, + * we would want to make sure that the user has been granted access to + * this data. However, REFRESH MAT VIEW is only able to be run by the + * owner of the mat view (or a superuser) and therefore there is no + * need to check for access to data in the mat view. + */ ereport(ERROR, (errcode(ERRCODE_CARDINALITY_VIOLATION), errmsg("new data for \"%s\" contains duplicate rows without any null columns", diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 4899a278eb..5c1c1beb2b 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -65,6 +65,12 @@ int SessionReplicationRole = SESSION_REPLICATION_ROLE_ORIGIN; /* How many levels deep into trigger execution are we? */ static int MyTriggerDepth = 0; +/* + * Note that this macro also exists in executor/execMain.c. There does not + * appear to be any good header to put it into, given the structures that + * it uses, so we let them be duplicated. Be sure to update both if one needs + * to be changed, however. + */ #define GetModifiedColumns(relinfo, estate) \ (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->modifiedCols) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index b9f21c560f..20b3188dfd 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -56,6 +56,7 @@ #include "utils/acl.h" #include "utils/lsyscache.h" #include "utils/memutils.h" +#include "utils/rls.h" #include "utils/snapmgr.h" #include "utils/tqual.h" @@ -82,12 +83,23 @@ static void ExecutePlan(EState *estate, PlanState *planstate, DestReceiver *dest); static bool ExecCheckRTEPerms(RangeTblEntry *rte); static void ExecCheckXactReadOnly(PlannedStmt *plannedstmt); -static char *ExecBuildSlotValueDescription(TupleTableSlot *slot, +static char *ExecBuildSlotValueDescription(Oid reloid, + TupleTableSlot *slot, TupleDesc tupdesc, + Bitmapset *modifiedCols, int maxfieldlen); static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree); +/* + * Note that this macro also exists in commands/trigger.c. There does not + * appear to be any good header to put it into, given the structures that + * it uses, so we let them be duplicated. Be sure to update both if one needs + * to be changed, however. + */ +#define GetModifiedColumns(relinfo, estate) \ + (rt_fetch((relinfo)->ri_RangeTableIndex, (estate)->es_range_table)->modifiedCols) + /* end of local decls */ @@ -1607,15 +1619,24 @@ ExecConstraints(ResultRelInfo *resultRelInfo, { if (tupdesc->attrs[attrChk - 1]->attnotnull && slot_attisnull(slot, attrChk)) + { + char *val_desc; + Bitmapset *modifiedCols; + + modifiedCols = GetModifiedColumns(resultRelInfo, estate); + val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), + slot, + tupdesc, + modifiedCols, + 64); + ereport(ERROR, (errcode(ERRCODE_NOT_NULL_VIOLATION), errmsg("null value in column \"%s\" violates not-null constraint", NameStr(tupdesc->attrs[attrChk - 1]->attname)), - errdetail("Failing row contains %s.", - ExecBuildSlotValueDescription(slot, - tupdesc, - 64)), + val_desc ? errdetail("Failing row contains %s.", val_desc) : 0, errtablecol(rel, attrChk))); + } } } @@ -1624,15 +1645,23 @@ ExecConstraints(ResultRelInfo *resultRelInfo, const char *failed; if ((failed = ExecRelCheck(resultRelInfo, slot, estate)) != NULL) + { + char *val_desc; + Bitmapset *modifiedCols; + + modifiedCols = GetModifiedColumns(resultRelInfo, estate); + val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), + slot, + tupdesc, + modifiedCols, + 64); ereport(ERROR, (errcode(ERRCODE_CHECK_VIOLATION), errmsg("new row for relation \"%s\" violates check constraint \"%s\"", RelationGetRelationName(rel), failed), - errdetail("Failing row contains %s.", - ExecBuildSlotValueDescription(slot, - tupdesc, - 64)), + val_desc ? errdetail("Failing row contains %s.", val_desc) : 0, errtableconstraint(rel, failed))); + } } } @@ -1643,6 +1672,8 @@ void ExecWithCheckOptions(ResultRelInfo *resultRelInfo, TupleTableSlot *slot, EState *estate) { + Relation rel = resultRelInfo->ri_RelationDesc; + TupleDesc tupdesc = RelationGetDescr(rel); ExprContext *econtext; ListCell *l1, *l2; @@ -1673,14 +1704,24 @@ ExecWithCheckOptions(ResultRelInfo *resultRelInfo, * case (the opposite of what we do above for CHECK constraints). */ if (!ExecQual((List *) wcoExpr, econtext, false)) + { + char *val_desc; + Bitmapset *modifiedCols; + + modifiedCols = GetModifiedColumns(resultRelInfo, estate); + val_desc = ExecBuildSlotValueDescription(RelationGetRelid(rel), + slot, + tupdesc, + modifiedCols, + 64); + ereport(ERROR, (errcode(ERRCODE_WITH_CHECK_OPTION_VIOLATION), errmsg("new row violates WITH CHECK OPTION for \"%s\"", wco->viewname), - errdetail("Failing row contains %s.", - ExecBuildSlotValueDescription(slot, - RelationGetDescr(resultRelInfo->ri_RelationDesc), - 64)))); + val_desc ? errdetail("Failing row contains %s.", val_desc) : + 0)); + } } } @@ -1696,25 +1737,64 @@ ExecWithCheckOptions(ResultRelInfo *resultRelInfo, * dropped columns. We used to use the slot's tuple descriptor to decode the * data, but the slot's descriptor doesn't identify dropped columns, so we * now need to be passed the relation's descriptor. + * + * Note that, like BuildIndexValueDescription, if the user does not have + * permission to view any of the columns involved, a NULL is returned. Unlike + * BuildIndexValueDescription, if the user has access to view a subset of the + * column involved, that subset will be returned with a key identifying which + * columns they are. */ static char * -ExecBuildSlotValueDescription(TupleTableSlot *slot, +ExecBuildSlotValueDescription(Oid reloid, + TupleTableSlot *slot, TupleDesc tupdesc, + Bitmapset *modifiedCols, int maxfieldlen) { StringInfoData buf; + StringInfoData collist; bool write_comma = false; + bool write_comma_collist = false; int i; + AclResult aclresult; + bool table_perm = false; + bool any_perm = false; - /* Make sure the tuple is fully deconstructed */ - slot_getallattrs(slot); + /* + * Check if RLS is enabled and should be active for the relation; if so, + * then don't return anything. Otherwise, go through normal permission + * checks. + */ + if (check_enable_rls(reloid, GetUserId(), true) == RLS_ENABLED) + return NULL; initStringInfo(&buf); appendStringInfoChar(&buf, '('); + /* + * Check if the user has permissions to see the row. Table-level SELECT + * allows access to all columns. If the user does not have table-level + * SELECT then we check each column and include those the user has SELECT + * rights on. Additionally, we always include columns the user provided + * data for. + */ + aclresult = pg_class_aclcheck(reloid, GetUserId(), ACL_SELECT); + if (aclresult != ACLCHECK_OK) + { + /* Set up the buffer for the column list */ + initStringInfo(&collist); + appendStringInfoChar(&collist, '('); + } + else + table_perm = any_perm = true; + + /* Make sure the tuple is fully deconstructed */ + slot_getallattrs(slot); + for (i = 0; i < tupdesc->natts; i++) { + bool column_perm = false; char *val; int vallen; @@ -1722,37 +1802,76 @@ ExecBuildSlotValueDescription(TupleTableSlot *slot, if (tupdesc->attrs[i]->attisdropped) continue; - if (slot->tts_isnull[i]) - val = "null"; - else + if (!table_perm) { - Oid foutoid; - bool typisvarlena; + /* + * No table-level SELECT, so need to make sure they either have + * SELECT rights on the column or that they have provided the + * data for the column. If not, omit this column from the error + * message. + */ + aclresult = pg_attribute_aclcheck(reloid, tupdesc->attrs[i]->attnum, + GetUserId(), ACL_SELECT); + if (bms_is_member(tupdesc->attrs[i]->attnum - FirstLowInvalidHeapAttributeNumber, + modifiedCols) || aclresult == ACLCHECK_OK) + { + column_perm = any_perm = true; - getTypeOutputInfo(tupdesc->attrs[i]->atttypid, - &foutoid, &typisvarlena); - val = OidOutputFunctionCall(foutoid, slot->tts_values[i]); + if (write_comma_collist) + appendStringInfoString(&collist, ", "); + else + write_comma_collist = true; + + appendStringInfoString(&collist, NameStr(tupdesc->attrs[i]->attname)); + } } - if (write_comma) - appendStringInfoString(&buf, ", "); - else - write_comma = true; - - /* truncate if needed */ - vallen = strlen(val); - if (vallen <= maxfieldlen) - appendStringInfoString(&buf, val); - else + if (table_perm || column_perm) { - vallen = pg_mbcliplen(val, vallen, maxfieldlen); - appendBinaryStringInfo(&buf, val, vallen); - appendStringInfoString(&buf, "..."); + if (slot->tts_isnull[i]) + val = "null"; + else + { + Oid foutoid; + bool typisvarlena; + + getTypeOutputInfo(tupdesc->attrs[i]->atttypid, + &foutoid, &typisvarlena); + val = OidOutputFunctionCall(foutoid, slot->tts_values[i]); + } + + if (write_comma) + appendStringInfoString(&buf, ", "); + else + write_comma = true; + + /* truncate if needed */ + vallen = strlen(val); + if (vallen <= maxfieldlen) + appendStringInfoString(&buf, val); + else + { + vallen = pg_mbcliplen(val, vallen, maxfieldlen); + appendBinaryStringInfo(&buf, val, vallen); + appendStringInfoString(&buf, "..."); + } } } + /* If we end up with zero columns being returned, then return NULL. */ + if (!any_perm) + return NULL; + appendStringInfoChar(&buf, ')'); + if (!table_perm) + { + appendStringInfoString(&collist, ") = "); + appendStringInfoString(&collist, buf.data); + + return collist.data; + } + return buf.data; } diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c index 32697ddaac..4b921fa596 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -1323,8 +1323,10 @@ retry: (errcode(ERRCODE_EXCLUSION_VIOLATION), errmsg("could not create exclusion constraint \"%s\"", RelationGetRelationName(index)), - errdetail("Key %s conflicts with key %s.", - error_new, error_existing), + error_new && error_existing ? + errdetail("Key %s conflicts with key %s.", + error_new, error_existing) : + errdetail("Key conflicts exist."), errtableconstraint(heap, RelationGetRelationName(index)))); else @@ -1332,8 +1334,10 @@ retry: (errcode(ERRCODE_EXCLUSION_VIOLATION), errmsg("conflicting key value violates exclusion constraint \"%s\"", RelationGetRelationName(index)), - errdetail("Key %s conflicts with existing key %s.", - error_new, error_existing), + error_new && error_existing ? + errdetail("Key %s conflicts with existing key %s.", + error_new, error_existing) : + errdetail("Key conflicts with existing key."), errtableconstraint(heap, RelationGetRelationName(index)))); } diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index 8f8e291fb8..7669130e2b 100644 --- a/src/backend/rewrite/rowsecurity.c +++ b/src/backend/rewrite/rowsecurity.c @@ -52,6 +52,7 @@ #include "utils/acl.h" #include "utils/lsyscache.h" #include "utils/rel.h" +#include "utils/rls.h" #include "utils/syscache.h" #include "tcop/utility.h" @@ -115,7 +116,7 @@ prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index) return false; /* Determine the state of RLS for this, pass checkAsUser explicitly */ - rls_status = check_enable_rls(rte->relid, rte->checkAsUser); + rls_status = check_enable_rls(rte->relid, rte->checkAsUser, false); /* If there is no RLS on this table at all, nothing to do */ if (rls_status == RLS_NONE) @@ -455,84 +456,6 @@ process_policies(List *policies, int rt_index, Expr **qual_eval, return; } -/* - * check_enable_rls - * - * Determine, based on the relation, row_security setting, and current role, - * if RLS is applicable to this query. RLS_NONE_ENV indicates that, while - * RLS is not to be added for this query, a change in the environment may change - * that. RLS_NONE means that RLS is not on the relation at all and therefore - * we don't need to worry about it. RLS_ENABLED means RLS should be implemented - * for the table and the plan cache needs to be invalidated if the environment - * changes. - * - * Handle checking as another role via checkAsUser (for views, etc). - */ -int -check_enable_rls(Oid relid, Oid checkAsUser) -{ - HeapTuple tuple; - Form_pg_class classform; - bool relrowsecurity; - Oid user_id = checkAsUser ? checkAsUser : GetUserId(); - - tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); - if (!HeapTupleIsValid(tuple)) - return RLS_NONE; - - classform = (Form_pg_class) GETSTRUCT(tuple); - - relrowsecurity = classform->relrowsecurity; - - ReleaseSysCache(tuple); - - /* Nothing to do if the relation does not have RLS */ - if (!relrowsecurity) - return RLS_NONE; - - /* - * Check permissions - * - * If the relation has row level security enabled and the row_security GUC - * is off, then check if the user has rights to bypass RLS for this - * relation. Table owners can always bypass, as can any role with the - * BYPASSRLS capability. - * - * If the role is the table owner, then we bypass RLS unless row_security - * is set to 'force'. Note that superuser is always considered an owner. - * - * Return RLS_NONE_ENV to indicate that this decision depends on the - * environment (in this case, what the current values of user_id and - * row_security are). - */ - if (row_security != ROW_SECURITY_FORCE - && (pg_class_ownercheck(relid, user_id))) - return RLS_NONE_ENV; - - /* - * If the row_security GUC is 'off' then check if the user has permission - * to bypass it. Note that we have already handled the case where the user - * is the table owner above. - * - * Note that row_security is always considered 'on' when querying - * through a view or other cases where checkAsUser is true, so skip this - * if checkAsUser is in use. - */ - if (!checkAsUser && row_security == ROW_SECURITY_OFF) - { - if (has_bypassrls_privilege(user_id)) - /* OK to bypass */ - return RLS_NONE_ENV; - else - ereport(ERROR, - (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), - errmsg("insufficient privilege to bypass row security."))); - } - - /* RLS should be fully enabled for this relation. */ - return RLS_ENABLED; -} - /* * check_role_for_policy - * determines if the policy should be applied for the current role diff --git a/src/backend/utils/adt/ri_triggers.c b/src/backend/utils/adt/ri_triggers.c index e6dd441507..f6bec8be9b 100644 --- a/src/backend/utils/adt/ri_triggers.c +++ b/src/backend/utils/adt/ri_triggers.c @@ -43,6 +43,7 @@ #include "parser/parse_coerce.h" #include "parser/parse_relation.h" #include "miscadmin.h" +#include "utils/acl.h" #include "utils/builtins.h" #include "utils/fmgroids.h" #include "utils/guc.h" @@ -50,6 +51,7 @@ #include "utils/lsyscache.h" #include "utils/memutils.h" #include "utils/rel.h" +#include "utils/rls.h" #include "utils/snapmgr.h" #include "utils/syscache.h" #include "utils/tqual.h" @@ -3197,6 +3199,9 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo, bool onfk; const int16 *attnums; int idx; + Oid rel_oid; + AclResult aclresult; + bool has_perm = true; if (spi_err) ereport(ERROR, @@ -3215,37 +3220,75 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo, if (onfk) { attnums = riinfo->fk_attnums; + rel_oid = fk_rel->rd_id; if (tupdesc == NULL) tupdesc = fk_rel->rd_att; } else { attnums = riinfo->pk_attnums; + rel_oid = pk_rel->rd_id; if (tupdesc == NULL) tupdesc = pk_rel->rd_att; } - /* Get printable versions of the keys involved */ - initStringInfo(&key_names); - initStringInfo(&key_values); - for (idx = 0; idx < riinfo->nkeys; idx++) + /* + * Check permissions- if the user does not have access to view the data in + * any of the key columns then we don't include the errdetail() below. + * + * Check if RLS is enabled on the relation first. If so, we don't return + * any specifics to avoid leaking data. + * + * Check table-level permissions next and, failing that, column-level + * privileges. + */ + + if (check_enable_rls(rel_oid, GetUserId(), true) != RLS_ENABLED) { - int fnum = attnums[idx]; - char *name, + aclresult = pg_class_aclcheck(rel_oid, GetUserId(), ACL_SELECT); + if (aclresult != ACLCHECK_OK) + { + /* Try for column-level permissions */ + for (idx = 0; idx < riinfo->nkeys; idx++) + { + aclresult = pg_attribute_aclcheck(rel_oid, attnums[idx], + GetUserId(), + ACL_SELECT); + + /* No access to the key */ + if (aclresult != ACLCHECK_OK) + { + has_perm = false; + break; + } + } + } + } + + if (has_perm) + { + /* Get printable versions of the keys involved */ + initStringInfo(&key_names); + initStringInfo(&key_values); + for (idx = 0; idx < riinfo->nkeys; idx++) + { + int fnum = attnums[idx]; + char *name, *val; - name = SPI_fname(tupdesc, fnum); - val = SPI_getvalue(violator, tupdesc, fnum); - if (!val) - val = "null"; + name = SPI_fname(tupdesc, fnum); + val = SPI_getvalue(violator, tupdesc, fnum); + if (!val) + val = "null"; - if (idx > 0) - { - appendStringInfoString(&key_names, ", "); - appendStringInfoString(&key_values, ", "); + if (idx > 0) + { + appendStringInfoString(&key_names, ", "); + appendStringInfoString(&key_values, ", "); + } + appendStringInfoString(&key_names, name); + appendStringInfoString(&key_values, val); } - appendStringInfoString(&key_names, name); - appendStringInfoString(&key_values, val); } if (onfk) @@ -3254,9 +3297,12 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo, errmsg("insert or update on table \"%s\" violates foreign key constraint \"%s\"", RelationGetRelationName(fk_rel), NameStr(riinfo->conname)), - errdetail("Key (%s)=(%s) is not present in table \"%s\".", - key_names.data, key_values.data, - RelationGetRelationName(pk_rel)), + has_perm ? + errdetail("Key (%s)=(%s) is not present in table \"%s\".", + key_names.data, key_values.data, + RelationGetRelationName(pk_rel)) : + errdetail("Key is not present in table \"%s\".", + RelationGetRelationName(pk_rel)), errtableconstraint(fk_rel, NameStr(riinfo->conname)))); else ereport(ERROR, @@ -3265,8 +3311,11 @@ ri_ReportViolation(const RI_ConstraintInfo *riinfo, RelationGetRelationName(pk_rel), NameStr(riinfo->conname), RelationGetRelationName(fk_rel)), + has_perm ? errdetail("Key (%s)=(%s) is still referenced from table \"%s\".", key_names.data, key_values.data, + RelationGetRelationName(fk_rel)) : + errdetail("Key is still referenced from table \"%s\".", RelationGetRelationName(fk_rel)), errtableconstraint(fk_rel, NameStr(riinfo->conname)))); } diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c index 69c8d3ad45..9a26a4efc5 100644 --- a/src/backend/utils/cache/plancache.c +++ b/src/backend/utils/cache/plancache.c @@ -60,13 +60,13 @@ #include "optimizer/prep.h" #include "parser/analyze.h" #include "parser/parsetree.h" -#include "rewrite/rowsecurity.h" #include "storage/lmgr.h" #include "tcop/pquery.h" #include "tcop/utility.h" #include "utils/inval.h" #include "utils/memutils.h" #include "utils/resowner_private.h" +#include "utils/rls.h" #include "utils/snapmgr.h" #include "utils/syscache.h" diff --git a/src/backend/utils/misc/Makefile b/src/backend/utils/misc/Makefile index 449d5b47ea..378b77e0bc 100644 --- a/src/backend/utils/misc/Makefile +++ b/src/backend/utils/misc/Makefile @@ -14,7 +14,7 @@ include $(top_builddir)/src/Makefile.global override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS) -OBJS = guc.o help_config.o pg_rusage.o ps_status.o \ +OBJS = guc.o help_config.o pg_rusage.o ps_status.o rls.o \ superuser.o timeout.o tzparser.o # This location might depend on the installation directories. Therefore diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 931993c25b..95727776d3 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -62,7 +62,6 @@ #include "replication/syncrep.h" #include "replication/walreceiver.h" #include "replication/walsender.h" -#include "rewrite/rowsecurity.h" #include "storage/bufmgr.h" #include "storage/dsm_impl.h" #include "storage/standby.h" @@ -80,6 +79,7 @@ #include "utils/plancache.h" #include "utils/portal.h" #include "utils/ps_status.h" +#include "utils/rls.h" #include "utils/snapmgr.h" #include "utils/tzparser.h" #include "utils/xml.h" diff --git a/src/backend/utils/misc/rls.c b/src/backend/utils/misc/rls.c new file mode 100644 index 0000000000..066ac21a58 --- /dev/null +++ b/src/backend/utils/misc/rls.c @@ -0,0 +1,114 @@ +/*------------------------------------------------------------------------- + * + * rls.c + * RLS-related utility functions. + * + * Portions Copyright (c) 1996-2015, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * + * IDENTIFICATION + * src/backend/utils/misc/rls.c + * + *------------------------------------------------------------------------- +*/ +#include "postgres.h" + +#include "access/htup.h" +#include "access/htup_details.h" +#include "catalog/pg_class.h" +#include "miscadmin.h" +#include "utils/acl.h" +#include "utils/elog.h" +#include "utils/rls.h" +#include "utils/syscache.h" + + +extern int check_enable_rls(Oid relid, Oid checkAsUser, bool noError); + +/* + * check_enable_rls + * + * Determine, based on the relation, row_security setting, and current role, + * if RLS is applicable to this query. RLS_NONE_ENV indicates that, while + * RLS is not to be added for this query, a change in the environment may change + * that. RLS_NONE means that RLS is not on the relation at all and therefore + * we don't need to worry about it. RLS_ENABLED means RLS should be implemented + * for the table and the plan cache needs to be invalidated if the environment + * changes. + * + * Handle checking as another role via checkAsUser (for views, etc). + * + * If noError is set to 'true' then we just return RLS_ENABLED instead of doing + * an ereport() if the user has attempted to bypass RLS and they are not + * allowed to. This allows users to check if RLS is enabled without having to + * deal with the actual error case (eg: error cases which are trying to decide + * if the user should get data from the relation back as part of the error). + */ +int +check_enable_rls(Oid relid, Oid checkAsUser, bool noError) +{ + HeapTuple tuple; + Form_pg_class classform; + bool relrowsecurity; + Oid user_id = checkAsUser ? checkAsUser : GetUserId(); + + tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); + if (!HeapTupleIsValid(tuple)) + return RLS_NONE; + + classform = (Form_pg_class) GETSTRUCT(tuple); + + relrowsecurity = classform->relrowsecurity; + + ReleaseSysCache(tuple); + + /* Nothing to do if the relation does not have RLS */ + if (!relrowsecurity) + return RLS_NONE; + + /* + * Check permissions + * + * If the relation has row level security enabled and the row_security GUC + * is off, then check if the user has rights to bypass RLS for this + * relation. Table owners can always bypass, as can any role with the + * BYPASSRLS capability. + * + * If the role is the table owner, then we bypass RLS unless row_security + * is set to 'force'. Note that superuser is always considered an owner. + * + * Return RLS_NONE_ENV to indicate that this decision depends on the + * environment (in this case, what the current values of user_id and + * row_security are). + */ + if (row_security != ROW_SECURITY_FORCE + && (pg_class_ownercheck(relid, user_id))) + return RLS_NONE_ENV; + + /* + * If the row_security GUC is 'off' then check if the user has permission + * to bypass it. Note that we have already handled the case where the user + * is the table owner above. + * + * Note that row_security is always considered 'on' when querying + * through a view or other cases where checkAsUser is true, so skip this + * if checkAsUser is in use. + */ + if (!checkAsUser && row_security == ROW_SECURITY_OFF) + { + if (has_bypassrls_privilege(user_id)) + /* OK to bypass */ + return RLS_NONE_ENV; + else + if (noError) + return RLS_ENABLED; + else + ereport(ERROR, + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE), + errmsg("insufficient privilege to bypass row security."))); + } + + /* RLS should be fully enabled for this relation. */ + return RLS_ENABLED; +} diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index b6e302fc9c..b8494a2f86 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -3508,6 +3508,7 @@ comparetup_index_btree(const SortTuple *a, const SortTuple *b, { Datum values[INDEX_MAX_KEYS]; bool isnull[INDEX_MAX_KEYS]; + char *key_desc; /* * Some rather brain-dead implementations of qsort (such as the one in @@ -3518,13 +3519,15 @@ comparetup_index_btree(const SortTuple *a, const SortTuple *b, Assert(tuple1 != tuple2); index_deform_tuple(tuple1, tupDes, values, isnull); + + key_desc = BuildIndexValueDescription(state->indexRel, values, isnull); + ereport(ERROR, (errcode(ERRCODE_UNIQUE_VIOLATION), errmsg("could not create unique index \"%s\"", RelationGetRelationName(state->indexRel)), - errdetail("Key %s is duplicated.", - BuildIndexValueDescription(state->indexRel, - values, isnull)), + key_desc ? errdetail("Key %s is duplicated.", key_desc) : + errdetail("Duplicate keys exist."), errtableconstraint(state->heapRel, RelationGetRelationName(state->indexRel)))); } diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h index 240f987a3a..8d19bfdf4a 100644 --- a/src/include/rewrite/rowsecurity.h +++ b/src/include/rewrite/rowsecurity.h @@ -34,40 +34,6 @@ typedef struct RowSecurityDesc List *policies; /* list of row security policies */ } RowSecurityDesc; -/* GUC variable */ -extern int row_security; - -/* Possible values for row_security GUC */ -typedef enum RowSecurityConfigType -{ - ROW_SECURITY_OFF, /* RLS never applied- error thrown if no priv */ - ROW_SECURITY_ON, /* normal case, RLS applied for regular users */ - ROW_SECURITY_FORCE /* RLS applied for superusers and table owners */ -} RowSecurityConfigType; - -/* - * Used by callers of check_enable_rls. - * - * RLS could be completely disabled on the tables involved in the query, - * which is the simple case, or it may depend on the current environment - * (the role which is running the query or the value of the row_security - * GUC- on, off, or force), or it might be simply enabled as usual. - * - * If RLS isn't on the table involved then RLS_NONE is returned to indicate - * that we don't need to worry about invalidating the query plan for RLS - * reasons. If RLS is on the table, but we are bypassing it for now, then - * we return RLS_NONE_ENV to indicate that, if the environment changes, - * we need to invalidate and replan. Finally, if RLS should be turned on - * for the query, then we return RLS_ENABLED, which means we also need to - * invalidate if the environment changes. - */ -enum CheckEnableRlsResult -{ - RLS_NONE, - RLS_NONE_ENV, - RLS_ENABLED -}; - typedef List *(*row_security_policy_hook_type)(CmdType cmdtype, Relation relation); @@ -76,6 +42,4 @@ extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook; extern bool prepend_row_security_policies(Query* root, RangeTblEntry* rte, int rt_index); -extern int check_enable_rls(Oid relid, Oid checkAsUser); - #endif /* ROWSECURITY_H */ diff --git a/src/include/utils/rls.h b/src/include/utils/rls.h new file mode 100644 index 0000000000..867faa05ff --- /dev/null +++ b/src/include/utils/rls.h @@ -0,0 +1,58 @@ +/*------------------------------------------------------------------------- + * + * rls.h + * Header file for Row Level Security (RLS) utility commands to be used + * with the rowsecurity feature. + * + * Copyright (c) 2007-2015, PostgreSQL Global Development Group + * + * src/include/utils/rls.h + * + *------------------------------------------------------------------------- + */ +#ifndef RLS_H +#define RLS_H + +/* GUC variable */ +extern int row_security; + +/* Possible values for row_security GUC */ +typedef enum RowSecurityConfigType +{ + ROW_SECURITY_OFF, /* RLS never applied- error thrown if no priv */ + ROW_SECURITY_ON, /* normal case, RLS applied for regular users */ + ROW_SECURITY_FORCE /* RLS applied for superusers and table owners */ +} RowSecurityConfigType; + +/* + * Used by callers of check_enable_rls. + * + * RLS could be completely disabled on the tables involved in the query, + * which is the simple case, or it may depend on the current environment + * (the role which is running the query or the value of the row_security + * GUC- on, off, or force), or it might be simply enabled as usual. + * + * If RLS isn't on the table involved then RLS_NONE is returned to indicate + * that we don't need to worry about invalidating the query plan for RLS + * reasons. If RLS is on the table, but we are bypassing it for now, then + * we return RLS_NONE_ENV to indicate that, if the environment changes, + * we need to invalidate and replan. Finally, if RLS should be turned on + * for the query, then we return RLS_ENABLED, which means we also need to + * invalidate if the environment changes. + * + * Note that RLS_ENABLED will also be returned if noError is true + * (indicating that the caller simply want to know if RLS should be applied + * for this user but doesn't want an error thrown if it is; this is used + * by other error cases where we're just trying to decide if data from the + * table should be passed back to the user or not). + */ +enum CheckEnableRlsResult +{ + RLS_NONE, + RLS_NONE_ENV, + RLS_ENABLED +}; + +extern int check_enable_rls(Oid relid, Oid checkAsUser, bool noError); + +#endif /* RLS_H */ diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/expected/privileges.out index 5359dd8536..74b0450f19 100644 --- a/src/test/regress/expected/privileges.out +++ b/src/test/regress/expected/privileges.out @@ -381,6 +381,37 @@ SELECT atest6 FROM atest6; -- ok (0 rows) COPY atest6 TO stdout; -- ok +-- check error reporting with column privs +SET SESSION AUTHORIZATION regressuser1; +CREATE TABLE t1 (c1 int, c2 int, c3 int check (c3 < 5), primary key (c1, c2)); +GRANT SELECT (c1) ON t1 TO regressuser2; +GRANT INSERT (c1, c2, c3) ON t1 TO regressuser2; +GRANT UPDATE (c1, c2, c3) ON t1 TO regressuser2; +-- seed data +INSERT INTO t1 VALUES (1, 1, 1); +INSERT INTO t1 VALUES (1, 2, 1); +INSERT INTO t1 VALUES (2, 1, 2); +INSERT INTO t1 VALUES (2, 2, 2); +INSERT INTO t1 VALUES (3, 1, 3); +SET SESSION AUTHORIZATION regressuser2; +INSERT INTO t1 (c1, c2) VALUES (1, 1); -- fail, but row not shown +ERROR: duplicate key value violates unique constraint "t1_pkey" +UPDATE t1 SET c2 = 1; -- fail, but row not shown +ERROR: duplicate key value violates unique constraint "t1_pkey" +INSERT INTO t1 (c1, c2) VALUES (null, null); -- fail, but see columns being inserted +ERROR: null value in column "c1" violates not-null constraint +DETAIL: Failing row contains (c1, c2) = (null, null). +INSERT INTO t1 (c3) VALUES (null); -- fail, but see columns being inserted or have SELECT +ERROR: null value in column "c1" violates not-null constraint +DETAIL: Failing row contains (c1, c3) = (null, null). +INSERT INTO t1 (c1) VALUES (5); -- fail, but see columns being inserted or have SELECT +ERROR: null value in column "c2" violates not-null constraint +DETAIL: Failing row contains (c1) = (5). +UPDATE t1 SET c3 = 10; -- fail, but see columns with SELECT rights, or being modified +ERROR: new row for relation "t1" violates check constraint "t1_c3_check" +DETAIL: Failing row contains (c1, c3) = (1, 10). +SET SESSION AUTHORIZATION regressuser1; +DROP TABLE t1; -- test column-level privileges when involved with DELETE SET SESSION AUTHORIZATION regressuser1; ALTER TABLE atest6 ADD COLUMN three integer; diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 1bb31326bb..21817d8b75 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -295,7 +295,6 @@ INSERT INTO document VALUES (10, 33, 1, current_user, 'hoge'); SET SESSION AUTHORIZATION rls_regress_user1; INSERT INTO document VALUES (8, 44, 1, 'rls_regress_user1', 'my third manga'); -- Must fail with unique violation, revealing presence of did we can't see ERROR: duplicate key value violates unique constraint "document_pkey" -DETAIL: Key (did)=(8) already exists. SELECT * FROM document WHERE did = 8; -- and confirm we can't see it did | cid | dlevel | dauthor | dtitle -----+-----+--------+---------+-------- @@ -1683,7 +1682,6 @@ EXPLAIN (COSTS OFF) WITH cte1 AS (SELECT * FROM t1 WHERE f_leak(b)) SELECT * FRO WITH cte1 AS (UPDATE t1 SET a = a + 1 RETURNING *) SELECT * FROM cte1; --fail ERROR: new row violates WITH CHECK OPTION for "t1" -DETAIL: Failing row contains (1, cfcd208495d565ef66e7dff9f98764da). WITH cte1 AS (UPDATE t1 SET a = a RETURNING *) SELECT * FROM cte1; --ok a | b ----+---------------------------------- @@ -1702,7 +1700,6 @@ WITH cte1 AS (UPDATE t1 SET a = a RETURNING *) SELECT * FROM cte1; --ok WITH cte1 AS (INSERT INTO t1 VALUES (21, 'Fail') RETURNING *) SELECT * FROM cte1; --fail ERROR: new row violates WITH CHECK OPTION for "t1" -DETAIL: Failing row contains (21, Fail). WITH cte1 AS (INSERT INTO t1 VALUES (20, 'Success') RETURNING *) SELECT * FROM cte1; --ok a | b ----+--------- diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/privileges.sql index a0ff953c90..f97a75a5fd 100644 --- a/src/test/regress/sql/privileges.sql +++ b/src/test/regress/sql/privileges.sql @@ -256,6 +256,31 @@ UPDATE atest5 SET one = 1; -- fail SELECT atest6 FROM atest6; -- ok COPY atest6 TO stdout; -- ok +-- check error reporting with column privs +SET SESSION AUTHORIZATION regressuser1; +CREATE TABLE t1 (c1 int, c2 int, c3 int check (c3 < 5), primary key (c1, c2)); +GRANT SELECT (c1) ON t1 TO regressuser2; +GRANT INSERT (c1, c2, c3) ON t1 TO regressuser2; +GRANT UPDATE (c1, c2, c3) ON t1 TO regressuser2; + +-- seed data +INSERT INTO t1 VALUES (1, 1, 1); +INSERT INTO t1 VALUES (1, 2, 1); +INSERT INTO t1 VALUES (2, 1, 2); +INSERT INTO t1 VALUES (2, 2, 2); +INSERT INTO t1 VALUES (3, 1, 3); + +SET SESSION AUTHORIZATION regressuser2; +INSERT INTO t1 (c1, c2) VALUES (1, 1); -- fail, but row not shown +UPDATE t1 SET c2 = 1; -- fail, but row not shown +INSERT INTO t1 (c1, c2) VALUES (null, null); -- fail, but see columns being inserted +INSERT INTO t1 (c3) VALUES (null); -- fail, but see columns being inserted or have SELECT +INSERT INTO t1 (c1) VALUES (5); -- fail, but see columns being inserted or have SELECT +UPDATE t1 SET c3 = 10; -- fail, but see columns with SELECT rights, or being modified + +SET SESSION AUTHORIZATION regressuser1; +DROP TABLE t1; + -- test column-level privileges when involved with DELETE SET SESSION AUTHORIZATION regressuser1; ALTER TABLE atest6 ADD COLUMN three integer;