PQinitSSL broken in some use cases

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