Hi Justin,

* Justin Erenkrantz ([EMAIL PROTECTED]) wrote:
> 
> I think you mean adding -I<...> to CPPFLAGS not to CFLAGS.  That should be 
> portable and supported everywhere.  It's a C preprocessor flag not a flag 
> for the compiler.  Autoconf will evaluate ac_compile which includes 
> $CPPFLAGS. You should be able to use APR_ADDTO on CPPFLAGS for the explicit 
> path case. (Since the user specified it, we always need to add it to 
> CPPFLAGS.)

I was just getting ready to launch into a spiel about how none of the
documented flags seemed to be honoured by AC_CHECK_HEADERS and how I'd
looked at the generated configure script to see that cpp invocation
looked very different to compiler invocation, blah blah blah. Anyway, I
will spare you that spiel because having double checked I see that
you're absolutely right and I'm now wondering how I ended up so derailed
in the first place. I guess something else must have been broken whilst
I was trying that out and I never twigged. Anyway yes, putting the
include paths into CPPFLAGS makes header checking work. Thanks :-)

[snip]
> >  Apache's builtin tests (which are obviously OK because Apache links
> >  fine) should occur before the AC_CHECK_LIB()s for "ssl" and "crypto".
> >  See "Step 3" of my changes to acinclude.m4.
[snip]
> 
> A much easier route would be to include `$apr_config --libs` in that 
> section. Those libraries would be in there.  When we go to actually link, 
> the libtool dependencies will have them there.  But, you could add those to 
> LIBS temporarily - just remove them when you are done trying to link.

Well, this is exactly what I was looking for. Unfortunately it seems not
to work as-is. What happens is that if I remove my explicit
AC_CHECK_LIB() calls for dl, socket, nsl, etc and do the following;

   ...
   saved_LIBS=$LIBS
   LIBS="$LIBS `$apr_config --libs`"
   AC_CHECK_LIB(crypto, SSLeay_version)
   AC_CHECK_LIB(ssl, SSL_CTX_new)
   LIBS=$saved_LIBS
   ...

then strangely the -lcrypto and -lssl checks report success *yet* they
do not find their way into the linker flags (so apache fails to link
after compiling). I can only imagine 2 reasons for this; either my
restoring of LIBS to its original form obliterates the result of the
AC_CHECK_LIB() macros, or the fact the linker flags that -lcrypto and
-lssl depend have disappeared (and didn't occur in AC_CHECK_LIB() form)
mean that autoconf does away with -lcrypto and -lssl some point later. I
suspect the former as the latter seems just too weird a possibility.

> ap_platform_runtime_link_flag is '-R' on those platforms that need a 
> special flag to indicate where to look at run-time for libraries.  (Solaris 
> is the predominate case.)  Some platforms use '-rpath' as well.  -L is not 
> enough.

I figured it must be something of that sort from the existing code's
comments, thanks for clarifying. I mainly wanted to be sure it wasn't
producing something to compensate for the fact autoconf's own mechanisms
weren't being used. Can I assume then that I'm right to parrot the
existing code's production of the additional LDFLAGS entry if that flag
is set?

> >- I'm tagging "-DHAVE_OPENSSL" or "-DHAVE_SSLC" directly onto CFLAGS
> >  rather than using anything like AC_DEFINE because the latter
> >  possibility would require HAVE_OPENSSL and HAVE_SSLC to be stubbed
> >  into an appropriate "something.h.in" file. If you prefer not to have
> >  such stuff polluting CFLAGS then please suggest an appropriate ".in"
> >  file for me to hook into.
> 
> Why is having HAVE_OPENSSL or HAVE_SSLC in ap_config_auto.h a bad thing?  I 
> don't understand.

I didn't say it was a bad thing, on the contrary - I think it's a good
thing, I just wasn't sure which (if any) .in file would have been the
appropriate one. It's a big-ass tree of code that I have only very
selective familiarity with. :-) Anyway, following your comments, I
noticed that declaring the symbols in the top-level acconfig.h and
running "./buildconf" eventually propogating those values into
ap_config_auto.h.in and everything works. I'm happy with that, is it
what you had in mind?

> >-AC_DEFUN(APACHE_CHECK_SSL_TOOLKIT,[
> >+AC_DEFUN(APACHE_CHECK_SSL_TOOLKIT_OLD,[
> 
> Just toss the old APACHE_CHECK_SSL_TOOLKIT.  No need to keep the old 
> version around.

Yeah, but I'm leaving it like this while hashing out the patch details
because it makes the diff readable (if you remove the old version and
produce a diff, you'll see what I mean).

> >+    AC_MSG_CHECKING(for OpenSSL version)
> >+    AC_TRY_COMPILE([#include <openssl/opensslv.h>],
> >+[#if !defined(OPENSSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < 
> >0x0090609f
> >+#error "invalid openssl version"
> >+#endif],
> >+      [dnl Replace this with OPENSSL_VERSION_TEXT from opensslv.h?
> >+      AC_MSG_RESULT(OK)],
> 
> Yes, it should indicate the version found somehow.  I believe we did that 
> before.  I think it's worthwhile to have.

I'm all for it, but would rather not guess as to the preferred mechanism
for slurping that text out of the header file (especially as we're not
supposed to assume anything about its location). I imagine the proper
way is to have a compile test spit the string out and then capture it
somehow. No doubt an autoconf-guru knows the relevant macro trick, but
on my own I'm more likely to wildly overcomplicate this code and bring
in portability problems. Unless someone tells me the Apache-Approved(tm)
way to do this, I'd rather just leave the comment there to guide someone
else if they feel so moved afterwards.

> > 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 
> 
> Just remove them altogether.

Agreed.

> The rest of it looks good.  -- justin

Thanks for the feedback. I've incorporated all the above and attached a
new patch. Does this seem OK? There are a couple of notes I want to
leave you with;

- I've incorporated the use of `$apr_config --libs` as you suggested and
  so my patch is currently broken, but I want to head in the right
  direction. Any ideas why the successfully reported -lssl -lcrypto
  flags disappear from the generated Makefile's?

- I've moved the version checks into the header check steps so I don't
  have to save and restore CPPFLAGS twice.

- Seeing as CPPFLAGS seems ideal for header *and* compilation checks, is
  INCLUDES still the appropriate place to APR_ADDTO() any required
  include path once the tests are done?

Cheers,
Geoff

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

Index: acconfig.h
===================================================================
RCS file: /home/cvspublic/httpd-2.0/acconfig.h,v
retrieving revision 1.1
diff -u -r1.1 acconfig.h
--- acconfig.h  31 Jan 2002 14:51:37 -0000      1.1
+++ acconfig.h  6 Mar 2003 21:38:19 -0000
@@ -1,2 +1,8 @@
 /* Define this if struct tm has a field tm_gmtoff */
 #undef HAVE_GMTOFF
+
+/* Define this if we are building with OpenSSL */
+#undef HAVE_OPENSSL
+
+/* Define this if we are building with SSL-C */
+#undef HAVE_SSLC
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        6 Mar 2003 21:38:19 -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,108 @@
   fi
   APR_ADDTO(LIBS, [-lssl -lcrypto])
   ap_cv_ssltk="$ap_ssltk_base"
+fi
+])
+
+dnl
+dnl APACHE_CHECK_SSL_TOOLKIT (new version)
+dnl
+dnl Configure for the detected openssl/ssl-c toolkit installation, giving
+dnl preference to "--with-ssl=<path>" if it was specified.
+dnl
+AC_DEFUN(APACHE_CHECK_SSL_TOOLKIT,[
+if test "x$ap_ssltk_configured" = "x"; then
+  dnl initialise the variables we use
+  ap_ssltk_base=""
+  ap_ssltk_inc=""
+  ap_ssltk_lib=""
+  ap_ssltk_type=""
+
+  dnl Determine the SSL/TLS toolkit's 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), [
+    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 Run header and version checks
+  saved_CPPFLAGS=$CPPFLAGS
+  if test "x$ap_ssltk_base" != "x"; then
+    ap_ssltk_inc="-I$ap_ssltk_base/include"
+    CPPFLAGS="$CPPFLAGS $ap_ssltk_inc"
+  fi
+  AC_CHECK_HEADERS([sslc.h], [ap_ssltk_type="sslc"], [])
+  if test "x$ap_ssltk_type" = "x"; then
+    AC_CHECK_HEADERS([openssl/opensslv.h openssl/ssl.h], [ap_ssltk_type="openssl"], 
[])
+    if test "x$ap_ssltk_type" = "x"; then
+      AC_MSG_ERROR([No SSL/TLS headers were available])
+    fi
+    dnl so it's OpenSSL - report, then test for a good version
+    echo "... SSL/TLS support configuring for OpenSSL"
+    AC_MSG_CHECKING(for OpenSSL version)
+    AC_TRY_COMPILE([#include <openssl/opensslv.h>],
+[#if !defined(OPENSSL_VERSION_NUMBER) || OPENSSL_VERSION_NUMBER < 0x0090609f
+#error "invalid openssl version"
+#endif],
+      [dnl Replace this with OPENSSL_VERSION_TEXT from opensslv.h?
+      AC_MSG_RESULT(OK)],
+      [AC_MSG_RESULT([not encouraging])
+      echo "WARNING: OpenSSL version may contain security vulnerabilities!"])
+  else
+    dnl so it's SSL-C - report, then test anything relevant
+    echo "... SSL/TLS support configuring for SSL-C"
+    AC_MSG_CHECKING(for SSL-C version)
+    dnl FIXME: we currently don't check anything for SSL-C
+    AC_MSG_RESULT([OK, but I didn't really check])
+  fi
+  dnl restore
+  CPPFLAGS=$saved_CPPFLAGS
+
+  dnl Run library checks
+  saved_LDFLAGS=$LDFLAGS
+  saved_LIBS=$LIBS
+  if test "x$ap_ssltk_base" != "x"; then
+    if test -d "$ap_ssltk_base/lib"; then
+      ap_ssltk_lib="$ap_ssltk_base/lib"
+    else
+      ap_ssltk_lib="$ap_ssltk_base"
+    fi
+    LDFLAGS="$LDFLAGS -L$ap_ssltk_lib"
+  fi
+  dnl make sure "other" flags are available so libcrypto and libssl can link
+  LIBS="$LIBS `$apr_config --libs`"
+  AC_CHECK_LIB(crypto, SSLeay_version)
+  AC_CHECK_LIB(ssl, SSL_CTX_new)
+  dnl restore
+  LDFLAGS=$saved_LDFLAGS
+  LIBS=$saved_LIBS
+
+  dnl Adjust apache's configuration based on what we found above.
+  dnl (a) define preprocessor symbols
+  if test "$ap_ssltk_type" = "openssl"; then
+    AC_DEFINE(HAVE_OPENSSL)
+  else
+    AC_DEFINE(HAVE_SSLC)
+  fi
+  dnl (b) hook up include paths
+  if test "x$ap_ssltk_inc" != "x"; then
+    APR_ADDTO(INCLUDES, [$ap_ssltk_inc])
+  fi
+  dnl (c) hook up linker paths
+  if test "x$ap_ssltk_lib" != "x"; then
+    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
+  fi
 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       6 Mar 2003 21:38:20 -0000
@@ -77,8 +77,6 @@
 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 #  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       6 Mar 2003 21:38:20 -0000
@@ -107,23 +107,7 @@
 
 #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>
-#ifdef SSL_EXPERIMENTAL_ENGINE
-#include <engine.h>
-#endif
-
 #include "ssl_toolkit_compat.h"
-
-#ifdef HAVE_SSL_X509V3_H
-#include <x509v3.h>
-#endif
 
 /* mod_ssl headers */
 #include "ssl_expr.h"
Index: modules/ssl/ssl_toolkit_compat.h
===================================================================
RCS file: /home/cvspublic/httpd-2.0/modules/ssl/ssl_toolkit_compat.h,v
retrieving revision 1.28
diff -u -r1.28 ssl_toolkit_compat.h
--- modules/ssl/ssl_toolkit_compat.h    3 Feb 2003 17:53:13 -0000       1.28
+++ modules/ssl/ssl_toolkit_compat.h    6 Mar 2003 21:38:20 -0000
@@ -55,7 +55,20 @@
  * between OpenSSL and RSA sslc
  */
 
-#ifdef OPENSSL_VERSION_NUMBER
+#ifdef HAVE_OPENSSL
+
+/* OpenSSL headers */
+#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>
+#include <openssl/x509v3.h>
+#ifdef SSL_EXPERIMENTAL_ENGINE
+#include <openssl/engine.h>
+#endif
 
 /*
  * rsa sslc uses incomplete types for most structures
@@ -123,6 +136,19 @@
 
 #else /* RSA sslc */
 
+/* SSL-C headers */
+#include <ssl.h>
+#include <err.h>
+#include <x509.h>
+#include <pem.h>
+#include <crypto.h>
+#include <evp.h>
+#include <rand.h>
+
+#if SSLC_VERSION > 0x1FFF
+#include <x509v3.h>
+#endif
+
 /* sslc does not support this function, OpenSSL has since 9.5.1 */
 #define RAND_status() 1
 
@@ -160,6 +186,9 @@
 #define PEM_F_DEF_CALLBACK PEM_F_DEF_CB
 #endif
 
+/* Note: this test is no longer used to mess with NO_SSL_X509V3_H and
+ * HAVE_SSL_X509V3_H, instead we include x509v3.h further up using the opposite
+ * test (SSLC_VERSION > 0x1FFF). */
 #if SSLC_VERSION < 0x2000
 
 #define X509_STORE_CTX_set_depth(st, d)    
@@ -171,8 +200,6 @@
 #define modssl_set_verify(ssl, verify, cb) \
     SSL_set_verify(ssl, verify)
 
-#define NO_SSL_X509V3_H
-
 #endif
 
 /* BEGIN GENERATED SECTION */
@@ -208,10 +235,6 @@
 #ifndef modssl_set_verify
 #define modssl_set_verify(ssl, verify, cb) \
     SSL_set_verify(ssl, verify, cb)
-#endif
-
-#ifndef NO_SSL_X509V3_H
-#define HAVE_SSL_X509V3_H
 #endif
 
 #endif /* SSL_TOOLKIT_COMPAT_H */

Reply via email to