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)

Mon, 28 Jul 2003 18:01:13 -0400
Christopher Browne <cbbrowne@libertyrms.info>
Why READ ONLY transactions?
> >>>>    - 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
Tue, 29 Jul 2003 22:05:34 -0700
Sean Chittenden <sean@chittenden.org>
[PATCH] Re: Why READ ONLY transactions?
Sean,

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

Way nifty.   

I vote in favor of this patch (suitably documented & debugged) for 7.5.

-- 
Josh Berkus
Aglio Database Solutions
San Francisco

Wed, 30 Jul 2003 08:27:47 -0700
Josh Berkus <josh@agliodbs.com>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
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

Wed, 30 Jul 2003 11:31:55 -0400
Tom Lane <tgl@sss.pgh.pa.us>
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

Wed, 30 Jul 2003 14:10:26 -0700
Sean Chittenden <sean@chittenden.org>
Re: [PATCHES] [PATCH] Re: Why READ ONLY transactions?
> > 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:
> 
> Way nifty.   
> 
> I vote in favor of this patch (suitably documented & debugged) for 7.5.

Heh, there ain't much to debug: it's pretty straight forward.  I ran
all the use cases/syntaxes I could think of and they worked as
expected.  It's a pretty chump little ditty that I originally wrote
for the sake of the 7.4 PR, but it's proving to be quite useful here
in my tree...  though I like the name "jail_read_only_transactions"
more...  patch updated for new name.

-sc

-- 
Sean Chittenden

Wed, 30 Jul 2003 14:33:35 -0700
Sean Chittenden <sean@chittenden.org>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
> > > 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:
> > 
> > Way nifty.   
> > 
> > I vote in favor of this patch (suitably documented & debugged) for 7.5.
> 
> Heh, there ain't much to debug: it's pretty straight forward.  I ran
> all the use cases/syntaxes I could think of and they worked as
> expected.  It's a pretty chump little ditty that I originally wrote
> for the sake of the 7.4 PR, but it's proving to be quite useful here
> in my tree...  though I like the name "jail_read_only_transactions"
> more...  patch updated for new name.

Err..  and attached.  -sc

-- 
Sean Chittenden
Wed, 30 Jul 2003 14:40:05 -0700
Sean Chittenden <sean@chittenden.org>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
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

Wed, 30 Jul 2003 18:22:55 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [PATCHES] [PATCH] Re: Why READ ONLY transactions? 
> >> 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

Wed, 30 Jul 2003 16:07:59 -0700
Sean Chittenden <sean@chittenden.org>
Re: [PATCHES] [PATCH] Re: Why READ ONLY transactions?
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

Wed, 30 Jul 2003 19:29:37 -0400
Tom Lane <tgl@sss.pgh.pa.us>
Re: [PATCHES] [PATCH] Re: Why READ ONLY transactions? 
> >> 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

Wed, 30 Jul 2003 16:42:31 -0700
Sean Chittenden <sean@chittenden.org>
Re: [PATCHES] [PATCH] Re: Why READ ONLY transactions?

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

Wed, 30 Jul 2003 21:29:37 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [PATCH] Re: Why READ ONLY transactions?

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

Wed, 30 Jul 2003 21:43:22 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [PATCHES] [PATCH] Re: Why READ ONLY transactions?
Sean Chittenden writes:

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

A database is already secure from a new user by default: He cannot read or
write or create anything except temporary tables and possibly the public
schema.  Setting him "read only" isn't going to change anything, because
he still can't actually read anything.  Before he can do that, the
administrator needs to grant him SELECT privileges.  And after that,
there is still no difference between "read only" and "read write", because
the user still can't write anything.

-- 
Peter Eisentraut   peter_e@gmx.net

Thu, 31 Jul 2003 11:01:57 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY
 transactions?
Bruce Momjian writes:

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

No, it would break the SQL standard.

-- 
Peter Eisentraut   peter_e@gmx.net

Thu, 31 Jul 2003 11:02:01 +0200 (CEST)
Peter Eisentraut <peter_e@gmx.net>
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY
 transactions?

This has been saved for the 7.5 release:

	http:/momjian.postgresql.org/cgi-bin/pgpatches2

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

Sean Chittenden wrote:
> > > > 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:
> > > 
> > > Way nifty.   
> > > 
> > > I vote in favor of this patch (suitably documented & debugged) for 7.5.
> > 
> > Heh, there ain't much to debug: it's pretty straight forward.  I ran
> > all the use cases/syntaxes I could think of and they worked as
> > expected.  It's a pretty chump little ditty that I originally wrote
> > for the sake of the 7.4 PR, but it's proving to be quite useful here
> > in my tree...  though I like the name "jail_read_only_transactions"
> > more...  patch updated for new name.
> 
> Err..  and attached.  -sc
> 
> -- 
> Sean Chittenden

[ Attachment, skipping... ]

> 
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org

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

Sat, 16 Aug 2003 23:33:14 -0400 (EDT)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Sean Chittenden wrote:
> > > > 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:
> > > 
> > > Way nifty.   
> > > 
> > > I vote in favor of this patch (suitably documented & debugged) for 7.5.
> > 
> > Heh, there ain't much to debug: it's pretty straight forward.  I ran
> > all the use cases/syntaxes I could think of and they worked as
> > expected.  It's a pretty chump little ditty that I originally wrote
> > for the sake of the 7.4 PR, but it's proving to be quite useful here
> > in my tree...  though I like the name "jail_read_only_transactions"
> > more...  patch updated for new name.
> 
> Err..  and attached.  -sc

I assume this patch is to control this way of breaking out of a
read-only transaction:

	test=> START TRANSACTION READ ONLY;
	START TRANSACTION
	test=> CREATE TABLE x(y INT);
	ERROR:  transaction IS read-only
	test=> COMMIT;
	COMMIT

	test=> START TRANSACTION READ ONLY;
	START TRANSACTION
	test=> SET transaction_read_only = FALSE;
	SET
	test=> CREATE TABLE x (y INT);
	CREATE TABLE
	test=> COMMIT;
	COMMIT

This seems like a valuable feature, as others have mentioned.  However,
should it also prevent changes to default_transaction_read_only?

What is the use case for this functionality?

Seems someone could easily break out of this by doing:

	test=> START TRANSACTION READ ONLY;
	START TRANSACTION
	test=> COMMIT;
	COMMIT

	test=> START TRANSACTION;
	START TRANSACTION
	test=> CREATE TABLE x (y INT);
	CREATE TABLE

This shows that default_transaction_read_only probably has to be
restricted too by the same variable.

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

Sun, 30 Nov 2003 23:10:09 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Bruce,

> This seems like a valuable feature, as others have mentioned.  However,
> should it also prevent changes to default_transaction_read_only?
>
> What is the use case for this functionality?

I thought that this was rejected thouroughly by Tom some months ago.  He 
argued pretty strongly that READ ONLY transactions were *not* a security 
feature and that trying to make them one would work very poorly.

-- 
Josh Berkus
Aglio Database Solutions
San Francisco

Mon, 1 Dec 2003 10:59:50 -0800
Josh Berkus <josh@agliodbs.com>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Josh Berkus wrote:
> Bruce,
> 
> > This seems like a valuable feature, as others have mentioned.  However,
> > should it also prevent changes to default_transaction_read_only?
> >
> > What is the use case for this functionality?
> 
> I thought that this was rejected thouroughly by Tom some months ago.  He 
> argued pretty strongly that READ ONLY transactions were *not* a security 
> feature and that trying to make them one would work very poorly.

I remember something like that, but I thought the patch was the result
of that discussion.  Tom?

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

Mon, 1 Dec 2003 14:12:46 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Josh Berkus wrote:
>> I thought that this was rejected thouroughly by Tom some months ago.  He 
>> argued pretty strongly that READ ONLY transactions were *not* a security 
>> feature and that trying to make them one would work very poorly.

> I remember something like that, but I thought the patch was the result
> of that discussion.  Tom?

Hm, I can't find anything in the archives in which I said that.  I did
argue that using GUC to control a security feature would be a mistake:
http://archives.postgresql.org/pgsql-patches/2003-07/msg00198.php
and after watching Bruce struggle with trying to make logging-related
GUC settings secure, I think my point is pretty much proved ;-).
I don't want to see more cruft like that added to the GUC logic.

Another thing to think about is that the semantics of START TRANSACTION
READ ONLY are constrained by the SQL standard, and they are not exactly
"read only" in the traditional sense (eg, you can still create and use
temp tables).  If we go down this path, I would be unsurprised to run
into a showstopper conflict between what's needed for reasonably
"secure" behavior and what the spec dictates.  It would be less risky
to use some other approach, if we are really interested in creating
read-only users.

So I'm still of the opinion I gave in the above-mentioned thread, that
I'd rather make "read only user" be a concept driven by a flag in the
user's pg_shadow entry.

			regards, tom lane

Mon, 01 Dec 2003 14:57:05 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions? 
> > Josh Berkus wrote:
> >> I thought that this was rejected thouroughly by Tom some months ago.  He 
> >> argued pretty strongly that READ ONLY transactions were *not* a security 
> >> feature and that trying to make them one would work very poorly.
> 
> > I remember something like that, but I thought the patch was the result
> > of that discussion.  Tom?
> 
> Hm, I can't find anything in the archives in which I said that.  I
> did argue that using GUC to control a security feature would be a
> mistake:
> http://archives.postgresql.org/pgsql-patches/2003-07/msg00198.php
> and after watching Bruce struggle with trying to make
> logging-related GUC settings secure, I think my point is pretty much
> proved ;-).  I don't want to see more cruft like that added to the
> GUC logic.

http://archives.postgresql.org/pgsql-patches/2003-07/msg00204.php

Sure sounds like you said READ ONLY xacts can't be used for security.  :)

> Another thing to think about is that the semantics of START
> TRANSACTION READ ONLY are constrained by the SQL standard, and they
> are not exactly "read only" in the traditional sense (eg, you can
> still create and use temp tables).  If we go down this path, I would
> be unsurprised to run into a showstopper conflict between what's
> needed for reasonably "secure" behavior and what the spec dictates.
> It would be less risky to use some other approach, if we are really
> interested in creating read-only users.

Hence the term, "security policy."  I want read only
users/transactions, but I also need temp tables to work and for
transactions to be committed out of temp tables into the real tables
via a proc with elevated privs.  Other people who don't want to have
malicious read only users fill up the disk may want TEMP tables to be
disabled.

> So I'm still of the opinion I gave in the above-mentioned thread,
> that I'd rather make "read only user" be a concept driven by a flag
> in the user's pg_shadow entry.

I think a boolean "read only" user flag will fall well short of
letting admins finely tune the database's behavior given the example
above.  I think using ALTER USER [username] SET is a much better
mechanism for securing users than setting a boolean in pg_shadow.
Taking the boolean in pg_shadow to its extreme, we'll either get to
the point where we've got a gazillion different columns (think of how
nasty MySQL's mysql database and it's host/user/table/db is) that are
unneeded 99% of the time.  <sarcasm>To avoid that, we could get smart
and replace the single boolean value with an int4 "options" field
where we could toggle various bits to mean different things.  Bit 1
would be read only.  Bit 2 would be allow temp tables.  Then we could
teach admins to xor bits and negate bits and hope that no one makes a
mistake, thus opening up their DB to abuse because the admin made a
mistake because instead of bit 15, they flipped bit 14, nevermind that
we'll have made the assumption that every DBA has a good working
understanding of binary.</sarcasm>

The patch doesn't prevent write(2), but this tunable isn't used to
prevent writing to disk, it's meant to prevent changes to the database
by a given user.  If you want a truly read-only database (in the case
of NFS), mount the filesystem as readonly (not sure if that works, but
it'd be a useful exercise).  One step better, centralize the
postmaster's write() calls and add a level of indirection with a few
function pointers.  If the backend is in read-only mode, use a
different func that aborts the transaction.

I think Tom's big objection is the abuse of the GUC system for
maintaining this information.  Having thought about this some, I think
the GUC system is pretty well suited for this and that Tom's objection
(correct me if I'm wrong here) is that GUC has a non-hierarchical
naming structure/convention.  With a hierarchical structure, lumping
of GUC variables becomes more reasonable and the naming is more
systematic.  Instead of, "jail_read_only_transaction=true" it'd be
"security.force_readonly=true" or "transaction.readonly_always=true".

-sc

-- 
Sean Chittenden

Mon, 1 Dec 2003 15:07:01 -0800
Sean Chittenden <sean@chittenden.org>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Sean Chittenden wrote:
> I think Tom's big objection is the abuse of the GUC system for
> maintaining this information.  Having thought about this some, I think
> the GUC system is pretty well suited for this and that Tom's objection
> (correct me if I'm wrong here) is that GUC has a non-hierarchical
> naming structure/convention.  With a hierarchical structure, lumping
> of GUC variables becomes more reasonable and the naming is more
> systematic.  Instead of, "jail_read_only_transaction=true" it'd be
> "security.force_readonly=true" or "transaction.readonly_always=true".

Agreed on the usefulness of GUC.  I had trouble adding security for
logging settings not because GUC wouldn't work but because the logging
control had to hit several different variables that all had different
API's.  It had to allow _increase_ for some variables and not others.

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

Mon, 1 Dec 2003 18:10:46 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Sean Chittenden <sean@chittenden.org> writes:
> http://archives.postgresql.org/pgsql-patches/2003-07/msg00204.php
> Sure sounds like you said READ ONLY xacts can't be used for security.  :)

Better read it again then.

> I think Tom's big objection is the abuse of the GUC system for
> maintaining this information.

Check.

> Having thought about this some, I think
> the GUC system is pretty well suited for this and that Tom's objection
> (correct me if I'm wrong here) is that GUC has a non-hierarchical
> naming structure/convention.

Not in the least.  My objection to using GUC for this is that it's not
designed to be non-subvertible; rather it's designed to allow settings
to come from nearly anywhere.  To get around that, you have to kluge it
horribly.  Poster child, once again, the cruft Bruce put into the
logging settings --- not only is that ugly, but I have very little
confidence that it doesn't still have holes.  Complexity is not a virtue
in security-related code; and any security expert will tell you that
having the same code serving both security- and non-security-related
goals is a recipe for disaster.  It's too easy to break security while
you are fooling with something you think is unrelated.

			regards, tom lane

Mon, 01 Dec 2003 18:17:16 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions? 
> > http://archives.postgresql.org/pgsql-patches/2003-07/msg00204.php
> > Sure sounds like you said READ ONLY xacts can't be used for security.  :)
> 
> Better read it again then.

Okay:

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

And "strong resist" in tgl-speak means, "over my dead body, it ain't
gunna happen."  :)

> > I think Tom's big objection is the abuse of the GUC system for
> > maintaining this information.
> 
> Check.
> 
> > Having thought about this some, I think the GUC system is pretty
> > well suited for this and that Tom's objection (correct me if I'm
> > wrong here) is that GUC has a non-hierarchical naming
> > structure/convention.
> 
> Not in the least.  My objection to using GUC for this is that it's
> not designed to be non-subvertible; rather it's designed to allow
> settings to come from nearly anywhere.  To get around that, you have
> to kluge it horribly.  Poster child, once again, the cruft Bruce put
> into the logging settings --- not only is that ugly, but I have very
> little confidence that it doesn't still have holes.  Complexity is
> not a virtue in security-related code; and any security expert will
> tell you that having the same code serving both security- and
> non-security-related goals is a recipe for disaster.  It's too easy
> to break security while you are fooling with something you think is
> unrelated.

Far be it for me to disagree with your points.  Can I clarify what
you're saying then with the following statement:

"A GUC-like system that is specific for containing security related
settings would be okay, but GUC as it stands in its current
incarnation, should not (at least with any illusion of providing
security) be used for anything that is security related."

And if I'm wrong in those assertions, can you comment on how you would
do this with a tunable definition of "read only?"  And if you agree
with the above statement, do you have any thoughts on improving GUC so
that it could potentially be more secure or secure enough?  Anything
that is written in C clobbers any attempt at being secure.  What in
side of the backend do you trust?  -sc

-- 
Sean Chittenden

Mon, 1 Dec 2003 16:15:39 -0800
Sean Chittenden <sean@chittenden.org>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY transactions?
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I assume this patch is to control this way of breaking out of a
> read-only transaction:
> [...]
> This seems like a valuable feature, as others have mentioned.

Why is this feature valuable?

A "read only user" is still able to easily DOS the server, consume
arbitrary disk space[1], and prevent other users from accessing data
(using LOCK, for example). It has been a long-standing fact that
giving a user the ability to execute arbitrary SQL is a security hole;
if you plan to change that, ISTM that a lot more work is necessary.

-Neil

[1] Whether they are allowed to create temp tables or not: plenty of
other parts of the executor use temporary storage.


Tue, 02 Dec 2003 01:30:11 -0500
Neil Conway <neilc@samurai.com>
Re: [PATCH] Re: [pgsql-advocacy] Why READ ONLY