On 14/05/2013 9:11 p.m., Tsantilas Christos wrote:
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.
And Squid built with the same options will also encounter this identical
library problem when its run as "/usr/bin/squid" or whatever.
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)...
I would go with -W,-rpath=/opt/openSSL/lib so that nothing like libtool
can play games with the ordering. Some versions try to be overly-smart.
We also need to ensure that the main squid binaries are also built with
the same extra options when --with-openssl is given parameters. So a
SSLFLAGS global may be needed creating.
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?
Probably not, and the issue existing for bin/squid still needs it later
anyway.
It is time I suppose to ask, are you wanting to fix all of these known
OpenSSL hack bugs in one update, or do them separately?
If you want do them separately then you have IMHO fixed the bug 3816
issue already, this path problem is a second bug todo, and bug 3759 is
still waiting a fix.
NP: bug 3759 is hitting the entire RHEL hierarchy of distros now.
http://build.squid-cache.org/job/3.HEAD-amd64-centos-6/lastFailedBuild/console,
http://bugs.squid-cache.org/show_bug.cgi?id=3759
Amos
