You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2003/05/19 16:48:47 UTC
cvs commit: httpd-2.0/modules/ssl config.m4 mod_ssl.c mod_ssl.h ssl_engine_config.c ssl_engine_init.c ssl_toolkit_compat.h
wrowe 2003/05/19 07:48:47
Modified: modules/ssl config.m4 mod_ssl.c mod_ssl.h
ssl_engine_config.c ssl_engine_init.c
ssl_toolkit_compat.h
Log:
Roll away the SSL_EXPERIMENTAL_ENGINE test in favor of testing for the
ENGINE_init() function in config.m4, and rely on HAVE_ENGINE_INIT instead.
Reviewed by: Ben Laurie (concept)
Revision Changes Path
1.14 +1 -0 httpd-2.0/modules/ssl/config.m4
Index: config.m4
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/config.m4,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -r1.13 -r1.14
--- config.m4 19 May 2003 14:43:20 -0000 1.13
+++ config.m4 19 May 2003 14:48:47 -0000 1.14
@@ -79,6 +79,7 @@
APACHE_CHECK_SSL_TOOLKIT
AC_CHECK_FUNCS(SSL_set_state)
AC_CHECK_FUNCS(SSL_set_cert_store)
+ AC_CHECK_FUNCS(ENGINE_init)
])
dnl # end of module specific part
1.79 +1 -1 httpd-2.0/modules/ssl/mod_ssl.c
Index: mod_ssl.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/mod_ssl.c,v
retrieving revision 1.78
retrieving revision 1.79
diff -u -r1.78 -r1.79
--- mod_ssl.c 23 Feb 2003 17:12:43 -0000 1.78
+++ mod_ssl.c 19 May 2003 14:48:47 -0000 1.79
@@ -116,7 +116,7 @@
SSL_CMD_SRV(SessionCache, TAKE1,
"SSL Session Cache storage "
"(`none', `dbm:/path/to/file')")
-#ifdef SSL_EXPERIMENTAL_ENGINE
+#ifdef HAVE_ENGINE_INIT
SSL_CMD_SRV(CryptoDevice, TAKE1,
"SSL external Crypto Device usage "
"(`builtin', `...')")
1.135 +1 -14 httpd-2.0/modules/ssl/mod_ssl.h
Index: mod_ssl.h
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/mod_ssl.h,v
retrieving revision 1.134
retrieving revision 1.135
diff -u -r1.134 -r1.135
--- mod_ssl.h 16 May 2003 18:22:44 -0000 1.134
+++ mod_ssl.h 19 May 2003 14:48:47 -0000 1.135
@@ -64,19 +64,6 @@
#ifndef __MOD_SSL_H__
#define __MOD_SSL_H__
-/*
- * Optionally enable the experimental stuff, but allow the user to
- * override the decision which experimental parts are included by using
- * CFLAGS="-DSSL_EXPERIMENTAL_xxxx_IGNORE".
- */
-#ifdef SSL_EXPERIMENTAL
-#ifdef SSL_ENGINE
-#ifndef SSL_EXPERIMENTAL_ENGINE_IGNORE
-#define SSL_EXPERIMENTAL_ENGINE
-#endif
-#endif
-#endif /* SSL_EXPERIMENTAL */
-
/*
* Power up our brain...
*/
@@ -414,7 +401,7 @@
void *pTmpKeys[SSL_TMP_KEY_MAX];
apr_hash_t *tPublicCert;
apr_hash_t *tPrivateKey;
-#ifdef SSL_EXPERIMENTAL_ENGINE
+#ifdef HAVE_ENGINE_INIT
char *szCryptoDevice;
#endif
struct {
1.78 +2 -2 httpd-2.0/modules/ssl/ssl_engine_config.c
Index: ssl_engine_config.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_config.c,v
retrieving revision 1.77
retrieving revision 1.78
diff -u -r1.77 -r1.78
--- ssl_engine_config.c 29 Mar 2003 02:18:43 -0000 1.77
+++ ssl_engine_config.c 19 May 2003 14:48:47 -0000 1.78
@@ -108,7 +108,7 @@
mc->tVHostKeys = apr_hash_make(pool);
mc->tPrivateKey = apr_hash_make(pool);
mc->tPublicCert = apr_hash_make(pool);
-#ifdef SSL_EXPERIMENTAL_ENGINE
+#ifdef HAVE_ENGINE_INIT
mc->szCryptoDevice = NULL;
#endif
@@ -510,7 +510,7 @@
return NULL;
}
-#ifdef SSL_EXPERIMENTAL_ENGINE
+#ifdef HAVE_ENGINE_INIT
const char *ssl_cmd_SSLCryptoDevice(cmd_parms *cmd,
void *dcfg,
const char *arg)
1.113 +2 -2 httpd-2.0/modules/ssl/ssl_engine_init.c
Index: ssl_engine_init.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_init.c,v
retrieving revision 1.112
retrieving revision 1.113
diff -u -r1.112 -r1.113
--- ssl_engine_init.c 3 Apr 2003 04:54:20 -0000 1.112
+++ ssl_engine_init.c 19 May 2003 14:48:47 -0000 1.113
@@ -270,7 +270,7 @@
/*
* SSL external crypto device ("engine") support
*/
-#ifdef SSL_EXPERIMENTAL_ENGINE
+#ifdef HAVE_ENGINE_INIT
ssl_init_Engine(base_server, p);
#endif
@@ -351,7 +351,7 @@
* Support for external a Crypto Device ("engine"), usually
* a hardware accellerator card for crypto operations.
*/
-#ifdef SSL_EXPERIMENTAL_ENGINE
+#ifdef HAVE_ENGINE_INIT
void ssl_init_Engine(server_rec *s, apr_pool_t *p)
{
SSLModConfigRec *mc = myModConfig(s);
1.33 +1 -1 httpd-2.0/modules/ssl/ssl_toolkit_compat.h
Index: ssl_toolkit_compat.h
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_toolkit_compat.h,v
retrieving revision 1.32
retrieving revision 1.33
diff -u -r1.32 -r1.33
--- ssl_toolkit_compat.h 1 May 2003 13:10:36 -0000 1.32
+++ ssl_toolkit_compat.h 19 May 2003 14:48:47 -0000 1.33
@@ -66,7 +66,7 @@
#include <openssl/evp.h>
#include <openssl/rand.h>
#include <openssl/x509v3.h>
-#ifdef SSL_EXPERIMENTAL_ENGINE
+#ifdef HAVE_ENGINE_INIT
#include <openssl/engine.h>
#endif
Re: cvs commit: httpd-2.0/modules/ssl ...
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 03:12 PM 5/27/2003, Geoff Thorpe wrote:
>> >But the better solution is to re-arrange the configuration
>> >order so the dependencies are already satisfied.
>>
>> No doubt, a project for another day (at least for my crazy schedule.)
>
>It's possiby a trivial change, and equally possibly a nightmare. BTW: Does
>this problem crop up with any other modules? Ie. do other modules have
>linker dependencies that are satisfied implicitly by apr? If so, they
>must surely face similar issues if they attempt to do any autoconf checks
>during configuration (and in which case, the issue of re-ordering the
>configuration system would solve a class of problems greater than just
>those of modssl).
Hmmm... there are relatively few opportunities - things like db support in
apr-util, zlib support in mod_deflate, etc.
>Actually continuing on that line of thinking, there's an orthogonal
>cleanup I could suggest, but one that might make later rearrangements
>less confusing; is it worth moving the SSL-specific M4 code out of the
>top-level and into modules/ssl/config.m4? This seems to be one of the
>ways in which the ssl module is (perhaps needlessly) unique in the way it
>is wired up to the rest of the tree. Just junk-food for thought.
Well, that's one way.
But it breaks our ability to wire up ab.c at the same time - my next set
of effort was to activate ab.c in the 'correct' pattern we used for mod_ssl.
>Of course, laziness/carelessness played its part too, as
>you've observed. <gulp> If it's any consolation, I am now more painfully
>aware of SSL-C implications than I would have ever chosen to be. :-)
This is another gleaming example of why two branch development has
become effective for this project :-) No users were hurt in this progressive
and cooperative development effort. Shoot down the bugs first, then review
the unintended consequences later.
>I'll (mostly) hold my tongue - all I'll let slip is that I wrote
>ENGINE_init() and can imagine ways in which it could one-day be NOP'd out
>of existence.
Now you have a reason not to :-) As long as we have parallel 0.9.7 and
optional with 0.9.6 support for ENGINE, this will remain a pita. By the way,
our result handling from the ENGINE_by_id() is bogus and disallows actually
enabling any specific engine. I'm working on it once I've got all the build magic
finished - but if you beat me to it tonight I owe you a couple of beers at the
next AC event or wherever we next meet up :-)
>Of course, even if those possibilities did eventuate to be
>the "Right Way" forward for openssl, my current schedule puts them easily
>out of the reckoning for this decade at least...
Amen to that, brother :-)
Bill
Re: cvs commit: httpd-2.0/modules/ssl ...
Posted by Geoff Thorpe <ge...@geoffthorpe.net>.
Hi there,
On May 27, 2003 01:17 pm, William A. Rowe, Jr. wrote:
> At 02:46 PM 5/25/2003, Geoff Thorpe wrote:
[snip]
> >OK, I can't say anything of much usefulness about SSL-C as I have no
> >experience with it. What I would suggest is that clean SSL-C support
> > in Apache2 may be useful to many people, but there are none who
> > benefit from it quite so much as RSA themselves. IIRC, it was from
> > RSA that the original mod_ssl tweaks for SSL-C originated
>
> Excuse me? No... those have been the fruits of Doug MacEachern's,
> Madhu's and my own efforts. RSA has been helpful from a support
> perspective, but didn't actually port any code that *I* am aware of.
Ah, perhaps I am mistaken then - my understanding had been that RSA had
submitted patches to Ralf for inclusion in mod_ssl (back before apache-2
factored into things). Even so, considering the Covalent angle I can see
that there may have been more than one effort underway anyhow. Whatever
the exact history, I certainly meant no offense.
> The problem from RSA's perspective is that they don't distribute
> mod_ssl, as far as I'm aware of, nor do they 'support' it. They
> support an SSL toolkit. Do whatever you will with it, and scream to
> them for negotiation bugs, or other toolkit operational or functional
> bugs. Covalent via Doug and myself, and HP via Madhu, have an interest
> in mod_ssl interoperating with the RSA toolkit, so all three of us have
> invested blood, sweat and tears to clean up some of the crufty stuff
> (occasionally introducing extra-crufty bits.)
Sure, I think the effort involved is pretty self-evident. I was merely
suggesting that perhaps RSA could be convinced to invest some of their
own resources to tweaking and testing the SSL-C integration. After all,
can you point me to a more widely installed SSL/TLS-enabled server
application than Apache? Read: "you are (or would appear to be) a large
part of their market". My own interest in the ssl configuration didn't
strictly come from my involvement in openssl, but rather the hacking I
was doing on apache-2 integration with distcache. However, when I saw
that some work on the general ssl/tls configuration was needed, it was
pretty obvious to me that the best thing to do is grab the bull by the
horns. Maybe RSA don't care, or maybe they do care but don't want to get
involved, but both would be hard to imagine under the circumstances. BTW:
I wasn't trying to dismiss the efforts of you, Doug, Madhu, nor anyone
else. On the contrary, I think your jobs have been (and will continue to
be) decidedly tricky in ways they needn't be if those on the "inside" of
the SSL-C code contributed their own know-how. Just covering the
backwards-compatibility problems must be a nightmare and this is
presumably something "they" could sort out quite trivially.
But hey, I'm the last guy who wants to advance SSL-C's cause, so I'll
leave it there. :-)
> >> In any case, we won't 'HAVE' ENGINE_init if we don't inject ssl
> >> libs, so I don't have any problem moving those detection macros into
> >> the 'protected' reconfiguration. That still leaves me with the
> >> question of how we can test on a platform like solaris, where
> >> OpenSSL needs libraries like -lsocket just to perform a test compile
> >> of
> >> ENGINE_init().
> >
> >Well this works inside the protected configuration for the reason that
> > it pulls in the apr linker flags, whatever they may be (eg. -ldl
> > -lsocket -lnsl [etc]).
>
> Ok, I see that now, and have committed the code to httpd-2.1 with all
> due credit to y'rself, thanks :-)
Pleasure. :-)
> >But the better solution is to re-arrange the configuration
> >order so the dependencies are already satisfied.
>
> No doubt, a project for another day (at least for my crazy schedule.)
It's possiby a trivial change, and equally possibly a nightmare. BTW: Does
this problem crop up with any other modules? Ie. do other modules have
linker dependencies that are satisfied implicitly by apr? If so, they
must surely face similar issues if they attempt to do any autoconf checks
during configuration (and in which case, the issue of re-ordering the
configuration system would solve a class of problems greater than just
those of modssl).
Actually continuing on that line of thinking, there's an orthogonal
cleanup I could suggest, but one that might make later rearrangements
less confusing; is it worth moving the SSL-specific M4 code out of the
top-level and into modules/ssl/config.m4? This seems to be one of the
ways in which the ssl module is (perhaps needlessly) unique in the way it
is wired up to the rest of the tree. Just junk-food for thought.
> >Well this certainly slipped through the cracks - apologies for my
> > part, I don't have SSL-C so I was largely flying blind in that
> > respect.
>
> In the future, please grep -r "BOGUS_SYMBOL" * before you submit
> this sort of patch, it makes it alot easier to see what's out there.
> Then it's not so confusing to search the archives for the discussion of
> why on earth the code is out there.
Sure, though this case was slightly misleading because there were other
rational reasons for why those checks might have been there, and that's
partly the reason why I didn't notice the *other* reason for their
existence. Of course, laziness/carelessness played its part too, as
you've observed. <gulp> If it's any consolation, I am now more painfully
aware of SSL-C implications than I would have ever chosen to be. :-)
> Right, but there still might be worthwhile information in the pkgconfig
> that we aren't aware of. I was mostly wondering what code is around to
> slurp out the useful details from the pkgconfig contents. In this case
> I agree, it seems we can keep plugging along.
Yep, if apache2+modules/ssl compiles and links today, then pkgconfig can't
give us anything we don't already have - though it is certainly a wise
idea for solidifying the code for later.
> >Well yes, but as I pointed out - if a future version redefines
> > ENGINE_init as a backward compatible macro (or simply removes it,
> > heaven forbid) then you're still going to hurt, whether the autoconf
> > test fails or not. In this case, it seems you want AC_TRY_LINK. But
> > that's perhaps a bit pedantic of me. I'll drop this one, your call
> > :-)
>
> I can't see this being deprecated into a macro - there are no _init
> calls in the ssl toolkits that devolve to no-ops. One of the
> distinguishing features of _init functions (in general) is that they
> can be used to create hard ties to library packages (e.g. using
> dlopen/dlsym to 'acquire' some library package.) This is why
> ENGINE_init is possibly the safest named symbol to test.
I'll (mostly) hold my tongue - all I'll let slip is that I wrote
ENGINE_init() and can imagine ways in which it could one-day be NOP'd out
of existence. Of course, even if those possibilities did eventuate to be
the "Right Way" forward for openssl, my current schedule puts them easily
out of the reckoning for this decade at least...
Cheers,
Geoff
--
Geoff Thorpe
geoff@geoffthorpe.net
http://www.geoffthorpe.net/
Re: cvs commit: httpd-2.0/modules/ssl ...
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 02:46 PM 5/25/2003, Geoff Thorpe wrote:
>On May 24, 2003 09:38 am, William A. Rowe, Jr. wrote:
>> Well, in the SSLC case, HAVE_SSL_SET_STATE was required because
>> you can't even renegotiate without toggling the state. I tried a
>> pretty simple hack, replacing the set state code (which on OpenSSL is
>> simply a bit tweak into what should be opaque data) with the
>> SSL_renegotate() function across toolkits (with the patch applied to
>> 2_0_BRANCH).
>>
>> That didn't work, as several of the perl-framework ssl/ tests started
>> failing. Considering that SSL_set_state a source patch on top of the
>> SSLC toolkit, and most SSLC users work from binary distributions, this
>> was never clean in the first place. Since ApacheSSL uses the
>> SSL_renegotiate() function, and it's support in both toolkits, this is
>> the right direction, I just missed something important (which I will
>> revisit as I find some cycles again.)
>
>OK, I can't say anything of much usefulness about SSL-C as I have no
>experience with it. What I would suggest is that clean SSL-C support in
>Apache2 may be useful to many people, but there are none who benefit from
>it quite so much as RSA themselves. IIRC, it was from RSA that the
>original mod_ssl tweaks for SSL-C originated
Excuse me? No... those have been the fruits of Doug MacEachern's, Madhu's
and my own efforts. RSA has been helpful from a support perspective, but didn't
actually port any code that *I* am aware of.
> - so it begs the question;
>are RSA involved in maintaining this code at all? It seems they should be
>doing as much as possible to avoid bitrot in the SSL-C hooks, and they'd
>be far better placed than anyone to make it happen (and test it)?
The significant problem with mod_ssl's history is that the code reaches deep
into the 'private', 'internals' of the library implementation. Ben Laurie, Madhu
and I are refactoring the various bits that are too transparent into the guts of
the library. It really should be the library's job to expose the appropriate
interfaces... indeed they are in OpenSSL and SSL-C (somewhat differently
in places.) But many of the appropriate 'methods' were introduced after
they were first needed by mod_ssl, and that code was never refactored to
approach the library cleanly.
The problem from RSA's perspective is that they don't distribute mod_ssl,
as far as I'm aware of, nor do they 'support' it. They support an SSL toolkit.
Do whatever you will with it, and scream to them for negotiation bugs, or other
toolkit operational or functional bugs. Covalent via Doug and myself, and
HP via Madhu, have an interest in mod_ssl interoperating with the RSA
toolkit, so all three of us have invested blood, sweat and tears to clean up
some of the crufty stuff (occasionally introducing extra-crufty bits.)
Hopefully others will grab their favorite toolkit and propose patches to extend
the module. This isn't terribly compatible with the socket-based SSL approach,
which is why Brad Nicholes had to deploy a different module for Netware. And
though they are defined on Win32 (and supported on WinCE) the extended SSL
WinSock foo is unimplemented (or generally not documented) across our Win32
supported platforms, so Win32 porters have gone the mod_ssl route.
>> In any case, we won't 'HAVE' ENGINE_init if we don't inject ssl libs,
>> so I don't have any problem moving those detection macros into the
>> 'protected' reconfiguration. That still leaves me with the question of
>> how we can test on a platform like solaris, where OpenSSL needs
>> libraries like -lsocket just to perform a test compile of
>> ENGINE_init().
>
>Well this works inside the protected configuration for the reason that it
>pulls in the apr linker flags, whatever they may be (eg. -ldl -lsocket
>-lnsl [etc]).
Ok, I see that now, and have committed the code to httpd-2.1 with all
due credit to y'rself, thanks :-)
>But the better solution is to re-arrange the configuration
>order so the dependencies are already satisfied.
No doubt, a project for another day (at least for my crazy schedule.)
>> >But we cannot quit testing for the SSL_set_state sort of functions (I
>> > don't know if you did or not) because I for one enjoy forcing SSL-C
>> > to do what it should have done in the first place :-) Please don't
>> > clobber function detection in your final patch, please!
>
>Well this certainly slipped through the cracks - apologies for my part, I
>don't have SSL-C so I was largely flying blind in that respect.
In the future, please grep -r "BOGUS_SYMBOL" * before you submit
this sort of patch, it makes it alot easier to see what's out there. Then
it's not so confusing to search the archives for the discussion of why
on earth the code is out there.
>> Perhaps we should go ahead and use the pkgconfig if it exists, or fall
>> back on our own autoconfig if it doesn't. Would that be a win?
>
>Maybe, but I think elegance dictates that the first priority would be to
>get the existing configuration steps ordered by dependency. The fact that
>apache-2 compiles and links indicates that there are no missing linker
>flags missing once the configuration has completed - it's only during
>configuration itself that there are one or two moments when the state of
>the flags can be left inconsistent. Later, if you then want to detect and
>add some logic to pull flags out of openssl.pc (and/or any equivalent for
>SSL-C), you could presumably use some macro to merge them in and catch
>any necessary flags you might otherwise be missing.
Right, but there still might be worthwhile information in the pkgconfig that
we aren't aware of. I was mostly wondering what code is around to slurp
out the useful details from the pkgconfig contents. In this case I agree, it
seems we can keep plugging along.
>> Now if you test HAVE_ENGINE_H, of course it will find engine.h.
>> However, if you test compile for ENGINE_init, against -lssl, it will
>> link against only the FIRST libssl and then try compiling for that
>> function. It won't try binding to more than one of any given library!
>
>Well yes, but as I pointed out - if a future version redefines ENGINE_init
>as a backward compatible macro (or simply removes it, heaven forbid) then
>you're still going to hurt, whether the autoconf test fails or not. In
>this case, it seems you want AC_TRY_LINK. But that's perhaps a bit
>pedantic of me. I'll drop this one, your call :-)
I can't see this being deprecated into a macro - there are no _init calls
in the ssl toolkits that devolve to no-ops. One of the distinguishing
features of _init functions (in general) is that they can be used to create
hard ties to library packages (e.g. using dlopen/dlsym to 'acquire' some
library package.) This is why ENGINE_init is possibly the safest named
symbol to test.
Bill
Re: cvs commit: httpd-2.0/modules/ssl ...
Posted by Geoff Thorpe <ge...@geoffthorpe.net>.
Hello,
On May 24, 2003 09:38 am, William A. Rowe, Jr. wrote:
> At 02:31 PM 5/19/2003, Geoff Thorpe wrote:
> >The support I coded up used autoconfisms for testing the various
> > things, AC_CHECK_HEADER(S), AC_CHECK_LIB, etc. The previous support
> > simply scanned hard-coded paths and did various shell-based
> > examinations to *find* header and lib paths. My reading at the time
> > of those
> >AC_CHECK_FUNCS macros in modules/ssl/config.m4 was that they existed
> >merely to provide some verification that what had been "found"
> > actually worked. If there was any other significance to them, it was
> > too subtle for me. :-)
>
> Well, in the SSLC case, HAVE_SSL_SET_STATE was required because
> you can't even renegotiate without toggling the state. I tried a
> pretty simple hack, replacing the set state code (which on OpenSSL is
> simply a bit tweak into what should be opaque data) with the
> SSL_renegotate() function across toolkits (with the patch applied to
> 2_0_BRANCH).
>
> That didn't work, as several of the perl-framework ssl/ tests started
> failing. Considering that SSL_set_state a source patch on top of the
> SSLC toolkit, and most SSLC users work from binary distributions, this
> was never clean in the first place. Since ApacheSSL uses the
> SSL_renegotiate() function, and it's support in both toolkits, this is
> the right direction, I just missed something important (which I will
> revisit as I find some cycles again.)
OK, I can't say anything of much usefulness about SSL-C as I have no
experience with it. What I would suggest is that clean SSL-C support in
Apache2 may be useful to many people, but there are none who benefit from
it quite so much as RSA themselves. IIRC, it was from RSA that the
original mod_ssl tweaks for SSL-C originated - so it begs the question;
are RSA involved in maintaining this code at all? It seems they should be
doing as much as possible to avoid bitrot in the SSL-C hooks, and they'd
be far better placed than anyone to make it happen (and test it)?
> In the HAVE_SSL_CERT_STORE, that's an add-in patch on any toolkit.
> because we have only a single SSL_CTX*, one can't toggle the CA within
> the per-dir context within a threaded server. With SSL_cert_store, one
> can then toggle the CA within each SSL*. So this too was not a 'noop.'
OK, yep - I see this now.
> The choice of checking HAVE_ENGINE_INIT over forcing the user to
> toggle the experminental-foo should be pretty obvious.
Sure, I only question how the presence of ENGINE support should be
detected - it's pretty clear that having experimental flags all over the
place defeats both autoconf and common sense.
> >Because that's what was happening for me when I was writing the
> > autoconf code to begin with. I had to start saving and restoring
> > variables so that I could mangle them while testing (eg. pulling in
> > the apr linker flags temporarily). Running further ssl tests outside
> > that save/restore macro block before the rest of configuration
> > process is complete seems more fragile to my eye [...]
>
> Yup - I can see the benefit. But it's fundamentally broken autoconf
> style if we leave invalid flags sitting around. Sure, we need to add
> -l references to libraries that don't yet exist. But I don't know that
> we want to do that before the autodetection is finished?
I totally agree here - as I said way back when I was hacking on this, the
ideal thing would be that those "low level" flags already be configured
before we go bumbling into the ssl configuration. However, my
self-imposed terms of reference were confined to the ssl macro as I don't
pretend to understand the whole configuration system (particularly w.r.t.
apr). You're right that this workaround may be ok inside the save/restore
block but that it results in incomplete flags being configured - and this
is what subsequent AC_CHECK_FUNC(SSL_**) checks can stub their toes on.
That this problem was already present in the previous ssl configuration
code made me feel less bad about not correcting it :-)
> In any case, we won't 'HAVE' ENGINE_init if we don't inject ssl libs,
> so I don't have any problem moving those detection macros into the
> 'protected' reconfiguration. That still leaves me with the question of
> how we can test on a platform like solaris, where OpenSSL needs
> libraries like -lsocket just to perform a test compile of
> ENGINE_init().
Well this works inside the protected configuration for the reason that it
pulls in the apr linker flags, whatever they may be (eg. -ldl -lsocket
-lnsl [etc]). But the better solution is to re-arrange the configuration
order so the dependencies are already satisfied.
> >I merely wanted to draw your attention to the changes I'd made before,
> >as the revert and changes you made did not show whether or not the
> >previous changes were on your radar.
>
> I was aware of them (I'm working in both trees) but no, I hadn't
> studied the earlier discussion/history. I'm proceeding to do so. But
> I'm not really keen on the operating philisophy 'the commit broke
> something, back it out!' I'm trying to work forward from the effort you
> and Madhu invested.
>
> So the simple answer is yes - we must support autoconf checks (but we
> can likely eliminate one of the existing ones) and your patch broke it.
> What is most disturbing is the fact that the committed code ignored
> the last paragraph of the following earlier thread;
>
> >Date: Wed, 12 Mar 2003 23:03:15 -0600
> >To: dev@httpd.apache.org
>
> From: "William A. Rowe, Jr." <wr...@rowe-clan.net>
>
> >Subject: RE: [PATCH] openssl configuration (v2)
> >
> >Madhu, I really like the gist of this patch.
> >
> >I'm trying to get rolling 2.0.45 out the door, so I haven't had time
> > to watch your conversation. I have my own toolkit patch for SSL-C
> > 2.3 that probably breaks earlier SSL-C flavors. This is why I
> > haven't had time to investigate/respond - spending too much time in
> > firefighting mode :-/ [...]
> >But we cannot quit testing for the SSL_set_state sort of functions (I
> > don't know if you did or not) because I for one enjoy forcing SSL-C
> > to do what it should have done in the first place :-) Please don't
> > clobber function detection in your final patch, please!
Well this certainly slipped through the cracks - apologies for my part, I
don't have SSL-C so I was largely flying blind in that respect.
> >> >I'd suggest that the ENGIEN test be inserted into
> >> >APACHE_CHECK_SSL_TOOLKIT macro,
> >
> >Cool. If nothing else, that'd probably keep things more self-contained
> > and with less potential for surprises.
>
> We can move the existing SSL-C tests that were removed back into the
> SSLC path of the config section, and the ENGINE_init into the OpenSSL
> branch of the code. (SSLC has an entirely different engine schema.)
>
> This solution still gives me grief though, when I try to build for
> static -lssl and -lcrypto on OpenSSL 0.9.7 on Solaris. The installed
> lib/pkgconfing/ file openssl.pc tells us exactly what libraries we
> should be adding, and even goes as far as to spell out the other
> goodies we are 'autodetecting'.
>
> Perhaps we should go ahead and use the pkgconfig if it exists, or fall
> back on our own autoconfig if it doesn't. Would that be a win?
Maybe, but I think elegance dictates that the first priority would be to
get the existing configuration steps ordered by dependency. The fact that
apache-2 compiles and links indicates that there are no missing linker
flags missing once the configuration has completed - it's only during
configuration itself that there are one or two moments when the state of
the flags can be left inconsistent. Later, if you then want to detect and
add some logic to pull flags out of openssl.pc (and/or any equivalent for
SSL-C), you could presumably use some macro to merge them in and catch
any necessary flags you might otherwise be missing.
> >Well the previous --with-ssl=... logic was broken which is why I ended
> > up hacking in this code to begin with. I'm fully aware of the need to
> > support building against arbitrary SSL installations (and/or
> > uninstalled build-trees). I'm not sure that AC_CHECK_FUNC() is more
> > resistant against inadvertently linking with unintended libraries
> > than AC_CHECK_HEADER() is with unintended headers.
>
> Huh? Of course it does, silly... consider the following, where ENGINE
> only exists in the system path, but we request to use the myssl flavor
> that does not support ENGINE...
>
> /usr/myssl/openssl/include/openssl/openssl.h
> /usr/myssl/openssl/lib/libssl.a {missing ENGINE_init}
>
> /usr/include/openssl/openssl.h
> /usr/include/openssl/engine.h
> /usr/lib/libssl.a {containing ENGINE_init}
>
> Now if you test HAVE_ENGINE_H, of course it will find engine.h.
> However, if you test compile for ENGINE_init, against -lssl, it will
> link against only the FIRST libssl and then try compiling for that
> function. It won't try binding to more than one of any given library!
Well yes, but as I pointed out - if a future version redefines ENGINE_init
as a backward compatible macro (or simply removes it, heaven forbid) then
you're still going to hurt, whether the autoconf test fails or not. In
this case, it seems you want AC_TRY_LINK. But that's perhaps a bit
pedantic of me. I'll drop this one, your call :-)
> Again, I think we are setting up some -l's just a little too early in
> our config scripts, but that's a bigger issue. For today I agree.
Yes, I think this is the basic problem. The saving/restoring tricks used
in the ssl config macro (and in loads of other typical autoconf usage) is
a valid approach for trying certain things out with the facitilty to
rollback. What one should *not* do is use such trickery to temporarily
correct a problem in the way the configuration process is organised, and
unfortunately that's the case here.
> >Well, if you want to stick with that, go ahead. You'll still need to
> > check for the ENGINE header anyway because you need to know whether
> > to include it or not. As I say, API compatibility doesn't require
> > that ENGINE_init() remains a function, only that it remains defined
> > in the headers. For the short term, you're safe one way or the other.
>
> As I say, on linux this worked. On solaris, we still need the extra
> libs. This is why I'm like to attack the pkgconfig method instead, when
> that openssl.pc datum file is available to us.
Well I agree openssl.pc may be useful down the line to make the whole
configuration more robust for the future. However the fact apache links
on solaris today (and on oodles of other platforms as well) means you're
not missing any linker flags, they're just not being assembled in the
right order. Pulling in openssl's linker dependencies in-place using the
current apr trickery, or doing the same thing via some external mechanism
like openssl.pc, is a band-aid on top of the real problem - namely that
openssl/ssl-c support should be configured after its dependencies rather
than before. The question is, is it actually *possible* to re-arrange
this given the current configuration system?
Cheers,
Geoff
--
Geoff Thorpe
geoff@geoffthorpe.net
http://www.geoffthorpe.net/
Re: cvs commit: httpd-2.0/modules/ssl ...
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 02:31 PM 5/19/2003, Geoff Thorpe wrote:
>Hey there,
>
>On May 19, 2003 01:01 pm, William A. Rowe, Jr. wrote:
>>
>> Then what exactly did we replace the checks for SSL_set_cert_store
>> and SSL_set_state with? Apparently we didn't, and the last refactoring
>> was most broken on that point.
>
>The support I coded up used autoconfisms for testing the various things,
>AC_CHECK_HEADER(S), AC_CHECK_LIB, etc. The previous support simply
>scanned hard-coded paths and did various shell-based examinations to
>*find* header and lib paths. My reading at the time of those
>AC_CHECK_FUNCS macros in modules/ssl/config.m4 was that they existed
>merely to provide some verification that what had been "found" actually
>worked. If there was any other significance to them, it was too subtle
>for me. :-)
Well, in the SSLC case, HAVE_SSL_SET_STATE was required because
you can't even renegotiate without toggling the state. I tried a pretty simple
hack, replacing the set state code (which on OpenSSL is simply a bit tweak
into what should be opaque data) with the SSL_renegotate() function across
toolkits (with the patch applied to 2_0_BRANCH).
That didn't work, as several of the perl-framework ssl/ tests started failing.
Considering that SSL_set_state a source patch on top of the SSLC toolkit,
and most SSLC users work from binary distributions, this was never clean
in the first place. Since ApacheSSL uses the SSL_renegotiate() function,
and it's support in both toolkits, this is the right direction, I just missed
something important (which I will revisit as I find some cycles again.)
In the HAVE_SSL_CERT_STORE, that's an add-in patch on any toolkit.
because we have only a single SSL_CTX*, one can't toggle the CA within
the per-dir context within a threaded server. With SSL_cert_store, one
can then toggle the CA within each SSL*. So this too was not a 'noop.'
The choice of checking HAVE_ENGINE_INIT over forcing the user to
toggle the experminental-foo should be pretty obvious.
>> >They actually risk to fail unnecessarily because of the
>> >issues with dependencies on apr - see the APACHE_CHECK_SSL_TOOLKIT
>> >implementation.
>>
>> How is that?
>
>Because that's what was happening for me when I was writing the autoconf
>code to begin with. I had to start saving and restoring variables so that
>I could mangle them while testing (eg. pulling in the apr linker flags
>temporarily). Running further ssl tests outside that save/restore macro
>block before the rest of configuration process is complete seems more
>fragile to my eye [...]
Yup - I can see the benefit. But it's fundamentally broken autoconf style
if we leave invalid flags sitting around. Sure, we need to add -l references
to libraries that don't yet exist. But I don't know that we want to do that
before the autodetection is finished?
In any case, we won't 'HAVE' ENGINE_init if we don't inject ssl libs, so
I don't have any problem moving those detection macros into the 'protected'
reconfiguration. That still leaves me with the question of how we can test
on a platform like solaris, where OpenSSL needs libraries like -lsocket just
to perform a test compile of ENGINE_init().
>I merely wanted to draw your attention to the changes I'd made before,
>as the revert and changes you made did not show whether or not the
>previous changes were on your radar.
I was aware of them (I'm working in both trees) but no, I hadn't studied the
earlier discussion/history. I'm proceeding to do so. But I'm not really keen
on the operating philisophy 'the commit broke something, back it out!' I'm
trying to work forward from the effort you and Madhu invested.
So the simple answer is yes - we must support autoconf checks (but we
can likely eliminate one of the existing ones) and your patch broke it.
What is most disturbing is the fact that the committed code ignored
the last paragraph of the following earlier thread;
>Date: Wed, 12 Mar 2003 23:03:15 -0600
>To: dev@httpd.apache.org
>From: "William A. Rowe, Jr." <wr...@rowe-clan.net>
>Subject: RE: [PATCH] openssl configuration (v2)
>
>Madhu, I really like the gist of this patch.
>
>I'm trying to get rolling 2.0.45 out the door, so I haven't had time to
>watch your conversation. I have my own toolkit patch for SSL-C 2.3
>that probably breaks earlier SSL-C flavors. This is why I haven't had
>time to investigate/respond - spending too much time in firefighting mode :-/
>[...]
>But we cannot quit testing for the SSL_set_state sort of functions (I don't
>know if you did or not) because I for one enjoy forcing SSL-C to do what
>it should have done in the first place :-) Please don't clobber function
>detection in your final patch, please!
Back to the present day,
>> >I'd suggest that the ENGIEN test be inserted into
>> >APACHE_CHECK_SSL_TOOLKIT macro,
>
>Cool. If nothing else, that'd probably keep things more self-contained and
>with less potential for surprises.
We can move the existing SSL-C tests that were removed back into the
SSLC path of the config section, and the ENGINE_init into the OpenSSL
branch of the code. (SSLC has an entirely different engine schema.)
This solution still gives me grief though, when I try to build for static -lssl
and -lcrypto on OpenSSL 0.9.7 on Solaris. The installed lib/pkgconfing/
file openssl.pc tells us exactly what libraries we should be adding, and
even goes as far as to spell out the other goodies we are 'autodetecting'.
Perhaps we should go ahead and use the pkgconfig if it exists, or fall
back on our own autoconfig if it doesn't. Would that be a win?
>> >and it would probably make more sense to
>> >test for the openssl/engine.h header rather than the ENGINE_init()
>> >function (eg. future versions might redefine ENGINE_init as a
>> >backwards-compatibility macro rather than an actual function, but that
>> >would fail the current autoconf test).
>>
>> Sorry, but that is a no-go. This has already come up on the curl list.
>>
>> The problem is that someone may have several flavors of SSL installed
>> on a given box, and that we may pick up another installed header rather
>> than the one *selected* by --with-ssl={/path}. This can't occur as
>> easily with tests against explicit -l{ib}s.
>
>Well the previous --with-ssl=... logic was broken which is why I ended up
>hacking in this code to begin with. I'm fully aware of the need to
>support building against arbitrary SSL installations (and/or uninstalled
>build-trees). I'm not sure that AC_CHECK_FUNC() is more resistant against
>inadvertently linking with unintended libraries than AC_CHECK_HEADER() is
>with unintended headers.
Huh? Of course it does, silly... consider the following, where ENGINE only
exists in the system path, but we request to use the myssl flavor that does
not support ENGINE...
/usr/myssl/openssl/include/openssl/openssl.h
/usr/myssl/openssl/lib/libssl.a {missing ENGINE_init}
/usr/include/openssl/openssl.h
/usr/include/openssl/engine.h
/usr/lib/libssl.a {containing ENGINE_init}
Now if you test HAVE_ENGINE_H, of course it will find engine.h. However,
if you test compile for ENGINE_init, against -lssl, it will link against only
the FIRST libssl and then try compiling for that function. It won't try binding
to more than one of any given library!
>Anyway, like I say - I'm just highlighting a
>couple of things in case they'd previously gone unnoticed or
>unconsidered, the rest is your call.
>
>> Because we add the proper -L{sslpath} and -l{libs} at the end of
>> APACHE_CHECK_SSL_TOOLKIT()'s invocation, we certainly better
>> be safe using AC_CHECK_FUNC() or our entire build is broken.
>
>Well, the problem I was observing is that it's fine that "-lssl -lcrypto"
>has been added with any appropriate "-L<path>"s, but that doesn't mean
>the linker can resolve. If the subsequent system crud required to resolve
>is later brought in from apr or wherever, then things will just work at
>compile-time, but it doesn't mean that they'll work outside
>APACHE_CHECK_SSL_TOOLKIT but before the rest of configuration is
>complete. This is why there is saving and restoring of LIBS in the macro,
>it allows me to temporarily pull in the apr linker flags (-ldl -lsocket
>...) so things resolve.
Again, I think we are setting up some -l's just a little too early in our config
scripts, but that's a bigger issue. For today I agree.
>> Just for your amusement, we choose ENGINE_init instead of some
>> ENGINE_load_all_engines function because the other modules within
>> engine support require -ldl or -ldld or whatnot to also link.
>> ENGINE_init was a far simpler function to test.
>
>Well, if you want to stick with that, go ahead. You'll still need to check
>for the ENGINE header anyway because you need to know whether to include
>it or not. As I say, API compatibility doesn't require that ENGINE_init()
>remains a function, only that it remains defined in the headers. For the
>short term, you're safe one way or the other.
As I say, on linux this worked. On solaris, we still need the extra libs.
This is why I'm like to attack the pkgconfig method instead, when that
openssl.pc datum file is available to us.
Bill
Re: cvs commit: httpd-2.0/modules/ssl ...
Posted by Geoff Thorpe <ge...@geoffthorpe.net>.
Hey there,
On May 19, 2003 01:01 pm, William A. Rowe, Jr. wrote:
> >The AC_CHECK_FUNCS stuff is not really useful here because of the
> >overhauls made in acinclude.h (which was why they were removed in the
> >first place).
>
> Then what exactly did we replace the checks for SSL_set_cert_store
> and SSL_set_state with? Apparently we didn't, and the last refactoring
> was most broken on that point.
The support I coded up used autoconfisms for testing the various things,
AC_CHECK_HEADER(S), AC_CHECK_LIB, etc. The previous support simply
scanned hard-coded paths and did various shell-based examinations to
*find* header and lib paths. My reading at the time of those
AC_CHECK_FUNCS macros in modules/ssl/config.m4 was that they existed
merely to provide some verification that what had been "found" actually
worked. If there was any other significance to them, it was too subtle
for me. :-)
> >They actually risk to fail unnecessarily because of the
> >issues with dependencies on apr - see the APACHE_CHECK_SSL_TOOLKIT
> >implementation.
>
> How is that?
Because that's what was happening for me when I was writing the autoconf
code to begin with. I had to start saving and restoring variables so that
I could mangle them while testing (eg. pulling in the apr linker flags
temporarily). Running further ssl tests outside that save/restore macro
block before the rest of configuration process is complete seems more
fragile to my eye - but your experience with this build system is more
extensive than mine so I'll defer. I merely wanted to draw your attention
to the changes I'd made before, as the revert and changes you made did
not show whether or not the previous changes were on your radar.
> >I'd suggest that the ENGIEN test be inserted into
> >APACHE_CHECK_SSL_TOOLKIT macro,
>
> That could be done...
Cool. If nothing else, that'd probably keep things more self-contained and
with less potential for surprises.
> >and it would probably make more sense to
> >test for the openssl/engine.h header rather than the ENGINE_init()
> >function (eg. future versions might redefine ENGINE_init as a
> >backwards-compatibility macro rather than an actual function, but that
> >would fail the current autoconf test).
>
> Sorry, but that is a no-go. This has already come up on the curl list.
>
> The problem is that someone may have several flavors of SSL installed
> on a given box, and that we may pick up another installed header rather
> than the one *selected* by --with-ssl={/path}. This can't occur as
> easily with tests against explicit -l{ib}s.
Well the previous --with-ssl=... logic was broken which is why I ended up
hacking in this code to begin with. I'm fully aware of the need to
support building against arbitrary SSL installations (and/or uninstalled
build-trees). I'm not sure that AC_CHECK_FUNC() is more resistant against
inadvertently linking with unintended libraries than AC_CHECK_HEADER() is
with unintended headers. Anyway, like I say - I'm just highlighting a
couple of things in case they'd previously gone unnoticed or
unconsidered, the rest is your call.
> Because we add the proper -L{sslpath} and -l{libs} at the end of
> APACHE_CHECK_SSL_TOOLKIT()'s invocation, we certainly better
> be safe using AC_CHECK_FUNC() or our entire build is broken.
Well, the problem I was observing is that it's fine that "-lssl -lcrypto"
has been added with any appropriate "-L<path>"s, but that doesn't mean
the linker can resolve. If the subsequent system crud required to resolve
is later brought in from apr or wherever, then things will just work at
compile-time, but it doesn't mean that they'll work outside
APACHE_CHECK_SSL_TOOLKIT but before the rest of configuration is
complete. This is why there is saving and restoring of LIBS in the macro,
it allows me to temporarily pull in the apr linker flags (-ldl -lsocket
...) so things resolve.
> Just for your amusement, we choose ENGINE_init instead of some
> ENGINE_load_all_engines function because the other modules within
> engine support require -ldl or -ldld or whatnot to also link.
> ENGINE_init was a far simpler function to test.
Well, if you want to stick with that, go ahead. You'll still need to check
for the ENGINE header anyway because you need to know whether to include
it or not. As I say, API compatibility doesn't require that ENGINE_init()
remains a function, only that it remains defined in the headers. For the
short term, you're safe one way or the other.
> Certainly an alternative, but not a really effective one due to the
> points I mentioned above. This issue already came up with curl, which
> used the header trick before it broke enough users.
I'm rattling this off in a hurry before I leave, so I haven't had a chance
to examine the issue you mentioned about curl - I'll try to do so later.
However, I think "getting the wrong lib" is just as easy as "getting the
wrong header", and expecting ENGINE_init() to be a function may be an
unnecessary requirement. One thing that occurs to me from your comments
is that a "Good Idea" would be to try to compare version information from
the output of a linked executable with that found in the headers. This is
autoconfery beyond my means however, though I'd try to help anyone who
was better versed in M4.
> If an SSL toolkit comes up with an alternate function to ENGINE_init
> we can deal with it then. Note that I'm not against also using
> HAVE_OPENSSL_ENGINE_H (which is the clean way of doing
> what you attempted above),
<doh> yes indeed.
> but we need to check ENGINE_init
> to avoid the situation where crufty headers are hanging out
> in /usr/local/include.
but crufty libs hang out in /usr/lib too ...
> The biggest problem with dropping SSL_set_state, SSL_set_cert_store,
> and now ENGINE_init is that all three may be introduced by tossing
> in the appropriate patches to openssl or sslc. That is, we could be
> looking at either 0.9.6-engine or a vanilla flavor.
Absolutely, I'm all for autoconf tests for these things if required, but
in the right place. However the version checks for openssl (at least) are
currently at a point where the SSL ones *do* exist and the ENGINE_init()
is 1-1 with the presence of openssl/engine.h. I can't speak for SSLC
though.
> If we have really broken things to the point where AutoConf cannot
> use it's simplest macros, then we have a bigger issue to examine.
I had to pull in apr flags temporarily and restore LIBS later to do
precisely what you're talking about. I wouldn't say that meant things
were broken, but I think it's a good argument for putting any further
ssl-related tests in the same save/restore block so they don't fall prey
to the same problems (eg. if ENGINE_init() does exist but won't link
because of -ldl, it'll fail outside the save/restore block but succeed
inside it).
Cheers,
Geoff
--
Geoff Thorpe
geoff@geoffthorpe.net
http://www.geoffthorpe.net/
Re: cvs commit: httpd-2.0/modules/ssl ...
Posted by "William A. Rowe, Jr." <wr...@apache.org>.
At 10:31 AM 5/19/2003, you wrote:
>> --- config.m4 19 May 2003 14:43:20 -0000 1.13
>> +++ config.m4 19 May 2003 14:48:47 -0000 1.14
>> + AC_CHECK_FUNCS(ENGINE_init)
>
>The AC_CHECK_FUNCS stuff is not really useful here because of the
>overhauls made in acinclude.h (which was why they were removed in the
>first place).
Then what exactly did we replace the checks for SSL_set_cert_store
and SSL_set_state with? Apparently we didn't, and the last refactoring
was most broken on that point.
>They actually risk to fail unnecessarily because of the
>issues with dependencies on apr - see the APACHE_CHECK_SSL_TOOLKIT
>implementation.
How is that?
>I'd suggest that the ENGIEN test be inserted into
>APACHE_CHECK_SSL_TOOLKIT macro,
That could be done...
>and it would probably make more sense to
>test for the openssl/engine.h header rather than the ENGINE_init()
>function (eg. future versions might redefine ENGINE_init as a
>backwards-compatibility macro rather than an actual function, but that
>would fail the current autoconf test).
Sorry, but that is a no-go. This has already come up on the curl list.
The problem is that someone may have several flavors of SSL installed
on a given box, and that we may pick up another installed header rather
than the one *selected* by --with-ssl={/path}. This can't occur as easily
with tests against explicit -l{ib}s.
Because we add the proper -L{sslpath} and -l{libs} at the end of
APACHE_CHECK_SSL_TOOLKIT()'s invocation, we certainly better
be safe using AC_CHECK_FUNC() or our entire build is broken.
Just for your amusement, we choose ENGINE_init instead of some
ENGINE_load_all_engines function because the other modules within
engine support require -ldl or -ldld or whatnot to also link. ENGINE_init
was a far simpler function to test.
>As for APACHE_CHECK_SSL_TOOLKIT,
>I'm glancing at viewcvs as I type this and I'd suggest putting the test
>somewhere in the openssl-part of the section marked "dnl Run header and
>version checks", just before the version check. Eg. (this is not diff -u,
>it's me improvising and avoiding line-breaks ...);
>
><untested>
> ....
> AC_CHECK_HEADERS([openssl/opensslv.h openssl/ssl.h],....
> 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_CHECK_HEADERS([openssl/engine.h],
>+ [AC_DEFINE(HAVE_ENGINE)], [])
> AC_MSG_CHECKING(for OpenSSL version)
> AC_TRY_COMPILE([#include <openssl/opensslv.h>]
> ....
></untested>
Certainly an alternative, but not a really effective one due to the points
I mentioned above. This issue already came up with curl, which used
the header trick before it broke enough users.
If an SSL toolkit comes up with an alternate function to ENGINE_init
we can deal with it then. Note that I'm not against also using
HAVE_OPENSSL_ENGINE_H (which is the clean way of doing
what you attempted above), but we need to check ENGINE_init
to avoid the situation where crufty headers are hanging out
in /usr/local/include.
The biggest problem with dropping SSL_set_state, SSL_set_cert_store,
and now ENGINE_init is that all three may be introduced by tossing
in the appropriate patches to openssl or sslc. That is, we could be
looking at either 0.9.6-engine or a vanilla flavor.
If we have really broken things to the point where AutoConf cannot
use it's simplest macros, then we have a bigger issue to examine.
Bill
Re: cvs commit: httpd-2.0/modules/ssl ...
Posted by Geoff Thorpe <ge...@geoffthorpe.net>.
Hi there,
This and the previous commit caught my attention;
On May 19, 2003 10:48 am, wrowe@apache.org wrote:
> wrowe 2003/05/19 07:48:47
>
> Modified: modules/ssl config.m4 mod_ssl.c mod_ssl.h
> ssl_engine_config.c ssl_engine_init.c
> ssl_toolkit_compat.h
> Log:
> Roll away the SSL_EXPERIMENTAL_ENGINE test in favor of testing for
> the ENGINE_init() function in config.m4, and rely on HAVE_ENGINE_INIT
> instead.
>
> Reviewed by: Ben Laurie (concept)
>
> Revision Changes Path
> 1.14 +1 -0 httpd-2.0/modules/ssl/config.m4
>
> Index: config.m4
> ===================================================================
> RCS file: /home/cvs/httpd-2.0/modules/ssl/config.m4,v
> retrieving revision 1.13
> retrieving revision 1.14
> diff -u -r1.13 -r1.14
> --- config.m4 19 May 2003 14:43:20 -0000 1.13
> +++ config.m4 19 May 2003 14:48:47 -0000 1.14
> @@ -79,6 +79,7 @@
> APACHE_CHECK_SSL_TOOLKIT
> AC_CHECK_FUNCS(SSL_set_state)
> AC_CHECK_FUNCS(SSL_set_cert_store)
> + AC_CHECK_FUNCS(ENGINE_init)
> ])
>
> dnl # end of module specific part
The AC_CHECK_FUNCS stuff is not really useful here because of the
overhauls made in acinclude.h (which was why they were removed in the
first place). They actually risk to fail unnecessarily because of the
issues with dependencies on apr - see the APACHE_CHECK_SSL_TOOLKIT
implementation. I'd suggest that the ENGIEN test be inserted into
APACHE_CHECK_SSL_TOOLKIT macro, and it would probably make more sense to
test for the openssl/engine.h header rather than the ENGINE_init()
function (eg. future versions might redefine ENGINE_init as a
backwards-compatibility macro rather than an actual function, but that
would fail the current autoconf test). As for APACHE_CHECK_SSL_TOOLKIT,
I'm glancing at viewcvs as I type this and I'd suggest putting the test
somewhere in the openssl-part of the section marked "dnl Run header and
version checks", just before the version check. Eg. (this is not diff -u,
it's me improvising and avoiding line-breaks ...);
<untested>
....
AC_CHECK_HEADERS([openssl/opensslv.h openssl/ssl.h],....
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_CHECK_HEADERS([openssl/engine.h],
+ [AC_DEFINE(HAVE_ENGINE)], [])
AC_MSG_CHECKING(for OpenSSL version)
AC_TRY_COMPILE([#include <openssl/opensslv.h>]
....
</untested>
Cheers,
Geoff
--
Geoff Thorpe
geoff@geoffthorpe.net
http://www.geoffthorpe.net/