Re: [PATCHES] [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 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] [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 [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] [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. sarcasmTo 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 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [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 [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] [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 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] Why READ ONLY
Bruce Momjian [EMAIL PROTECTED] 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. ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] 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 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] 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 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] [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. -sc -- Sean Chittenden ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] 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 ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] 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 [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] [PATCH] Re: [pgsql-advocacy] 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 [EMAIL PROTECTED] 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 [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup.| Newtown Square, Pennsylvania 19073 ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
