On Thu, Oct 1, 2015 at 3:42 AM, Andres Freund <and...@anarazel.de> wrote: > Yes, that what I think as well. At this point we'll already have > executed insert rls stuff on the EXCLUDED tuple: > /* > * Check any RLS INSERT WITH CHECK policies > * > * ExecWithCheckOptions() will skip any WCOs which are not of > the kind > * we are looking for at this point. > */ > if (resultRelInfo->ri_WithCheckOptions != NIL) > ExecWithCheckOptions(WCO_RLS_INSERT_CHECK, > > resultRelInfo, slot, estate); > and before executing the actual projection we also checked the existing > tuple: > ExecWithCheckOptions(WCO_RLS_CONFLICT_CHECK, resultRelInfo, > mtstate->mt_existing, > mtstate->ps.state); > > after the update triggers have, if applicable run, we run the the normal > checks there as well because it's just ExecUpdate() > if (resultRelInfo->ri_WithCheckOptions != NIL) > ExecWithCheckOptions(WCO_RLS_UPDATE_CHECK, > > resultRelInfo, slot, estate); > > so I do indeed think that there's no point in layering RLS above > EXCLUDED.
I see your point, I think. It might be a problem if we weren't already making the statement error out, but we are. However, we're checking the excluded tuple (the might-be-inserted, might-be-excluded tuple that reflects before row insert trigger effects) with WCO_RLS_INSERT_CHECK, not WCO_RLS_UPDATE_CHECK. The WCO_RLS_UPDATE_CHECK applies to the tuple to be appended to the relation (the tuple that an UPDATE makes supersede some existing tuple, a new row version). We all seem to be in agreement that excluded.* ought to be subject to column-level privilege enforcement, mostly due to possible leaks with before row insert triggers (these could be SoD; a malicious UPSERT could be written a certain way). None of the checks in the code above are the exact RLS equivalent of the principle we have for column privileges, AFAICT, because update-applicable policies (everything but insert-applicable policies, actually) are not checked against the excluded tuple. Shouldn't select-applicable policies also be applied to the excluded tuples, just as with UPDATE ... FROM "join from" tables, which excluded is kinda similar to? I'm not trying to be pedantic; I just don't grok the underlying principles here. Couldn't a malicious WHERE clause leak the excluded.* tuple contents (and cause the UPDATE to not proceed) before the WCO_RLS_CONFLICT_CHECK call site was reached, while also preventing it from ever actually being reached (with a malicious function that returns false after stashing excluded.* elsewhere)? You can put volatile functions in UPDATE WHERE clauses, even if it is generally a bad idea. Perhaps I'm simply not following you here, though. I think that this is one challenge with having per-command policies with a system that checks permissions dynamically (not during parse analysis). Note that I'm not defending the status quo of the master branch -- I'm just a little uneasy about what the ideal, least surprising behavior is here. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers