Обсуждение: 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