Thread: PQinitSSL broken in some use cases
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. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/
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. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
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. -- Andrew Chernow eSilo, LLC every bit counts http://www.esilo.com/
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
On Tue, Feb 10, 2009 at 9:32 AM, Magnus Hagander <magnus@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
Merlin Moncure wrote: > On Tue, Feb 10, 2009 at 9:32 AM, Magnus Hagander <magnus@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
>> 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
Robert Haas <robertmhaas@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
Tom Lane wrote: > Robert Haas <robertmhaas@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
On Tue, Feb 10, 2009 at 10:54 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@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
Robert Haas <robertmhaas@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
Magnus Hagander wrote: > Merlin Moncure wrote: > > On Tue, Feb 10, 2009 at 9:32 AM, Magnus Hagander <magnus@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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Tue, Feb 10, 2009 at 11:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@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
> 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. ...Robert
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/
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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian <bruce@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
Merlin Moncure wrote: > On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian <bruce@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 <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
On Tue, Feb 10, 2009 at 11:14 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@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
On Tue, Feb 10, 2009 at 11:54 AM, Bruce Momjian <bruce@momjian.us> wrote: > Merlin Moncure wrote: >> On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian <bruce@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
On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure <mmoncure@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
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/
On Tue, Feb 10, 2009 at 12:03 PM, Dave Page <dpage@pgadmin.org> wrote: > On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure <mmoncure@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
On Tue, Feb 10, 2009 at 12:03 PM, Merlin Moncure <mmoncure@gmail.com> wrote: > On Tue, Feb 10, 2009 at 11:54 AM, Bruce Momjian <bruce@momjian.us> wrote: >> Merlin Moncure wrote: >>> On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian <bruce@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
On Tue, Feb 10, 2009 at 5:05 PM, Merlin Moncure <mmoncure@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
Merlin Moncure wrote: > On Tue, Feb 10, 2009 at 12:03 PM, Dave Page <dpage@pgadmin.org> wrote: >> On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure <mmoncure@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
On Tue, Feb 10, 2009 at 1:02 PM, Magnus Hagander <magnus@hagander.net> wrote: > Merlin Moncure wrote: >> On Tue, Feb 10, 2009 at 12:03 PM, Dave Page <dpage@pgadmin.org> wrote: >>> On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure <mmoncure@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