Re: [PATCHES] [PATCH] Re: Why READ ONLY transactions?

> >> I would NOT call it a "security" provision, as it is fairly
> >> easily defeated using SET TRANSACTION.
>
> > Um, why not make it an actual full blown security feature by
> > applying the following patch?
>
> It's not intended to be a security measure, and I would strongly
> resist any attempt to make it so along the lines you propose.

Intended or not, it does work.

> I do not want to try to base real security on GUC settings. The GUC
> mechanism is not designed to be unsubvertible, it's designed to
> allow convenient administration of a bunch of settings.

I agree that permissions of objects or anything specific should remain
outside of the GUC system, however for global GUC like things, such as
the default mode of a transaction (READ ONLY/READ WRITE), this is
perfect (I think of PostgreSQL's GUC system the same way I do
FreeBSD's sysctl MIB system or Linux's /proc file system: useful for
global things, in appropriate for anything fine grained). I was
thinking about that this morning, a better name would be
"jail_read_only_transactions" as the GUC contains a user to a read
only transaction. It could be confusing in that it doesn't force a
transaction to be read only.

> In any case, we already have mechanisms for preventing specific
> users from altering data: that's what GRANT/REVOKE are for. I don't
> think anyone would have bothered with START TRANSACTION READ ONLY if
> it weren't required by the SQL spec.

Ah, but this falls on its face when you want a user who does have
write access to tables to go through a fixed procedure before opening
up the DB for write access (logging of SELECTs will always require
some goo). To prevent a user who does have write access
(INSERT/UPDATE/DELETE) to tables from modifying tables before they've
started a transaction inside of the database system (different than
BEGIN, custom function start_txn() that sets up the database for
logging), in every function, I used to have to test to see if the
txn_id was in the temp table. With the posted patch, I can ensure a
fully auditable and exceedingly secure database (except for malicious
DBAs) that prevents any kind of unlogged abuse. Here's what I'm
planning on doing in my tree:

-- Username joe is any non-dba
ALTER USER joe SET transaction_force_read_only TO TRUE;
ALTER USER joe SET default_transaction_read_only TO TRUE;
CREATE FUNCTION public.start_txn()
RETURNS BIGINT
EXTERNAL SECURITY DEFINER
AS '
-- Pulls a txn_id from a sequence and stuff value into a temp
-- table that a user doesn't have write access to. Once
-- transaction ID is stored, change the transaction from READ
-- ONLY to READ WRITE. Return the txn_id to the user.
' LANGUAGE 'plpgsql';

Before, I had to have every function that modified data test to see if
a txn_id existed in the session temp table. Now, by relying on the
transaction's mode, I only have to test for that on the tables that I
log when there is SELECT activity, which will cut the number of lines
of pl/PgSQL code by about 1200-1500 lines[1]!

At the very least, it's an easier way of guaranteeing a READ ONLY
database. Securing a database with GRANT/REVOKE can be tedious and
error prone. In the case of a PHP Web shop/hacker that hasn't a clue
about quoting data before sending queries to the backend, this is a
nice safety blanket that takes a few seconds to setup (create user
www, alter user, alter user && *poof*, www user is secure). Right
now, to secure a user, you have to REVOKE INSERT, UPDATE, DELETE on
all tables, schemas and functions running as SECURITY DEFINER that
modify data, whereas jail_read_only_transactions is a simple and
effective blanket. IMHO, this is a huge 2nd safety belt that's easy
to apply, even though you're right, people _should_ rely on
GRANT/REVOKE.... though GRANT/REVOKE doesn't work in some situations
as mentioned above.

-sc

[1] Pl/PgSQL code + surrounding white space (* >300):

PERFORM TRUE FROM [temp_tbl]...;
IF NOT FOUND THEN
RAISE EXCEPTION ''my error message'';
END IF;

--
Sean Chittenden