Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions? - Mailing list pgsql-patches

From Sean Chittenden
Subject Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Date
Msg-id 20030730211026.GB34647@perrin.int.nxad.com
Whole thread Raw
In response to Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
List pgsql-patches
> >> 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

pgsql-patches by date:

Previous
From: Tom Lane
Date:
Subject: Re: array expression NULL fix [was: [HACKERS] odd behavior/possible bug]
Next
From: Sean Chittenden
Date:
Subject: Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?