On 14/05/2013 11:13 p.m., Tsantilas Christos wrote:
> On 05/14/2013 12:52 PM, Amos Jeffries wrote:
>> 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.
> Yes.
> However squid binary builds with the correct options.
I'm not seeing which ones. But if you know, they need to be added to the
conftest.
>> 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.
> With compiler flags we do not have such problems. We are not using an
> SSLFLAGS but we are updating the CPPFLAGS directly, so we are sure that
> the include dir is the correct one.
> The CPPFLAGS used in generated makefiles too so the squid binaries build
> correctly.
Thats all done before the new check macro is run though, right? so
CPPFLAGS setup by --enable-ssl and --with-openssl should be used for
both squid and conftest.
> Currently, after squid build, the system admin have to update the ld.so
> configuration or set LD_LIBRARY_PATH to load the correct openSSL libraries.
That is what I mean. If they have to do that then we are not setting
CPPFLAGS quite right. They should only have to build --with-openssl=X to
use the X library installation. Nothing else special.
> This is also for Kerveros, LDAP libraries, XML libraries, and expat
> libraries. Probably this is requires a separate patch fixing all of these...
Yes I think we are missing a while pile of potential -Wl,-rpath=X
settings which our ./configure.ac should be adding when relevant.
Like I said this is separate issue to the bug you were fixing. So if you
want to leave it out for now that is fine.
> We can just add comment after configure script finishes to inform user
> that he have to update the ld.so configuration too....
I dont think its worth doing anything that special for one library and
not all of them.
>
>>
>>> 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
> Aggr... The bug3759 it is related to bug3232 which force us to build a
> patch which used the new openSSL interface for TXT_DB api and a
> workaround because the new interface was buggy.
> But I am not sure that just checking if new API exist or not is enough.
>
> I will provide a fix for this on a separate patch.
> Is there any know Fedore/redhat openSSL package which has these problem?
> I need to look on their patch before fix this problem...
All of them from 0.9.8 (f or g IIRC) to the 1.0.0c as far as I'm aware.
1.0.0d was the officially changed OpenSSL version.
Our BuildFarm CentOS machine is using the stock package from CentOS 6 so
we can test the fix there. If you would like a login to the node to
trial things ask Kinkie.
> Other checks we are doing using OpenSSL version are:
> 1) Use "SSL_METHOD *" or "const SSL_METHOD *" with SSL*_method()
> functions (0.1.x requires const)
> 2) check if TLSv1_1_client_method() and TLSv1_2_client_method() exists.
> These functions added on openSSL-1.0.1 release.
>
> We may want to add them too in configure script.
Maybe. IIRC these have not been a problem with people back-porting or
mangling the OpenSSL APIs, its just been a matter of the official
releases not supporting them before a certain number.
So, going back to your last patch submission. Please move the AC_DEFUN
to acinclude/lib-checks.m4 where all our library hack tests are supposed
to be defined, and remove the new .m4 file creation. Once that is done I
am happy for you to commit with "--fixes squid:3816". Then lets continue
the thread for the rest of the mess.
Amos
Received on Tue May 14 2013 - 12:00:54 MDT
This archive was generated by hypermail 2.2.0 : Tue May 14 2013 - 12:00:09 MDT