Re: [HACKERS] proposal: schema variables - Mailing list pgsql-hackers

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.

> 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>
"""


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.


-- 
Fabien.