Dear OpenSSL development community,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=revThis 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.ccvs 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_chil_dynlock_fallback.patch
Description: Binary data
openssl_chil_nolocks_donotbreak.patch
Description: Binary data
smime.p7s
Description: S/MIME cryptographic signature
