Re: [HACKERS] PQinitSSL broken in some use casesf
On Sun, Mar 29, 2009 at 1:56 PM, Bruce Momjian br...@momjian.us wrote: I think this is where we got stuck because extending libpq with a new function is a larger API change, and not having a clear plan of what initialization stuff we might need in the future, it seems unwise, and also perhaps overkill. right, maybe this is a case of 'the perfect being the enemy of the good'...I have no qualms with fixing it now with the two argument version. The stuff I'd like to see is a separate issue, I guess. merlin -- 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] PQinitSSL broken in some use casesf
Tom Lane wrote:
I personally would be happy with the two-argument function solution.
I modified my previous patch to use a two-argument function solution.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: doc/src/sgml/libpq.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.280
diff -C6 -r1.280 libpq.sgml
*** doc/src/sgml/libpq.sgml 28 Mar 2009 01:36:11 - 1.280
--- doc/src/sgml/libpq.sgml 30 Mar 2009 13:29:48 -
***
*** 6179,6190
--- 6179,6219
!-- If this URL changes replace it with a URL to www.archive.org. --
See ulink
url=http://h71000.www7.hp.com/doc/83final/BA554_90007/ch04.html;/ulink
for details on the SSL API.
/para
+ para
+variablelist
+ varlistentry
+ term
+ functionPQinitSecure/function
+ indexterm
+primaryPQinitSecure/primary
+ /indexterm
+ /term
+
+ listitem
+ para
+Allows applications to select which secure components to initialize.
+synopsis
+ void PQinitSecure(int do_ssl, init do_crypto);
+/synopsis
+ /para
+
+ para
+When do_ssl is non-zero, OpenSSL's SSL library will be initialized.
+When do_crypto is non-zero, OpenSSL's Crypto library will be initialized.
+By default, both libraries are initialized. When SSL support is not
+compiled in, this function does nothing.
+ /para
+ /listitem
+ /varlistentry
+/variablelist
+ /para
+
table id=libpq-ssl-file-usage
titleLibpq/Client SSL File Usage/title
tgroup cols=3
thead
row
entryFile/entry
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.22
diff -C6 -r1.22 exports.txt
*** src/interfaces/libpq/exports.txt 22 Sep 2008 13:55:14 - 1.22
--- src/interfaces/libpq/exports.txt 30 Mar 2009 13:29:48 -
***
*** 149,154
--- 149,155
PQinstanceData147
PQsetInstanceData 148
PQresultInstanceData 149
PQresultSetInstanceData 150
PQfireResultCreateEvents 151
PQconninfoParse 152
+ PQinitSecure 153
Index: src/interfaces/libpq/fe-secure.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.121
diff -C6 -r1.121 fe-secure.c
*** src/interfaces/libpq/fe-secure.c 28 Mar 2009 18:48:55 - 1.121
--- src/interfaces/libpq/fe-secure.c 30 Mar 2009 13:29:49 -
***
*** 96,107
--- 96,108
static PostgresPollingStatusType open_client_SSL(PGconn *);
static void close_SSL(PGconn *);
static char *SSLerrmessage(void);
static void SSLerrfree(char *buf);
static bool pq_init_ssl_lib = true;
+ static bool pq_init_crypto_lib = true;
static SSL_CTX *SSL_context = NULL;
#ifdef ENABLE_THREAD_SAFETY
static int ssl_open_connections = 0;
#ifndef WIN32
***
*** 170,182
* initialized OpenSSL.
*/
void
PQinitSSL(int do_init)
{
#ifdef USE_SSL
! pq_init_ssl_lib = do_init;
#endif
}
/*
* Initialize global context
*/
--- 171,196
* initialized OpenSSL.
*/
void
PQinitSSL(int do_init)
{
#ifdef USE_SSL
! PQinitSecure(do_init, do_init);
! #endif
! }
!
! /*
! * Exported function to allow application to tell us which secure
! * components to initialize.
! */
! void
! PQinitSecure(int do_ssl, int do_crypto)
! {
! #ifdef USE_SSL
! pq_init_ssl_lib = do_ssl;
! pq_init_crypto_lib = do_crypto;
#endif
}
/*
* Initialize global context
*/
***
*** 820,831
--- 834,847
* message - no connection local setup is made.
*/
static int
init_ssl_system(PGconn *conn)
{
#ifdef ENABLE_THREAD_SAFETY
+ int num_ssl_conns = 0;
+
#ifdef WIN32
/* Also see similar code in fe-connect.c, default_threadlock() */
if (ssl_config_mutex == NULL)
{
while (InterlockedExchange(win32_ssl_create_mutex, 1) == 1)
/* loop, another thread own the lock */ ;
***
*** 837,849
InterlockedExchange(win32_ssl_create_mutex, 0);
}
#endif
if (pthread_mutex_lock(ssl_config_mutex))
return -1;
! if (pq_init_ssl_lib)
{
/*
* If necessary, set up an array to hold locks for OpenSSL. OpenSSL will
* tell us how big to make this array.
*/
if (pq_lockarray == NULL)
--- 853,874
InterlockedExchange(win32_ssl_create_mutex, 0);
}
#endif
if (pthread_mutex_lock(ssl_config_mutex))
return -1;
! /*
! * Increment connection count if we are responsible for
! * intiializing the SSL or Crypto library. Currently, only
! * crypto needs this during setup and cleanup. That may
! *
Re: [HACKERS] PQinitSSL broken in some use casesf
Andrew Chernow a...@esilo.com writes: I modified my previous patch to use a two-argument function solution. It sounds like everyone has converged on agreeing that this way is okay for 8.4? Object now or hold your peace ... 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] PQinitSSL broken in some use casesf
Bruce Momjian br...@momjian.us writes: Tom Lane wrote: It sounds like everyone has converged on agreeing that this way is okay for 8.4? Object now or hold your peace ... What are we doing with PQinitSSL()? Nothing, except improving the documentation. 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] PQinitSSL broken in some use casesf
Robert Haas robertmh...@gmail.com writes: This looks OK to me, except I think we should modify the documentation to PQinitSSL() to say that it you must not use both that function and PQinitSecure(), and explain that if you need to control initialization of libcrypto and libssl, you should use that function instead. Agreed, the docs need a bit more effort. I'll have a go at that. 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] PQinitSSL broken in some use casesf
On Mon, Mar 30, 2009 at 9:36 AM, Andrew Chernow a...@esilo.com wrote: Tom Lane wrote: I personally would be happy with the two-argument function solution. I modified my previous patch to use a two-argument function solution. This looks OK to me, except I think we should modify the documentation to PQinitSSL() to say that it you must not use both that function and PQinitSecure(), and explain that if you need to control initialization of libcrypto and libssl, you should use that function instead. ...Robert -- 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] PQinitSSL broken in some use casesf
Tom Lane wrote: Andrew Chernow a...@esilo.com writes: I modified my previous patch to use a two-argument function solution. It sounds like everyone has converged on agreeing that this way is okay for 8.4? Object now or hold your peace ... What are we doing with PQinitSSL()? -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: This looks OK to me, except I think we should modify the documentation to PQinitSSL() to say that it you must not use both that function and PQinitSecure(), and explain that if you need to control initialization of libcrypto and libssl, you should use that function instead. Agreed, the docs need a bit more effort. I'll have a go at that. Right; I saw no change to PSinitSSL documentation in the patch, which was my concern. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Tom Lane wrote: It sounds like everyone has converged on agreeing that this way is okay for 8.4? Object now or hold your peace ... What are we doing with PQinitSSL()? Nothing, except improving the documentation. My patch didn't change the docs at all. I wasn't sure what to do there. However, I did change the implementation but left behavior alone. It now calls PQinitSecure(do_init, do_init); -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
On Mon, Mar 30, 2009 at 10:22 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Mar 30, 2009 at 9:36 AM, Andrew Chernow a...@esilo.com wrote: Tom Lane wrote: I personally would be happy with the two-argument function solution. I modified my previous patch to use a two-argument function solution. This looks OK to me, except I think we should modify the documentation to PQinitSSL() to say that it you must not use both that function and PQinitSecure(), and explain that if you need to control initialization of libcrypto and libssl, you should use that function instead. do you think PQinitSSL should be deprecated? merlin -- 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] PQinitSSL broken in some use casesf
Merlin Moncure wrote: On Mon, Mar 30, 2009 at 10:22 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Mar 30, 2009 at 9:36 AM, Andrew Chernow a...@esilo.com wrote: Tom Lane wrote: I personally would be happy with the two-argument function solution. I modified my previous patch to use a two-argument function solution. This looks OK to me, except I think we should modify the documentation to PQinitSSL() to say that it you must not use both that function and PQinitSecure(), and explain that if you need to control initialization of libcrypto and libssl, you should use that function instead. do you think PQinitSSL should be deprecated? Well, I think having duplicate capability in an API is confusing, so yea. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Merlin Moncure mmonc...@gmail.com writes: On Mon, Mar 30, 2009 at 10:22 AM, Robert Haas robertmh...@gmail.com wrote: This looks OK to me, except I think we should modify the documentation to PQinitSSL() to say that it you must not use both that function and PQinitSecure(), and explain that if you need to control initialization of libcrypto and libssl, you should use that function instead. do you think PQinitSSL should be deprecated? I see no reason to deprecate it per se. I was planning to just define it as equivalent to PQinitSecure(do_init, do_init). BTW, unless someone objects I'd like to make the name of that function PQinitSecurity. The other looks like it's implying that it is replacing an insecure PQinit() function ... 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] PQinitSSL broken in some use casesf
Merlin Moncure wrote: On Mon, Mar 30, 2009 at 10:22 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Mar 30, 2009 at 9:36 AM, Andrew Chernow a...@esilo.com wrote: Tom Lane wrote: I personally would be happy with the two-argument function solution. I modified my previous patch to use a two-argument function solution. This looks OK to me, except I think we should modify the documentation to PQinitSSL() to say that it you must not use both that function and PQinitSecure(), and explain that if you need to control initialization of libcrypto and libssl, you should use that function instead. do you think PQinitSSL should be deprecated? FYI, libcrypto has been initializing with SSL in threaded builds since Postgres 8.0. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Bruce Momjian wrote: Merlin Moncure wrote: On Mon, Mar 30, 2009 at 10:22 AM, Robert Haas robertmh...@gmail.com wrote: On Mon, Mar 30, 2009 at 9:36 AM, Andrew Chernow a...@esilo.com wrote: Tom Lane wrote: I personally would be happy with the two-argument function solution. I modified my previous patch to use a two-argument function solution. This looks OK to me, except I think we should modify the documentation to PQinitSSL() to say that it you must not use both that function and PQinitSecure(), and explain that if you need to control initialization of libcrypto and libssl, you should use that function instead. do you think PQinitSSL should be deprecated? Well, I think having duplicate capability in an API is confusing, so yea. Maybe document that PQinitSSL(do_init) is now the same as PQinitSecure(do_init, do_init). -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Tom Lane wrote: BTW, unless someone objects I'd like to make the name of that function PQinitSecurity. Agreed. Although, both PQinitSecure and PQinitSecurity are very general names. Security can mean a lot of things. Maybe the name should be more particular, like PQinitOpenSSL. I think its okay to reference the openssl project name being how this function is designed to work with openssl's library split. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
On 30 mar 2009, at 17.24, Andrew Chernow a...@esilo.com wrote: Tom Lane wrote: BTW, unless someone objects I'd like to make the name of that function PQinitSecurity. Agreed. Although, both PQinitSecure and PQinitSecurity are very general names. Security can mean a lot of things. Maybe the name should be more particular, like PQinitOpenSSL. I think its okay to reference the openssl project name being how this function is designed to work with openssl's library split. +1 It's quite likely that well want to support another ssl library in the future. At least likely enough that any api we define should take it into consideration. /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] PQinitSSL broken in some use casesf
On Mon, Mar 30, 2009 at 11:56 AM, Magnus Hagander mag...@hagander.net wrote: On 30 mar 2009, at 17.24, Andrew Chernow a...@esilo.com wrote: Tom Lane wrote: BTW, unless someone objects I'd like to make the name of that function PQinitSecurity. Agreed. Although, both PQinitSecure and PQinitSecurity are very general names. Security can mean a lot of things. Maybe the name should be more particular, like PQinitOpenSSL. I think its okay to reference the openssl project name being how this function is designed to work with openssl's library split. +1 It's quite likely that well want to support another ssl library in the future. At least likely enough that any api we define should take it into consideration. +1. ...Robert -- 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] PQinitSSL broken in some use casesf
Magnus Hagander wrote: On 30 mar 2009, at 17.24, Andrew Chernow a...@esilo.com wrote: Tom Lane wrote: BTW, unless someone objects I'd like to make the name of that function PQinitSecurity. Agreed. Although, both PQinitSecure and PQinitSecurity are very general names. Security can mean a lot of things. Maybe the name should be more particular, like PQinitOpenSSL. I think its okay to reference the openssl project name being how this function is designed to work with openssl's library split. +1 It's quite likely that well want to support another ssl library in the future. At least likely enough that any api we define should take it into consideration. Are we sure we don't want to add a more general libpq initialization control at this time? PQinitOptions(PG_NO_SSL_INIT | PG_NO_CRYPTO_INIT); -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Bruce Momjian wrote: Magnus Hagander wrote: On 30 mar 2009, at 17.24, Andrew Chernow a...@esilo.com wrote: Tom Lane wrote: BTW, unless someone objects I'd like to make the name of that function PQinitSecurity. Agreed. Although, both PQinitSecure and PQinitSecurity are very general names. Security can mean a lot of things. Maybe the name should be more particular, like PQinitOpenSSL. I think its okay to reference the openssl project name being how this function is designed to work with openssl's library split. +1 It's quite likely that well want to support another ssl library in the future. At least likely enough that any api we define should take it into consideration. Are we sure we don't want to add a more general libpq initialization control at this time? PQinitOptions(PG_NO_SSL_INIT | PG_NO_CRYPTO_INIT); Could be an option but if we go down that path, it needs to be PG_NO_OPENSSL_SSL_INIT and PG_NO_OPENSSL_CRYPTO_INIT //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] PQinitSSL broken in some use casesf
Magnus Hagander mag...@hagander.net writes: Bruce Momjian wrote: Are we sure we don't want to add a more general libpq initialization control at this time? PQinitOptions(PG_NO_SSL_INIT | PG_NO_CRYPTO_INIT); Could be an option but if we go down that path, it needs to be PG_NO_OPENSSL_SSL_INIT and PG_NO_OPENSSL_CRYPTO_INIT And we get into the whole question of error handling, which is what shot down that proposal last time. 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] PQinitSSL broken in some use casesf
Tom Lane wrote: Magnus Hagander mag...@hagander.net writes: Bruce Momjian wrote: Are we sure we don't want to add a more general libpq initialization control at this time? PQinitOptions(PG_NO_SSL_INIT | PG_NO_CRYPTO_INIT); Could be an option but if we go down that path, it needs to be PG_NO_OPENSSL_SSL_INIT and PG_NO_OPENSSL_CRYPTO_INIT And we get into the whole question of error handling, which is what shot down that proposal last time. Can you remind me of the details? I don't remember that issue. Currently PQinitSSL() returns void, so I don't see an issue there. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Bruce Momjian br...@momjian.us writes: Tom Lane wrote: And we get into the whole question of error handling, which is what shot down that proposal last time. Can you remind me of the details? I don't remember that issue. Currently PQinitSSL() returns void, so I don't see an issue there. The point is exactly the same as the complaint about turning PQinitSSL's argument into a bitmask: if you are trying to define an extensible API then you need a way for the app to determine whether all the bits it passed were recognizable by the library. I think we should stick with the simple two-argument function and not try to design a solution for unknown problems. Otherwise we are right back at the point where the previous thread petered out for lack of consensus. 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] PQinitSSL broken in some use casesf
Tom Lane wrote: I think we should stick with the simple two-argument function and not +100 Generic PQinit concept was already punted several times. Using a bit mask for initsecure or something was also tried and rejected. The well is dry on this. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Tom Lane wrote: And we get into the whole question of error handling, which is what shot down that proposal last time. Can you remind me of the details? I don't remember that issue. Currently PQinitSSL() returns void, so I don't see an issue there. The point is exactly the same as the complaint about turning PQinitSSL's argument into a bitmask: if you are trying to define an extensible API then you need a way for the app to determine whether all the bits it passed were recognizable by the library. It seems having the init function return the bits it recognized would be the logical return value. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
On Mon, Mar 30, 2009 at 2:31 PM, Bruce Momjian br...@momjian.us wrote: Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Tom Lane wrote: And we get into the whole question of error handling, which is what shot down that proposal last time. Can you remind me of the details? I don't remember that issue. Currently PQinitSSL() returns void, so I don't see an issue there. The point is exactly the same as the complaint about turning PQinitSSL's argument into a bitmask: if you are trying to define an extensible API then you need a way for the app to determine whether all the bits it passed were recognizable by the library. It seems having the init function return the bits it recognized would be the logical return value. I'm disinclined to go this route because, as Tom pointed out upthread, there's no evidence at all that this problem is an instance of some more general class. If it turns out that we need to initialize other things, they're likely to be totally unrelated to SSL, and whether they are or not, they may not be things that can be indicated using a bitmask. For example, what happens if we someday add support for a separate SSL library called FooSSL, and we happen to need a char * to describe how it should be initialized? Or we might need a whole char * or int argument for some purpose unrelated to SSL. Then we'll have a PQinit() function that pretends to be a general-purpose initialization mechanism but really isn't. With enough wrangling, we might be able to convince ourselves that we've designed something which is general enough to cover all contingencies, but I'm not sure there's any good reason to put in that much work. Adding a new API call is not really that big a deal, especially given that we're retaining the old one for backward compatibility. If we discover we need something else in the future, we'll just add an API call for that, too. ...Robert -- 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] PQinitSSL broken in some use casesf
Bruce Momjian wrote: It seems having the init function return the bits it recognized would be the logical return value. That's what my first PQionitSecure patch did, actually it returned the bits it didn't understand. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Robert Haas wrote: Or we might need a whole char * or int argument for some purpose unrelated to SSL. Then we'll have a PQinit() function that pretends to be a general-purpose initialization mechanism but really isn't. I proposed a PQinit() that included a void data pointer argument. The idea was PQinit(which, data, data_len) would be called multiple times to init different things. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
On Mon, Mar 30, 2009 at 2:59 PM, Andrew Chernow a...@esilo.com wrote: Robert Haas wrote: Or we might need a whole char * or int argument for some purpose unrelated to SSL. Then we'll have a PQinit() function that pretends to be a general-purpose initialization mechanism but really isn't. I proposed a PQinit() that included a void data pointer argument. The idea was PQinit(which, data, data_len) would be called multiple times to init different things. I guess that'd work but it might be overkill. ...Robert -- 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] PQinitSSL broken in some use casesf
Robert Haas robertmh...@gmail.com writes: I guess that'd work but it might be overkill. The real bottom line is that *all* the proposals for generic init functions are overkill. We have no evidence that we need one and no certainty about what the requirements for it would be if we did. I think we should just do PQinitOpenSSL(2 args) and be done with it. 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] PQinitSSL broken in some use casesf
On Mon, Mar 30, 2009 at 3:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I guess that'd work but it might be overkill. The real bottom line is that *all* the proposals for generic init functions are overkill. We have no evidence that we need one and no certainty about what the requirements for it would be if we did. I think we should just do PQinitOpenSSL(2 args) and be done with it. I was pushing for generic PQinit, but have come around and agree with this point of view. merlin -- 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] PQinitSSL broken in some use casesf
Merlin Moncure wrote: On Mon, Mar 30, 2009 at 3:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: I guess that'd work but it might be overkill. The real bottom line is that *all* the proposals for generic init functions are overkill. ?We have no evidence that we need one and no certainty about what the requirements for it would be if we did. I think we should just do PQinitOpenSSL(2 args) and be done with it. I was pushing for generic PQinit, but have come around and agree with this point of view. OK, we are _go_ then. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Andrew Chernow a...@esilo.com writes: I modified my previous patch to use a two-argument function solution. Applied with minor changes (rename to PQinitOpenSSL per discussion, some comment and docs improvements). 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] PQinitSSL broken in some use casesf
Bruce Momjian br...@momjian.us writes: Merlin Moncure wrote: I think we should just do PQinitOpenSSL(2 args) and be done with it. I was pushing for generic PQinit, but have come around and agree with this point of view. OK, we are _go_ then. Done. Let's hope this thread stays dead. 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] PQinitSSL broken in some use casesf
Bruce Momjian br...@momjian.us writes: Andrew Chernow wrote: Adding PQinitSSL(new_value) seem reasonable to me. My only complaint has been that the API user has no way of knowing if the function understood their request. I think doing PQinitSSL(new_value) is probably the least invasive change to solve this, which is why I suggested it. It does have a compile-time check by referencing the #define. You're missing the point, which is that it isn't certain whether the right thing happens at runtime. It would be very hard to debug the failure if an app compiled against new headers was run with an old shlib. The separate two-argument function would avoid that: the failure would manifest as called function doesn't exist, which would at least make it obvious what was wrong. I personally would be happy with the two-argument function solution. Now, that only addresses the specific problem of libcrypto vs libssl. IIUC, Merlin's current thought is that we should be looking for a more general solution. But it seems a bit dangerous to try to design a general solution when we have only one example to work from. 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] PQinitSSL broken in some use casesf
Tom Lane wrote: I personally would be happy with the two-argument function solution. The patch I submitted pretty much does this, except it uses a flags argument instead of 2 fixed arguments. It can be easily changed to support the 2 argument idea: 1. Change prototype to: void PQinitSecure(int ssl, int crypto); - previous return value can go away, new version can be void. 2. Remove PG_SECURE_SSL, PG_SECURE_CRYPTO from libpq-fe.h 3. Modify wrapping code in PQinitSSL to use new prototype. - either pass 0,0 or 1,1, to PQinitSecure. Outside of that, the patch goes unchanged. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Tom Lane wrote: Bruce Momjian br...@momjian.us writes: Andrew Chernow wrote: Adding PQinitSSL(new_value) seem reasonable to me. My only complaint has been that the API user has no way of knowing if the function understood their request. I think doing PQinitSSL(new_value) is probably the least invasive change to solve this, which is why I suggested it. It does have a compile-time check by referencing the #define. You're missing the point, which is that it isn't certain whether the right thing happens at runtime. It would be very hard to debug the failure if an app compiled against new headers was run with an old shlib. The separate two-argument function would avoid that: the failure would manifest as called function doesn't exist, which would at least make it obvious what was wrong. I personally would be happy with the two-argument function solution. Now, that only addresses the specific problem of libcrypto vs libssl. IIUC, Merlin's current thought is that we should be looking for a more general solution. But it seems a bit dangerous to try to design a general solution when we have only one example to work from. I think this is where we got stuck because extending libpq with a new function is a larger API change, and not having a clear plan of what initialization stuff we might need in the future, it seems unwise, and also perhaps overkill. FYI, libcrypto is initialized only in threaded libpq builds, and non-zero calls to PQinitSSL were always no-ops because they just enabled the default behavior. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
On Fri, Mar 27, 2009 at 9:38 PM, Bruce Momjian br...@momjian.us wrote: I have applied the attached patch which does several things: o documents that libssl _and_ libcrypto initialization is turned off by PQinitSSL(0) o clarified cases where this behavior is important o added comments that the CRYPTO_set_* calls reference libcrypto, not libssl I think we can now say that the current behavior is not a bug because it is documented, even though the PQinitSSL() function name is inaccurate. It is still a bug in the sense that it is impossible to properly initialize crypto features in some scenarios. A doc patch (which I argued is the best way to go for 8.4) fails to properly raise the seriousness of the issue and also fails to suggest a workaround. I think a proper way to document this issue would be something like this: If your application initializes libcrypto, but not libssl, you must not call PQinitSSL(1) because it will overwrite your libcrypto initialization. In order to safely use libpq in your application, you must include ssl headers and call the following functions: #include openssl/ssl.h #include openssl/conf.h OPENSSL_config(NULL); SSL_library_init(); SSL_load_error_strings(); PQinitSSL(0); In order to initialize libpq properly for SSL connections. I think there is a good argument that PQinitSSL(X) where X 1 would work fine for more fine-grained control. The new libpq init function idea was interesting, but having a documented solution for WSAStartup()/WSACleanup() usage, we now don't have another libpq init use-case so it is hard to suggest a new libpq function. This feature when discussed at the time was not enough _by itself_ to support a PQinit feature (I agree with this reasoning), but surely should be considered as valid supporting evidence that a library initialization feature is useful. IOW, the whole of the argument is equal to the sum of its parts. (yes, we have an agenda here: we were not happy that our events patch could not establish behavior at library initialization time). merlin -- 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] PQinitSSL broken in some use casesf
On Sat, Mar 28, 2009 at 9:23 AM, Merlin Moncure mmonc...@gmail.com wrote: It is still a bug in the sense that it is impossible to properly initialize crypto features in some scenarios. A doc patch (which I Meant to say: 'your doc patch merlin -- 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] PQinitSSL broken in some use casesf
Merlin Moncure wrote: It is still a bug in the sense that it is impossible to properly initialize crypto features in some scenarios. A doc patch (which I argued is the best way to go for 8.4) fails to properly raise the seriousness of the issue and also fails to suggest a workaround. I think a proper way to document this issue would be something like this: If your application initializes libcrypto, but not libssl, you must not call PQinitSSL(1) because it will overwrite your libcrypto initialization. In order to safely use libpq in your application, you must include ssl headers and call the following functions: #include openssl/ssl.h #include openssl/conf.h OPENSSL_config(NULL); SSL_library_init(); SSL_load_error_strings(); PQinitSSL(0); In order to initialize libpq properly for SSL connections. I think there is a good argument that PQinitSSL(X) where X 1 would work fine for more fine-grained control. ?The new libpq init function idea was interesting, but having a documented solution for WSAStartup()/WSACleanup() usage, we now don't have another libpq init use-case so it is hard to suggest a new libpq function. This feature when discussed at the time was not enough _by itself_ to support a PQinit feature (I agree with this reasoning), but surely should be considered as valid supporting evidence that a library initialization feature is useful. IOW, the whole of the argument is equal to the sum of its parts. (yes, we have an agenda here: we were not happy that our events patch could not establish behavior at library initialization time). Well, we are not the Make Merlin Happy club. ;-) Making usable software requires adjustments on everyone's part. I don't think we have enough demand to hardcode those details in our documentation. Personally, I feel adding #defines for PQinitSSL is the right approach, and I think that gives enough checks at compile time, but it doesn't guard against cases where the application is built against one version of libpq but is run against an older version, which I think can happen. My personal opinion is that adding #defines to PQinitSSL() is the right level of fix, and if we ever implement a libpq init mechanism we can convert PGinitSSL to a macro. I am attaching a sample patch for feedback. However, as I mentioned above, this is not the Make Bruce Happy club either so I need support from other developers that this is the right fix. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/libpq.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v retrieving revision 1.280 diff -c -c -r1.280 libpq.sgml *** doc/src/sgml/libpq.sgml 28 Mar 2009 01:36:11 - 1.280 --- doc/src/sgml/libpq.sgml 28 Mar 2009 19:06:45 - *** *** 6172,6181 If your application initializes literallibssl/ or literallibcrypto/ libraries and applicationlibpq/application is built with acronymSSL/ support, you should call !functionPQinitSSL(0)/ to tell applicationlibpq/application that the literallibssl/ and literallibcrypto/ libraries have been initialized by your application so applicationlibpq/application will not initialize those libraries. !-- If this URL changes replace it with a URL to www.archive.org. -- See ulink url=http://h71000.www7.hp.com/doc/83final/BA554_90007/ch04.html;/ulink --- 6172,6185 If your application initializes literallibssl/ or literallibcrypto/ libraries and applicationlibpq/application is built with acronymSSL/ support, you should call !functionPQinitSSL(PG_NO_INIT_SSL_CRYPTO)/ to tell applicationlibpq/application that the literallibssl/ and literallibcrypto/ libraries have been initialized by your application so applicationlibpq/application will not initialize those libraries. +To have applicationlibpq/application initialize only +literallibssl/, use functionPQinitSSL(PG_INIT_SSL_ONLY)/, and +use functionPQinitSSL(PG_INIT_CRYPTO_ONLY)/ to initialize only +literallibcrypto/. !-- If this URL changes replace it with a URL to www.archive.org. -- See ulink url=http://h71000.www7.hp.com/doc/83final/BA554_90007/ch04.html;/ulink Index: src/interfaces/libpq/fe-secure.c === RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v retrieving revision 1.121 diff -c -c -r1.121 fe-secure.c *** src/interfaces/libpq/fe-secure.c 28 Mar 2009 18:48:55 - 1.121 --- src/interfaces/libpq/fe-secure.c 28 Mar 2009 19:06:45 - *** *** 99,104 --- 99,105 static void SSLerrfree(char *buf); static bool pq_init_ssl_lib = true; + static bool
Re: [HACKERS] PQinitSSL broken in some use casesf
Bruce Momjian br...@momjian.us writes: Well, we are not the Make Merlin Happy club. ;-) Merlin and Andrew were the ones complaining initially. If they feel that a proposed patch doesn't fix the problem, then I'd say that it isn't fixing the problem. My personal opinion is that adding #defines to PQinitSSL() is the right level of fix, and if we ever implement a libpq init mechanism we can convert PGinitSSL to a macro. I am attaching a sample patch for feedback. This is just a rehash of one of the patches that was discussed earlier. There wasn't consensus for it then, and there's not now. 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] PQinitSSL broken in some use casesf
Tom Lane wrote: This is just a rehash of one of the patches that was discussed earlier. There wasn't consensus for it then, and there's not now. I am personally out of ideas. It feels like this issue has been beaten to death. There are only a few ways to do it and I believe they have all been put on the table. Any suggestions? -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Andrew Chernow wrote: Tom Lane wrote: This is just a rehash of one of the patches that was discussed earlier. There wasn't consensus for it then, and there's not now. I am personally out of ideas. It feels like this issue has been beaten to death. There are only a few ways to do it and I believe they have all been put on the table. Any suggestions? Well, at least the current behavior is documented now. I personally thought the #define was the best minimal fix because it only added a few lines to the docs and the C code. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
On Sat, Mar 28, 2009 at 4:26 PM, Bruce Momjian br...@momjian.us wrote: Andrew Chernow wrote: Tom Lane wrote: This is just a rehash of one of the patches that was discussed earlier. There wasn't consensus for it then, and there's not now. I am personally out of ideas. It feels like this issue has been beaten to death. There are only a few ways to do it and I believe they have all been put on the table. Any suggestions? Well, at least the current behavior is documented now. You mean, with your proposed documentation patch? (if you meant my proposed advice, then ignore the following paragraph) Not only does it not suggest the problem, it actually misinforms you...it suggests you call PQinitSSL(0) when your application initializes literallibssl/ or literallibcrypto/ libraries. The operative word being 'or'. If you follow the advice after only having initialized the libcrypto library, you would get a failure trying to use libpq/SSL. merlin -- 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] PQinitSSL broken in some use casesf
On Sat, Mar 28, 2009 at 3:51 PM, Tom Lane t...@sss.pgh.pa.us wrote: Bruce Momjian br...@momjian.us writes: Well, we are not the Make Merlin Happy club. ;-) Merlin and Andrew were the ones complaining initially. If they feel that a proposed patch doesn't fix the problem, then I'd say that it isn't fixing the problem. +1. My personal opinion is that adding #defines to PQinitSSL() is the right level of fix, and if we ever implement a libpq init mechanism we can convert PGinitSSL to a macro. I am attaching a sample patch for feedback. This is just a rehash of one of the patches that was discussed earlier. There wasn't consensus for it then, and there's not now. OK, I read this patch. Wazzamatterwithit? AIUI, we need to define an API that allows libssl and libcrypto to be initialized or not initialized in any combination. We can do this by modifying the meaning of the argument to PQinitSSL(), by adding a new API function, by creating some more general initialization function, or by some other method. It doesn't REALLY matter. I think I'm personally of the opinion that PQinitSSL(int) should be left alone and we should define PQinitSSLCrypto(int, int), just to avoid the possibility of difficult-to-debug backward-compatibility problems, but the number of people calling PQinitSSL(2) in existing applications is doubtless vanishingly small, because there is probably no reason to call it with a non-constant argument, so who is going to pick 2 rather than 1? So if someone has some sort of principled objection to adding a new API call, then let's just modify the behavior of the existing call instead and move on. Is there more substance here than meets the eye? ...Robert -- 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] PQinitSSL broken in some use casesf
Robert Haas wrote: Is there more substance here than meets the eye? No, you about summed it up. We need a way to init libssl and libcrypto in any combo. Along the way, PQinit() was discussed which may have muddied the waters. I prefer leaving the PQinitSSL function alone, thus my patch that implements PQinitSecure(flags). Adding PQinitSSL(new_value) seem reasonable to me. My only complaint has been that the API user has no way of knowing if the function understood their request. An older libpq would treat any non-zero argument as one, which would silently fail/mis-behave from a new apps perspective. Not sure this can be solved. In the end, anyway you do it will have an issue or two. I agree that it really doesn't matter, all methods would probably do the trick. Andrew Chernow eSilo, LLC -- 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] PQinitSSL broken in some use casesf
Andrew Chernow wrote: Robert Haas wrote: Is there more substance here than meets the eye? No, you about summed it up. We need a way to init libssl and libcrypto in any combo. Along the way, PQinit() was discussed which may have muddied the waters. I prefer leaving the PQinitSSL function alone, thus my patch that implements PQinitSecure(flags). Adding PQinitSSL(new_value) seem reasonable to me. My only complaint has been that the API user has no way of knowing if the function understood their request. An older libpq would treat any non-zero argument as one, which would silently fail/mis-behave from a new apps perspective. Not sure this can be solved. In the end, anyway you do it will have an issue or two. I agree that it really doesn't matter, all methods would probably do the trick. I think doing PQinitSSL(new_value) is probably the least invasive change to solve this, which is why I suggested it. It does have a compile-time check by referencing the #define. We never documented the valid values to PQinitSSL(), and no one ever reported this as a bug, so the existing use of PQinitSSL() is probably very small. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Merlin Moncure wrote: On Tue, Feb 10, 2009 at 5:02 PM, Bruce Momjian br...@momjian.us wrote: Merlin Moncure wrote: PQinitSSL(0) was specifically designed to allow applications to set up SSL on their own. How does this not work properly? this has nothing to do with who initializes ssl. this is all about *crypto*. remember, crypto and ssl are two separate libraries. The application or library in question may not even link with ssl or use ssl headers. The problem is PQinitSSL (re-) initializes crypto without asking if that's ok. PQinitSSL(false) initializes crypto? Please point me to exact function calls that are the problem? Everything is very vague. nooo, you are not listening :-) PQinitSSL(0) initializes libpq for ssl but leaves crypto and ssl initialization to the app PQinitSSL(1) initializes libpq, crypto, and ssl libraries Now, consider an app that uses libcrypto for its own requirements *but not libssl*. It initializes libcrypto, passing its own lock vector, etc. It cannot however initialize ssl because it does not link with ssl, or include ssl headers. There are no ssl functions to call, and it wouldn't make sense to expect the app to do this even if there were. Now, if this app also has libpq dependency, it needs a way to tell libpq: 'i have already initialized the crypto library, but could you please set up libssl'. otherwise you end up re-initializing libcrypto with different lock vector which is very bad if there are any locks already in use, which is quite likely. There is no way to do that with libpqso you see that no matter how you call PQinitSSL, the application is broken in some way. Passing 0 breaks because ssl never ends up getting set up, and passing 1 breaks because libcrypto's locks get messed up. The main problem is that libpq PQinitSSL makes broad (and extremely dangerous assumption) that it is the only one interested in libcrypto lock vector. In short, it's broken. I am back to looking at this. I dropped off this discussion back in February because I felt people didn't want to answer questions I had, but now it seems we have to close this out somehow. I have applied the attached patch which does several things: o documents that libssl _and_ libcrypto initialization is turned off by PQinitSSL(0) o clarified cases where this behavior is important o added comments that the CRYPTO_set_* calls reference libcrypto, not libssl I think we can now say that the current behavior is not a bug because it is documented, even though the PQinitSSL() function name is inaccurate. In fact, 8.4 is the first time we are documenting the valid parameter value to PQinitSSL(), in 8.3 we have: to inside applicationlibpq/application), you can use functionPQinitSSL(int)/ to tell applicationlibpq/application that the acronymSSL/ library has already been initialized by your application. So we have some flexibility in defining how it behaves, and this also illustrates how little the function is used because no one ever complained about it. I think there is a good argument that PQinitSSL(X) where X 1 would work fine for more fine-grained control. The new libpq init function idea was interesting, but having a documented solution for WSAStartup()/WSACleanup() usage, we now don't have another libpq init use-case so it is hard to suggest a new libpq function. I am figuring we have to keep the current behavior and see what happens after 8.4; the new documentation should make the behavior clear and perhaps trigger other users to report suggestions. I assume this item is closed for 8.4 unless I hear otherwise. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: doc/src/sgml/libpq.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v retrieving revision 1.279 diff -c -c -r1.279 libpq.sgml *** doc/src/sgml/libpq.sgml 23 Mar 2009 01:45:29 - 1.279 --- doc/src/sgml/libpq.sgml 28 Mar 2009 01:17:21 - *** *** 6169,6179 /para para !If you are using acronymSSL/ inside your application (in addition !to inside applicationlibpq/application), you can call !functionPQinitSSL(int)/ with literal0/ to tell !applicationlibpq/application that the acronymSSL/ library !has already been initialized by your application. !-- If this URL changes replace it with a URL to www.archive.org. -- See ulink url=http://h71000.www7.hp.com/doc/83final/BA554_90007/ch04.html;/ulink --- 6169,6181 /para para !If your application initializes literallibssl/ or !literallibcrypto/ libraries and applicationlibpq/application !is built with acronymSSL/ support, you should call !
Re: [HACKERS] PQinitSSL broken in some use casesf
Bruce Momjian wrote: I think there is a good argument that PQinitSSL(X) where X 1 would work fine for more fine-grained control. The new libpq init function idea was interesting, but having a documented solution for WSAStartup()/WSACleanup() usage, we now don't have another libpq init use-case so it is hard to suggest a new libpq function. If you look back through the list, the PQinit idea was offered up several times while discussing WSA* stuff. There were few buyers. I don't see how having or not having a documented solution for WSA* usage would make a bit of difference. I am figuring we have to keep the current behavior and see what happens after 8.4; the new documentation should make the behavior clear and perhaps trigger other users to report suggestions. This is not a battle I find worth fighting. But I am having trouble staying completely quiet; I typically have this issue when I disagree :) This patch merely documents the problem, when another fully documented working patch fixed it; following the discussions on the list. http://archives.postgresql.org//pgsql-hackers/2009-02/msg01018.php Was this reviewed and/or rejected? Andrew Chernow -- 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] PQinitSSL broken in some use casesf
Andrew Chernow wrote: Bruce Momjian wrote: I think there is a good argument that PQinitSSL(X) where X 1 would work fine for more fine-grained control. The new libpq init function idea was interesting, but having a documented solution for WSAStartup()/WSACleanup() usage, we now don't have another libpq init use-case so it is hard to suggest a new libpq function. If you look back through the list, the PQinit idea was offered up several times while discussing WSA* stuff. There were few buyers. I don't see how having or not having a documented solution for WSA* usage would make a bit of difference. It only means we don't have _another_ use for a more general libpq init function. I am figuring we have to keep the current behavior and see what happens after 8.4; the new documentation should make the behavior clear and perhaps trigger other users to report suggestions. This is not a battle I find worth fighting. But I am having trouble staying completely quiet; I typically have this issue when I disagree :) This patch merely documents the problem, when another fully documented working patch fixed it; following the discussions on the list. http://archives.postgresql.org//pgsql-hackers/2009-02/msg01018.php Was this reviewed and/or rejected? Comments Tom made were that there was no consensus on the proper fix/direction, and I agree. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Andrew Chernow wrote:
Andrew Chernow wrote:
Maybe better, have it return a zero/nonzero error code; where one
of the
possibilities for failure is you passed a bit I didn't understand.
Why not just return those bit(s) instead of an arbitrary code? How
about:
-1 = error (if it ever does anything that can fail)
0 = success (all bits known)
0 = unknown bits (remaining known bits *have* been set)
I attached a patch that implements the above, using PQinitSecure as the
function name.
Fixed a bug in the patch. I forgot to make PQinitSSL update the new
pg_initcryptolib flag. My fix was to make PQinitSSL proxy to PQinitSecure.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: doc/src/sgml/libpq.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.278
diff -C6 -r1.278 libpq.sgml
*** doc/src/sgml/libpq.sgml 11 Feb 2009 04:08:47 - 1.278
--- doc/src/sgml/libpq.sgml 19 Feb 2009 16:32:11 -
***
*** 6174,6185
--- 6174,6215
applicationlibpq/application that the acronymSSL/ library
has already been initialized by your application.
!-- If this URL changes replace it with a URL to www.archive.org. --
See ulink
url=http://h71000.www7.hp.com/doc/83final/BA554_90007/ch04.html;/ulink
for details on the SSL API.
+
+variablelist
+ varlistentry
+ term
+ functionPQinitSecure/function
+ indexterm
+primaryPQinitSecure/primary
+ /indexterm
+ /term
+
+ listitem
+ para
+Allows applications to select which secure components to initialize.
+synopsis
+ int PQinitSecure(int flags);
+/synopsis
+ /para
+
+ para
+The flags argument can be any of the following: PG_SECURE_SSL,
+PG_SECURE_CRYPTO. PG_SECURE_SSL will initialize the SSL portion of
+the OpenSSL library. PG_SECURE_CRYPTO will initialize the crypto
+portion of the OpenSSL library. The function returns the bits it
+did not understand or zero indicating it understood all bits in flags.
+If an error occurs, such as calling this function without SSL
+support enabled, -1 is returned.
+ /para
+ /listitem
+ /varlistentry
+/variablelist
/para
table id=libpq-ssl-file-usage
titleLibpq/Client SSL File Usage/title
tgroup cols=3
thead
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.22
diff -C6 -r1.22 exports.txt
*** src/interfaces/libpq/exports.txt22 Sep 2008 13:55:14 - 1.22
--- src/interfaces/libpq/exports.txt19 Feb 2009 16:32:11 -
***
*** 149,154
--- 149,155
PQinstanceData147
PQsetInstanceData 148
PQresultInstanceData 149
PQresultSetInstanceData 150
PQfireResultCreateEvents 151
PQconninfoParse 152
+ PQinitSecure 153
\ No newline at end of file
Index: src/interfaces/libpq/fe-secure.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.119
diff -C6 -r1.119 fe-secure.c
*** src/interfaces/libpq/fe-secure.c28 Jan 2009 15:06:47 - 1.119
--- src/interfaces/libpq/fe-secure.c19 Feb 2009 16:32:11 -
***
*** 96,107
--- 96,108
static PostgresPollingStatusType open_client_SSL(PGconn *);
static void close_SSL(PGconn *);
static char *SSLerrmessage(void);
static void SSLerrfree(char *buf);
static bool pq_initssllib = true;
+ static bool pq_initcryptolib = true;
static SSL_CTX *SSL_context = NULL;
#ifdef ENABLE_THREAD_SAFETY
static int ssl_open_connections = 0;
#ifndef WIN32
***
*** 170,186
*initialized OpenSSL.
*/
void
PQinitSSL(int do_init)
{
#ifdef USE_SSL
! pq_initssllib = do_init;
#endif
}
/*
*Initialize global context
*/
int
pqsecure_initialize(PGconn *conn)
{
int r = 0;
--- 171,205
*initialized OpenSSL.
*/
void
PQinitSSL(int do_init)
{
#ifdef USE_SSL
! (void) PQinitSecure(do_init ? (PG_SECURE_SSL | PG_SECURE_CRYPTO) : 0);
#endif
}
/*
+ *Exported function to allow application to tell us which secure
+ * components to initialize.
+ */
+ int
+ PQinitSecure(int flags)
+ {
+ int code = -1;
+
+ #ifdef USE_SSL
+ pq_initssllib = flags PG_SECURE_SSL ? true : false;
+ pq_initcryptolib = flags PG_SECURE_CRYPTO ? true : false;
+ code = flags ~(PG_SECURE_SSL | PG_SECURE_CRYPTO);
+ #endif
+
+ return code;
+ }
+
+ /*
*Initialize global context
*/
int
pqsecure_initialize(PGconn *conn)
Re: [HACKERS] PQinitSSL broken in some use casesf
Andrew Chernow wrote:
Maybe better, have it return a zero/nonzero error code; where one of the
possibilities for failure is you passed a bit I didn't understand.
Why not just return those bit(s) instead of an arbitrary code? How about:
-1 = error (if it ever does anything that can fail)
0 = success (all bits known)
0 = unknown bits (remaining known bits *have* been set)
I attached a patch that implements the above, using PQinitSecure as the
function name.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: doc/src/sgml/libpq.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.278
diff -C6 -r1.278 libpq.sgml
*** doc/src/sgml/libpq.sgml 11 Feb 2009 04:08:47 - 1.278
--- doc/src/sgml/libpq.sgml 18 Feb 2009 15:22:04 -
***
*** 6174,6185
--- 6174,6215
applicationlibpq/application that the acronymSSL/ library
has already been initialized by your application.
!-- If this URL changes replace it with a URL to www.archive.org. --
See ulink
url=http://h71000.www7.hp.com/doc/83final/BA554_90007/ch04.html;/ulink
for details on the SSL API.
+
+variablelist
+ varlistentry
+ term
+ functionPQinitSecure/function
+ indexterm
+primaryPQinitSecure/primary
+ /indexterm
+ /term
+
+ listitem
+ para
+Allows applications to select which secure components to initialize.
+synopsis
+ int PQinitSecure(int flags);
+/synopsis
+ /para
+
+ para
+The flags argument can be any of the following: PG_SECURE_SSL,
+PG_SECURE_CRYPTO. PG_SECURE_SSL will initialize the SSL portion of
+the OpenSSL library. PG_SECURE_CRYPTO will initialize the crypto
+portion of the OpenSSL library. The function returns the bits it
+did not understand or zero indicating it understood all bits in flags.
+If an error occurs, such as calling this function without SSL
+support enabled, -1 is returned.
+ /para
+ /listitem
+ /varlistentry
+/variablelist
/para
table id=libpq-ssl-file-usage
titleLibpq/Client SSL File Usage/title
tgroup cols=3
thead
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.22
diff -C6 -r1.22 exports.txt
*** src/interfaces/libpq/exports.txt22 Sep 2008 13:55:14 - 1.22
--- src/interfaces/libpq/exports.txt18 Feb 2009 15:22:04 -
***
*** 149,154
--- 149,155
PQinstanceData147
PQsetInstanceData 148
PQresultInstanceData 149
PQresultSetInstanceData 150
PQfireResultCreateEvents 151
PQconninfoParse 152
+ PQinitSecure 153
\ No newline at end of file
Index: src/interfaces/libpq/fe-secure.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.119
diff -C6 -r1.119 fe-secure.c
*** src/interfaces/libpq/fe-secure.c28 Jan 2009 15:06:47 - 1.119
--- src/interfaces/libpq/fe-secure.c18 Feb 2009 15:22:04 -
***
*** 96,107
--- 96,108
static PostgresPollingStatusType open_client_SSL(PGconn *);
static void close_SSL(PGconn *);
static char *SSLerrmessage(void);
static void SSLerrfree(char *buf);
static bool pq_initssllib = true;
+ static bool pq_initcryptolib = true;
static SSL_CTX *SSL_context = NULL;
#ifdef ENABLE_THREAD_SAFETY
static int ssl_open_connections = 0;
#ifndef WIN32
***
*** 175,186
--- 176,205
#ifdef USE_SSL
pq_initssllib = do_init;
#endif
}
/*
+ *Exported function to allow application to tell us which secure
+ * components to initialize.
+ */
+ int
+ PQinitSecure(int flags)
+ {
+ int code = -1;
+
+ #ifdef USE_SSL
+ pq_initssllib = flags PG_SECURE_SSL ? true : false;
+ pq_initcryptolib = flags PG_SECURE_CRYPTO ? true : false;;
+ code = flags ~(PG_SECURE_SSL | PG_SECURE_CRYPTO);
+ #endif
+
+ return code;
+ }
+
+ /*
*Initialize global context
*/
int
pqsecure_initialize(PGconn *conn)
{
int r = 0;
***
*** 820,831
--- 839,852
* message - no connection local setup is made.
*/
static int
init_ssl_system(PGconn *conn)
{
#ifdef ENABLE_THREAD_SAFETY
+ int num_ssl_conns = 0;
+
#ifdef WIN32
/* Also see similar code in fe-connect.c, default_threadlock() */
if (ssl_config_mutex == NULL)
{
while (InterlockedExchange(win32_ssl_create_mutex, 1) == 1)
/* loop, another thread own the lock */ ;
***
***
Re: [HACKERS] PQinitSSL broken in some use casesf
Andrew Chernow a...@esilo.com writes: 4. int PQinitSecure(int init_mask) This works just like #3 but returns the bits it understood. It allows an app to determine if the loaded libpq supports a bit it has tried to use. Maybe better, have it return a zero/nonzero error code; where one of the possibilities for failure is you passed a bit I didn't understand. This would depend on whether we intend that the function would ever actually *do* anything, as opposed to just save its argument to be consulted later. Maybe that's not appropriate/necessary. 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] PQinitSSL broken in some use casesf
Maybe better, have it return a zero/nonzero error code; where one of the possibilities for failure is you passed a bit I didn't understand. Why not just return those bit(s) instead of an arbitrary code? How about: -1 = error (if it ever does anything that can fail) 0 = success (all bits known) 0 = unknown bits (remaining known bits *have* been set) -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Robert Haas wrote: On Feb 11, 2009, at 12:12 AM, Andrew Chernow a...@esilo.com wrote: Robert Haas wrote: I am not in love with the idea of using PQinitSSL(SOME_MAGIC_VALUE) The issue I see is the inability to toggle crypto initialization. I think crypto init and ssl init are 2 different things. Thus, I proposed the below: http://archives.postgresql.org/pgsql-hackers/2009-02/msg00488.php If that can be made transparent to existing apps, I'm all for it. ...Robert Should I create a patch implementing the PQinitCrypto idea? -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
On Fri, Feb 13, 2009 at 9:17 AM, Andrew Chernow a...@esilo.com wrote: Should I create a patch implementing the PQinitCrypto idea? I think that would be helpful. Seeing the code will give everyone a better idea of exactly what the proposed change is and whether it's acceptable. ...Robert -- 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] PQinitSSL broken in some use casesf
Robert Haas wrote:
On Fri, Feb 13, 2009 at 9:17 AM, Andrew Chernow a...@esilo.com wrote:
Should I create a patch implementing the PQinitCrypto idea?
I think that would be helpful. Seeing the code will give everyone a
better idea of exactly what the proposed change is and whether it's
acceptable.
...Robert
Patch attached.
One thing I noticed is the ssl_open_connections variable is ref counting
connections when pq_initssllib is true. But, it now only affects crypto
library init and cleanup calls. Point is, ref counting is only needed
if pq_initcryptolib is true and it should be renamed to
crypto_open_connections. I didn't do this in the patch. Its the same
old name and the counter is incremented if pq_initssllib or
pq_initcryptolib is true. Please advise.
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/
Index: src/interfaces/libpq/exports.txt
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v
retrieving revision 1.22
diff -C6 -r1.22 exports.txt
*** src/interfaces/libpq/exports.txt22 Sep 2008 13:55:14 - 1.22
--- src/interfaces/libpq/exports.txt13 Feb 2009 17:01:22 -
***
*** 149,154
--- 149,155
PQinstanceData147
PQsetInstanceData 148
PQresultInstanceData 149
PQresultSetInstanceData 150
PQfireResultCreateEvents 151
PQconninfoParse 152
+ PQinitCrypto 153
\ No newline at end of file
Index: src/interfaces/libpq/fe-secure.c
===
RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v
retrieving revision 1.119
diff -C6 -r1.119 fe-secure.c
*** src/interfaces/libpq/fe-secure.c28 Jan 2009 15:06:47 - 1.119
--- src/interfaces/libpq/fe-secure.c13 Feb 2009 17:01:22 -
***
*** 96,107
--- 96,108
static PostgresPollingStatusType open_client_SSL(PGconn *);
static void close_SSL(PGconn *);
static char *SSLerrmessage(void);
static void SSLerrfree(char *buf);
static bool pq_initssllib = true;
+ static bool pq_initcryptolib = true;
static SSL_CTX *SSL_context = NULL;
#ifdef ENABLE_THREAD_SAFETY
static int ssl_open_connections = 0;
#ifndef WIN32
***
*** 175,186
--- 176,199
#ifdef USE_SSL
pq_initssllib = do_init;
#endif
}
/*
+ *Exported function to allow application to tell us it's already
+ *initialized the OpenSSL Crypto library.
+ */
+ void
+ PQinitCrypto(int do_init)
+ {
+ #ifdef USE_SSL
+ pq_initcryptolib = do_init;
+ #endif
+ }
+
+ /*
*Initialize global context
*/
int
pqsecure_initialize(PGconn *conn)
{
int r = 0;
***
*** 820,831
--- 833,846
* message - no connection local setup is made.
*/
static int
init_ssl_system(PGconn *conn)
{
#ifdef ENABLE_THREAD_SAFETY
+ int num_ssl_conns = 0;
+
#ifdef WIN32
/* Also see similar code in fe-connect.c, default_threadlock() */
if (ssl_config_mutex == NULL)
{
while (InterlockedExchange(win32_ssl_create_mutex, 1) == 1)
/* loop, another thread own the lock */ ;
***
*** 837,849
InterlockedExchange(win32_ssl_create_mutex, 0);
}
#endif
if (pthread_mutex_lock(ssl_config_mutex))
return -1;
! if (pq_initssllib)
{
/*
* If necessary, set up an array to hold locks for OpenSSL.
OpenSSL will
* tell us how big to make this array.
*/
if (pq_lockarray == NULL)
--- 852,873
InterlockedExchange(win32_ssl_create_mutex, 0);
}
#endif
if (pthread_mutex_lock(ssl_config_mutex))
return -1;
! /*
!* Increment connection count if we are responsible for
!* intiializing the SSL or Crypto library. Currently, only
!* crypto needs this during setup and cleanup. That may
!* change in the future so we always update the counter.
!*/
! if (pq_initssllib || pq_initcryptolib)
! num_ssl_conns = ssl_open_connections++;
!
! if (pq_initcryptolib)
{
/*
* If necessary, set up an array to hold locks for OpenSSL.
OpenSSL will
* tell us how big to make this array.
*/
if (pq_lockarray == NULL)
***
*** 865,877
pthread_mutex_unlock(ssl_config_mutex);
return -1;
}
}
}
! if (ssl_open_connections++ == 0)
{
/* These are only required for threaded SSL
Re: [HACKERS] PQinitSSL broken in some use casesf
On Fri, Feb 13, 2009 at 12:06 PM, Andrew Chernow a...@esilo.com wrote: Patch attached. One thing I noticed is the ssl_open_connections variable is ref counting connections when pq_initssllib is true. But, it now only affects crypto library init and cleanup calls. Point is, ref counting is only needed if pq_initcryptolib is true and it should be renamed to crypto_open_connections. I didn't do this in the patch. Its the same old name and the counter is incremented if pq_initssllib or pq_initcryptolib is true. Please advise. I'll review this in more detail when I have a chance, but it certainly won't be committable without doc changes, and it's probably best if you write those and include them in the patch. ...Robert -- 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] PQinitSSL broken in some use casesf
Robert Haas wrote: On Fri, Feb 13, 2009 at 12:06 PM, Andrew Chernow a...@esilo.com wrote: Patch attached. One thing I noticed is the ssl_open_connections variable is ref counting connections when pq_initssllib is true. But, it now only affects crypto library init and cleanup calls. Point is, ref counting is only needed if pq_initcryptolib is true and it should be renamed to crypto_open_connections. I didn't do this in the patch. Its the same old name and the counter is incremented if pq_initssllib or pq_initcryptolib is true. Please advise. I'll review this in more detail when I have a chance, but it certainly won't be committable without doc changes, and it's probably best if you write those and include them in the patch. ...Robert Okay. Added docs in the same place PQinitSSL is. Apparently, initssl doesn't have formal func docs. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/ Index: doc/src/sgml/libpq.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/libpq.sgml,v retrieving revision 1.278 diff -C6 -r1.278 libpq.sgml *** doc/src/sgml/libpq.sgml 11 Feb 2009 04:08:47 - 1.278 --- doc/src/sgml/libpq.sgml 13 Feb 2009 18:11:40 - *** *** 60,72 functionPQsetdbLogin/. Note that these functions will always return a non-null object pointer, unless perhaps there is too little memory even to allocate the structnamePGconn/ object. The functionPQstatus/ function should be called to check whether a connection was successfully made before queries are sent via the connection object. ! note para On Windows, there is a way to improve performance if a single database connection is repeatedly started and shutdown. Internally, libpq calls WSAStartup() and WSACleanup() for connection startup and shutdown, respectively. WSAStartup() increments an internal --- 60,72 functionPQsetdbLogin/. Note that these functions will always return a non-null object pointer, unless perhaps there is too little memory even to allocate the structnamePGconn/ object. The functionPQstatus/ function should be called to check whether a connection was successfully made before queries are sent via the connection object. ! note para On Windows, there is a way to improve performance if a single database connection is repeatedly started and shutdown. Internally, libpq calls WSAStartup() and WSACleanup() for connection startup and shutdown, respectively. WSAStartup() increments an internal *** *** 6169,6181 para If you are using acronymSSL/ inside your application (in addition to inside applicationlibpq/application), you can call functionPQinitSSL(int)/ with literal0/ to tell applicationlibpq/application that the acronymSSL/ library !has already been initialized by your application. !-- If this URL changes replace it with a URL to www.archive.org. -- See ulink url=http://h71000.www7.hp.com/doc/83final/BA554_90007/ch04.html;/ulink for details on the SSL API. /para --- 6169,6184 para If you are using acronymSSL/ inside your application (in addition to inside applicationlibpq/application), you can call functionPQinitSSL(int)/ with literal0/ to tell applicationlibpq/application that the acronymSSL/ library !has already been initialized by your application. Additionally, you !can call functionPQinitCrypto(int)/ with literal0/ to tell !applicationlibpq/application that the acronymCrypto/ library has !already been initialized by your application. !-- If this URL changes replace it with a URL to www.archive.org. -- See ulink url=http://h71000.www7.hp.com/doc/83final/BA554_90007/ch04.html;/ulink for details on the SSL API. /para Index: src/interfaces/libpq/exports.txt === RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/exports.txt,v retrieving revision 1.22 diff -C6 -r1.22 exports.txt *** src/interfaces/libpq/exports.txt22 Sep 2008 13:55:14 - 1.22 --- src/interfaces/libpq/exports.txt13 Feb 2009 18:11:40 - *** *** 149,154 --- 149,155 PQinstanceData147 PQsetInstanceData 148 PQresultInstanceData 149 PQresultSetInstanceData 150 PQfireResultCreateEvents 151 PQconninfoParse 152 + PQinitCrypto 153 \ No newline at end of file Index: src/interfaces/libpq/fe-secure.c === RCS file: /projects/cvsroot/pgsql/src/interfaces/libpq/fe-secure.c,v retrieving revision 1.119 diff -C6 -r1.119 fe-secure.c *** src/interfaces/libpq/fe-secure.c28 Jan 2009 15:06:47 - 1.119 ---
Re: [HACKERS] PQinitSSL broken in some use casesf
Andrew Chernow wrote: Robert Haas wrote: On Fri, Feb 13, 2009 at 12:06 PM, Andrew Chernow a...@esilo.com wrote: Patch attached. One thing I noticed is the ssl_open_connections variable is ref counting connections when pq_initssllib is true. But, it now only affects crypto library init and cleanup calls. Point is, ref counting is only needed if pq_initcryptolib is true and it should be renamed to crypto_open_connections. I didn't do this in the patch. Its the same old name and the counter is incremented if pq_initssllib or pq_initcryptolib is true. Please advise. I'll review this in more detail when I have a chance, but it certainly won't be committable without doc changes, and it's probably best if you write those and include them in the patch. One problem with this patch is that a libpq app using PQinitSSL(0) is under the assumption that this shuts off ssl init and crypto init. That app might be doing its own crypto init which would be overwritten by libpq because the app is unaware of PQinitCrypto (if and when it eventually links with 8.4 libpq). This feels like a very uncommon situation, but a possible gotcha. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Andrew Chernow wrote:
Andrew Chernow wrote:
Robert Haas wrote:
On Fri, Feb 13, 2009 at 12:06 PM, Andrew Chernow a...@esilo.com wrote:
Patch attached.
One thing I noticed is the ssl_open_connections variable is ref
counting
connections when pq_initssllib is true. But, it now only affects
crypto
library init and cleanup calls. Point is, ref counting is only
needed if
pq_initcryptolib is true and it should be renamed to
crypto_open_connections. I didn't do this in the patch. Its the
same old
name and the counter is incremented if pq_initssllib or
pq_initcryptolib is
true. Please advise.
I'll review this in more detail when I have a chance, but it certainly
won't be committable without doc changes, and it's probably best if
you write those and include them in the patch.
One problem with this patch is that a libpq app using PQinitSSL(0) is
under the assumption that this shuts off ssl init and crypto init. That
app might be doing its own crypto init which would be overwritten by
libpq because the app is unaware of PQinitCrypto (if and when it
eventually links with 8.4 libpq). This feels like a very uncommon
situation, but a possible gotcha.
(sorry I keep posting)
This feels like a very uncommon situation
I take that back. Not so sure it is uncommon, any threaded libpq app
would probably get bit if they called PQinitSSL. On top of that, it
could take up to a year before complaints start rolling in, as 8.4 hits
the distros. Yuck.
I now think the the orignal suggestion of PQinitSSLExtended is better
than PQinitCrypto. With PQinitSSLExtended, PQinitSSL needs a minor
implementation adjustment but the behvior remains the same. The
extended version is probably:
/* IMHO appending Ex is a little nicer */
void PQinitSSLEx(int ssl_init, int crypto_init);
/* PQinitSSL wraps PQinitSSLEx */
void PQinitSSL(int do_init)
{
PQinitSSLEx(do_init, do_init);
}
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.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] PQinitSSL broken in some use casesf
Andrew Chernow a...@esilo.com writes: One problem with this patch is that a libpq app using PQinitSSL(0) is under the assumption that this shuts off ssl init and crypto init. That app might be doing its own crypto init which would be overwritten by libpq because the app is unaware of PQinitCrypto (if and when it eventually links with 8.4 libpq). This feels like a very uncommon situation, but a possible gotcha. I take that back. Not so sure it is uncommon, I agree, we should *not* change the existing behavior for either case. This probably means that an independent PQinitCrypto function is the wrong thing, even though it would've been the cleanest solution if we were starting from scratch. At this point I like Merlin's proposal of a third parameter value to PQinitSSL the best. /* IMHO appending Ex is a little nicer */ void PQinitSSLEx(int ssl_init, int crypto_init); Ugh, ugh, ugh. We do not do Ex around here --- uninterpretable abbreviations are not helpful to the reader. Call it Extended if you must have it. Also, this definition feels a bit wrong --- it's not possible for all four cases to be valid, is it? 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] PQinitSSL broken in some use casesf
At this point I like Merlin's proposal of a third parameter value to PQinitSSL the best. I'm not opposed to it, although I don't think it is as clean as a new function. Also, this definition feels a bit wrong --- it's not possible for all four cases to be valid, is it? Yes it is. PQinitSSLExtended(0, 0); // don't init anything, PQinitSSL(0) PQinitSSLExtended(1, 0); // init ssl, don't init crypto PQinitSSLExtended(0, 1); // don't init ssl, init crypto PQinitSSLExtended(1, 1); // init both, default behavior, PQinitSSL(1) -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Andrew Chernow wrote: At this point I like Merlin's proposal of a third parameter value to PQinitSSL the best. I'm not opposed to it, although I don't think it is as clean as a new function. Also, this definition feels a bit wrong --- it's not possible for all four cases to be valid, is it? Yes it is. PQinitSSLExtended(0, 0); // don't init anything, PQinitSSL(0) PQinitSSLExtended(1, 0); // init ssl, don't init crypto PQinitSSLExtended(0, 1); // don't init ssl, init crypto PQinitSSLExtended(1, 1); // init both, default behavior, PQinitSSL(1) Maybe the argument to PQinitSSLExtended should be a bit mask, making this version more extendable ... PG_INITSSL, PG_INITCRYPTO? Also, how about calling this PQinitSecure(int flags), since SSL is only one thing it can init. This is just like merlin's suggestion but without hacking the existing PQinitSSL. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Andrew Chernow a...@esilo.com writes: Also, this definition feels a bit wrong --- it's not possible for all four cases to be valid, is it? Yes it is. PQinitSSLExtended(0, 0); // don't init anything, PQinitSSL(0) PQinitSSLExtended(1, 0); // init ssl, don't init crypto PQinitSSLExtended(0, 1); // don't init ssl, init crypto PQinitSSLExtended(1, 1); // init both, default behavior, PQinitSSL(1) I know what you're thinking the flags should mean, I'm saying that it's not possible for the third case to be sane. It implies that the application initialized ssl but not crypto, which isn't possible. 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] PQinitSSL broken in some use casesf
Tom Lane wrote: Andrew Chernow a...@esilo.com writes: Also, this definition feels a bit wrong --- it's not possible for all four cases to be valid, is it? Yes it is. PQinitSSLExtended(0, 0); // don't init anything, PQinitSSL(0) PQinitSSLExtended(1, 0); // init ssl, don't init crypto PQinitSSLExtended(0, 1); // don't init ssl, init crypto PQinitSSLExtended(1, 1); // init both, default behavior, PQinitSSL(1) I know what you're thinking the flags should mean, I'm saying that it's not possible for the third case to be sane. It implies that the application initialized ssl but not crypto, which isn't possible. Or that the application called PQinitSSLExtended(0, 1) and then initialized SSL itself, which is sane. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Andrew Chernow a...@esilo.com writes: Maybe the argument to PQinitSSLExtended should be a bit mask, making this version more extendable ... PG_INITSSL, PG_INITCRYPTO? +1 for thinking ahead to the next time, but is a bit mask the right thing? 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] PQinitSSL broken in some use casesf
On Fri, Feb 13, 2009 at 3:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Andrew Chernow a...@esilo.com writes: Maybe the argument to PQinitSSLExtended should be a bit mask, making this version more extendable ... PG_INITSSL, PG_INITCRYPTO? +1 for thinking ahead to the next time, but is a bit mask the right thing? What would you suggest? struct pointer? varargs? If you are down with the struct pointer idea, maybe we could revive the idea of PQinit: http://archives.postgresql.org/pgsql-hackers/2009-01/msg01349.php We could fix up the windows issue at the same time, and leave room for other things. Fixing windows WSA issues or installing an event proc hook into the library :D. merlin -- 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] PQinitSSL broken in some use casesf
Merlin Moncure mmonc...@gmail.com writes: On Fri, Feb 13, 2009 at 3:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: +1 for thinking ahead to the next time, but is a bit mask the right thing? What would you suggest? struct pointer? varargs? Not sure. By definition, we're trying to predict an unforeseen requirement, and that's always going to be tough. I'm not too thrilled about a struct pointer, because that will introduce the problem which version of the struct is the client passing?. A varargs arrangement could cover almost anything, but it also seems like overkill. I don't actually have an idea that sounds better than a bitmask, I just wanted to see if anyone else did. BTW, the bitmask isn't perfect either --- doesn't it just reintroduce the problem already complained of with your idea for PQinitSSL? That is, how does the client know whether the function recognized all the bits it passed? 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] PQinitSSL broken in some use casesf
Tom Lane wrote:
is, how does the client know whether the function recognized all the
bits it passed?
How about returning the bits it could set?
int mask = PG_INITSSL | PG_INITCRYPTO;
if (!(PQinitSecure(mask) PG_INITCRYPTO))
; // no support for crypto
...OR...
consider a generic PQinit call per system/object/etc..
int PQinit(int which, void *data);
int mask = PG_SECURE_SSL | PG_SECURE_CRYPTO;
PQinit(PG_INIT_SECURE, mask); // or PG_INIT_OPENSSL
xxx_t xxx = {0, blah, 12};
PQinit(PG_INIT_xxx, xxx);
--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.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] PQinitSSL broken in some use casesf
BTW, the bitmask isn't perfect either --- doesn't it just reintroduce the problem already complained of with your idea for PQinitSSL? That is, how does the client know whether the function recognized all the bits it passed? Well, if we add the PQgetLibraryVersion() function I suggested upthread, then it can check that first. I find it difficult to believe that isn't a good idea independently of how we solve this particular problem. ...Robert -- 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] PQinitSSL broken in some use casesf
On Fri, Feb 13, 2009 at 4:53 PM, Robert Haas robertmh...@gmail.com wrote: BTW, the bitmask isn't perfect either --- doesn't it just reintroduce the problem already complained of with your idea for PQinitSSL? That is, how does the client know whether the function recognized all the bits it passed? Well, if we add the PQgetLibraryVersion() function I suggested upthread, then it can check that first. I find it difficult to believe that isn't a good idea independently of how we solve this particular problem. I'd prefer PQversion() as a name. Also, it doesn't necessarily handle the issue directly. For example, it doesn't tell you which bits are valid...you have to guess. Also, do we really need a function for this? Is the generic init worth discussion or a non starter? I guess we have a scheduling problem here...I think the ssl problem is serious enough to warrant a fast-track into 8.4, but maybe it's 'too much' to hash out a generic library initialization routine this late in the cycle. merlin -- 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] PQinitSSL broken in some use casesf
Robert Haas robertmh...@gmail.com writes: BTW, the bitmask isn't perfect either --- doesn't it just reintroduce the problem already complained of with your idea for PQinitSSL? That is, how does the client know whether the function recognized all the bits it passed? Well, if we add the PQgetLibraryVersion() function I suggested upthread, then it can check that first. I find it difficult to believe that isn't a good idea independently of how we solve this particular problem. Personally, I still subscribe to the PNG design theory that conditionalizing code on overall version numbers isn't a very good idea. Version information should be tied as tightly as possible to the particular behavior for which it applies. http://www.w3.org/TR/PNG-Rationale.html#R.Chunk-naming-conventions It's even worse when the available version numbers aren't really overall, which would be the case here. If we had such a function you can bet your bottom dollar that people would test it to distinguish behaviors that are really server-side, and then get burnt in the field when their app gets used with mismatched server and libpq versions. For the problem at hand, if we think it's important for an app to be able to tell whether libpq supports a particular initialization behavior, then adding a new function to provide that behavior seems like the best bet to me. So maybe that says that designing for extensibility in the function signature is misguided anyhow ... 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] PQinitSSL broken in some use casesf
Personally, I still subscribe to the PNG design theory that conditionalizing code on overall version numbers isn't a very good idea. Version information should be tied as tightly as possible to the particular behavior for which it applies. http://www.w3.org/TR/PNG-Rationale.html#R.Chunk-naming-conventions I don't disagree with you, but it's not always easy to separate things that cleanly. pg_dump doesn't try to distinguish server capabilities, just server versions (and I'm sure you'd scream bloody murder if someone submitted a patch to refactor it... because that patch would be hideous). It's even worse when the available version numbers aren't really overall, which would be the case here. If we had such a function you can bet your bottom dollar that people would test it to distinguish behaviors that are really server-side, and then get burnt in the field when their app gets used with mismatched server and libpq versions. I think that could be largely addressed through documentation. There is no helpful for terminal stupidity anyway. The reality is, right now we have a way to distinguish server versions, but not client versions. That doesn't seem particularly logical or consistent. Testing for client versions lets you do things like check for breakage known to exist in versions X, where there may be no difference in the API, just the behavior. It also reduces the need to make every non-backward-compatible change an API change. Many people will only ever link against the most recent version and will not care about backward-compatibility. It doesn't hurt to provide a hook to those who do care. For the problem at hand, if we think it's important for an app to be able to tell whether libpq supports a particular initialization behavior, then adding a new function to provide that behavior seems like the best bet to me. So maybe that says that designing for extensibility in the function signature is misguided anyhow ... It could certainly be overdone. ...Robert -- 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] PQinitSSL broken in some use casesf
So far, we have the below ideas: 1. void PQinitSSL(MAGIC_VALUES) The idea is to use some magic values to add more options, converting the do_init argument from a flag to an enumeration. The idea is choose magic values that would most likely never be used by existing libpq applications. The downside to this method is there is no way to detect if the request was honored: meaning a new app passes a magic value to an older libpq. 2. void PQinitSecure(int init_ssl, int int_crypto) Another proposed name was PQinitSSLExtended or PQinitOpenSSL. This leaves the existing PQinitSSL alone. The problem here is that it can't be extended like the current PQinitSSL, so we fixed today's problem but created it again for future (sounds like government). That leads us to #3. 3. void PQinitSecure(int init_mask) Works just like #2 but uses a bit mask, like PG_INITSSL or something, instead of a fixed set of flags. It trys to make the function more extendable. Although, we bang into the #1 problem again - new apps using new bits that older libpqs are not aware of. There is no way to know if the request was honored. This leads us to #4 (#5 was also a suggested fix). 4. int PQinitSecure(int init_mask) This works just like #3 but returns the bits it understood. It allows an app to determine if the loaded libpq supports a bit it has tried to use. I have not heard any comments on this, I am still looking for its faults. You could claim its akward, but I've seen much worse. An alternative is to add a second argument, int *support_mask, instead of returning it. Or, make the single argument an input and output value .. (int *mask). 5. ??? PQgetLibraryVersion(???) Another proposed name was PQversion. This was suggested as a way of fixing the faults of #1 and #3. The idea is to use the libpq version to determine if a particular init bit is supported. 6. PQinit(int init_who, void *data) Revived from a recent WSACleanup thread. Appears to always be the last kid picked for the kick ball team, so I won't dive to deep. The idea is to create a single init function for libpq, that can handle init'n any module/feature/runtime-settings/etc.. Another name could be PQcontrol. I think that's everything. Are there any favorites or combos? Personally, I'm not really stuck on any of them. I'd just like to see the feature added. If I had to choose, it would be #4. A close second would be #1 + #5 (using PQversion as the name). -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
On Feb 11, 2009, at 12:12 AM, Andrew Chernow a...@esilo.com wrote: Robert Haas wrote: I am not in love with the idea of using PQinitSSL(SOME_MAGIC_VALUE) The issue I see is the inability to toggle crypto initialization. I think crypto init and ssl init are 2 different things. Thus, I proposed the below: http://archives.postgresql.org/pgsql-hackers/2009-02/msg00488.php If that can be made transparent to existing apps, I'm all for it. ...Robert -- 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] PQinitSSL broken in some use casesf
Robert Haas wrote: On Feb 11, 2009, at 12:12 AM, Andrew Chernow a...@esilo.com wrote: Robert Haas wrote: I am not in love with the idea of using PQinitSSL(SOME_MAGIC_VALUE) The issue I see is the inability to toggle crypto initialization. I think crypto init and ssl init are 2 different things. Thus, I proposed the below: http://archives.postgresql.org/pgsql-hackers/2009-02/msg00488.php If that can be made transparent to existing apps, I'm all for it. Defaulting the crypto initialization to TRUE should make this transparent. The downside is that this breaks backwards compatibility, since an unknown symbol is being referenced. Although, that only occurs when PQinitCrypto is actually used. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Andrew Chernow wrote: Bruce Momjian wrote: Andrew Chernow wrote: I am using a library that links with and initializes libcrypto (ie. CRYPTO_set_locking_callback) but not SSL. This causes problems even when using PQinitSSL(FALSE) because things like SSL_library_init(); are not called (unless I manually call them, copy and paste code from fe-secure.c which may change). If libpq does init ssl, it overwrites (and breaks) the other library's crypto. Shouldn't crypto and ssl init be treated as two different things? If not, how does one determine a version portable way of initializing SSL in a manner required by libpq? Lots of apps using encryption but don't necessarily use ssl, so they need to know how to init ssl for libpq. I didn't realize they were could be initialized separately, so we really don't have an answer for you. This is the first time I have heard of this requirement. Just bringing it to everyones attention. I have no idea how common this use case is or if it deserves a patch. From your comments, it sounds uncommon. How we came across this: We have an internal library that links with libcrypto.so but not libssl.so. The library uses digests and ciphers from libcrypto. It initializes libcrypto for thread safety and seeds the PRNG. So, one of our applications is linking with both libpq and this library; which caused the conflict. How we worked around it: We solved it by copying the SSL init sequence from fe-secure.c. Doesn't seem like something that would change very often. So we init_our_library(), PQinitSSL(0) and then do a few lines of SSL init stuff. Seems unusual, but certainly not nearly impossible. But we're back to the discussions around the WSA code - our API provides no really good place to do this, so perhaps we should just clearly document how it's done and how to work around 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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 9:32 AM, Magnus Hagander mag...@hagander.net wrote: How we worked around it: We solved it by copying the SSL init sequence from fe-secure.c. Doesn't seem like something that would change very often. So we init_our_library(), PQinitSSL(0) and then do a few lines of SSL init stuff. Seems unusual, but certainly not nearly impossible. But we're back to the discussions around the WSA code - our API provides no really good place to do this, so perhaps we should just clearly document how it's done and how to work around it? I'm not so sure that's appropriate in this case. I think the existing libpq behavior is simply wrong...crypto and ssl are two separate libraries and PQinitSSL does not expose the necessary detail. This is going to break apps in isolated but spectacular fashion when they link to both pq and crypto for different reasons. maybe invent a special value to PQinitSSL for ssl only init? merlin -- 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] PQinitSSL broken in some use casesf
Merlin Moncure wrote: On Tue, Feb 10, 2009 at 9:32 AM, Magnus Hagander mag...@hagander.net wrote: How we worked around it: We solved it by copying the SSL init sequence from fe-secure.c. Doesn't seem like something that would change very often. So we init_our_library(), PQinitSSL(0) and then do a few lines of SSL init stuff. Seems unusual, but certainly not nearly impossible. But we're back to the discussions around the WSA code - our API provides no really good place to do this, so perhaps we should just clearly document how it's done and how to work around it? I'm not so sure that's appropriate in this case. I think the existing libpq behavior is simply wrong...crypto and ssl are two separate libraries and PQinitSSL does not expose the necessary detail. This is going to break apps in isolated but spectacular fashion when they link to both pq and crypto for different reasons. They could, but nobody has reported it yet, so it's not a common scenario. maybe invent a special value to PQinitSSL for ssl only init? We could do that, I guess. However, if an application passes this in to an old version of libpq, there is no way to know that it didn't know about 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] PQinitSSL broken in some use casesf
maybe invent a special value to PQinitSSL for ssl only init? We could do that, I guess. However, if an application passes this in to an old version of libpq, there is no way to know that it didn't know about it. Well, you could create PQinitSSLExtended, but, as you say, the use case is pretty narrow... It would help if there were a PQgetLibraryVersion() function. ...Robert -- 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] PQinitSSL broken in some use casesf
Robert Haas robertmh...@gmail.com writes: We could do that, I guess. However, if an application passes this in to an old version of libpq, there is no way to know that it didn't know about it. Well, you could create PQinitSSLExtended, but, as you say, the use case is pretty narrow... It would help if there were a PQgetLibraryVersion() function. Help how? There is nothing an app can do to work around the problem AFAICS. Or if there were, we should just document it and not change the code --- the use case for this is evidently too narrow to justify complicating libpq's API even more. 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] PQinitSSL broken in some use casesf
Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: We could do that, I guess. However, if an application passes this in to an old version of libpq, there is no way to know that it didn't know about it. Well, you could create PQinitSSLExtended, but, as you say, the use case is pretty narrow... It would help if there were a PQgetLibraryVersion() function. Help how? There is nothing an app can do to work around the problem AFAICS. Or if there were, we should just document it and not change the code --- the use case for this is evidently too narrow to justify complicating libpq's API even more. Sure, there is a way to work around it in this case. By manually initializing ssl+crypto even though you only need crypto, and then tell libpq you have taken care of the initialization. //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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 10:54 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: We could do that, I guess. However, if an application passes this in to an old version of libpq, there is no way to know that it didn't know about it. Well, you could create PQinitSSLExtended, but, as you say, the use case is pretty narrow... It would help if there were a PQgetLibraryVersion() function. Help how? There is nothing an app can do to work around the problem AFAICS. Or if there were, we should just document it and not change the code --- the use case for this is evidently too narrow to justify complicating libpq's API even more. It would let you assert that you were running against a version of libpq that has the functionality that you are attempting to use, thus eliminating the risk of silent failure. ...Robert -- 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] PQinitSSL broken in some use casesf
Robert Haas robertmh...@gmail.com writes: Well, you could create PQinitSSLExtended, but, as you say, the use case is pretty narrow... It would help if there were a PQgetLibraryVersion() function. Help how? There is nothing an app can do to work around the problem AFAICS. Or if there were, we should just document it and not change the code --- the use case for this is evidently too narrow to justify complicating libpq's API even more. It would let you assert that you were running against a version of libpq that has the functionality that you are attempting to use, thus eliminating the risk of silent failure. If that's all you want, then PQinitSSLExtended is a perfectly good answer. Your app will fail to link if you try to use a library version that hasn't got it. I think documenting the workaround is a sufficient answer though. 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] PQinitSSL broken in some use casesf
Magnus Hagander wrote: Merlin Moncure wrote: On Tue, Feb 10, 2009 at 9:32 AM, Magnus Hagander mag...@hagander.net wrote: How we worked around it: We solved it by copying the SSL init sequence from fe-secure.c. Doesn't seem like something that would change very often. So we init_our_library(), PQinitSSL(0) and then do a few lines of SSL init stuff. Seems unusual, but certainly not nearly impossible. But we're back to the discussions around the WSA code - our API provides no really good place to do this, so perhaps we should just clearly document how it's done and how to work around it? I'm not so sure that's appropriate in this case. I think the existing libpq behavior is simply wrong...crypto and ssl are two separate libraries and PQinitSSL does not expose the necessary detail. This is going to break apps in isolated but spectacular fashion when they link to both pq and crypto for different reasons. They could, but nobody has reported it yet, so it's not a common scenario. Agreed. Would someone remind me why turning off ssl initialization in libpq does not work for this case? -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 11:14 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Well, you could create PQinitSSLExtended, but, as you say, the use case is pretty narrow... It would help if there were a PQgetLibraryVersion() function. Help how? There is nothing an app can do to work around the problem AFAICS. Or if there were, we should just document it and not change the code --- the use case for this is evidently too narrow to justify complicating libpq's API even more. It would let you assert that you were running against a version of libpq that has the functionality that you are attempting to use, thus eliminating the risk of silent failure. If that's all you want, then PQinitSSLExtended is a perfectly good answer. Your app will fail to link if you try to use a library version that hasn't got it. I agree. I was thinking that there might not be enough interest in this feature to add an API call just to support it, but I thought PQgetVersion() might be a more general solution. I think documenting the workaround is a sufficient answer though. I don't have a strong opinion on that one way or the other, but Andrew seemed to be concerned that he was cut-and-pasting a fair amount of code. ...Robert -- 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] PQinitSSL broken in some use casesf
Tom Lane wrote: If that's all you want, then PQinitSSLExtended is a perfectly good answer. How about PQinitCrypto(bool do_init), which would default to TRUE if never called. PQinitSSL already handles the SSL part, just need control over initializing crypto. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
Andrew Chernow wrote: Tom Lane wrote: If that's all you want, then PQinitSSLExtended is a perfectly good answer. How about PQinitCrypto(bool do_init), which would default to TRUE if never called. PQinitSSL already handles the SSL part, just need control over initializing crypto. Folks, we need a lot more demand before we add functions to libpq. -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
Robert Haas wrote: Would someone remind me why turning off ssl initialization in libpq does not work for this case? That initializes both libcrypto and libssl. The problem arises when libcrypto has been initialized but libssl has not. So initialize ssl in your application? What is the problem? -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: Would someone remind me why turning off ssl initialization in libpq does not work for this case? That initializes both libcrypto and libssl. The problem arises when libcrypto has been initialized but libssl has not. So initialize ssl in your application? What is the problem? then libpq doesn't work. merlin -- 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] PQinitSSL broken in some use casesf
Merlin Moncure wrote: On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: Would someone remind me why turning off ssl initialization in libpq does not work for this case? That initializes both libcrypto and libssl. The problem arises when libcrypto has been initialized but libssl has not. So initialize ssl in your application? What is the problem? then libpq doesn't work. Why? -- Bruce Momjian br...@momjian.ushttp://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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 11:14 AM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Well, you could create PQinitSSLExtended, but, as you say, the use case is pretty narrow... It would help if there were a PQgetLibraryVersion() function. Help how? There is nothing an app can do to work around the problem AFAICS. Or if there were, we should just document it and not change the code --- the use case for this is evidently too narrow to justify complicating libpq's API even more. It would let you assert that you were running against a version of libpq that has the functionality that you are attempting to use, thus eliminating the risk of silent failure. If that's all you want, then PQinitSSLExtended is a perfectly good answer. Your app will fail to link if you try to use a library version that hasn't got it. I think documenting the workaround is a sufficient answer though. I don't think you can get way with that this time. wsa cleanup was a mainly harmless side effect. This is a nasty 'maybe it works, maybe it doesn't' virtually impossible to debug problem. We caught it on a particular platform (windows, iirc) when deep in our code a mutex call deadlocked when it shouldn't have, after weeks of working ok. debugging nightmare. PQinitSSL is *broken*. It's always been broken. Since it already takes a parameter, I say add a special switch...the backwards compatibility danger doesn't seem too bad. merlin -- 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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 11:54 AM, Bruce Momjian br...@momjian.us wrote: Merlin Moncure wrote: On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: Would someone remind me why turning off ssl initialization in libpq does not work for this case? That initializes both libcrypto and libssl. The problem arises when libcrypto has been initialized but libssl has not. So initialize ssl in your application? What is the problem? then libpq doesn't work. PQinitSSL is required if you want to make any ssl calls (it does some libpq setup beyond the ssl library initialization). merlin -- 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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure mmonc...@gmail.com wrote: PQinitSSL is *broken*. It's always been broken. Since it already takes a parameter, I say add a special switch...the backwards compatibility danger doesn't seem too bad. Add a switch to what? I get very nervous for our Windows users when people start talking about changing the libpq API (for those that don't know, Windows doesn't have DLL versioning like Unix - so any non-backwards compatible API change really needs a corresponding filename change to avoid pain and suffering). -- Dave Page EnterpriseDB UK: http://www.enterprisedb.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] PQinitSSL broken in some use casesf
Bruce Momjian wrote: Andrew Chernow wrote: Tom Lane wrote: If that's all you want, then PQinitSSLExtended is a perfectly good answer. How about PQinitCrypto(bool do_init), which would default to TRUE if never called. PQinitSSL already handles the SSL part, just need control over initializing crypto. Folks, we need a lot more demand before we add functions to libpq. Honestly, I'm not suggesting that a function is added. If others decide to do that, I think the function's purpose should be to init crypto. We don't need another way to init ssl. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 12:03 PM, Dave Page dp...@pgadmin.org wrote: On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure mmonc...@gmail.com wrote: PQinitSSL is *broken*. It's always been broken. Since it already takes a parameter, I say add a special switch...the backwards compatibility danger doesn't seem too bad. Add a switch to what? I get very nervous for our Windows users when people start talking about changing the libpq API (for those that don't know, Windows doesn't have DLL versioning like Unix - so any non-backwards compatible API change really needs a corresponding filename change to avoid pain and suffering). PQinitSSL(SSL_ONLY) or something, where the constant is carefully chosen to not be accidentally passed in by older libpq users. merlin -- 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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 12:03 PM, Merlin Moncure mmonc...@gmail.com wrote: On Tue, Feb 10, 2009 at 11:54 AM, Bruce Momjian br...@momjian.us wrote: Merlin Moncure wrote: On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian br...@momjian.us wrote: Robert Haas wrote: Would someone remind me why turning off ssl initialization in libpq does not work for this case? That initializes both libcrypto and libssl. The problem arises when libcrypto has been initialized but libssl has not. So initialize ssl in your application? What is the problem? then libpq doesn't work. PQinitSSL is required if you want to make any ssl calls (it does some libpq setup beyond the ssl library initialization). that was worded badly. Rather, PQinitSSL is required if you need to use ssl features withing libpq. merlin -- 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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 5:05 PM, Merlin Moncure mmonc...@gmail.com wrote: PQinitSSL(SSL_ONLY) or something, where the constant is carefully chosen to not be accidentally passed in by older libpq users. Ahh, OK. That would be painless. -- Dave Page EnterpriseDB UK: http://www.enterprisedb.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] PQinitSSL broken in some use casesf
Merlin Moncure wrote: On Tue, Feb 10, 2009 at 12:03 PM, Dave Page dp...@pgadmin.org wrote: On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure mmonc...@gmail.com wrote: PQinitSSL is *broken*. It's always been broken. Since it already takes a parameter, I say add a special switch...the backwards compatibility danger doesn't seem too bad. Add a switch to what? I get very nervous for our Windows users when people start talking about changing the libpq API (for those that don't know, Windows doesn't have DLL versioning like Unix - so any non-backwards compatible API change really needs a corresponding filename change to avoid pain and suffering). PQinitSSL(SSL_ONLY) or something, where the constant is carefully chosen to not be accidentally passed in by older libpq users. So how are you planinng to deal with it when your application passes that to a version of libpq that doesn't support 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] PQinitSSL broken in some use casesf
On Tue, Feb 10, 2009 at 1:02 PM, Magnus Hagander mag...@hagander.net wrote: Merlin Moncure wrote: On Tue, Feb 10, 2009 at 12:03 PM, Dave Page dp...@pgadmin.org wrote: On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure mmonc...@gmail.com wrote: PQinitSSL is *broken*. It's always been broken. Since it already takes a parameter, I say add a special switch...the backwards compatibility danger doesn't seem too bad. Add a switch to what? I get very nervous for our Windows users when people start talking about changing the libpq API (for those that don't know, Windows doesn't have DLL versioning like Unix - so any non-backwards compatible API change really needs a corresponding filename change to avoid pain and suffering). PQinitSSL(SSL_ONLY) or something, where the constant is carefully chosen to not be accidentally passed in by older libpq users. So how are you planinng to deal with it when your application passes that to a version of libpq that doesn't support it? well, either nothing, which is no worse off than we are now, or backpatch the fix. probably nothing :-) merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
