Bug 622003

Description of problem:

The CHIL Engine (used to access Thales/nCipher cryptographic hardware) sets an ex_data table entry in OpenSSL, with a function pointer to the cleanup function located in the CHIL engine binary.

When a calling program unloads the Engine, the cleanup function pointer is not cleared.  When the calling program loads the Engine again, a second function pointer is added to the ex_data cleanup stack, leaving the first one in place but likely pointing to invalid memory.  This crashes the calling program as soon as it attempts to clean up the ex_data  entry.  


Version-Release number of selected component (if applicable):
 
Any OpenSSL 0.9.8 RPM.
Any OpenSSL version from openssl.org

How reproducible:

Apache 2.2.x, as supplied by Red Hat or downloaded from apache.org, does this double library load and triggers this issue any time the Engine library text does not land in the same memory location second time around, which is most of the time.

Steps to Reproduce:
1. Install Thales nCipher HSM
2. Install nCSS Software
3. Add /opt/nfast/toolkits/hwcrhk to /etc/ld.so.conf.d/nfast, run ldconfig -l (if I recall)
4. Edit /etc/httpd/conf/httpd.conf (if I recall) to add SSLCryptoDevice chil anywhere
5. /etc/init.d/httpd start.  
  
Actual results:

httpd server segfaults.

Expected results:

httpd server does not segfault.

Additional info:

This patch to the openssl upstream fixes the issue:

http://cvs.openssl.org/filediff?f=openssl/engines/e_chil.c&v1=1.1.2.9&v2=1.1.2.10

--- e_chil.c	2010/03/24 23:42:30	1.1.2.9
+++ e_chil.c	2010/05/26 16:16:49	1.1.2.10
@@ -111,11 +111,10 @@
 #ifndef OPENSSL_NO_RSA
 /* RSA stuff */
 static int hwcrhk_rsa_mod_exp(BIGNUM *r, const BIGNUM *I, RSA *rsa, BN_CTX *ctx);
-#endif
-#ifndef OPENSSL_NO_RSA
 /* This function is aliased to mod_exp (with the mont stuff dropped). */
 static int hwcrhk_mod_exp_mont(BIGNUM *r, const BIGNUM *a, const BIGNUM *p,
 		const BIGNUM *m, BN_CTX *ctx, BN_MONT_CTX *m_ctx);
+static int hwcrhk_rsa_finish(RSA *rsa);
 #endif
 
 #ifndef OPENSSL_NO_DH
@@ -135,10 +134,6 @@
 	UI_METHOD *ui_method, void *callback_data);
 static EVP_PKEY *hwcrhk_load_pubkey(ENGINE *eng, const char *key_id,
 	UI_METHOD *ui_method, void *callback_data);
-#ifndef OPENSSL_NO_RSA
-static void hwcrhk_ex_free(void *obj, void *item, CRYPTO_EX_DATA *ad,
-	int ind,long argl, void *argp);
-#endif
 
 /* Interaction stuff */
 static int hwcrhk_insert_card(const char *prompt_info,
@@ -193,7 +188,7 @@
 	hwcrhk_rsa_mod_exp,
 	hwcrhk_mod_exp_mont,
 	NULL,
-	NULL,
+	hwcrhk_rsa_finish,
 	0,
 	NULL,
 	NULL,
@@ -603,7 +598,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:
@@ -1081,6 +1076,21 @@
 	{
 	return hwcrhk_mod_exp(r, a, p, m, ctx);
 	}
+
+static int hwcrhk_rsa_finish(RSA *rsa)
+	{
+	HWCryptoHook_RSAKeyHandle *hptr;
+	int ret;
+	hptr = RSA_get_ex_data(rsa, hndidx_rsa);
+	if (hptr)
+                {
+                ret = p_hwcrhk_RSAUnloadKey(*hptr, NULL);
+                OPENSSL_free(hptr);
+		RSA_set_ex_data(rsa, hndidx_rsa, NULL);
+                }
+	return 1;
+	}
+
 #endif
 
 #ifndef OPENSSL_NO_DH
@@ -1139,34 +1149,6 @@
 	return 1;
 	}
 
-/* This cleans up an RSA KM key, called when ex_data is freed */
-#ifndef OPENSSL_NO_RSA
-static void hwcrhk_ex_free(void *obj, void *item, CRYPTO_EX_DATA *ad,
-	int ind,long argl, void *argp)
-{
-	char tempbuf[1024];
-	HWCryptoHook_ErrMsgBuf rmsg;
-#ifndef OPENSSL_NO_RSA
-	HWCryptoHook_RSAKeyHandle *hptr;
-#endif
-#if !defined(OPENSSL_NO_RSA)
-	int ret;
-#endif
-
-	rmsg.buf = tempbuf;
-	rmsg.size = sizeof(tempbuf);
-
-#ifndef OPENSSL_NO_RSA
-	hptr = (HWCryptoHook_RSAKeyHandle *) item;
-	if(hptr)
-                {
-                ret = p_hwcrhk_RSAUnloadKey(*hptr, NULL);
-                OPENSSL_free(hptr);
-                }
-#endif
-}
-#endif
-
 /* Mutex calls: since the HWCryptoHook model closely follows the POSIX model
  * these just wrap the POSIX functions and add some logging.
  */