On Mon, May 24, 2010, Sander Temme wrote:
>
> On May 21, 2010, at 5:58 PM, Dr. Stephen Henson wrote:
>
> > On Fri, May 21, 2010, Sander Temme wrote:
> >
> <..>
> >> What would be best?
> >>
> >
> > Unfortunately there is no way to do this with the existing ex_data API and
> > we'd rather avoid extending APIs in the stable branches if possible.
> >
> > My suggestion would be to follow the same route I did with the compression
> > ex_data: avoid the use of the ex_data free handler entirely and free up the
> > ex_data pointer elsewhere.
> >
> > Since this is an RSA structure you can add a "finish" handler to contain the
> > necessary functionality.
>
> The following fixes the problem for me on Red Hat:
>
> Index: engines/e_chil.c
> ===================================================================
> RCS file: /home/openssl/cvs/openssl/engines/e_chil.c,v
> retrieving revision 1.12
> diff -u -r1.12 e_chil.c
> --- engines/e_chil.c 24 Mar 2010 23:42:05 -0000 1.12
> +++ engines/e_chil.c 24 May 2010 22:24:28 -0000
> @@ -138,6 +138,7 @@
> #ifndef OPENSSL_NO_RSA
> static void hwcrhk_ex_free(void *obj, void *item, CRYPTO_EX_DATA *ad,
> int ind,long argl, void *argp);
> +static int hwcrhk_rsa_finish(RSA *rsa);
> #endif
>
> /* Interaction stuff */
> @@ -193,7 +194,7 @@
> hwcrhk_rsa_mod_exp,
> hwcrhk_mod_exp_mont,
> NULL,
> - NULL,
> + hwcrhk_rsa_finish,
> 0,
> NULL,
> NULL,
> @@ -602,7 +603,7 @@
> if (hndidx_rsa == -1)
> hndidx_rsa = RSA_get_ex_new_index(0,
> "nFast HWCryptoHook RSA key handle",
> - NULL, NULL, hwcrhk_ex_free);
> + NULL, NULL, NULL);
> #endif
> return 1;
> err:
> @@ -1162,6 +1163,36 @@
> }
> #endif
> }
> +
> +/*
> + * Cleanup function for RSA structures. This is a wrapper function
> + * around hwcrhk_ex_free, which was supposed to be registered as a
> + * free_func for the ex_data entry attached to RSA instances. Since
> + * the free_func is not robust when the ENGINE gets loaded multiple
> + * times, call it instead in a finish handler for RSA structures.
> + */
> +
> +int hwcrhk_rsa_finish(RSA *rsa)
> +{
> + /* The intention is that this is our index on the stack of
> + * CRYPTO_EX_DATA_FUNCS. This value is not used by
> + * hwcrhk_ex_free()
> + */
> + int index = 0;
> + void *item;
> + CRYPTO_EX_DATA *ad;
> +
> + /* Retrieve the ex_data data. This value is used by
> + * hwcrhk_ex_free() to unload the loaded key from the HSM(s)
> + */
> + ad = &rsa->ex_data;
> + item = CRYPTO_get_ex_data(ad, index);
> +
> + hwcrhk_ex_free(rsa, item, ad, 0, 0,
> + "nFast HWCryptoHook RSA key handle");
> +
> + return 1;
> +}
> #endif
>
> /* Mutex calls: since the HWCryptoHook model closely follows the POSIX model
>
>
>
> Attached patch with CHANGES entry against HEAD. Save for the CHANGES entry,
> this applies cleanly against 1.0.0 Stable and with a little fuzz against
> 0.9.8 Stable.
>
> Tested on that same CentOS 5.4 system with HEAD and 0.9.8-Stable HEAD against
> Apache HEAD (prefork).
>
>
I've committed a fix for this now. I took the opportunity to get rid of the
ex_data free function at the same time. Let me know if that works OK. Either
check the next snapshot or apply this patch:
http://cvs.openssl.org/chngview?cn=19661
Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [email protected]
Automated List Manager [email protected]
