A new version of the patch.
I will commit this patch to trunk....
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])
>
> 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.
>
> * 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_"...
>
> Amos
>
=== modified file 'acinclude/lib-checks.m4'
--- acinclude/lib-checks.m4 2012-05-27 23:46:56 +0000
+++ acinclude/lib-checks.m4 2013-05-14 13:22:57 +0000
@@ -77,20 +77,84 @@
AC_DEFUN([SQUID_CHECK_LIBIPHLPAPI],[
AC_CACHE_CHECK([for libIpHlpApi],squid_cv_have_libiphlpapi,[
SQUID_STATE_SAVE(iphlpapi)
LIBS="$LIBS -liphlpapi"
AC_LINK_IFELSE([AC_LANG_PROGRAM([[
#include <windows.h>
#include <winsock2.h>
#include <iphlpapi.h>
]], [[
MIB_IPNETTABLE i;
unsigned long isz=sizeof(i);
GetIpNetTable(&i,&isz,FALSE);
]])],
[squid_cv_have_libiphlpapi=yes
SQUID_STATE_COMMIT(iphlpapi)],
[squid_cv_have_libiphlpapi=no
SQUID_STATE_ROLLBACK(iphlpapi)])
])
SQUID_STATE_ROLLBACK(iphlpapi)
])
+
+dnl Checks whether the OpenSSL SSL_get_certificate crashes squid and if a
+dnl workaround can be used instead of using the SSL_get_certificate
+AC_DEFUN([SQUID_CHECK_OPENSSL_GETCERTIFICATE_WORKS],[
+ AH_TEMPLATE(SQUID_SSLGETCERTIFICATE_BUGGY, "Define to 1 if the SSL_get_certificate crashes squid")
+ AH_TEMPLATE(SQUID_USE_SSLGETCERTIFICATE_HACK, "Define to 1 to use squid workaround for SSL_get_certificate")
+ OLDLIBS="$LIBS"
+ LIBS="$LIBS $SSLLIB"
+ if test "x$SSLLIBDIR" != "x"; then
+ LIBS="$LIBS -Wl,-rpath -Wl,$SSLLIBDIR"
+ fi
+
+ AC_MSG_CHECKING(whether the SSL_get_certificate is buggy)
+ AC_RUN_IFELSE([
+ AC_LANG_PROGRAM(
+ [
+ #include <openssl/ssl.h>
+ #include <openssl/err.h>
+ ],
+ [
+ SSLeay_add_ssl_algorithms();
+ SSL_CTX *sslContext = SSL_CTX_new(SSLv3_method());
+ SSL *ssl = SSL_new(sslContext);
+ X509* cert = SSL_get_certificate(ssl);
+ return 0;
+ ])
+ ],
+ [
+ AC_MSG_RESULT([no])
+ ],
+ [
+ AC_DEFINE(SQUID_SSLGETCERTIFICATE_BUGGY, 1)
+ AC_MSG_RESULT([yes])
+ ],
+ [])
+
+ AC_MSG_CHECKING(whether the workaround for SSL_get_certificate works)
+ AC_RUN_IFELSE([
+ AC_LANG_PROGRAM(
+ [
+ #include <openssl/ssl.h>
+ #include <openssl/err.h>
+ ],
+ [
+ SSLeay_add_ssl_algorithms();
+ SSL_CTX *sslContext = SSL_CTX_new(SSLv3_method());
+ X509 ***pCert = (X509 ***)sslContext->cert;
+ X509 *sslCtxCert = pCert && *pCert ? **pCert : (X509 *)0x1;
+ if (sslCtxCert != NULL)
+ return 1;
+ return 0;
+ ])
+ ],
+ [
+ AC_MSG_RESULT([yes])
+ AC_DEFINE(SQUID_USE_SSLGETCERTIFICATE_HACK, 1)
+ ],
+ [
+ AC_MSG_RESULT([no])
+ ],
+[])
+
+LIBS=$OLDLIBS
+])
=== modified file 'configure.ac'
--- configure.ac 2013-01-22 06:29:59 +0000
+++ configure.ac 2013-05-14 13:31:28 +0000
@@ -1238,40 +1238,43 @@
[Define this to include code for SSL gatewaying support])
AC_MSG_NOTICE([Using OpenSSL MD5 implementation: ${with_openssl:=no}])
SQUID_DEFINE_BOOL(USE_OPENSSL,${with_openssl},
[Define this to make use of the OpenSSL libraries for MD5 calculation rather than Squid-supplied MD5 implementation or if building with SSL encryption])
if test "x$enable_ssl" = "xyes"; then
if test "x$SSLLIB" = "x"; then
SSLLIB="-lcrypto" # for MD5 routines
fi
# This is a workaround for RedHat 9 brain damage..
if test -d /usr/kerberos/include -a "x$SSLLIBDIR" = "x" -a -f /usr/include/openssl/kssl.h; then
AC_MSG_NOTICE([OpenSSL depends on Kerberos])
SSLLIBDIR="/usr/kerberos/lib"
CPPFLAGS="$CPPFLAGS -I/usr/kerberos/include"
fi
fi
if test "x$SSLLIBDIR" != "x" ; then
SSLLIB="-L$SSLLIBDIR $SSLLIB"
fi
AC_SUBST(SSLLIB)
+if test "x$with_openssl" = "xyes"; then
+SQUID_CHECK_OPENSSL_GETCERTIFICATE_WORKS
+fi
AC_ARG_ENABLE(forw-via-db,
AS_HELP_STRING([--enable-forw-via-db],[Enable Forw/Via database]), [
SQUID_YESNO([$enableval],[unrecognized argument to --enable-forw-via-db: $enableval])
])
SQUID_DEFINE_BOOL(USE_FORW_VIA_DB,${enable_forw_via_db:=no},
[Enable Forw/Via database])
AC_MSG_NOTICE([Forw/Via database enabled: $enable_forw_via_db])
AC_ARG_ENABLE(cache-digests,
AS_HELP_STRING([--enable-cache-digests],
[Use Cache Digests. See http://wiki.squid-cache.org/SquidFaq/CacheDigests]),
[
SQUID_YESNO($enableval,
[unrecognized argument to --enable-cache-digests: $enableval])
])
SQUID_DEFINE_BOOL(USE_CACHE_DIGESTS,${enable_cache_digests:=no},
[Use Cache Digests for locating objects in neighbor caches.])
AC_MSG_NOTICE([Cache Digests enabled: $enable_cache_digests])
=== modified file 'src/ssl/support.cc'
--- src/ssl/support.cc 2013-01-30 15:39:37 +0000
+++ src/ssl/support.cc 2013-05-14 13:33:47 +0000
@@ -1433,43 +1433,55 @@
SSL_CTX *
Ssl::generateSslContext(CertificateProperties const &properties, AnyP::PortCfg &port)
{
Ssl::X509_Pointer cert;
Ssl::EVP_PKEY_Pointer pkey;
if (!generateSslCertificate(cert, pkey, properties))
return NULL;
if (!cert)
return NULL;
if (!pkey)
return NULL;
return createSSLContext(cert, pkey, port);
}
bool Ssl::verifySslCertificate(SSL_CTX * sslContext, CertificateProperties const &properties)
{
+ // SSL_get_certificate is buggy in openssl versions 1.0.1d and 1.0.1e
+ // Try to retrieve certificate directly from SSL_CTX object
+#if SQUID_USE_SSLGETCERTIFICATE_HACK
+ X509 ***pCert = (X509 ***)sslContext->cert;
+ X509 * cert = pCert && *pCert ? **pCert : NULL;
+#elif SQUID_SSLGETCERTIFICATE_BUGGY
+ X509 * cert = NULL;
+ assert(0);
+#else
// Temporary ssl for getting X509 certificate from SSL_CTX.
Ssl::SSL_Pointer ssl(SSL_new(sslContext));
X509 * cert = SSL_get_certificate(ssl.get());
+#endif
+ if (!cert)
+ return false;
ASN1_TIME * time_notBefore = X509_get_notBefore(cert);
ASN1_TIME * time_notAfter = X509_get_notAfter(cert);
bool ret = (X509_cmp_current_time(time_notBefore) < 0 && X509_cmp_current_time(time_notAfter) > 0);
if (!ret)
return false;
return certificateMatchesProperties(cert, properties);
}
bool
Ssl::setClientSNI(SSL *ssl, const char *fqdn)
{
//The SSL_CTRL_SET_TLSEXT_HOSTNAME is a openssl macro which indicates
// if the TLS servername extension (SNI) is enabled in openssl library.
#if defined(SSL_CTRL_SET_TLSEXT_HOSTNAME)
if (!SSL_set_tlsext_host_name(ssl, fqdn)) {
const int ssl_error = ERR_get_error();
debugs(83, 3, "WARNING: unable to set TLS servername extension (SNI): " <<
ERR_error_string(ssl_error, NULL) << "\n");
return false;
