2018-08-23 10:17 GMT+02:00 Fabien COELHO <coe...@cri.ensmp.fr>: > > Hello Pavel, > > 2. holding some session based informations, that can be used in security >> definer functions. >> > > Hmmm, I see our disagreement. My point is that this feature is *NOT* fit > for security-related uses because if the transaction fails the variable > would keep the value it had if the transaction had not failed... > > 3. Because it is not transactional, then it allows write operation on read >> > > It is not transactional safe, but it is secure in sense a possibility to >> set a access rights. >> > > This is a misleading play on words. It is secure wrt to access right, but > unsecure wrt security purposes which is the only point for having such a > feature in the first place. > > I understand, so some patterns are not possible, but when you need hold >> some keys per session, then this simply solution can be good enough. >> > > Security vs "good enough in some cases" looks bad to me. >
We don't find a agreement, because you are concentrated on transation, me on session. And we have different expectations. > I think it is possible for some more complex patterns, >> > > I'm not sure of any pattern which would be correct wrt security if it > depends on the success of a transaction. > > but then developer should be smarter, and should to enocode state result >> to content of variable. >> > > I do not see how the developer can be smarter if they need a transactional > for security but they do not have it. > > There is strong benefit - read write access to variables is very cheap and >> fast. >> > > I'd say that PostgreSQL is about "ACID & security" first, not "cheap & > fast" first. > > I invite any patch to doc (or everywhere) with explanation and about >> possible risks. >> > > Hmmm... You are the one proposing the feature... > > Here is something, thanks for adjusting it to the syntax you are proposing > and inserting it where appropriate. Possibly in the corresponding CREATE > doc? > > """ > <caution> > <par> > Beware that session variables are not transactional. > This is a concern in a security context where the variable must be set to > some trusted value depending on the success of the transaction: > if the transaction fails, the variable keeps its trusted value unduly. > </par> > > <par> > For instance, the following pattern does NOT work: > > <programlisting> > CREATE USER auditer; > SET ROLE auditer; > CREATE SESSION VARIABLE is_audited BOOLEAN DEFAULT FALSE ...; > -- ensure that only "auditer" can write "is_audited": > REVOKE ... ON SESSION VARIABLE is_audited FROM ...; > > -- create an audit function > CREATE FUNCTION audit_session(...) SECURITY DEFINER AS $$ > -- record the session and checks in some place... > -- then tell it was done: > LET is_audited = TRUE; > $$; > > -- the intention is that other security definier functions can check that > -- the session is audited by checking on "is_audited", eg: > CREATE FUNCTION only_for_audited(...) SECURITY DEFINER AS $$ > IF NOT is_audited THEN RAISE "security error"; > -- do protected stuff here. > $$; > </programlisting> > > The above pattern can be attacked with the following approach: > <programlisting> > BEGIN; > SELECT audit_session(...); > -- success, "is_audited" is set... > ROLLBACK; > -- the audit login has been reverted, but "is_audited" retains its value. > > -- any subsequent operation believes wrongly that the session is audited, > -- but its logging has really been removed by the ROLLBACK. > > -- ok but should not: > SELECT only_for_audited(...); > </programlisting> > </par> > </caution> > """ > > It is good example of not supported pattern. It is not designed for this. I'll merge this doc. Note: I am not sure, if I have all relations to described issue, but if I understand well, then solution can be reset on transaction end, maybe reset on rollback. This is solvable, I'll look how it is complex. > > For the record, I'm "-1" on this feature as proposed, for what it's worth, > because of the misleading security implications. This feature would just > help people have their security wrong. > I respect your opinion - and I hope so integration of your proposed doc is good warning for users that would to use not transactional variable like transactional source. Regards Pavel > > -- > Fabien. > >