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]

Reply via email to