Hi,

I'll be mailing back here at another time about my work on integrating
"distcache" (www.distcache.org) support into Apache 2. I first need to
run a bunch of checks on my autoconf hooks into the Apache 2 build
system though ...

... which leads nicely on to why I *am* posting now. The distcache
exercise put me in direct contact with the openssl checks in apache
(acinclude.m4 + modules/ssl/config.m4), and so I noticed that the code
had been waiting in a very strange state for someone to find time to
sort it out.  so I've been hacking away at it and have attached a
(unified) diff of where I'm at.

This isn't ready for CVS yet, but it needs (now) the input of an apache
hacker to verify a few things and fix an issue to do with how the rest
of the build system is tied together. Also, given some of the weirdness
of the existing code, any "Right Way" of doing things is necessarily
going to be a big enough change that regressions have to be watched for.
Eg. the existing openssl checks just cycle through some preset
commonly-used system paths, and don't use any of the normal autoconf
mechanisms to find or test anything. It's obviously not supposed to be
this way, but I can't guess how disruptive it might be to suddenly
commit a correction to it.

Well I'll leave off by logging some info about the patch contents. Any
and all feedback would most welcome.

Cheers,
Geoff

Patch notes:
- I've left the existing APACHE_CHECK_SSL_TOOLKIT implementation but
  simply renamed it out of the way - this is mainly to make sure the
  diff is nice and clean and not an abomination caused by diffing two
  *almost* entirely different blocks of code against one another.
- All the hard-coded path checking (where on earth did that come from?)
  that was used before is gone and the autoconf macros AC_TRY_COMPILE
  and AC_TRY_LINK are used instead to verify headers and libraries. This
  needs a regression once-over, but can't possibly be a good thing to
  leave the way it was.
- The existing implementation doesn't try any compile or link tests,
  because right now any linking test would fail because the basic system
  library checks hadn't been performed in advance. I've put a FIXME
  there so someone can take a look at the issue - whatever Apache's
  existing library checks are is obviously sufficient, because Apache
  links :-) So it's just a matter of rearranging or reproducing all the
  standard -ldl -lnsl -lsocket [etc] checks to occur before
  APACHE_CHECK_SSL_TOOLKIT runs.
- Relative paths to "--with-ssl" were broken but now work. Before they
  would pass the configure checks if the paths were valid relative to
  the top-level directory, but would then fail (not surprisingly) when
  compiling inside modules/ssl/. Linking was also broken for the same
  reason.
- A path supplied to "--with-ssl" might (before) have been overriden by
  another linker path containing openssl libraries and yet silently
  succeeded. This can lead to mismatches between headers and libraries
  (with all the obvious linker or run-time problems that can lead to),
  and also has the failing that it succeeds whilst contradicting what
  was asked for. This can be pretty painful if there are important
  reasons why --with-ssl was specifying a specific build.
- --with-ssl supports both installation bases (where include and lib are
  subdirectories) as well as compiled openssl source trees (just the lib
  subdirectory). I think this was the case before too, but because of
  the other items mentioned, this now actually works.
- "openssl version" isn't used any more, because it requires that the
  openssl binary be installed. More accurate (and much easier) version
  checking is possible from openssl's "opensslv.h" header, and this has
  the added advantage that RPM type systems only need the
  "libopenssl***" package installed rather than the utils as well.
- The version check is less fragile (no regexp string games) and has
  been bumped to 0.9.6i from 0.9.6e because of the recent SSL/TLS
  vulnerability.
- openssl headers are now included with the "openssl/" directory prefix.
- modules/ssl/config.m4 no longer needs to check SSL_set_state and
  SSL_set_cert_store functions - the only possible use for these was
  because nothing up until that point had run any compiler or link tests
  on the guesses made by configure. That's now changed so these checks
  are pointless.

-- 
Geoff Thorpe
[EMAIL PROTECTED]
http://www.geoffthorpe.net/

Index: acinclude.m4
===================================================================
RCS file: /home/cvspublic/httpd-2.0/acinclude.m4,v
retrieving revision 1.136
diff -u -r1.136 acinclude.m4
--- acinclude.m4        17 Feb 2003 02:32:19 -0000      1.136
+++ acinclude.m4        27 Feb 2003 05:14:01 -0000
@@ -312,7 +312,7 @@
 ])
 
 dnl
-dnl APACHE_CHECK_SSL_TOOLKIT
+dnl APACHE_CHECK_SSL_TOOLKIT (old version)
 dnl
 dnl Find the openssl toolkit installation and check it for the right
 dnl version, then add its flags to INCLUDES and LIBS.  This should
@@ -320,7 +320,7 @@
 dnl and then AC_TRY_LINK to test the libraries directly for the version,
 dnl but that will require someone who knows how to program openssl.
 dnl
-AC_DEFUN(APACHE_CHECK_SSL_TOOLKIT,[
+AC_DEFUN(APACHE_CHECK_SSL_TOOLKIT_OLD,[
 if test "x$ap_ssltk_base" = "x"; then
   AC_MSG_CHECKING(for SSL/TLS toolkit base)
   ap_ssltk_base=""
@@ -421,6 +421,151 @@
   fi
   APR_ADDTO(LIBS, [-lssl -lcrypto])
   ap_cv_ssltk="$ap_ssltk_base"
+fi
+])
+
+dnl
+dnl APACHE_CHECK_SSL_TOOLKIT (new version)
+dnl
+dnl Find the openssl toolkit installation and check it for the right
+dnl version, then add its flags to INCLUDES and LIBS.
+dnl
+AC_DEFUN(APACHE_CHECK_SSL_TOOLKIT,[
+if test "x$ap_ssltk_configured" = "x"; then
+  dnl Before running autoconf tests for potentially mangled CFLAGS/LIBS, make a
+  dnl backup that we restore at the end. (We'll use APR_ADDTO() later, as/where
+  dnl needed).
+  saved_cflags=$CFLAGS
+  saved_libs=$LIBS
+  dnl initialise the variables that will be APR_ADDTO()'d later on
+  ap_ssltk_base=""
+  ap_ssltk_inc=""
+  ap_ssltk_lib=""
+
+  dnl Step 1: base directory, if any
+  AC_MSG_CHECKING(for SSL/TLS toolkit base)
+  AC_ARG_WITH(ssl, APACHE_HELP_STRING(--with-ssl=DIR,SSL/TLS toolkit (OpenSSL)), [
+    dnl If --with-ssl specifies a directory, we use that directory or fail
+    if test "x$withval" != "xyes" -a "x$withval" != "x"; then
+      dnl This ensures $withval is actually a directory and that it is absolute
+      ap_ssltk_base="`cd $withval ; pwd`"
+    fi
+  ])
+  if test "x$ap_ssltk_base" = "x"; then
+    AC_MSG_RESULT(none)
+  else
+    AC_MSG_RESULT($ap_ssltk_base)
+  fi
+
+  dnl Step 2: run header checks
+  AC_MSG_CHECKING(for SSL/TLS headers)
+  if test "x$ap_ssltk_base" != "x"; then
+    ap_ssltk_inc="-I$ap_ssltk_base/include"
+  fi
+  CFLAGS="$CFLAGS $ap_ssltk_inc"
+  dnl AC_CHECK_HEADER sucks because (a) it only runs the pre-compiler, (b) it
+  dnl won't observe changes to CPPFLAGS so we can't pass ap_ssltk_inc to it,
+  dnl and (c) AC_TRY_COMPILE achieves more than two AC_CHECK_HEADER()s does.
+  AC_TRY_COMPILE(
+[#include <openssl/opensslv.h>
+#include <openssl/ssl.h>],
+[SSL_CTX *foo = SSL_CTX_new((void*)0);], [],
+[AC_MSG_ERROR([OpenSSL headers are unusable])])
+  if test "x$ap_ssltk_inc" != "x"; then
+    AC_MSG_RESULT($ap_ssltk_inc)
+  else
+    AC_MSG_RESULT(yes)
+  fi
+
+  dnl FIXME: It is necessary (for normal autoconf arrangements) that the checks for 
the
+  dnl various system libraries take place before testing behaviour of optional
+  dnl libraries - *particularly those that could be static libraries*. In other words,
+  dnl before testing for things like openssl. The reason is that if any
+  dnl AC_CHECK_LIB() macros find libraries, those libraries are then automatically
+  dnl available to resolve linkage when testing behaviour of other libraries. The
+  dnl following checks are certainly required before "-lssl -lcrypto" can be expected
+  dnl to resolve on Linux, HPUX, and Solaris - at least with openssl 0.9.7 and the
+  dnl versions I've used of those systems. There are most likely other checks that'd 
be
+  dnl needed for other platforms so it would make sense to ensure that whatever 
Apache's
+  dnl existing code considers "adequate system library checks" are all finished and
+  dnl available before this SSL configuration takes place. That'd need an Apache 
hacker
+  dnl to sort out though, this build system is still a little too mysterious to me.
+  AC_CHECK_LIB(dl, dlopen) dnl Linux and friends
+  AC_CHECK_LIB(dld, shl_load) dnl HPUX (has no friends)
+  AC_CHECK_LIB(nsl, gethostent)
+  AC_CHECK_LIB(socket, socket)
+
+  dnl Step 3: run library checks (use AC_TRY_LINK instead of AC_CHECK_LIB, as
+  dnl it lets us control the precise order of LIBS items and provides a better
+  dnl test that all is well with openssl).
+  AC_MSG_CHECKING(for SSL/TLS libraries)
+  if test "x$ap_ssltk_base" != "x"; then
+    dnl Check if 'base' is an installation, otherwise try it as a compiled
+    dnl source tree
+    if test -d "$ap_ssltk_base/lib"; then
+      ap_ssltk_lib="$ap_ssltk_base/lib"
+    else
+      ap_ssltk_lib="$ap_ssltk_base"
+    fi
+  fi
+  dnl We keep ap_ssltk_lib free of "-L" stuff so "Step 6" remains less fiddly.
+  if test "x$ap_ssltk_lib" != "x"; then
+    LIBS="$LIBS -L$ap_ssltk_lib -lssl -lcrypto"
+  else
+    LIBS="$LIBS -lssl -lcrypto"
+  fi
+  AC_TRY_LINK([#include <openssl/ssl.h>],
+              [SSL_CTX *foo = SSL_CTX_new((void*)0);], [],
+              [AC_MSG_ERROR([OpenSSL libraries missing or unusable])])
+  if test "x$ap_ssltk_lib" != "x"; then
+    AC_MSG_RESULT($ap_ssltk_lib)
+  else
+    AC_MSG_RESULT(yes)
+  fi
+
+  dnl Step 4: run version checks
+  AC_MSG_CHECKING(for SSL/TLS version)
+  AC_TRY_COMPILE([#include <openssl/opensslv.h>],
+[#if !defined(OPENSSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < 0x0090609f
+#error "invalid openssl version"
+#endif], [],
+[AC_MSG_ERROR([OpenSSL versions prior to 0.9.6i have known security holes])])
+  dnl Perhaps look at replacing "ok" with a grep for OPENSSL_VERSION_TEXT from
+  dnl openssl/opensslv.h??
+  AC_MSG_RESULT(ok)
+
+  dnl Step 5: revert our CFLAGS and LIBS back to normal
+  CFLAGS=$saved_cflags
+  LIBS=$saved_libs
+
+  dnl Step 6: annotate the Apache build environment
+  if test "x$ap_ssltk_inc" != "x"; then
+    dnl I've removed the "/openssl" suffix from this now because I've also
+    dnl corrected modssl's includes. Eg. it uses #include <openssl/ssl.h>
+    dnl rather than #include <ssl.h>.
+    APR_ADDTO(INCLUDES, [$ap_ssltk_inc])
+  fi
+  if test "x$ap_ssltk_lib" != "x"; then
+    dnl This stuff is weird but follows the previous autoconf stuff for openssl
+    dnl so I'm choosing to produce what that produced. As for what
+    dnl $ap_platform_runtime_link_flag might be, who knows.
+    APR_ADDTO(LDFLAGS, [-L$ap_ssltk_lib])
+    if test "x$ap_platform_runtime_link_flag" != "x"; then
+      APR_ADDTO(LDFLAGS, [$ap_platform_runtime_link_flag$ap_ssltk_libdir])
+    fi
+    dnl *Unlike* the previous autoconf stuff however, I'm putting the "-L"
+    dnl flag here in LIBS also, otherwise it's too damned easy for the wrong
+    dnl openssl library to get linked in with unpredictable results. If the
+    dnl "-L" doesn't directly prefix the "-lssl -lcrypto", we don't know what
+    dnl other "-L" directives might influence the selection. If an apache
+    dnl hacker can convince his/herself that no non-Apache "-L"s will occur
+    dnl between the above LDFLAGS changes and these LIBS changes, they can
+    dnl then safely remove this.
+    APR_ADDTO(LIBS, [-L$ap_ssltk_lib])
+  fi
+  APR_ADDTO(LIBS, [-lssl -lcrypto])
+
+dnl Done
 fi
 ])
 
Index: modules/ssl/config.m4
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/ssl/config.m4,v
retrieving revision 1.11
diff -u -r1.11 config.m4
--- modules/ssl/config.m4       29 Mar 2002 07:36:01 -0000      1.11
+++ modules/ssl/config.m4       27 Feb 2003 05:14:02 -0000
@@ -77,8 +77,13 @@
 dnl #  hook module into the Autoconf mechanism (--enable-ssl option)
 APACHE_MODULE(ssl, [SSL/TLS support (mod_ssl)], $ssl_objs, , no, [
     APACHE_CHECK_SSL_TOOLKIT
-    AC_CHECK_FUNCS(SSL_set_state)
-    AC_CHECK_FUNCS(SSL_set_cert_store)
+    dnl These checks aren't really useful and could fail for silly reasons if
+    dnl ever the flags configured by APACHE_CHECK_SSL_TOOLKIT aren't in
+    dnl effect when these checks run (but are in effect during apache
+    dnl compilation). The version checks on openssl already make sure the
+    dnl below functions exist anyway.
+    dnl AC_CHECK_FUNCS(SSL_set_state)
+    dnl AC_CHECK_FUNCS(SSL_set_cert_store)
 ])
 
 dnl #  end of module specific part
Index: modules/ssl/mod_ssl.h
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/ssl/mod_ssl.h,v
retrieving revision 1.125
diff -u -r1.125 mod_ssl.h
--- modules/ssl/mod_ssl.h       23 Feb 2003 17:12:43 -0000      1.125
+++ modules/ssl/mod_ssl.h       27 Feb 2003 05:14:02 -0000
@@ -108,21 +108,21 @@
 #define MOD_SSL_VERSION AP_SERVER_BASEREVISION
 
 /* OpenSSL headers */
-#include <ssl.h>
-#include <err.h>
-#include <x509.h>
-#include <pem.h>
-#include <crypto.h>
-#include <evp.h>
-#include <rand.h>
+#include <openssl/ssl.h>
+#include <openssl/err.h>
+#include <openssl/x509.h>
+#include <openssl/pem.h>
+#include <openssl/crypto.h>
+#include <openssl/evp.h>
+#include <openssl/rand.h>
 #ifdef SSL_EXPERIMENTAL_ENGINE
-#include <engine.h>
+#include <openssl/engine.h>
 #endif
 
 #include "ssl_toolkit_compat.h"
 
 #ifdef HAVE_SSL_X509V3_H
-#include <x509v3.h>
+#include <openssl/x509v3.h>
 #endif
 
 /* mod_ssl headers */

Reply via email to