Option 2 works iff pthreads exists. -Kyle H
On Thu, Aug 28, 2008 at 11:45 PM, Sander Temme via RT <[EMAIL PROTECTED]> wrote: > This affects OpenSSL 0.9.8 and trunk. > > In engines/e_chil.c around line 594, the engine checks if it has mutex > callbacks to work with and, if not, errors out with the message: > > "You HAVE to add dynamic locking callbacks via > CRYPTO_set_dynlock_{create,lock,destroy}_callback()" > > There used to be a static lock (CRYPTO_LOCK_HWCRHK, #39 if I'm not > mistaken), but that was removed in 0.9.8 so if the calling application > does not set up dynamic locking scaffolding as described in > threads(3), it will error out when trying to load the chil engine. > This happens whether the calling application is threaded or not. Of > course it could tell the engine to disable the mutex callbacks, but > for instance all utilities supplied with openssl neither set the > callbacks nor disable them. Neither does, for instance, Apache. > > This means that a lot of popular applications error out when trying to > run with the chil engine. > > I have this week committed a patch to the Apache HTTP Server that sets > up the dynamic locking callbacks: > > http://svn.apache.org/viewvc?rev=687550&view=rev > > This resolves the issue on the Apache side. I would like to also > resolve it also on the OpenSSL side, and see two approaches: > > 1) Simply do away with the error like so: > > --- openssl/engines/e_chil.c 2008-08-17 17:22:54.000000000 -0700 > +++ openssl-nolocks/engines/e_chil.c 2008-08-21 19:15:37.000000000 -0700 > @@ -588,12 +588,6 @@ > hwcrhk_globals.mutex_release = hwcrhk_mutex_unlock; > hwcrhk_globals.mutex_destroy = hwcrhk_mutex_destroy; > } > - else if (CRYPTO_get_locking_callback() != NULL) > - { > - > HWCRHKerr(HWCRHK_F_HWCRHK_INIT,HWCRHK_R_LOCKING_MISSING); > - ERR_add_error_data(1,"You HAVE to add dynamic locking > callbacks > via CRYPTO_set_dynlock_{create,lock,destroy}_callback()"); > - goto err; > - } > } > > /* Try and get a context - if not, we may have a DSO but no > > This means that non-threaded applications no longer have to set > callbacks they don't need, and that anyone who loads the engine from > multithreaded code, frankly, gets what they ask for in the way of > random crashes and excruciating pain. However, the engine does not > need to be pedantic about this in the former, common, single-threaded > case. > > 2) Have the engine provide its own callbacks that get set in case the > application does not provide (presumably more suitable) alternatives: > > [EMAIL PROTECTED]:~/projects/openssl$ cvs diff engines/e_chil.c > cvs server: /home/openssl/cvs/CVSROOT/config: unrecognized keyword > 'UseNewInfoFmtStrings' > Index: engines/e_chil.c > =================================================================== > RCS file: /home/openssl/cvs/openssl/engines/e_chil.c,v > retrieving revision 1.5 > diff -u -r1.5 e_chil.c > --- engines/e_chil.c 18 Mar 2006 14:22:20 -0000 1.5 > +++ engines/e_chil.c 22 Aug 2008 05:54:45 -0000 > @@ -59,6 +59,7 @@ > > #include <stdio.h> > #include <string.h> > +#include <pthread.h> > #include <openssl/crypto.h> > #include <openssl/pem.h> > #include <openssl/dso.h> > @@ -103,6 +104,9 @@ > static int hwcrhk_mutex_lock(HWCryptoHook_Mutex*); > static void hwcrhk_mutex_unlock(HWCryptoHook_Mutex*); > static void hwcrhk_mutex_destroy(HWCryptoHook_Mutex*); > +static struct CRYPTO_dynlock_value *hwcrhk_dyn_create_function(const > char *file, int line); > +static void hwcrhk_dyn_lock_function(int mode, struct > CRYPTO_dynlock_value *l, const char *file, int line); > +static void hwcrhk_dyn_destroy_function(struct CRYPTO_dynlock_value > *l, const char *file, int line); > > /* BIGNUM stuff */ > static int hwcrhk_mod_exp(BIGNUM *r, const BIGNUM *a, const BIGNUM *p, > @@ -247,6 +251,11 @@ > int lockid; > }; > > +struct CRYPTO_dynlock_value > +{ > + pthread_mutex_t mutex; > +}; > + > /* hwcryptohook.h has some typedefs that turn > struct HWCryptoHook_PassphraseContextValue > into HWCryptoHook_PassphraseContext */ > @@ -590,9 +599,12 @@ > } > else if (CRYPTO_get_locking_callback() != NULL) > { > - > HWCRHKerr(HWCRHK_F_HWCRHK_INIT,HWCRHK_R_LOCKING_MISSING); > - ERR_add_error_data(1,"You HAVE to add dynamic locking > callbacks > via CRYPTO_set_dynlock_{create,lock,destroy}_callback()"); > - goto err; > + /* Upcalls haven't been set by the > + * application, use our own as > + * fallback. */ > +/* > CRYPTO_set_dynlock_create_callback(hwcrhk_dyn_create_function); */ > +/* > CRYPTO_set_dynlock_lock_callback(hwcrhk_dyn_lock_function); > */ > +/* > CRYPTO_set_dynlock_destroy_callback(hwcrhk_dyn_destroy_function); */ > } > } > > @@ -1199,6 +1211,33 @@ > CRYPTO_destroy_dynlockid(mt->lockid); > } > > +static struct CRYPTO_dynlock_value *hwcrhk_dyn_create_function(const > char *file, int line) > +{ > + struct CRYPTO_dynlock_value *value; > + > + value = (struct CRYPTO_dynlock_value *) > OPENSSL_malloc(sizeof(struct CRYPTO_dynlock_value)); > + > + if (!value) > + return NULL; > + pthread_mutex_init(&(value->mutex), NULL); > + return value; > +} > + > +static void hwcrhk_dyn_lock_function(int mode, struct > CRYPTO_dynlock_value *l, const char *file, int line) > +{ > + if (mode & CRYPTO_LOCK) > + pthread_mutex_lock(&(l->mutex)); > + else > + pthread_mutex_unlock(&(l->mutex)); > +} > + > +static void hwcrhk_dyn_destroy_function(struct CRYPTO_dynlock_value *l, > + const char *file, int line) > +{ > + pthread_mutex_destroy(&(l->mutex)); > + OPENSSL_free(l); > +} > + > static int hwcrhk_get_pass(const char *prompt_info, > int *len_io, char *buf, > HWCryptoHook_PassphraseContext *ppctx, > > > Patches also attached since the line endings in the mail will break > them. This code is a stripped version out of the Seals & Sea Lions > book, so perhaps permission should be sought from its authors before > it gets included. > > Would it be appropriate to introduce a dependency on a particular > threading implementation, something the locking upcalls very elegantly > avoid? > > What do you think? > > S. > > PS. I work for nCipher and am a member of the Apache Software > Foundation, > but speak for neither organization. > > -- > [EMAIL PROTECTED] http://www.temme.net/sander/ > PGP FP: 51B4 8727 466A 0BC3 69F4 B7B8 B2BE BC40 1529 24AF > > > > > > ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]