Re: [HACKERS] Protection from SQL injection
On Tue, 2008-04-29 at 16:01 -0400, Aidan Van Dyk wrote: Most of my published applications *are* simple, and I tend to consolidate as much of my business logic in the database as possible and a known set of queries shared by all the related apps, relying heavily on view, triggers, and functions, so the queries in my web-side and C-side applications really are very simple and straight forward. I a company I worked, we got ( almost ? ) the same result by doing all access using functions and REVOKE-ing frontend app users all privileges on anything else. So almost all sql issued by apps looks like SELECT * FROM some_func(p1, p2, ..., pn) This has a lot of nice properties, among others ability to do lots of database code fixing on live 27/4 apps without frontends never noticing. I purposely choose to have simple static queries in my apps. So a mode which rejects queries with literals/constants in them would catch bugs in my code. Hmm - maybe a mode where functions accept only parameters would be needed for enforcing this on current server code. Anyway, with pl/proxy partitioning/loadbalancing running on data-empty servers, code injection would be quite hard even without params-only mode. Those bugs really could be cosmetic, and still valid SQL queries, but one of them could be a valid one which could be an injection vector. Could we also get a mode, where PREPARE would only be allowed for queries of the form SELECT * FROM func(?,?,?,?,?); :) - Hannu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches
I updated the series of SE-PostgreSQL patches for the latest pgsql-8.4devel tree. [1/4] sepostgresql-pgace-8.4devel-3-r739.patch provides PGACE (PostgreSQL Access Control Extension) framework. http://sepgsql.googlecode.com/files/sepostgresql-pgace-8.4devel-3-r739.patch [2/4] sepostgresql-sepgsql-8.4devel-3-r739.patch provides SE-PostgreSQL feature, based on PGACE framework. http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r739.patch [3/4] sepostgresql-pg_dump-8.4devel-3-r739.patch enables to dump databases with security attribute. http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r739.patch [4/4] sepostgresql-policy-8.4devel-3-r739.patch provides the default security policy of SE-PostgreSQL. http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r739.patch We provide a quick overview for SE-PostgreSQL at: http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL Thanks, KaiGai Kohei wrote: The series of patches are the proposal of Security-Enhanced PostgreSQL (SE-PostgreSQL) for the upstreamed PostgreSQL 8.4 development cycle. [1/4] sepostgresql-pgace-8.4devel-3.patch provides PGACE (PostgreSQL Access Control Extension) framework http://sepgsql.googlecode.com/files/sepostgresql-pgace-8.4devel-3-r704.patch [2/4] sepostgresql-sepgsql-8.4devel-3.patch provides SE-PostgreSQL feature, based on PGACE framework. http://sepgsql.googlecode.com/files/sepostgresql-sepgsql-8.4devel-3-r704.patch [3/4] sepostgresql-pg_dump-8.4devel-3.patch enables pg_dump to dump database with security attribute. http://sepgsql.googlecode.com/files/sepostgresql-pg_dump-8.4devel-3-r704.patch [4/4] sepostgresql-policy-8.4devel-3.patch provides the default security policy for SE-PostgreSQL. http://sepgsql.googlecode.com/files/sepostgresql-policy-8.4devel-3-r704.patch We can provide a quick overview for SE-PostgreSQL at: http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL ENVIRONMENT --- Please confirm your environment. The followings are requriements of SE-PostgreSQL. * Fedora 8 or later system * SELinux is enabled and working * kernel-2.6.24 or later * selinux-policy and selinux-policy-devel v3.0.8 or later * libselinux, policycoreutils INSTALLATION $ tar jxvf postgresql-snapshot.tar.bz2 $ cd postgresql-snapshot $ patch -p1 ../sepostgresql-pgace-8.4devel-3.patch $ patch -p1 ../sepostgresql-sepgsql-8.4devel-3.patch $ patch -p1 ../sepostgresql-pg_dump-8.4devel-3.patch $ patch -p1 ../sepostgresql-policy-8.4devel-3.patch $ ./configure --enable-selinux $ make $ make -C contrib/sepgsql-policy $ su # make install # /usr/sbin/semodule -i contrib/sepgsql-policy/sepostgresql.pp (NOTE: semodule is a utility to load/unload security policy modules.) # /sbin/restorecon -R /usr/local/pgsql (NOTE: restorecon is a utilicy to initialize security context of files.) SETUP - # mkdir -p /opt/sepgsql # chown foo_user:var_group /opt/sepgsql # chcon -t postgresql_db_t /opt/sepgsql (NOTE: chcon is a utility to set up security context of files.) # exit $ /usr/sbin/run_init /usr/local/pgsql/bin/initdb -D /opt/sepgsql (NOTE: run_init is a utility to start a program, as if it is branched from init script.) $ /usr/local/pgsql/bin/pg_ctl -D /opt/sepgsql start SUMMARYS FOR EVERY PATCHES -- [1/4] - sepostgresql-pgace-8.4devel-3.patch This patch provides PGACE (PostgreSQL Access Control Extension) framework. It has a similar idea of LSM (Linu Security Module). It can provide a guest module several hooks at strategic points. The guest module can make its decision whether required actions should be allowed, or not. In addition, PGACE also provides falicilites to manage security attribute of database objects. Any tuple can have a its security attribute, and the guest module can refer it to control accesses. A more conprehensive memo at: http://code.google.com/p/sepgsql/wiki/WhatIsPGACE [2/4] - sepostgresql-sepgsql-8.4devel-3.patch This patch provides SE-PostgreSQL facilities based on PGACE. Security-Enhanced PostgreSQL (SE-PostgreSQL) is a security extension built in PostgreSQL, to provide system-wide consistency in access controls. It enables to apply a single unigied security policy of SELinux for both operating system and database management system. In addition, it also provides fine-grained mandatory access which includes column-/row- level non-bypassable access control even if privileged database users. Quick overview at: http://code.google.com/p/sepgsql/wiki/WhatIsSEPostgreSQL [3/4] - sepostgresql-pg_dump-8.4devel-3.patch This patch gives us a feature to dump database with security attribute. It is turned on with '--enable-selinux' option at pg_dump/pg_dumpall, when the server works as SE-
[HACKERS] pg_standby issue
I just saw this issue... so though of reporting it before I have to get rid of the environment. I `touch`d the trigger file and I saw the following message: Trigger file: /tmp/pg_standby.trigger.5444 Waiting for WAL file: 0001000B00F2 WAL file path : ../wal_archive//0001000B00F2 Restoring to... : pg_xlog/RECOVERYXLOG Sleep interval : 5 seconds Max wait interval : 0 forever Command for restore : cp ../wal_archive//0001000B00F2 pg_xlog/RECOVERYXLOG WAL file not present yet. Checking for trigger file...Keep archive history : 000100F1 and later running restore : OK Trigger file: /tmp/pg_standby.trigger.5444 Waiting for WAL file: 0001000B00F3 WAL file path : ../wal_archive//0001000B00F3 Restoring to... : pg_xlog/RECOVERYXLOG Sleep interval : 5 seconds Max wait interval : 0 forever Command for restore : cp ../wal_archive//0001000B00F3 pg_xlog/RECOVERYXLOG WAL file not present yet. Checking for trigger file... WAL file not present yet. Checking for trigger file... ~4 repetitions WAL file not present yet. Checking for trigger file... WAL file not present yet. Checking for trigger file... WAL file not present yet. Checking for trigger file...trigger file found Trigger file: /tmp/pg_standby.trigger.5444 Waiting for WAL file: 0001000B00F3 WAL file path : ../wal_archive//0001000B00F3 Restoring to... : pg_xlog/RECOVERYXLOG Sleep interval : 5 seconds Max wait interval : 0 forever Command for restore : cp ../wal_archive//0001000B00F3 pg_xlog/RECOVERYXLOG WAL file not present yet. Checking for trigger file... WAL file not present yet. Checking for trigger file... Here's the background info: This is a EnterpriseDB 8.2 installation (not very different from Postgres, architecturally (as if you didn't know that :) ) ). So I had got the pg_standby code from PG version 8.3 and got it compiled it under the 8.2 environment. Set up a hot standby using the following restore_command: restore_command = 'pg_standby -c -d -s 5 -w 0 -t /tmp/pg_standby.trigger.5444 ../wal_archive/ %f %p `perl pg_last_restart_point.pl` 2 pg_standby.log' Where pg_last_restart_point.pl is the same script I posted here a while back. The standby server is running on the same box as the master DB. This was just a test setup, I had done for testing, a few days ago. There hasn't been much activity (no activity on master DB): wc -l pg_standby.log 489122 pg_standby.log uniq pg_standby.log | wc -l 80 As you can see, before and after the 'trigger file found' message, pg_standby is waiting for the same file! The standby server is still running, waiting for pg_standby to return. If somebody wants to take a shot at debugging the issue, I'll leave the environment as it is for a few hours more before I try to touch the trigger file again... Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com EnterpriseDB http://www.enterprisedb.com Mail sent from my BlackLaptop device
Re: [HACKERS] Proposed patch - psql wraps at window width
Bruce Momjian [EMAIL PROTECTED] writes: Peter Eisentraut wrote: Am Dienstag, 29. April 2008 schrieb Bruce Momjian: Peter Eisentraut wrote: Am Dienstag, 29. April 2008 schrieb Bruce Momjian: We do look at COLUMNS if the ioctl() fails, but not for file/pipe output. This is quite a useless complication. Readline uses exactly the same ioctl() call to determine the columns, so if ioctl() were to fail, then COLUMNS would be unset or wrong as well. I was thinking about Win32 or binaries that don't have readline. These rules don't seem very consistent. You are mixing platform dependencies, build options, theoretical, unproven failures of kernel calls, none of which have anything to do with each other. For example, if readline weren't installed, then there would be no one who sets COLUMNS, so why look at it? If you want to allow users to set COLUMNS manually (possibly useful, see Greg Stark's arguments), then it should have priority over ioctl(), not the other way around. OK, two people like it, no one has objected. :-) I will work on making those changes. Thanks. Uh, precisely what changes are you referring to now? These rules don't seem very consistent doesn't sound like no one has objected to me. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's PostGIS support! -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Column storage positions
On Mon, Apr 2, 2007 at 2:44 AM, Phil Currier [EMAIL PROTECTED] wrote: On 4/1/07, Guillaume Smet [EMAIL PROTECTED] wrote: Phil, did you make any progress with your patch? Your results seemed very encouraging and your implementation interesting. IIRC, the problem was that you weren't interested in working on the visual/mysqlish column ordering. As the plan was to decouple column ordering in three different orderings, I don't think it's really a problem if your implementation doesn't support one of them (at least if it doesn't prevent us from having the visual one someday). I haven't done much with it since February, largely because my available free time evaporated. But I do intend to get back to it when I have a chance. But you're right, the storage position stuff I've worked on is completely independent from display positions, and certainly wouldn't prevent that being added separately. Is there any chance you keep us posted with your progress and post a preliminary patch exposing your design choices? This could allow other people to see if there are interesting results with their particular database and workload. Yeah, I'll try to clean things up and post a patch eventually. And if anyone feels like working on the display position piece, let me know; perhaps we could pool our efforts for 8.4. Hi Phil, Did you make any progress on this cleanup? It seems like a good timing to revive this project if we want it for 8.4. Thanks for your feedback. -- Guillaume -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protection from SQL injection
On Tue, 29 Apr 2008, Josh Berkus wrote: Did you guys miss Tom's comment up-thread? Postgres already does this if you use PQExecParams(). Keen. Now we just need to get the driver developers to implement it. I imagine Java does. The JDBC driver takes a multi-command statement and splits it up to be able to use the extended query protocol. So the JDBC driver is actually doing the reverse of your suggestion. For us it was a decision to ease the transition from V2 to V3 protocol and not break code that used to work. Kris Jurka -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposed patch - psql wraps at window width
This patch is blocking other work -- for instance, the PrintTable API and two patches that depend on it. Could we get the main hunks committed soon, with the policy bits stripped out? That way, discussion on the behavior can continue until we reach an agreement, and others can work on other patches. (Policy bits stripped out could mean selecting the wrapped behavior only explicitly and selecting the column width only on a \pset variable. All the hard bits about when to use $COLUMNS and the ioctl(), which are the controversial parts, can be deferred and are pretty localized changes AFAICS.) -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposed patch - psql wraps at window width
Peter Eisentraut wrote: Am Dienstag, 29. April 2008 schrieb Bruce Momjian: Peter Eisentraut wrote: Am Dienstag, 29. April 2008 schrieb Bruce Momjian: We do look at COLUMNS if the ioctl() fails, but not for file/pipe output. This is quite a useless complication. Readline uses exactly the same ioctl() call to determine the columns, so if ioctl() were to fail, then COLUMNS would be unset or wrong as well. I was thinking about Win32 or binaries that don't have readline. These rules don't seem very consistent. You are mixing platform dependencies, build options, theoretical, unproven failures of kernel calls, none of which have anything to do with each other. For example, if readline weren't installed, then there would be no one who sets COLUMNS, so why look at it? If you want to allow users to set COLUMNS manually (possibly useful, see Greg Stark's arguments), then it should have priority over ioctl(), not the other way around. OK, two people like it, no one has objected. :-) I will work on making those changes. Thanks. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protection from SQL injection
On Tue, Apr 29, 2008 at 09:02:30PM -0400, Gregory Stark wrote: Did you guys miss Tom's comment up-thread? Postgres already does this if you use PQExecParams(). I did, yes. Thanks for the clue. OTOH, I do see the OP's point that it'd be nice if the DBA could enforce this rule. Maybe a way of insisting on PQExecParams() instead of anything else? A -- Andrew Sullivan [EMAIL PROTECTED] +1 503 667 4564 x104 http://www.commandprompt.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protection from SQL injection
Hi, How many people are using literals in Java? Not sure if I understood the question... In Java most people use constants (final static). 'Checkstyle' can find 'magic numbers' in the source code. If the constants feature was very important in SQL, people would have requested it, and it would be in the SQL standard by now. There is a workaround: user defined functions. Disabling multi-statement commands Disabling multi-statement commands just limits the effect of SQL injection. Disabling literals actually protects from SQL injection. Both features are important. ( almost ? ) the same result by doing all access using functions This also doesn't protect from SQL injection, it only limits the effect. Half a security measure is almost always worse than none at all Cars and houses have locks. Locks can't fully protect you. Do they give the illusion security? Maybe. But it's definitely better to have them. headlines: New PostgreSQL feature breaks 99% applications Not if it's disabled by default. What about New PostgreSQL feature offers 95% protection from SQL injection? The developers and admins who know about this feature and want to use it... quality produced by this ppl is higher than average and less likely to have such basic faults. Maybe. I found some problems in my code when enabling this feature, and I thought I was save (or paranoid :-). Regards, Thomas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposed patch - psql wraps at window width
Alvaro Herrera [EMAIL PROTECTED] writes: This patch is blocking other work -- for instance, the PrintTable API and two patches that depend on it. Could we get the main hunks committed soon, with the policy bits stripped out? That way, discussion on the behavior can continue until we reach an agreement, and others can work on other patches. This patch seems sufficiently controversial that commit now is the very last thing that should happen to it. I suggest committing the PrintTable stuff and not worrying about whether that breaks the wrap patch. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protection from SQL injection
Andrew Dunstan wrote: Tino Wildenhain wrote: Hi, In C the best practice is to use #define for constants. In C++ you have 'const', in Java 'static final'. Unfortunately the 'named constant' concept doesn't exist in SQL. I think that's a mistake. I suggest to support CREATE CONSTANT ... VALUE ... and DROP CONSTANT ..., example: CREATE CONSTANT STATE_ACTIVE VALUE 'active'. of course you mean: CREATE CONSTANT state_active TEXT VALUE 'active'; ? ;) Why does he mean that? Manifest constants are not typed in plenty of languages. Well but in this case we want them to prevent easy sql injection and therefore arbitrary macro expansion like in those plenty of languages does not seem like a good idea to me. Cheers Tino -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protection from SQL injection
Could we also get a mode, where PREPARE would only be allowed for queries of the form SELECT * FROM func(?,?,?,?,?); :) Actually, that is similar to the concept of global prepared statements that I proposed some time ago, but I will not have time to write the patch, alas... Idea was that the DBA can create a list of SQL statements (with privileges about who can execute them, just like functions) which are prepared on-demand at the first EXECUTE by the client. This would enhance performance (but for performance I like the idea of caching plans better). It would be pretty cumbersome, though, to execute dynamic SQL like the typical search query... -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protection from SQL injection
Hi, Constants are just convenience: instead of constants, user defined functions can be used. This already works, however it's a bit verbose: CREATE FUNCTION STATE_ACTIVE() RETURNS VARCHAR AS $$ BEGIN RETURN 'active'; END; $$ LANGUAGE PLPGSQL; Usage is almost the same: SELECT * FROM USERS WHERE STATE=STATE_ACTIVE(); therefore arbitrary macro expansion like in those plenty of languages does not seem like a good idea to me. This is _not_ macro expansion as in C '#define'. Constants are typed, as in C++ 'const' and Java 'static final'. The question is only: should the user explicitly state the data type, or should the data type be deduced from the value. Both is possible: CREATE CONSTANT STATE_ACTIVE VALUE 'active'; CREATE CONSTANT STATE_ACTIVE TEXT VALUE 'active'; Regards, Thomas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposed patch - psql wraps at window width
Tom Lane wrote: This patch seems sufficiently controversial that commit now is the very last thing that should happen to it. I suggest committing the PrintTable stuff and not worrying about whether that breaks the wrap patch. regards, tom lane\ AFIK, the only thing that's controversial about the patch is how to turn it on and off -- the actual core code appears to be inflaming no passions. And it's the core code that presents merge issues. Could it be committed with a hidden enable syntax, for the interim? Or even no enable syntax, just to have it in the code base? \pset format wrap-beta-test on -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposed patch - psql wraps at window width
Bryce Nesbitt [EMAIL PROTECTED] writes: AFIK, the only thing that's controversial about the patch is how to turn it on and off -- the actual core code appears to be inflaming no passions. And it's the core code that presents merge issues. Well, personally I haven't read the core code yet, since it's not commit fest yet ;-). I don't know whether there are any issues there, but it wouldn't surprise me given the number of issues in the control code. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Internal design of MERGE, with Rules
MERGE looks like it may need some new infrastructure to support it,
depending upon the implementation route. Guidance and discussion
requested from main hackers, if possible.
This is a separate post because there's no further discussion here on
the external behaviour of MERGE, or its concurrency/locking.
Assumptions
---
* MERGE must be a parameterisable, preparable command. The parameters
might be anywhere in the statement i.e. in the USING clause, or in the
action statements, or both.
* MERGE must work like any other statement, e.g. multi-statement
requests must work also e.g. UPDATE...; MERGE ... ;INSERT ...
* MERGE works on tables only, not views. PostgreSQL doesn't support
updateable views, so this isn't a problem to solve as part of this
implementation.
* MERGE contains possibly multiple INSERTs, UPDATEs and DELETEs, each of
which can invoke a rule to generate even more commands.
* We would not add MERGE to the list of possible events that invoke a
Rule. (The data changing events would remain as INSERT, UPDATE, DELETE).
However, a MERGE statement might be used *within* a Rule, giving a tree
of plans rather than a single plan or a list of plans. (I can't see any
way or any reason to say no allowed to that...)
Implications
To fully support prepared queries, MERGE needs to be a query not a
utility command as I had originally envisaged. MERGE internally consists
of one driving query that is always executed in full to retrieve all
tuples generated, plus a number of other queries that may or may not get
executed according to the evaluation of the WHEN clauses.
Rule re-writing happens at the plan level. That means we need to
recognise MERGE as being a new form of query: a query that has multiple
other optional queries buried within it, each of which can be
independently re-written. Only by doing that can we apply the rules
correctly to the various parts of MERGE.
That means we probably need to introduce new infrastructure in the tcop
or executor modules to handle queries-within-queries. This isn't
special-casing MERGE so much as introducing infrastructure for a new
class of query, such as MERGE, REPLACE, INSERT ELSE UPDATE. (Merge
itself does cover almost all cases of this type of query, but we'd be
able to fairly easily support all of the different syntax).
MERGE would then be represented by a query that has many side
queries (so called so we don't confused calling them sub-queries). Each
side query can be rewritten into a list of queries. Any of those queries
could be a MERGE itself, so we might make each Query a tree of Queries.
The main query and the side queries are related, in that the values used
for the side queries come from the main query. The side queries'
external references will be replaced by parameters, in addition to other
parameters already supplied. So the side queries are parameterised, even
if the MERGE statement wasn't parameterised. The main driving query will
be executed once, the side queries once per row (if at all).
Let's look back at the infrastructure aspects. My options so far for
implementation are
0. Write MERGE in PL/pgSQL. Err, no.
1. We treat the MERGE similar to an Update-with-conditional-effects and
have the MERGE's main plan deliver tuples that we then act on during
ExecutePlan() using a new ExecMerge() function. We would preserve the
text of the side queries and execute them using SPI from within
ExecMerge(). That takes care of the rule rewriting, since we effectively
don't know what's happening inside SPI. Feels ugly, is all I can say,
though actually very similar to the way RI works.
2. Similar to (1), but instead of using SPI we start another portal from
within ExecMerge() function. Portals within portals. Seems very likely
for something to break in an unexpected way.
3. We treat a MERGE as a new kind of portal query. Any query containing
a MERGE will be forced to be a PORTAL_MULTI_QUERY. We then introduce a
new query processing routine for MERGE, ProcessQueryWithActions() rather
than running ProcessQuery() during PortalRunMulti(). This can then
execute the merge-query similarly to a cursor, retrieving tuples that
are then fed to the side queries that then run in separate portals.
Doing it this way will mean that we don't need to run the executor
recursively, we just switch from main query to retrieve next row and
then execute the appropriate side query. The executor level calls will
look almost exactly as it would if we wrote MERGE in PL/pgSQL - fetching
from a main cursor and then executing side queries as appropriate. The
hacking will have similar-ish effects to that induced by the changes for
RETURNING.
Something like
ExecutorStart(mainQueryDesc, 0);
for (;;)
{
/* FETCH 1 */
ExecutorRun(mainQueryDesc, ForwardScanDirection, 1L);
if (mainQueryDesc-estate-es_processed == 0)
break;
//identify side query
Re: [HACKERS] Protection from SQL injection
On Wed, Apr 30, 2008 at 8:52 PM, Thomas Mueller [EMAIL PROTECTED] wrote: Hi, Constants are just convenience: instead of constants, user defined functions can be used. This already works, however it's a bit verbose: CREATE FUNCTION STATE_ACTIVE() RETURNS VARCHAR AS $$ BEGIN RETURN 'active'; END; $$ LANGUAGE PLPGSQL; Usage is almost the same: SELECT * FROM USERS WHERE STATE=STATE_ACTIVE(); therefore arbitrary macro expansion like in those plenty of languages does not seem like a good idea to me. This is _not_ macro expansion as in C '#define'. Constants are typed, as in C++ 'const' and Java 'static final'. The question is only: should the user explicitly state the data type, or should the data type be deduced from the value. Both is possible: CREATE CONSTANT STATE_ACTIVE VALUE 'active'; CREATE CONSTANT STATE_ACTIVE TEXT VALUE 'active'; Maybe we can extend the SQL's WITH clause do declare the constant along with the query, and not separate from the query. WITH CONSTANT c_jobrole = 'clerk', CONSTANT c_dept = 10 SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept; and let postgres allow literals only in the WITH clause. Also, IMHO, the type of the expression should be automatically deduced. The right hand side should be an expression and not just a string or numeric literal. For eg. the above query can be written as: WITH CONSTANT c_jobrole = 'clerk', CONSTANT c_deptname = 'FINANCE'::text, CONSTANT c_dept = (SELECT dname FROM dept WHERE dname = c_deptname) SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept; so the expression can be CAST'd into appropriate type wherever needed. Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com EnterpriseDB http://www.enterprisedb.com Mail sent from my BlackLaptop device
Re: [HACKERS] Protection from SQL injection
Gurjeet Singh [EMAIL PROTECTED] writes: Maybe we can extend the SQL's WITH clause do declare the constant along with the query, and not separate from the query. WITH CONSTANT c_jobrole = 'clerk', CONSTANT c_dept = 10 SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept; [ scratches head... ] And that will provide SQL injection protection how? Anyway, you hardly need new syntax to do that, I'd expect WITH SELECT 'clerk' AS c_jobrole ... to accomplish it just fine. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protection from SQL injection
On Wed, Apr 30, 2008 at 10:58 PM, Tom Lane [EMAIL PROTECTED] wrote: Gurjeet Singh [EMAIL PROTECTED] writes: Maybe we can extend the SQL's WITH clause do declare the constant along with the query, and not separate from the query. WITH CONSTANT c_jobrole = 'clerk', CONSTANT c_dept = 10 SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept; [ scratches head... ] And that will provide SQL injection protection how? Well, if the the query was: WITH CONSTANT c_jobrole = value from a FORM text field, CONSTANT c_dept = 10 SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept; And if the attack supplied a value 'clerk OR 1=1' the final query (after replacing constants) would look like this: SELECT * FROM emp WHERE jobrole = 'clerk OR 1=1' and deptno = 10; The attacker was not able to inject any new code there. (reiterates: and let postgres allow literals only in the WITH clause) Anyway, you hardly need new syntax to do that, I'd expect WITH SELECT 'clerk' AS c_jobrole ... to accomplish it just fine. I am not sure I understood this example. Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com EnterpriseDB http://www.enterprisedb.com Mail sent from my BlackLaptop device
[HACKERS] TidScan needs handling of a corner cases
Hi All, I noticed that the TidScan fails to identify when the requested block is not in the relation. Consider this (pg_class has 6 blocks): postgres=# explain analyze select ctid, * from pg_class where ctid in ( '(6,1)' ); ERROR: could not read block 6 of relation 1663/11511/1259: read only 0 of 8192 bytes Also, it is known that 0 is not a valid row-offset in the block, but the tid input function accepts this value (it rejects -ve values). For the same setup: postgres=# explain analyze select ctid, * from pg_class where ctid in ( '(6,0)' ); QUERY PLAN Tid Scan on pg_class (cost=0.00..4.01 rows=1 width=211) (actual time=0.009..0.009 rows=0 loops=1) TID Cond: (ctid = '(6,0)'::tid) Total runtime: 0.130 ms (3 rows) Can we safely fix these? First one by ignoring the request if requested_block = RelationGetNumberOfBlocks(), and second one by accepting only non-zero positive values in the tid input function. Best regards -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com EnterpriseDB http://www.enterprisedb.com Mail sent from my BlackLaptop device
Re: [HACKERS] Protection from SQL injection
* Gurjeet Singh [EMAIL PROTECTED] [080430 13:38]: Well, if the the query was: WITH CONSTANT c_jobrole = value from a FORM text field, CONSTANT c_dept = 10 SELECT * FROM emp WHERE jobrole = c_jobrole and deptno = c_dept; And if the attack supplied a value 'clerk OR 1=1' the final query (after replacing constants) would look like this: SELECT * FROM emp WHERE jobrole = 'clerk OR 1=1' and deptno = 10; The attacker was not able to inject any new code there. (reiterates: and let postgres allow literals only in the WITH clause) Sure, but you've only changed the attack vector: set value from a form text field to be: something' SELECT 1; DELETE FROM users; WITH c_jobrole = '1 And you get: WITH CONSTANT cjobrole = 'someting' SELECT 1; DELETE FROM users; WITH c_jobrole = '1' SELECT * FROM emp WHERE jobrole = c_jobrole; Sure, if the value from form is properly escaped and sanitized, it wouldn't be a problem. But if you are sure that all data is properly escaped and sanitized, then the whole don't allow literals extra protection isn't needed either, and you don't need to go looking for ways to avoid the literals in queries. -- Aidan Van Dyk Create like a god, [EMAIL PROTECTED] command like a king, http://www.highrise.ca/ work like a slave. signature.asc Description: Digital signature
Re: [HACKERS] Proposed patch - psql wraps at window width
Gregory Stark wrote: [No I wasn't thinking of that, that's an interesting case too though I think we might need to think a bit harder about cases that wrap poorly. If you have long column headings we could wrap those too. But what if you have enough space for just a few characters per column and you have long text fields in those columns?] I've been using this patch for months. After the first week, I switched to the behavior above (the wrap code gives up at a certain point). It is a lot better, try it for yourself. If you try to cram too much stuff onto a line it's going to become an even more unreadable mess of very very tall columns. Wrapping column headings only gets you so far, delaying the inevitable a little bit. A patch to squeeze out extra header space, or abbreviate headers would be an advance. -Bryce \pset format aligned autowrap \pset format aligned 80 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protection from SQL injection
On Wed, Apr 30, 2008 at 10:20:09AM -0400, Andrew Sullivan wrote:
On Tue, Apr 29, 2008 at 09:02:30PM -0400, Gregory Stark wrote:
Did you guys miss Tom's comment up-thread? Postgres already does this if you
use PQExecParams().
I did, yes. Thanks for the clue. OTOH, I do see the OP's point that
it'd be nice if the DBA could enforce this rule. Maybe a way of
insisting on PQExecParams() instead of anything else?
Create a function somewhere:
void PQexec()
{
die();
}
And it will override the one in the shared library. In other languages
subclassing should be able to provide the same effect.
Have a nice day,
--
Martijn van Oosterhout [EMAIL PROTECTED] http://svana.org/kleptog/
Please line up in a tree and maintain the heap invariant while
boarding. Thank you for flying nlogn airlines.
signature.asc
Description: Digital signature
Re: [HACKERS] TidScan needs handling of a corner cases
Gurjeet Singh [EMAIL PROTECTED] writes: postgres=# explain analyze select ctid, * from pg_class where ctid in ( '(6,1)' ); ERROR: could not read block 6 of relation 1663/11511/1259: read only 0 of 8192 bytes I was about to say that throwing an error for this is just fine, but on closer investigation we didn't throw an error before 8.3; the behavioral change is because mdread() now throws an error for out-of-bounds read. So for consistency with historical behavior this probably needs to be fixed. Also, it is known that 0 is not a valid row-offset in the block, but the tid input function accepts this value (it rejects -ve values). For the same setup: Not sure what you think needs to be fixed in this case? Again, backwards compatibility seems a reason not to change it; and I sure don't see why you'd want to throw an error for this case but not the other one. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Protection from SQL injection
Martijn van Oosterhout [EMAIL PROTECTED] writes:
void PQexec()
{
die();
}
Actually disabling PQexec seems like a pretty poor decision; it breaks
working code to no purpose. Also, doing it on the client side means
you're at risk of some clients being secure and some not. I thought
what we were discussing was a server-side GUC parameter that would
disallow more than one SQL statement per PQexec.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Proposed patch - psql wraps at window width
Re: [HACKERS] Proposed patch - psql wraps at window width
Tom Lane wrote: Well, personally I haven't read the core code yet, since it's not commit fest yet ;-). I don't know whether there are any issues there, but it wouldn't surprise me given the number of issues in the control code. regards, tom lane I'm biased because I wrote the patch. However -- I think the core is commitable (it won't break anything else, or have complex dependencies or bloat). The issue of $COLUMNS was identified as unresolved at the time the patch was submitted, and I think it's the only bit in serious question. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Internal design of MERGE, with Rules
On Wed, Apr 30, 2008 at 04:58:52PM +0100, Simon Riggs wrote: That means we probably need to introduce new infrastructure in the tcop or executor modules to handle queries-within-queries. This isn't special-casing MERGE so much as introducing infrastructure for a new class of query, such as MERGE, REPLACE, INSERT ELSE UPDATE. (Merge itself does cover almost all cases of this type of query, but we'd be able to fairly easily support all of the different syntax). MERGE would then be represented by a query that has many side queries (so called so we don't confused calling them sub-queries). Why make them special cases? (I'm sure there's a good reason!) I've sometimes wanted to be able to put DML statements inside SELECT statements. The following is a slightly reasonable example: INSERT INTO ilog (i,ts,n) SELECT i, now(), COUNT(*) FROM ( INSERT INTO bar (x,y) SELECT 5, n FROM baz WHERE i = 10 RETURNING i) x GROUP BY i; Hum, that implies a nasty schema (ilog.ts should be in bar). Ah well, I hope you get the idea. The simplest example I can think of is: SELECT * FROM (INSERT INTO foo (n) VALUES (1) RETURNING 1) x; Hum, I think I may have just thought of why. What would: SELECT (INSERT INTO foo (n) VALUES (f.n) RETURNING 1) FROM foo f; be expected to do? I'm thinking of nasty cases where the outer code reading foo never stops because there's new stuff being inserted ahead of it. But then again, you can put the insert into a stored procedure and it does indeed do something sensible. Sam -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches
Folks, For hackers who don't understand security frameworks, I'm going to make a strong case for KaiGai's patch. Because of my current presentation series, I've been talking to PostgreSQL users about security features around the world for the last several months, and there's a *lot* of interest in security framework support. Not only are existing users interested in it, but some potential users (security agencies, banks) who didn't use PostgreSQL before have come to talk to me becuase of SE-Postgres. Further, I've asked the TrustedSolaris folks to take a look at KaiGai's implementation to see if it was generic enough for them to build on as a test of whether SE-Postgres was too specific to SE-Linux; the answer has been a tentative yes, it's generic. So it would be much better to have this functionality be mainstream rather than a fork. If it does get bounced, please do it becuase of code quality and not because nobody is asking for this. Thanks! -- --Josh Josh Berkus PostgreSQL @ Sun San Francisco -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches
Josh Berkus wrote: Folks, So it would be much better to have this functionality be mainstream rather than a fork. If it does get bounced, please do it becuase of code quality and not because nobody is asking for this. +1 Joshua D. Drake -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches
On 01/05/2008, Josh Berkus [EMAIL PROTECTED] wrote: Further, I've asked the TrustedSolaris folks to take a look at KaiGai's implementation to see if it was generic enough for them to build on as a test of whether SE-Postgres was too specific to SE-Linux; the answer has been a tentative yes, it's generic. So it would be much better to have this functionality be mainstream rather than a fork. If it does get bounced, please do it becuase of code quality and not because nobody is asking for this. Not a hacker, just a curious reader ... are there equivalent frameworks for the other supported platforms? E.g. MacOS, *BSD, Windows? Are the similarities between those (if they exist) close enough not to introduce a maintenance nightmare? Cheers, Andrej -- Please don't top post, and don't use HTML e-Mail :} Make your quotes concise. http://www.american.edu/econ/notes/htmlmail.htm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches
On Thu, 1 May 2008, Andrej Ricnik-Bay wrote: Not a hacker, just a curious reader ... are there equivalent frameworks for the other supported platforms? E.g. MacOS, *BSD, Windows? SELinux is a Linux implementation of ideas from an earlier NSA project named Flask. There is port of another variant of that, Flask/TE, that is making its way into the BSD variants via a project called SEBSD. TrustedBSD, Darwin (OS X), and OpenSolaris all have projects in this area already (the Solaris one just launched last month). A good starter page is http://www.trustedbsd.org/sebsd.html Particularly given the common heritage, I suspect that the PostgreSQL side of all these projects will be similar, and that once those hooks are in place it will just be a matter of tying them into the higher levels of the other framework. It would be too ambitious to target all of them all at once for a first pass, but it may be worth a look at the fundamentals of SEBSD to make sure the right hooks look like they're in place. Windows has this thing called Group Policy that's supposedly leaped forward for Windows Server 2008. They are now advertising it as like SELinux, but better. The presentation PDF I just read on that subject sounds like something written by the crazy guy at Broadway 57th street I used to walk by, as he talked on fruit as if they were his cell phone. It's such a deluded and wildly misguided bit of sales fluff that you can't take it seriously, and the whole thing just leaves me feeling sorry for them instead. -- * Greg Smith [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
