Thread: Why READ ONLY transactions?

Robert Treat wrote:
> On Sat, 2003-07-26 at 21:31, Gavin Sherry wrote:
>>>>    - Read only transactions, bringing a greater level of security to web and
>>>>      enterprise applications by protecting data from modification.

>> This should be removed. Even though I added it to the press release,
>> I've just realised it's not really a security measure against SQL
>> injection since injected code can just specify 'SET TRANSACTION READ
>> WRITE'. We should still mention it, but not as a security measure.

> Aside from spec compliance, whats the bonus for having it then? Or put
> a better way, why/when would I want to use this?

If I am writing a "report program" that isn't supposed to do any
updates to anything, then I would be quite happy to set things to
READ-ONLY as it means that I won't _accidentally_ do updates.

It's like adding a pair of suspenders to your wardrobe.  You can
_always_, if you really try, get your pants to fall down, but this
provides some protection.

I would NOT call it a "security" provision, as it is fairly easily
defeated using SET TRANSACTION.

But it's a nice discipline...

We can tell people:

 "When you are writing report programs, start them off by setting
 transactions to be READ-ONLY.  That means that you won't modify data
 by accident."

That's a useful sort of "Best Practice," for those organizations that
are into that sort of thing.
--
let name="cbbrowne" and tld="libertyrms.info" in name ^ "@" ^ tld;;
<http://dev6.int.libertyrms.com/>
Christopher Browne
(416) 646 3304 x124 (land)

> >>>>    - Read only transactions, bringing a greater level of
> >>>>    security to web and enterprise applications by protecting
> >>>>    data from modification.
>
> >> This should be removed. Even though I added it to the press
> >> release, I've just realised it's not really a security measure
> >> against SQL injection since injected code can just specify 'SET
> >> TRANSACTION READ WRITE'. We should still mention it, but not as a
> >> security measure.
>
> > Aside from spec compliance, whats the bonus for having it then? Or
> > put a better way, why/when would I want to use this?
>
> If I am writing a "report program" that isn't supposed to do any
> updates to anything, then I would be quite happy to set things to
> READ-ONLY as it means that I won't _accidentally_ do updates.
>
> It's like adding a pair of suspenders to your wardrobe.  You can
> _always_, if you really try, get your pants to fall down, but this
> provides some protection.
>
> 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?  This gives PostgreSQL real read only
transactions that users can't escape from.  Notes about the patch:

*) If the GUC transaction_force_read_only is set to FALSE, nothing
   changes in PostgreSQL's behavior.  The default is FALSE, letting
   users change from READ ONLY to READ WRITE at will.

*) If transaction_force_read_only is TRUE, this sandboxes the
   connection for the remainder of the connection if the session is
   set to read only.  The following bits apply:

   a) if you're a super user, you can change transaction_read_only.

   b) if you're not a super user, you can change transaction_read_only
      to true.

   c) if you're not a super user, you can always change
      transaction_read_only from false to true.  If
      transaction_force_read_only is true, you can't change
      transaction_read_only from true to false.

   d) If transaction_force_read_only is TRUE, but
      transaction_read_only is FALSE, the transaction is still READ
      WRITE.

   e) Only super users can change transaction_force_read_only.


Basically, if you want to permanently prevent a user from ever being
able to get in a non-read only transaction, do:

\c [dbname] [db_superuser]
BEGIN;
ALTER USER test SET default_transaction_read_only TO TRUE;
ALTER USER test SET transaction_force_read_only TO TRUE;
COMMIT;

-- To test:
regression=# \c regression test
regression=> SHOW transaction_read_only;
 transaction_read_only
-----------------------
 on
(1 row)

regression=> SHOW transaction_force_read_only;
 transaction_force_read_only
-----------------------------
 on
(1 row)

regression=> SET transaction_read_only TO FALSE;
ERROR:  Insufficient privileges to SET transaction_read_only TO FALSE


It's also possible to set transaction_force_read_only in
postgresql.conf making it possible to create read only databases to
non-superusers by starting postgresql with
default_transaction_read_only and transaction_force_read_only set to
TRUE.  If this patch is well received, I'll quickly bang out some
documentation for this new GUC.  From a security stand point, this is
a nifty feature.  -sc

--
Sean Chittenden

Sean Chittenden <sean@chittenden.org> writes:
>> 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.  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.

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.

            regards, tom lane

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

Sean Chittenden <sean@chittenden.org> writes:
>> 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.

No, you just haven't thought of a way to get around it yet.  When you do
think of one, you'll be wanting us to contort the GUC system to plug the
loophole.  We've already got a horrid mess in there for the LOG_XXX
variables, and I don't want to add more.

I'm not objecting to the idea of being able to make users read-only.
I'm objecting to using GUC for it.  Send in a patch that, say, adds a
bool column to pg_shadow, and I'll be happy.

            regards, tom lane

> >> 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.
>
> No, you just haven't thought of a way to get around it yet.  When
> you do think of one, you'll be wanting us to contort the GUC system
> to plug the loophole.  We've already got a horrid mess in there for
> the LOG_XXX variables, and I don't want to add more.
>
> I'm not objecting to the idea of being able to make users read-only.
> I'm objecting to using GUC for it.  Send in a patch that, say, adds
> a bool column to pg_shadow, and I'll be happy.

How is that any different than ALTER USER [username] SET
jail_read_only_transactions TO true?  It sets something in
pg_shadow.useconfig column, which is permanent.  Ultimately, the
XactReadOnly variable is going to be used and the
assign_transaction_read_only() function in guc.c will be necessary
too.  Would a different GUC that required only one ALTER USER
statement make you happier?  If so, then how about:

ALTER USER [username] SET readonly TO TRUE;
ALTER USER [username] SET read_only TO TRUE;
ALTER USER [username] SET readonly_user TO TRUE;
ALTER USER [username] SET read_only_user TO TRUE;

When "read_only_user" is set to true, it changes
transaction_read_only, default_transaction_read_only, and
jail_read_only_transactions all to TRUE.  The read_only_user GUC does
nothing if set to FALSE and can only be set by the superuser.

If I were to add a specific syntax for this, what would you like?

ALTER USER [username] [READONLY|NOTREADONLY];

??  Use of the GUC infrastructure makes much more sense and is loads
cleaner, IMHO.

Use of GUC is also going to be much more lightweight than adding a new
syntax for making accounts read only, esp since the read only
transaction infrastructure is already GUC backed.

Is your objection that GUC doesn't scale well?  If so, it shouldn't
too hard for me to change GUC to use a hash lookup instead of a linear
scan (that'd be something I'd do for 7.5).  If it's that GUC is a flat
namespace, I'm very inclined agree that it's limited in that regard
and a full MIB tree would be preferred.  Ex:

ALTER USER [username] SET user.readonly = TRUE;
ALTER USER [username] SET user.dba = TRUE;
ALTER USER [username] SET user.create_database = TRUE;
ALTER USER [username] SET user.create_user = TRUE;
ALTER USER [username] SET user.security.ssl.require = TRUE;
ALTER USER [username] SET user.security.ssl.rsa_cert = "text cert authenticating this user";
ALTER USER [username] SET user.security.ssl.sslv2_enable = FALSE;
ALTER USER [username] SET user.security.ssl.sslv3_require = TRUE;
ALTER USER [username] SET user.schema = [schema1, schema2, schema3, public];
ALTER USER [username] SET user.security.see_other_schemas = FALSE;
ALTER USER [username] SET database.name = "some non-existent dbname";

New databases, once created, would also show up in the MIB hierarchy,
allowing things like:

ALTER USER [username] SET database.[dbname].readonly TO TRUE;

That last option, for example, would let users connect to a centrally
hosted database, but would spoof the dbname that the user would see
via CURRENT_DATABASE.  I could imagine it being useful for hosted DB
environments wherein you want to give the user the illusion of a
private database.  Same with user.security.see_other_schemas.

Where the textual OIDs would be converted to their numeric
counterparts and then stored with their value in useconfig.  Now that
PostgreSQL has slick array support (thanks Joe!), the various user
options could be stored as array elements in the
pg_catalog.pg_shadow.useconfig column.  Imagine adding a syntax for
every feature that a user could have vs. setting user features via a
GUC/MIB system.  I'd take the MIB system any day of the week and twice
on Friday given the resulting reduction of bloat to gram.y.

-sc

--
Sean Chittenden

Sean Chittenden <sean@chittenden.org> writes:
>> I'm not objecting to the idea of being able to make users read-only.
>> I'm objecting to using GUC for it.  Send in a patch that, say, adds
>> a bool column to pg_shadow, and I'll be happy.

> How is that any different than ALTER USER [username] SET
> jail_read_only_transactions TO true?  It sets something in
> pg_shadow.useconfig column, which is permanent.

But it has to go through a mechanism that is designed and built to allow
that value to be overridden from other places.  I think using GUC for
this is just asking for trouble.  Even if there is no security hole
today, it's very easy to imagine future changes in GUC that would
unintentionally create one.

            regards, tom lane

> >> I'm not objecting to the idea of being able to make users
> >> read-only.  I'm objecting to using GUC for it.  Send in a patch
> >> that, say, adds a bool column to pg_shadow, and I'll be happy.
>
> > How is that any different than ALTER USER [username] SET
> > jail_read_only_transactions TO true?  It sets something in
> > pg_shadow.useconfig column, which is permanent.
>
> But it has to go through a mechanism that is designed and built to
> allow that value to be overridden from other places.  I think using
> GUC for this is just asking for trouble.  Even if there is no
> security hole today, it's very easy to imagine future changes in GUC
> that would unintentionally create one.

*nods* Anything that goes outside of SetConfigOption(), however, is
incorrectly setting a GUC value and can't really be prevented.  At the
C level, nothing is safe and there's no way to make things 100% secure
(except for possibly by moving PostgreSQL over into protected mode).
If PostgreSQL can't trust itself, who can it trust?

If you're worried about someone setting JailReadOnlyXacts or
XactsReadOnly in a C extension, then let me hide those two variables
away in their own file, declare them static, and provide accessor
methods to the variables.  It doesn't prevent someone from changing
their values if they know the address, but it'll at least prevent
someone from #include'ing a header and mucking with things.  Would
moving things into their own files and declaring them static be a
sufficient compromise?  I'll declare the accessor functions inline
too, that way there should be zero loss of performance given
XactReadOnly is frequently used. -sc

--
Sean Chittenden

If we change default_transaction_read_only to PGC_USERLIMIT, the
administrator can turn it on and off, but an ordinary user can only turn
it on, but not off.

Would that help?

---------------------------------------------------------------------------

Sean Chittenden wrote:
-- Start of PGP signed section.
> > >>>>    - Read only transactions, bringing a greater level of
> > >>>>    security to web and enterprise applications by protecting
> > >>>>    data from modification.
> >
> > >> This should be removed. Even though I added it to the press
> > >> release, I've just realised it's not really a security measure
> > >> against SQL injection since injected code can just specify 'SET
> > >> TRANSACTION READ WRITE'. We should still mention it, but not as a
> > >> security measure.
> >
> > > Aside from spec compliance, whats the bonus for having it then? Or
> > > put a better way, why/when would I want to use this?
> >
> > If I am writing a "report program" that isn't supposed to do any
> > updates to anything, then I would be quite happy to set things to
> > READ-ONLY as it means that I won't _accidentally_ do updates.
> >
> > It's like adding a pair of suspenders to your wardrobe.  You can
> > _always_, if you really try, get your pants to fall down, but this
> > provides some protection.
> >
> > 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?  This gives PostgreSQL real read only
> transactions that users can't escape from.  Notes about the patch:
>
> *) If the GUC transaction_force_read_only is set to FALSE, nothing
>    changes in PostgreSQL's behavior.  The default is FALSE, letting
>    users change from READ ONLY to READ WRITE at will.
>
> *) If transaction_force_read_only is TRUE, this sandboxes the
>    connection for the remainder of the connection if the session is
>    set to read only.  The following bits apply:
>
>    a) if you're a super user, you can change transaction_read_only.
>
>    b) if you're not a super user, you can change transaction_read_only
>       to true.
>
>    c) if you're not a super user, you can always change
>       transaction_read_only from false to true.  If
>       transaction_force_read_only is true, you can't change
>       transaction_read_only from true to false.
>
>    d) If transaction_force_read_only is TRUE, but
>       transaction_read_only is FALSE, the transaction is still READ
>       WRITE.
>
>    e) Only super users can change transaction_force_read_only.
>
>
> Basically, if you want to permanently prevent a user from ever being
> able to get in a non-read only transaction, do:
>
> \c [dbname] [db_superuser]
> BEGIN;
> ALTER USER test SET default_transaction_read_only TO TRUE;
> ALTER USER test SET transaction_force_read_only TO TRUE;
> COMMIT;
>
> -- To test:
> regression=# \c regression test
> regression=> SHOW transaction_read_only;
>  transaction_read_only
> -----------------------
>  on
> (1 row)
>
> regression=> SHOW transaction_force_read_only;
>  transaction_force_read_only
> -----------------------------
>  on
> (1 row)
>
> regression=> SET transaction_read_only TO FALSE;
> ERROR:  Insufficient privileges to SET transaction_read_only TO FALSE
>
>
> It's also possible to set transaction_force_read_only in
> postgresql.conf making it possible to create read only databases to
> non-superusers by starting postgresql with
> default_transaction_read_only and transaction_force_read_only set to
> TRUE.  If this patch is well received, I'll quickly bang out some
> documentation for this new GUC.  From a security stand point, this is
> a nifty feature.  -sc
>
> --
> Sean Chittenden

[ Attachment, skipping... ]
-- End of PGP section, PGP failed!

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Tom, have you considered using PGC_USERLIMIT for the existing
default_transaction_read_only variable?  You could allow admins to turn
it on and off, but non-admins could only turn it on.

---------------------------------------------------------------------------

Tom Lane wrote:
> Sean Chittenden <sean@chittenden.org> writes:
> >> I'm not objecting to the idea of being able to make users read-only.
> >> I'm objecting to using GUC for it.  Send in a patch that, say, adds
> >> a bool column to pg_shadow, and I'll be happy.
>
> > How is that any different than ALTER USER [username] SET
> > jail_read_only_transactions TO true?  It sets something in
> > pg_shadow.useconfig column, which is permanent.
>
> But it has to go through a mechanism that is designed and built to allow
> that value to be overridden from other places.  I think using GUC for
> this is just asking for trouble.  Even if there is no security hole
> today, it's very easy to imagine future changes in GUC that would
> unintentionally create one.
>
>             regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
>       subscribe-nomail command to majordomo@postgresql.org so that your
>       message can get through to the mailing list cleanly
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073