Re: PQinitSSL broken in some use casesf

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(at)momjian(dot)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(at)hagander(dot)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(at)hagander(dot)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(at)gmail(dot)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(at)gmail(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)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(at)gmail(dot)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(at)hagander(dot)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(at)momjian(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)momjian(dot)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(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)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(at)momjian(dot)us> wrote:
> Merlin Moncure wrote:
>> On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian <bruce(at)momjian(dot)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(at)gmail(dot)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(at)pgadmin(dot)org> wrote:
> On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure <mmoncure(at)gmail(dot)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(at)gmail(dot)com> wrote:
> On Tue, Feb 10, 2009 at 11:54 AM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> Merlin Moncure wrote:
>>> On Tue, Feb 10, 2009 at 11:52 AM, Bruce Momjian <bruce(at)momjian(dot)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(at)gmail(dot)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(at)pgadmin(dot)org> wrote:
>> On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure <mmoncure(at)gmail(dot)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(at)hagander(dot)net> wrote:
> Merlin Moncure wrote:
>> On Tue, Feb 10, 2009 at 12:03 PM, Dave Page <dpage(at)pgadmin(dot)org> wrote:
>>> On Tue, Feb 10, 2009 at 4:57 PM, Merlin Moncure <mmoncure(at)gmail(dot)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