Re: A new workaround for bug 3816: ssl_crtd crash with OpenSSL v...

On 05/14/2013 06:55 AM, Amos Jeffries wrote:
> On 14/05/2013 6:28 a.m., Tsantilas Christos wrote:
>> I am attaching a fix.
>> Still needs some discussion.
>> This patch does the following two checks:
>> 1) Checks if the SSL_get_certificate is buggy
>> 2) Checks it he workaround can be enabled.
>>
>> Inside squid:
>> If the workaround can be used, enable it
>> else if the SSL_get_certificate is not buggy, use it
>> else hit an assertion
>>
>> I select this approach:
>> 1) because the workaround is significant faster than using the
>> SSL_get_certificate
>> 2) to avoid the segfault if the SSL_get_certificate is buggy .
>>
>> Problems:
>> I had problem with the LD_LIBRARY_PATH. For example if the user does
>> not want to use system libraries and use openSSL SDK installed under a
>> non standard directory, the test program will run with system libraries.
>> To avoid this someone should use the LD_LIBRARY_PATH in configure script:
>> ./configure --with-openssl=/path/to/openssl/
>> LD_LIBRARY_PATH=/path/to/openssl/
>>
>> I do not like this option, so in the test I am using the -wl,-rpath
>> compiler option to pass the openSSL libraries path.
>> But this option does not looks good too..
>>
>> Also we may want to harden the workaround test to use a hardcoded
>> certificate instead of a NULL certificate. (I attached an example in a
>> previous mail)
>>
>> Regards,
>> Christos
>
> Looks like good progress.
>
> Have you tried moving the m4_include statement after AC_SUBST(SSLLIB)?
> The the m4_include will expand the file in-place inside configure.ac.
>
> Have you tried passing the flags as an argument to the check macro? eg.
> SQUID_CHECK_OPENSSL_GETCERTIFICATE_WORKS([$SSLLIB])

The $SSLLIB variable can be used without problem in the check. This is
not a problem. The problem is different. Imagine the following scenario:

1) The user has installed in your system a buggy version of openSSL. The
configure script finds the SSL_get_certificate buggy
2) Installs a new version under the /opt/openSSL/
3) Runs again the configure script:
     ./configure --with-openssl=/opt/openSSL/
 The check will be compiled using:
   g++ -o conftest test.cc -I/opt/openSSL/include -L/opt/openSSL/lib -lssl
 But it will run as
    ./conftest
This is means that the conftest will use the system ssl libraries
installed under the "/usr/lib/". The test for SSL_get_certificate will
fail again in this case.

To avoid this problem I used in this patch the -rpath:
   g++ -o conftest test.cc -I/opt/openSSL/include -L/opt/openSSL/lib
-lssl -Wl,-rpath -Wl,/opt/openSSL/lib

Is this acceptable? Maybe the -rpath is not available in all systems (it
is a linker parameter, not a g++ parameter)...

An alternate maybe is to set the LD_RUN_PATH or LD_LIBRARY_PATH
environment variable before run the check and unset it after the test
finishes:

OLD_LD_RUN_PATH="$LD_RUN_PATH"
if test "x$SSLLIBDIR" != "x"; then
     LD_RUN_PATH="$LD_RUN_PATH:$SSLLIBDIR"
fi
SQUID_CHECK_OPENSSL_GETCERTIFICATE_WORKS
LD_RUN_PATH="$OLD_LD_RUN_PATH"

But still we may have problems. Are the LD_RUN_PATH or LD_LIBRARY_PATH
environment variables available in all systems?

>
> partial audit:
> * Provided the m4_include is not sensitive to location I would like this
> AC_DEFUN to be in acinclude/lib-checks.m4 though along with the other
> library hack checks. If location is sensitive we will be forced to use a
> separate .m4 file though.

OK.

>
> * Also the .cc code does not need to use "#if defined(" when the squid
> code is in explicit control of the macro definitino 0/1/absent state.
> Just use "#if SQUID_"...

OK.

>
> Amos
Received on Tue May 14 2013 - 09:11:43 MDT