Switching SSL_library_init with OPENSSL_init_ssl

[aogriffiths]

As explained here tsiv/ccminer-cryptonight#25

Tested on Debian GNU/Linux 9 (stretch)

[squid-prbot]

Can one of the admins verify this patch?

[yadij]

OK to test

[yadij]

While this change is a good one, the relevant check should not ever need to run on Debian machines since it provides pkg-config integration.

Do you have pkg-config installed? what does "pkg-config -- cflags openssl" return?

What exact ./configure parameters did you use to enable OpenSSL support in Squid?
if you used a Debian OpenSSL package, which one?
if you use a custom OpenSSL build, what path did you install it into?

[rousskov]

I agree that some changes are needed here, but the proposed fix needs more work or, at the very least, an explanation/assurances that it cannot make things worse for some of the supported environments.

OpenSSL documentation says: If you are using OpenSSL 1.0.2 or below, then you would use SSL_library_init. Could this change break Squid support for some older OpenSSL versions that are currently supported?

To avoid these complex questions/issues, perhaps we should use a less version-dependent function call here? And a function call actually used by Squid itself?

[neheb]

This will break OpenSSL < 1.1. Better to check for some other function like SSL_CTX_new(NULL)

[rousskov]

Untested. Per @neheb comment:

Suggested change

	AC_CHECK_LIB(ssl,[OPENSSL_init_ssl],[LIBOPENSSL_LIBS="-lssl $LIBOPENSSL_LIBS"],[
	AC_CHECK_LIB(ssl,[SSL_CTX_new],[LIBOPENSSL_LIBS="-lssl $LIBOPENSSL_LIBS"],[

[yadij]

I expect we should continue with the existing SSL_library_init test as per the OpenSSL documentation for the v1.0.2 and prior library versions which might lack pkg-config integration. Newer libraries certainly should all be providing that better method of determining compiler and linker settings.

[rousskov]

I expect we should continue with the existing SSL_library_init test as per the OpenSSL documentation for the v1.0.2 and prior library versions

The documentation I cited invites the opposite conclusion IMO -- ./configure should not be using SSL_library_init( ) because that function is version-dependent. Another reason not to use that function in a ./configure test is because the function is unused in modern Squids.

The SSL_CTX_new() function that @neheb and I suggested does not suffer from these problems AFAICT. It may suffer from some other, unknown to me problems. I do not know why @aogriffiths ignored our suggestion.

Newer libraries certainly should all be providing [pkg-config configuration].

The library does provide pkg-config configuration, but it may not be working in a specific build environment (for whatever reason, including the lack of pkg-config installation). If it were all working, this pull request would not exist. As long as we support a version-agnostic AC_CHECK_LIB() test, we should not rely on a version-specific function inside that test.

Alternatively, we could drop support for environments without (working) pkg-config or even make the AC_CHECK_LIB() test version-specific, but all those controversial/complex alternatives are outside this simple PR scope (and can be implemented independently from this PR).

[fxcoudert]

Hi, Homebrew (https://github.com/Homebrew/homebrew-core) maintainer here.

I can confirm that the current squid configure checks fail with OpenSSL 1.1.1c, because SSL_library_init was removed. Checking for another function, such as SSL_CTX_new, works fine and allows the build to succeed.

[rousskov]

Closing in favor of #470 which has an active developer behind it and should solve the problem that this PR was working on. Thank you.