Re: [HACKERS] statement timeout vs dump/restore

2008-05-05 Thread Zeugswetter Andreas OSB sIT

  Do we want the following:
 
  1. pg_dump issues set statement_timeout = 0; to the 
 database prior to 
  taking its copy of data (yes/no/default-but-switchable)
  2. pg_dump/pg_restore issue set statement_timeout = 0; in 
 text mode 
  output (yes/no/default-but-switchable)
  3. pg_restore issues set statement_timeout = 0; to the 
 database in 
  restore mode (yes/no/default-but-switchable)
 
 I think yes for all three.  There was some handwaving about someone
 maybe not wanting it, but an utter lack of convincing use-cases; so
 I see no point in going to the effort of providing a switch.
 
 Note that 2 and 3 are actually the same thing (if you think they are
 not, then you are putting the behavior in the wrong place).

I thought a proper fix for 3 would not depend on 2 ?

Andreas

-- 
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 - LDAPS support for servers on port 636 w/o TLS

2008-05-05 Thread Andreas Pflug

Tom Lane wrote:

stephen layland [EMAIL PROTECTED] writes:
  

I've written a quick patch against the head branch (8.4DEV, but it also
works with 8.1.3 sources) to fix LDAP authentication support to
work with LDAPS servers that do not need start TLS.   I'd be interested
to hear your opinions on this.



Not being an LDAP user, I'm not very qualified to comment on the details
here, but ...

  

My solution was to create a boolean config variable called
ldap_use_start_tls which the user can toggle whether or not
start tls is necessary.



... I really don't like using a GUC variable to determine the
interpretation of entries in pg_hba.conf.  A configuration file exists
to set configuration, it shouldn't need help from a distance.  Also,
doing it this way means that if several different LDAP servers are
referenced in different pg_hba.conf entries, they'd all have to have
the same encryption behavior.

I think a better idea is to embed the flag in the pg_hba.conf entry
itself.  Perhaps something like ldapso: instead of ldaps: to
indicate old secure ldap protocol, or include another parameter
in the URL body.
  
With ldaps on port 636 STARTTLS should NEVER be issued, so the protocol 
identifier ldaps should be sufficient as do not issue STARTTLS flag. 
IMHO the current pg_hba.conf implementation doesn't follow the usual 
nomenclatura; ldap with TLS is still ldap. Using ldaps as indicator for 
ldap with tls over port 389 is misleading for anyone familiar with ldap.


Regards,
Andreas


--
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 - LDAPS support for servers on port 636 w/o TLS

2008-05-05 Thread Magnus Hagander
Tom Lane wrote:
 I think a better idea is to embed the flag in the pg_hba.conf entry
 itself.  Perhaps something like ldapso: instead of ldaps: to
 indicate old secure ldap protocol, or include another parameter
 in the URL body.

FWIW, I'm working on a proposal to change how pg_hba.conf deals with
the parameter field to make it easier to do things like this, by
using a name/value pair setup instead. The LDAP url is one reason -
it's hacky enough already *before* we add this kind of option to it...

//Magnus

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

2008-05-05 Thread Darren Reed

This might seem sillly, but...

...are comments going to be considered statements for the purpose of 
this knob?

(hoping the anwer is yes)

Darren


--
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] statement timeout vs dump/restore

2008-05-05 Thread Andrew Dunstan



Zeugswetter Andreas OSB sIT wrote:

Do we want the following:
  
1. pg_dump issues set statement_timeout = 0; to the 
  
database prior to 


taking its copy of data (yes/no/default-but-switchable)
2. pg_dump/pg_restore issue set statement_timeout = 0; in 
  
text mode 


output (yes/no/default-but-switchable)
3. pg_restore issues set statement_timeout = 0; to the 
  
database in 


restore mode (yes/no/default-but-switchable)
  

I think yes for all three.  There was some handwaving about someone
maybe not wanting it, but an utter lack of convincing use-cases; so
I see no point in going to the effort of providing a switch.

Note that 2 and 3 are actually the same thing (if you think they are
not, then you are putting the behavior in the wrong place).



I thought a proper fix for 3 would not depend on 2 ?


  


I'm sure we could separate the two if we wanted to. Since we don't it's 
been put in the most natural spot, which does both.


cheers

andrew

--
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 - LDAPS support for servers on port 636 w/o TLS

2008-05-05 Thread David Boreham

Andreas Pflug wrote:
With ldaps on port 636 STARTTLS should NEVER be issued, so the 
protocol identifier ldaps should be sufficient as do not issue 
STARTTLS flag. IMHO the current pg_hba.conf implementation doesn't 
follow the usual nomenclatura; ldap with TLS is still ldap. Using 
ldaps as indicator for ldap with tls over port 389 is misleading for 
anyone familiar with ldap.
I agree. ldaps:: should mean plain SSL without StartTLS. ldap:: should 
mean a plain text connection,

unless some additional configuration directive enables StartTLS.

There has been some discussion in the past about including (or not) this 
configuration state in the url :


http://www.openldap.org/lists/openldap-devel/200202/msg00070.html



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

2008-05-05 Thread Tom Lane
Darren Reed [EMAIL PROTECTED] writes:
 This might seem sillly, but...
 ...are comments going to be considered statements for the purpose of 
 this knob?
 (hoping the anwer is yes)

Are you trying to say we should forbid comments?  No thank you.

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

2008-05-05 Thread Chris Browne
[EMAIL PROTECTED] (Florian Weimer) writes:
 * Thomas Mueller:

 What do you think about it? Do you think it makes sense to implement
 this security feature in PostgreSQL as well?

 Can't this be implemented in the client library, or a wrapper around it?
 A simple approximation would be to raise an error when you encounter a
 query string that isn't contained in some special configuration file.

This could be implemented in a client library, but that means that
you're still entirely as vulnerable; any client that chooses not to
use that library won't be protected.

It would be a mighty attractive thing to have something at the server
level to protect against the problem.
-- 
let name=cbbrowne and tld=linuxfinances.info in String.concat @ 
[name;tld];;
http://linuxdatabases.info/info/lsf.html
If you add a couple of i's to Microsoft's stock ticker symbol, you get
'misfit'.  This is, of course, not a coincidence.

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

2008-05-05 Thread Darren Reed

Tom Lane wrote:

Darren Reed [EMAIL PROTECTED] writes:
  

This might seem sillly, but...
...are comments going to be considered statements for the purpose of 
this knob?

(hoping the anwer is yes)



Are you trying to say we should forbid comments?  No thank you.
  


No.

When psql (for example) parses this:
COMMIT;BEGIN;-- Hi
it will generate individual commands with postgres (the server) over
the connection.  ie. psql sends COMMIT; waits, then sends 'BEGIN;,
waits, etc.

When you do this in perl:
$db-prepare(COMMIT;BEGIN;--);
one single command string (the entire string above) is sent to the server.

How often do people code comments into prepare statements in perl
or the equivalent in java, ruby, etc?

Do you put comments in your perl prepare statements?

If comments count as a statement, at the server end, then the 
multi-statement
disabling also disables another attack vector - slightly: you can no 
longer attack

using this as your username:
' OR 1=1;--

This raises another point - preventing muilti-statement attacks work so
long as the hacker string isn't broken up into multiple statements by the
client side.  Passing said string to /bin/sh, which then passes it to psql
would successfully enable the attack even with this knob turned on.  But
few people are likely to be using a design that is that slow.

Darren


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

2008-05-05 Thread Andrew Dunstan



Chris Browne wrote:

[EMAIL PROTECTED] (Florian Weimer) writes:
  

* Thomas Mueller:



What do you think about it? Do you think it makes sense to implement
this security feature in PostgreSQL as well?
  

Can't this be implemented in the client library, or a wrapper around it?
A simple approximation would be to raise an error when you encounter a
query string that isn't contained in some special configuration file.



This could be implemented in a client library, but that means that
you're still entirely as vulnerable; any client that chooses not to
use that library won't be protected.

It would be a mighty attractive thing to have something at the server
level to protect against the problem.
  


If by the problem you mean SQL injection attacks in general, I 
strongly suspect you are chasing a mirage.


Someone mentioned upthread that the best way to secure the database was 
to require that all access be via stored procedures. You can actually go 
quite a logn way down that road. I have seen it done quite successfully 
in a Fortune 50 company. In effect you are getting out of the game of 
catching insecure operations by not allowing your user direct access to 
them at all. (This approach also has huge benefits in allowing 
optimisation without disturbing client code).


But this requires work by the DBA. I have some ideas about how we could 
make it a whole lot easier and more workable, but they probably don't 
belong in this thread.


cheers

andrew

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


[HACKERS] Exposing quals

2008-05-05 Thread David Fetter
Folks,

I'd like to make it possible to see qualifiers from inside functions
in PostgreSQL.  For my particular case, and for dblink, it would
allow user-space code to some remove bottlenecks which make currently
make inter-DBMS communication extremely inefficient.

The current code returns one (potentially) big string with the quals
ANDed together.  I'm thinking that the new code would:

1.  Return just a pointer.

2.  Add some functions like rsinfo_get_node_tree_str() which can turn
same into (in this case, a string, but others might get tree
structures) on demand from client code.

3.  Add wrapper functions in each untrusted PL (trusted PLs shouldn't
be doing anything that needs this).

4.  Document the above.

Please find attached the patch, and thanks to Neil Conway and Korry
Douglas for the code, and to Jan Wieck for helping me hammer out the
scheme above.  Mistakes are all mine ;)

Comments?  Questions?

Cheers,
David.
-- 
David Fetter [EMAIL PROTECTED] http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: [EMAIL PROTECTED]

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Index: contrib/dblink/dblink.c
===
RCS file: /projects/cvsroot/pgsql/contrib/dblink/dblink.c,v
retrieving revision 1.73
diff -c -c -r1.73 dblink.c
*** contrib/dblink/dblink.c 4 Apr 2008 17:02:56 -   1.73
--- contrib/dblink/dblink.c 8 Apr 2008 18:29:05 -
***
*** 752,757 
--- 752,758 
char   *conname = NULL;
remoteConn *rconn = NULL;
boolfail = true;/* default to backward 
compatible */
+   ReturnSetInfo   *rsi;
  
/* create a function context for cross-call persistence */
funcctx = SRF_FIRSTCALL_INIT();
***
*** 826,831 
--- 827,843 
elog(ERROR, wrong number of arguments);
}
  
+   if (sql  rsi-qual)
+   {
+   char *quals = rsinfo_get_qual_str(rsi);
+   char *qualifiedQuery = palloc(strlen(sql) + strlen( 
WHERE ) +
+   
  strlen(quals) + 1);
+ 
+   sprintf(qualifiedQuery, %s WHERE %s, sql, quals);
+ 
+   sql = qualifiedQuery;
+   }
+ 
if (!conn)
DBLINK_CONN_NOT_AVAIL;
  
Index: src/backend/executor/execQual.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/executor/execQual.c,v
retrieving revision 1.228
diff -c -c -r1.228 execQual.c
*** src/backend/executor/execQual.c 25 Mar 2008 22:42:43 -  1.228
--- src/backend/executor/execQual.c 8 Apr 2008 18:29:06 -
***
*** 45,50 
--- 45,51 
  #include funcapi.h
  #include miscadmin.h
  #include nodes/makefuncs.h
+ #include optimizer/clauses.h
  #include optimizer/planmain.h
  #include parser/parse_expr.h
  #include utils/acl.h
***
*** 1415,1420 
--- 1416,1440 
return result;
  }
  
+ /*
+  *
+  * Get either an empty string or a batch of qualifiers.
+  *
+  */
+ char *
+ rsinfo_get_qual_str(ReturnSetInfo *rsinfo)
+ {
+   Node*qual;
+   List*context;
+ 
+   if (rsinfo-qual == NIL)
+   return pstrdup();
+ 
+   qual = (Node *) make_ands_explicit(rsinfo-qual);
+   context = deparse_context_for_plan(NULL, NULL, rsinfo-rtable);
+ 
+   return deparse_expression(qual, context, false, false);
+ }
  
  /*
   *ExecMakeTableFunctionResult
***
*** 1426,1431 
--- 1446,1452 
  Tuplestorestate *
  ExecMakeTableFunctionResult(ExprState *funcexpr,
ExprContext *econtext,
+   List *qual, List 
*rtable,
TupleDesc expectedDesc,
TupleDesc *returnDesc)
  {
***
*** 1458,1463 
--- 1479,1486 
InitFunctionCallInfoData(fcinfo, NULL, 0, NULL, (Node *) rsinfo);
rsinfo.type = T_ReturnSetInfo;
rsinfo.econtext = econtext;
+   rsinfo.qual = qual;
+   rsinfo.rtable = rtable;
rsinfo.expectedDesc = expectedDesc;
rsinfo.allowedModes = (int) (SFRM_ValuePerCall | SFRM_Materialize);
rsinfo.returnMode = SFRM_ValuePerCall;
Index: src/backend/executor/nodeFunctionscan.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/executor/nodeFunctionscan.c,v
retrieving revision 1.46
diff -c -c -r1.46 nodeFunctionscan.c
*** src/backend/executor/nodeFunctionscan.c 29 Feb 2008 02:49:39 -   

Re: [HACKERS] pgstat SRF?

2008-05-05 Thread Magnus Hagander
Magnus Hagander wrote:
 Magnus Hagander wrote:
  Tom Lane wrote:
   Magnus Hagander [EMAIL PROTECTED] writes:
While looking over the statistics-for-functions patch
(http://archives.postgresql.org/pgsql-patches/2008-03/msg00300.php),
I came back to a thought I've had before - why do we keep one
function per column for pgstat functions, instead of using a set
returning function? Is there some actual reason for this, or is
it just legacy from a time when it was (much) harder to write
SRFs?
   
   I think it's so that you can build your own stats views instead of
   being compelled to select the data someone thought was good for
   you.
  
  You can still do that if it's an SRF. You could even make the SRF
  take an optional argument to either return a single value (filtered
  the same way the individual functions are) or when this one is set
  to NULL, return the whole table. 
  
  It would make the overhead a lot lower in the most common case
  (SELECT
  * FROM pg_stat_somethingorother), while only adding a little in
  the other cases, I think.
  
  Though I'm not sure that overhead is big enough to care about in the
  first place, but if you're VIEWs are longish it could be...
 
 Actually, looking at this once more, the interface to the functions
 sucked more than I thought. They're not actually accepting procpid as
 parameters, but just an index into the current array in pgstats..
 Basically, they're not supposed to be used in any other way than
 accessing all the rows at once :-)
 
 Attached is a version of the functions required for pg_stat_activity
 implemented as a SRF instead of different functions. A quick benchmark
 (grabbing the VIEW 10,000 times on a system with about 500 active
 backends) shows it's about 20% faster than the function-per-value
 approach, but the runtime per view is still very quick as it is today.
 (And most of what overhead there is most likely comes from reading the
 stats file)
 
 However, it also implements the lookup-by-PID functionality that IMHO
 makes a lot more sense than lookup-by-backend-array-index. This is
 obviously a lot more performant than querying the VIEW for all rows -
 something that might be a big win for monitoring apps that look for
 info about a single backend.
 
 Unsure if we want to go ahead and convert all functions, but I think
 we can make a good argument for making *new* stats views (like the
 ones about functions that in the queue) based on SRFs instead. It
 also has the nice side-effect of less rows in the system tables ;)
 
 Comments?

Unless there are any objections I'll complete this patch along with the
proper documentation, and apply it for now. I will look at converting
some further pgstats stuff into SRFs as well, assuming they show
similar results..

//Magnus

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

2008-05-05 Thread Greg Sabino Mullane

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


 How often do people code comments into prepare statements in perl
 or the equivalent in java, ruby, etc?

 Do you put comments in your perl prepare statements?

Does it matter? It shouldn't. They are comments.

 If comments count as a statement, at the server end, then the
 multi-statement disabling also disables another attack vector -
 slightly: you can no longer attack using this as your username:
  ' OR 1=1;--

Using placeholders and other best practices removes such attacks
completely.

I mostly agree with some other people in this thread that the
'disable multi-line switch' is marginally useful at best, and provides
a false sense of security. But let's not confuse the issue with
examples like the above. Otherwise I'll point out yet again that this
whole things a solution in search of a problem. Poorly written apps
will remain poorly written apps, no matter what server-side bandaids
we try to apply.

- --
Greg Sabino Mullane [EMAIL PROTECTED]
PGP Key: 0x14964AC8 200805051559
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8

-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAkgfZzcACgkQvJuQZxSWSsjAoACg6UKhb2r94khikeOfT2cUOGhD
vh0AoIY/8dSH4tkmsLxl2Jkpbn7/u3+4
=hGCo
-END PGP SIGNATURE-



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

2008-05-05 Thread Gregory Stark
Bruce Momjian [EMAIL PROTECTED] writes:

 OK, so COLUMNS should take precedence.  I assume this is going to
 require us to read the COLUMNS enviroment variable in psql _before_
 readline sets it, and that COLUMNS will only affect screen output, like
 ioctl().  Is that consistent?

What are you talking about? screen output?

Are you even getting my emails? Please confirm that you receive this email.

-- 
  Gregory Stark
  EnterpriseDB  http://www.enterprisedb.com
  Ask me about EnterpriseDB's On-Demand Production Tuning

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

2008-05-05 Thread Tom Lane
KaiGai Kohei [EMAIL PROTECTED] writes:
 I updated the series of SE-PostgreSQL patches for the latest pgsql-8.4devel 
 tree.

I tried to do a bit of testing of this, but it does not work on current
Fedora 8, because the policy module doesn't build:

[EMAIL PROTECTED] sepgsql-policy]$ make
make -f /usr/share/selinux/devel/Makefile NAME=targeted
make[1]: Entering directory `/home/tgl/sepgsql/contrib/sepgsql-policy'
Compiling targeted sepostgresql module
/usr/bin/checkmodule:  loading policy configuration from tmp/sepostgresql.tmp
sepostgresql.te:349:ERROR 'syntax error' at token 
'corenet_tcp_recvfrom_labeled' on line 5675:
# NOTE: These changes are to be merged in the later releases.
corenet_tcp_recvfrom_labeled(sepgsql_server_type, sepgsql_client_type)
/usr/bin/checkmodule:  error(s) encountered while parsing configuration
make[1]: *** [tmp/sepostgresql.mod] Error 1
make[1]: Leaving directory `/home/tgl/sepgsql/contrib/sepgsql-policy'
make: *** [sepostgresql.pp] Error 2
[EMAIL PROTECTED] sepgsql-policy]$ 


In the meantime, here are some random comments after my failed test
build and a very fast scan through the patch:

The patch tries to re-add ipcclean to the source tree --- probably a
merge error?

autoconf complains about the description-free AC_DEFINEs

Doesn't compile warning-free if selinux not enabled ... for that matter,
it doesn't compile warning-free if selinux IS enabled.

No documentation updates whatsoever :-(

About half of the patch seems to be conditional on
#ifdef SECURITY_SYSATTR_NAME
and the other half on
#ifdef HAVE_SELINUX
This seems bizarre: is there really any chance that there are two
independently usable chunks of code here?  And why is it conditional
on a macro that is a field name, rather than conditional on a feature
macro?  That is, I'd expect to see something like
#ifdef ENABLE_SEPOSTGRES
throughout.

For that matter, what is the point of treating SECURITY_SYSATTR_NAME
as a configurable thing in the first place?  I can hardly imagine a
worse idea than a security-critical column having different names in
different installations.

The patch hasn't got a mode in which SELinux support is compiled in but
not active.  This is a good way to ensure that no one will ever ship
standard RPMs with the feature compiled in, because they will be entirely
nonfunctional for people who aren't interested in setting up SELinux.
I think you need an enable_sepostgres GUC, or something like that.
(Of course, the overhead of the per-row security column would probably
discourage anyone from wanting to use such a configuration anyway,
so maybe the point is moot.)

sepgsql-policy has got usability problems:
* It should pay attention to the configured installation PREFIX instead of
hardwiring a couple of random possible installation locations
* It can only support the build machine's SELINUXTYPE --- how am I supposed
to produce RPMs that support all available types?

The contents and use of sepgsqlGetTupleName() make it look like the
entire security scheme is based on object name alone.  That doesn't
even account for schemas, let alone overloaded function names.  Please
tell me this is not as broken-by-design as it looks.

I occasionally tell people try to make the patch look like it's always
been there.  This is pretty far from meeting that goal.  Random bits
of code that are commented PGACE: are obviously not intended to just
fit in.  You've generally ignored the Postgres code layout conventions
(pgindent will help to some extent but it's far from a panacea) and our
commenting conventions --- eg, hardly any of the functions have header
comments, and the ones that do follow conventions seen noplace in the
Postgres code, like using @ on parameter names.  In general the number
and quality of the comments is far below the standard for Postgres code,
and the lack of any implementation documentation isn't helping.

Another big problem, which I understand your motivation for but that
doesn't make the code any less ugly, is that you've got trivial bits
of code that're separated by two(!) levels of hook calls from where
they're actually being used.  Not only does that make it unreadable
but the files that actually do the work combine bits of code that
should be scattered across a lot of modules, causing those files to
be just horrid from a modularity standpoint --- they've got their
fingers stuck in everyplace.  If you want this to get applied you need
to start thinking of it as an integral part of the code, not an add-on.

Some other bits of add-on-itis:
* If you need a dedicated LWLock, declare it in lwlock.h.
* If you need a node type, declare it in nodes.h (T_SEvalItem is utterly
  broken)

Why in the world would you have security restrictions associated with
TOAST tuples?  Seems like all the interesting restrictions should be
on the parent table.

Don't randomly invent your own style of management of a postmaster
child process.  For one thing, this code doesn't cope with either

Re: [HACKERS] [0/4] Proposal of SE-PostgreSQL patches

2008-05-05 Thread Greg Smith

On Mon, 5 May 2008, Tom Lane wrote:


elog() should not be used for user-facing errors.  I couldn't easily
tell just which of the messages are likely to be seen by users and
which ones should be can't happen cases, but certainly there are
a whole lot of these that need to be ereport()s.  Likely there need
to be some new ERRCODEs too.


And it would be a nice step toward the scenarios I was asking about if 
there was a GUC variable for what level to log security violations at.  I 
realize now the tuple-level warnings are going into the SELinux logs 
rather than the PostgreSQL ones, but it should be easier to change policy 
violations that impact the server to something other than just ERROR.


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


Re: [HACKERS] Proposed patch - psql wraps at window width

2008-05-05 Thread Bruce Momjian
Bryce Nesbitt wrote:
 
  OK, so COLUMNS should take precedence.  I assume this is going to
  require us to read the COLUMNS environment variable in psql _before_
  readline sets it, and that COLUMNS will only affect screen output, like
  ioctl().  Is that consistent?

 This whole thing is confusing enough at the point, I think a complete 
 proposal needs to be articulated. It is hard to comment on a fragment of 
 an idea.
 
 The main cases to cover are:  (1) how to specify wrap for tty's

In order of precedence:

\pset columns
$COLUMNS
iotcl()

  (2) how to specify wrap for pipes

\pset columns

 (3) how to get wrapped on platforms that don't 
 have the ioctl (presumably windows without cygwin)   

\pset columns
$COLUMNS

 (4) how to set up 
 different defaults for tty's and pipes (e.g. wrap interactive tty's, but 
 leave output aligned for scripts).
 
 And perhaps, as a bonus comment on (5) the idea of having psql NOT 
 source .psqlrc

   -X
   --no-psqlrc

 I hope at some point someone will actually try the actual core wrapping 
 code, and comment on it.

I tested it and it worked well once I modified it.

Updated patch with clearer documentation that matches the above
behavior:

ftp://momjian.us/pub/postgresql/mypatches/wrap

FYI, I looked into 'ls -C' hanlding a little more and ls (GNU coreutils)
5.97 honors COLUMNS _only_ in file/pipe output, not for screen output. 
What the C code does is to read COLUMNS, then overwrite that value with
ioctl() if it works.

Do we want to follow that behavior?  ls has a '-w' option to specify the
width, like our \pset columns.  However, the manual page seems backwards:

   -w, --width=COLS
  assume screen width instead of current value

The GNU 'ls' manual does not mention COLUMNS.

BSD 'ls' used COLUMNS only when outputing to the screen, and ioctl
fails.  However, the BSD manual says:

 COLUMNSIf this variable contains a string representing a decimal
integer, it is used as the column position width for
displaying multiple-text-column output.  The ls utility
calculates how many pathname text columns to display based
on the width pro- vided.  (See -C.)

Again, I think the manual is wrong.

So it seem GNU ls and BSD ls are inconsistent, which I think means we
should design our API the best we can, rather than rely on how others
interpret COLUMNS.

-- 
  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] Proposed patch - psql wraps at window width

2008-05-05 Thread Bruce Momjian
Gregory Stark wrote:
 Bruce Momjian [EMAIL PROTECTED] writes:
 
  OK, so COLUMNS should take precedence.  I assume this is going to
  require us to read the COLUMNS enviroment variable in psql _before_
  readline sets it, and that COLUMNS will only affect screen output, like
  ioctl().  Is that consistent?
 
 What are you talking about? screen output?
 
 Are you even getting my emails? Please confirm that you receive this email.

Yes, I am getting your emails, but I have more than you to please.

Are others OK with $COLUMNS controlling screen output and file/pipe, or
perhaps COLUMNS controlling only file/pipe, as GNU ls does?  I have
heard a few people say they only want \pset columns to control
file/pipe.

I have outlined the GNU ls behavior in a email I just sent.

-- 
  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] Proposed patch - psql wraps at window width

2008-05-05 Thread Tom Lane
Bruce Momjian [EMAIL PROTECTED] writes:
 Are others OK with $COLUMNS controlling screen output and file/pipe, or
 perhaps COLUMNS controlling only file/pipe, as GNU ls does?  I have
 heard a few people say they only want \pset columns to control
 file/pipe.

I agree with the latter.  Anyone who is setting COLUMNS is going to set
it to reflect the *screen* width they want; it has zero to do with
what should happen for output going somewhere else than the screen.

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

2008-05-05 Thread Bruce Momjian
Bruce Momjian wrote:
 Gregory Stark wrote:
  Bruce Momjian [EMAIL PROTECTED] writes:
  
   OK, so COLUMNS should take precedence.  I assume this is going to
   require us to read the COLUMNS enviroment variable in psql _before_
   readline sets it, and that COLUMNS will only affect screen output, like
   ioctl().  Is that consistent?
  
  What are you talking about? screen output?
  
  Are you even getting my emails? Please confirm that you receive this email.
 
 Yes, I am getting your emails, but I have more than you to please.
 
 Are others OK with $COLUMNS controlling screen output and file/pipe, or
 perhaps COLUMNS controlling only file/pipe, as GNU ls does?  I have
 heard a few people say they only want \pset columns to control
 file/pipe.
 
 I have outlined the GNU ls behavior in a email I just sent.

I talked to Greg on IM and he had a new idea, related to my 'auto' idea
of using aligned/wrapped/expanded mode automatically.

His idea is that 'auto' mode (aligned/wrapped/expanded) is only for
output to the screen, and 'wrapped' is always wrapped. 'Wrapped' would
be affected by \pset columns, COLUMNS, or the terminal width for output
to screen, file, or pipe.  If no width could be derrmined for wrapped,
it would default to 72.

This gives us 'wrapped' that always wraps (if it can fit the headings),
and most people will use 'auto' in their psqlrc, and it affects only
terminal output.

-- 
  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] Proposed patch - psql wraps at window width

2008-05-05 Thread Bruce Momjian
Tom Lane wrote:
 Bruce Momjian [EMAIL PROTECTED] writes:
  Are others OK with $COLUMNS controlling screen output and file/pipe, or
  perhaps COLUMNS controlling only file/pipe, as GNU ls does?  I have
  heard a few people say they only want \pset columns to control
  file/pipe.
 
 I agree with the latter.  Anyone who is setting COLUMNS is going to set
 it to reflect the *screen* width they want; it has zero to do with
 what should happen for output going somewhere else than the screen.

OK, that's what I thought, and what I think Peter wants, and what the
posted patch does.

-- 
  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] [0/4] Proposal of SE-PostgreSQL patches

2008-05-05 Thread Tom Lane
Josh Berkus [EMAIL PROTECTED] writes:
 For hackers who don't understand security frameworks, I'm going to make a 
 strong case for KaiGai's patch.
 ...
 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.  

Well, let's be perfectly blunt here: this is an extremely sizable patch
that will be of interest to less than 1% of our users (and I think I'm
being generous in that estimate).  If it's going to get in, it's going
to have to not cost us anything much on reliability, maintainability,
or performance --- rank those however you please, there is someone out
there who will be most interested in any one of them.

My look at the code today convinced me that it's not going to get
applied without a whole lot of further work.  I think what we must
figure out in this commit-fest is whether to encourage KaiGai to start
doing that work, or to decide that it probably is a dead issue and he
shouldn't bother.

As to reliability: the issue that worries me the most was already raised
by Greg Smith --- this patch puts security checks and consequent catalog
lookups into some mighty low-level places that IMHO have no business
triggering catalog accesses.  If I'd been able to make the patch work
today, I'd have tested whether it could get through the regression tests
with CLOBBER_CACHE_ALWAYS defined.  I'd be happier if it were refactored
to put the security checks only into executor code paths, and not try to
enforce security restrictions against the system itself.  (Thought
experiment: what happens if SELinux denies access to a row of
pg_security to the code that is supposed to be checking security?
Or try this one: it looks to me like you can set up the system to
pretend to some user that the pg_class rows for certain indexes don't
exist, even though he has update rights on their parent table.  Instant
index corruption.)

As to maintainability: I already posted a lot of unhappy remarks on code
style and readability.  I don't think there's anything unfixable there,
but there's a lot of work to do to convert what was evidently written as
an arms-length add-on into a sensible part of core Postgres.

As to performance: frankly, I'm afraid the performance is abysmal.
This was what I was mainly hoping to check out if I could have gotten
it to build today.  We need to at least get some rough pgbench readings
before deciding whether it's worth pushing forward.

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 - LDAPS support for servers on port 636 w/o TLS

2008-05-05 Thread steve layland
Thank you all for your comments.  I was unaware the ldaps: scheme was
not supposed to be used for LDAP+TLS encryption, but it makes sense now
that you mention it.

There's a nice discussion about how the folks working on mod_ldap for
Apache worked this out way back in 2005:

http://mail-archives.apache.org/mod_mbox/httpd-dev/200501.mbox/[EMAIL PROTECTED]

Anyway, I think we've distilled the issue down to how to best enable TLS
for ldap:// connections.

By my reckoning, that means we can have:

1) per-hba.conf entry configuration where the configuration can
be:

a) of the ldap URL extension form mentioned by David
(!StartTLS).

b) key=value type of param string as suggested by Magnus

c) a specific URI scheme like ldap+tls:// like Tom
suggested.

d) a new authentication type ldaptls

2) per-postgres server configuration which can be:

a) an old LDAPTLS environment variable ? needs research

b) a server-wide GUC variable (along with TLSCERT
specifications?) as in the current patch

I'm open to other suggestions.

One other thing to keep in mind is how best to map database roles to
ldap Distinguished Name (dn) entries?

In other words, we need to take the user jimmy in

psql -U jimmy

and translate into an ldap authentication request for the distinguished
name that is entirely dependent on the site and ldap impl, example:

uid=jimmy,ou=people,dc=example,dc=com

I've racked my brain thinking of ways that this can fit cleanly in
hba.conf, but I haven't found anything I _really_ like (current patch 
and proposal 3 below are prob my favorites.) Any other
ideas/comments/suggestions?

# Current Functionality for reference - no tls control
hostdbname  all 127.0.0.0/32ldap 
ldap://ldap.example.com[:port]/ignored;uid=;ou=people,dc=example,dc=com;

# Current Functionality in patch (w/ server wide TLS control in GUC var)
# GUC var causes all ldap entries to use same authentication. can be
# applied to service lookup as well
hostdbname  all 127.0.0.0/32ldap 
ldap://ldap.example.com[:port]/ou=people,dc=example,dc=com;uid=;

# proposal 1 - RFC 2255 URI kind of yucky; scope, attributes, filter
# not actually used in simple authentication
hostdbname  all 127.0.0.0/32ldap 
ldap://ldap.example.com[:port]/uid=%u,ou=people,dc=example,dc=com???!StartTLS;

# proposal 1b - still RFC 2255 compliant, but semantically weird.  no
# filter is actually used in simple authentication
hostdbname  all 127.0.0.0/32ldap 
ldap://ldap.example.com[:port]/ou=people,dc=example,dc=com?one??(uid=%u)!StartTLS

# proposal 2 - psuedo-URI scheme; hacky but easy
hostdbname  all 127.0.0.0/32ldap 
ldap+tls://ldap.example.com[:port]/ou=people,dc=example,dc=com;uid=;

# proposal 3 - mod hba parsing, add new ldaptls auth type; reasonably
# easy and least invasive; 
hostdbname  all 127.0.0.0/32ldaptls 
ldap://ldap.example.com[:port]/ou=people,dc=example,dc=com;uid=;;

# proposal 4 - mod hba parsing
hostdbname  all 127.0.0.0/32ldap 
ldap://ldap.example.com[:port]/ou=people,dc=example,dc=com;uid=;; StartTLS

# proposal 5 - Magnum's key = value like idea (i'm guessing here,
# Magnum.  If I misinterpret, please explain)
hostdbname  all 127.0.0.0/32ldap 
ldap://ldap.example.com[:port]/ou=people,dc=example,dc=com;prefix=uid=;start_tls=1;

I have some radical ideas as well involving completely ripping out the
pg_hba.conf file but I'll leave that for another, more appropriate day.
:)

Thanks again for the feedback, and sorry for the verbosity.

-Steve (#postgresql rockpunk)


signature.asc
Description: Digital signature