As you know, 90% of us are very happy to run a rock solid SSL solution with
mod_ssl but there were 10% of us which still see segfaults from time to time
or even on a regular basis. And you also know that for months I'm searching
for the origin of these segfaults, but because I never could reproduce them I
never was able to fix it. Recently we at least discovered that it happens
inside OpenSSL by referencing some RSA structures. But until now we never
figured out the reason why those structures are messed up. Especially, as I
said, because I'm still unable to reproduce the problem myself.
But today David Kerry <[EMAIL PROTECTED]> mailed me in a private mail the
essential hint, I think. In short, he said "What I discovered was, it is these
temporary keys that contain the bad rsa->meth values.". And then after I went
over the source related to the temporary keys the last hours, it was obvious.
We stored the RSA temp keys and DH params over restarts as RSA* and DH*
structures (i.e. OpenSSL structures) instead of raw (and this way safe)
ASN.1/DER data. And because OpenSSL uses some static internal stuff which is
interlinked to these structures (the rsa->meth stuff!), although after a
restart the RSA structure itself survived, the pointers inside it then pointed
into nirvana - AT LEAST ON SOME PLATFORM. Most platforms map the stuff to the
same address on restarts, so the segfaults never occured for us. And even on
platforms were it can occur, it only occured if 40bit browsers reached the
website, of course (because for them the temp keys are mainly needed).
Puhhhh... I'm 100% sure that this subtle detail is the root of all evil. So
I've today completely rewritten the RSA temp key and DH param handling for us
and it now should avoid this problem. The remaining problem just is, that
for me it still works as good as before ;) Because I never can reproduce the
old problem. SO IT IS NOW YOUR TURN! Those of you who still receive
segfaults (and which seemed not be related to the session cache!) should give
the appended patch a try. I'm sure it will fix the problem plus minus the bugs
I've introduced with the patch itself. It's a patch against 2.4.7.
Please give me feedback.
Thanks.
Ralf S. Engelschall
[EMAIL PROTECTED]
www.engelschall.com
Index: mod_ssl.h
===================================================================
RCS file: /e/modssl/cvs/mod_ssl/pkg.apache/src/modules/ssl/mod_ssl.h,v
retrieving revision 1.117
diff -u -r1.117 mod_ssl.h
--- mod_ssl.h 1999/10/22 19:37:33 1.117
+++ mod_ssl.h 1999/11/03 15:28:29
@@ -383,6 +383,20 @@
#define SSL_AIDX_MAX (2)
/*
+ * Define IDs for the temporary RSA keys and DH params
+ */
+
+#define SSL_TKP_GEN (0)
+#define SSL_TKP_ALLOC (1)
+#define SSL_TKP_FREE (2)
+
+#define SSL_TKPIDX_RSA512 (0)
+#define SSL_TKPIDX_RSA1024 (1)
+#define SSL_TKPIDX_DH512 (2)
+#define SSL_TKPIDX_DH1024 (3)
+#define SSL_TKPIDX_MAX (4)
+
+/*
* Define the SSL options
*/
#define SSL_OPT_NONE (0)
@@ -506,10 +520,6 @@
pool *pPool;
BOOL bFixed;
int nInitCount;
- RSA *pRSATmpKey512;
- RSA *pRSATmpKey1024;
- DH *pDHTmpParam512;
- DH *pDHTmpParam1024;
int nSessionCacheMode;
char *szSessionCacheDataFile;
int nSessionCacheDataSize;
@@ -520,6 +530,8 @@
int nMutexFD;
int nMutexSEMID;
array_header *aRandSeed;
+ ssl_ds_table *tTmpKeys;
+ void *pTmpKeys[SSL_TKPIDX_MAX];
ssl_ds_table *tPublicCert;
ssl_ds_table *tPrivateKey;
struct {
@@ -627,6 +639,7 @@
/* module initialization */
void ssl_init_Module(server_rec *, pool *);
void ssl_init_SSLLibrary(void);
+void ssl_init_TmpKeysHandle(int, server_rec *, pool *);
void ssl_init_ConfigureServer(server_rec *, pool *, SSLSrvConfigRec *);
void ssl_init_CheckServers(server_rec *, pool *);
STACK_OF(X509_NAME)
Index: ssl_engine_config.c
===================================================================
RCS file: /e/modssl/cvs/mod_ssl/pkg.apache/src/modules/ssl/ssl_engine_config.c,v
retrieving revision 1.61
diff -u -r1.61 ssl_engine_config.c
--- ssl_engine_config.c 1999/08/25 07:03:48 1.61
+++ ssl_engine_config.c 1999/11/03 15:28:32
@@ -127,10 +127,6 @@
* initialize per-module configuration
*/
mc->nInitCount = 0;
- mc->pRSATmpKey512 = NULL;
- mc->pRSATmpKey1024 = NULL;
- mc->pDHTmpParam512 = NULL;
- mc->pDHTmpParam1024 = NULL;
mc->nSessionCacheMode = SSL_SCMODE_UNSET;
mc->szSessionCacheDataFile = NULL;
mc->nSessionCacheDataSize = 0;
@@ -143,6 +139,9 @@
mc->aRandSeed = ap_make_array(pPool, 4, sizeof(ssl_randseed_t));
mc->tPrivateKey = ssl_ds_table_make(pPool, sizeof(ssl_asn1_t));
mc->tPublicCert = ssl_ds_table_make(pPool, sizeof(ssl_asn1_t));
+ mc->tTmpKeys = ssl_ds_table_make(pPool, sizeof(ssl_asn1_t));
+
+ (void)memset(mc->pTmpKeys, 0, SSL_TKPIDX_MAX*sizeof(void *));
#ifdef SSL_VENDOR
mc->ctx = ap_ctx_new(pPool);
Index: ssl_engine_init.c
===================================================================
RCS file: /e/modssl/cvs/mod_ssl/pkg.apache/src/modules/ssl/ssl_engine_init.c,v
retrieving revision 1.88
diff -u -r1.88 ssl_engine_init.c
--- ssl_engine_init.c 1999/11/01 08:34:53 1.88
+++ ssl_engine_init.c 1999/11/03 15:32:09
@@ -234,6 +234,7 @@
#endif
if (mc->nInitCount == 1) {
ssl_pphrase_Handle(s, p);
+ ssl_init_TmpKeysHandle(SSL_TKP_GEN, s, p);
#ifndef WIN32
return;
#endif
@@ -262,38 +263,9 @@
ssl_log(s, SSL_LOG_INFO, "Init: Seeding PRNG with %d bytes of entropy", n);
/*
- * pre-generate the temporary RSA keys
+ * pre-generate the temporary RSA keys and DH params
*/
- if (mc->pRSATmpKey512 == NULL) {
- ssl_log(s, SSL_LOG_INFO, "Init: Generating temporary RSA private keys
(512/1024 bits)");
- mc->pRSATmpKey512 = RSA_generate_key(512, RSA_F4, NULL, NULL);
- if (mc->pRSATmpKey512 == NULL) {
- ssl_log(s, SSL_LOG_ERROR, "Init: Failed to generate temporary 512 bit RSA
private key");
- ssl_die();
- }
- mc->pRSATmpKey1024 = RSA_generate_key(1024, RSA_F4, NULL, NULL);
- if (mc->pRSATmpKey1024 == NULL) {
- ssl_log(s, SSL_LOG_ERROR, "Init: Failed to generate temporary 1024 bit
RSA private key");
- ssl_die();
- }
- }
-
- /*
- * pre-configure the temporary DH parameters
- */
- if (mc->pDHTmpParam512 == NULL) {
- ssl_log(s, SSL_LOG_INFO, "Init: Configuring temporary DH parameters (512/1024
bits)");
- mc->pDHTmpParam512 = ssl_dh_GetTmpParam(512);
- if (mc->pDHTmpParam512 == NULL) {
- ssl_log(s, SSL_LOG_ERROR, "Init: Failed to configure temporary 512 bit DH
parameters");
- ssl_die();
- }
- mc->pDHTmpParam1024 = ssl_dh_GetTmpParam(1024);
- if (mc->pDHTmpParam1024 == NULL) {
- ssl_log(s, SSL_LOG_ERROR, "Init: Failed to configure temporary 1024 bit
DH parameters");
- ssl_die();
- }
- }
+ ssl_init_TmpKeysHandle(SSL_TKP_ALLOC, s, p);
/*
* initialize servers
@@ -351,6 +323,137 @@
}
/*
+ * Handle the Temporary RSA Keys and DH Params
+ */
+void ssl_init_TmpKeysHandle(int action, server_rec *s, pool *p)
+{
+ SSLModConfigRec *mc = myModConfig();
+ ssl_asn1_t *asn1;
+ unsigned char *ucp;
+ RSA *rsa;
+ DH *dh;
+
+ /* Generate Keys and Params */
+ if (action == SSL_TKP_GEN) {
+ ssl_log(s, SSL_LOG_INFO, "Init: Generating temporary RSA private keys
+(512/1024 bits)");
+
+ /* generate 512 bit RSA key */
+ if ((rsa = RSA_generate_key(512, RSA_F4, NULL, NULL)) == NULL) {
+ ssl_log(s, SSL_LOG_ERROR, "Init: Failed to generate temporary 512 bit RSA
+private key");
+ ssl_die();
+ }
+ asn1 = (ssl_asn1_t *)ssl_ds_table_push(mc->tTmpKeys, ap_pstrdup(mc->pPool,
+"RSA:512"));
+ asn1->nData = i2d_RSAPrivateKey(rsa, NULL);
+ asn1->cpData = ap_palloc(mc->pPool, asn1->nData);
+ ucp = asn1->cpData; i2d_RSAPrivateKey(rsa, &ucp); /* 2nd arg increments */
+ RSA_free(rsa);
+
+ /* generate 1024 bit RSA key */
+ if ((rsa = RSA_generate_key(1024, RSA_F4, NULL, NULL)) == NULL) {
+ ssl_log(s, SSL_LOG_ERROR, "Init: Failed to generate temporary 1024 bit
+RSA private key");
+ ssl_die();
+ }
+ asn1 = (ssl_asn1_t *)ssl_ds_table_push(mc->tTmpKeys, ap_pstrdup(mc->pPool,
+"RSA:1024"));
+ asn1->nData = i2d_RSAPrivateKey(rsa, NULL);
+ asn1->cpData = ap_palloc(mc->pPool, asn1->nData);
+ ucp = asn1->cpData; i2d_RSAPrivateKey(rsa, &ucp); /* 2nd arg increments */
+ RSA_free(rsa);
+
+ ssl_log(s, SSL_LOG_INFO, "Init: Configuring temporary DH parameters (512/1024
+bits)");
+
+ /* import 512 bit DH param */
+ if ((dh = ssl_dh_GetTmpParam(512)) == NULL) {
+ ssl_log(s, SSL_LOG_ERROR, "Init: Failed to import temporary 512 bit DH
+parameters");
+ ssl_die();
+ }
+ asn1 = (ssl_asn1_t *)ssl_ds_table_push(mc->tTmpKeys, ap_pstrdup(mc->pPool,
+"DH:512"));
+ asn1->nData = i2d_DHparams(dh, NULL);
+ asn1->cpData = ap_palloc(mc->pPool, asn1->nData);
+ ucp = asn1->cpData; i2d_DHparams(dh, &ucp); /* 2nd arg increments */
+ /* do not free dh, it's static */
+
+ /* import 1024 bit DH param */
+ if ((dh = ssl_dh_GetTmpParam(1024)) == NULL) {
+ ssl_log(s, SSL_LOG_ERROR, "Init: Failed to import temporary 1024 bit DH
+parameters");
+ ssl_die();
+ }
+ asn1 = (ssl_asn1_t *)ssl_ds_table_push(mc->tTmpKeys, ap_pstrdup(mc->pPool,
+"DH:1024"));
+ asn1->nData = i2d_DHparams(dh, NULL);
+ asn1->cpData = ap_palloc(mc->pPool, asn1->nData);
+ ucp = asn1->cpData; i2d_DHparams(dh, &ucp); /* 2nd arg increments */
+ /* do not free dh, it's static */
+ }
+
+ /* Allocate Keys and Params */
+ else if (action == SSL_TKP_ALLOC) {
+ ssl_log(s, SSL_LOG_INFO, "Init: Configuring temporary RSA private keys
+(512/1024 bits)");
+
+ /* allocate 512 bit RSA key */
+ if ((asn1 = (ssl_asn1_t *)ssl_ds_table_get(mc->tTmpKeys, "RSA:512")) != NULL)
+{
+ ucp = asn1->cpData;
+ if ((mc->pTmpKeys[SSL_TKPIDX_RSA512] =
+ d2i_RSAPrivateKey(NULL, &ucp, asn1->nData)) == NULL) {
+ ssl_log(s, SSL_LOG_ERROR, "Init: Failed to configure temporary 512
+bit RSA private key");
+ ssl_die();
+ }
+ }
+
+ /* allocate 1024 bit RSA key */
+ if ((asn1 = (ssl_asn1_t *)ssl_ds_table_get(mc->tTmpKeys, "RSA:1024")) !=
+NULL) {
+ ucp = asn1->cpData;
+ if ((mc->pTmpKeys[SSL_TKPIDX_RSA1024] =
+ d2i_RSAPrivateKey(NULL, &ucp, asn1->nData)) == NULL) {
+ ssl_log(s, SSL_LOG_ERROR, "Init: Failed to configure temporary 1024
+bit RSA private key");
+ ssl_die();
+ }
+ }
+
+ ssl_log(s, SSL_LOG_INFO, "Init: Configuring temporary DH parameters (512/1024
+bits)");
+
+ /* allocate 512 bit DH param */
+ if ((asn1 = (ssl_asn1_t *)ssl_ds_table_get(mc->tTmpKeys, "DH:512")) != NULL) {
+ ucp = asn1->cpData;
+ if ((mc->pTmpKeys[SSL_TKPIDX_DH512] =
+ d2i_DHparams(NULL, &ucp, asn1->nData)) == NULL) {
+ ssl_log(s, SSL_LOG_ERROR, "Init: Failed to configure temporary 512
+bit DH parameters");
+ ssl_die();
+ }
+ }
+
+ /* allocate 1024 bit DH param */
+ if ((asn1 = (ssl_asn1_t *)ssl_ds_table_get(mc->tTmpKeys, "DH:1024")) != NULL)
+{
+ ucp = asn1->cpData;
+ if ((mc->pTmpKeys[SSL_TKPIDX_DH1024] =
+ d2i_DHparams(NULL, &ucp, asn1->nData)) == NULL) {
+ ssl_log(s, SSL_LOG_ERROR, "Init: Failed to configure temporary 1024
+bit DH parameters");
+ ssl_die();
+ }
+ }
+ }
+
+ /* Free Keys and Params */
+ else if (action == SSL_TKP_FREE) {
+ if (mc->pTmpKeys[SSL_TKPIDX_RSA512] != NULL) {
+ RSA_free((RSA *)mc->pTmpKeys[SSL_TKPIDX_RSA512]);
+ mc->pTmpKeys[SSL_TKPIDX_RSA512] = NULL;
+ }
+ if (mc->pTmpKeys[SSL_TKPIDX_RSA1024] != NULL) {
+ RSA_free((RSA *)mc->pTmpKeys[SSL_TKPIDX_RSA1024]);
+ mc->pTmpKeys[SSL_TKPIDX_RSA1024] = NULL;
+ }
+ if (mc->pTmpKeys[SSL_TKPIDX_DH512] != NULL) {
+ DH_free((DH *)mc->pTmpKeys[SSL_TKPIDX_DH512]);
+ mc->pTmpKeys[SSL_TKPIDX_DH512] = NULL;
+ }
+ if (mc->pTmpKeys[SSL_TKPIDX_DH1024] != NULL) {
+ DH_free((DH *)mc->pTmpKeys[SSL_TKPIDX_DH1024]);
+ mc->pTmpKeys[SSL_TKPIDX_DH1024] = NULL;
+ }
+ }
+ return;
+}
+
+/*
* Configure a particular server
*/
void ssl_init_ConfigureServer(server_rec *s, pool *p, SSLSrvConfigRec *sc)
@@ -860,6 +963,11 @@
*/
ssl_scache_kill(s);
ssl_mutex_kill(s);
+
+ /*
+ * Destroy the temporary keys and params
+ */
+ ssl_init_TmpKeysHandle(SSL_TKP_FREE, s, NULL);
/*
* Free the non-pool allocated structures
Index: ssl_engine_kernel.c
===================================================================
RCS file: /e/modssl/cvs/mod_ssl/pkg.apache/src/modules/ssl/ssl_engine_kernel.c,v
retrieving revision 1.110
diff -u -r1.110 ssl_engine_kernel.c
--- ssl_engine_kernel.c 1999/10/08 11:00:37 1.110
+++ ssl_engine_kernel.c 1999/11/03 15:28:38
@@ -1308,7 +1308,6 @@
*
* So we generated 512 and 1024 bit temporary keys on startup
* which we now just handle out on demand....
- *
*/
RSA *ssl_callback_TmpRSA(SSL *pSSL, int nExport, int nKeyLen)
{
@@ -1319,16 +1318,16 @@
if (nExport) {
/* It's because an export cipher is used */
if (nKeyLen == 512)
- rsa = mc->pRSATmpKey512;
+ rsa = (RSA *)mc->pTmpKeys[SSL_TKPIDX_RSA512];
else if (nKeyLen == 1024)
- rsa = mc->pRSATmpKey1024;
+ rsa = (RSA *)mc->pTmpKeys[SSL_TKPIDX_RSA1024];
else
/* it's too expensive to generate on-the-fly, so keep 1024bit */
- rsa = mc->pRSATmpKey1024;
+ rsa = (RSA *)mc->pTmpKeys[SSL_TKPIDX_RSA1024];
}
else {
/* It's because a sign-only certificate situation exists */
- rsa = mc->pRSATmpKey1024;
+ rsa = (RSA *)mc->pTmpKeys[SSL_TKPIDX_RSA1024];
}
return rsa;
}
@@ -1345,16 +1344,16 @@
if (nExport) {
/* It's because an export cipher is used */
if (nKeyLen == 512)
- dh = mc->pDHTmpParam512;
+ dh = (DH *)mc->pTmpKeys[SSL_TKPIDX_DH512];
else if (nKeyLen == 1024)
- dh = mc->pDHTmpParam1024;
+ dh = (DH *)mc->pTmpKeys[SSL_TKPIDX_DH1024];
else
/* it's too expensive to generate on-the-fly, so keep 1024bit */
- dh = mc->pDHTmpParam1024;
+ dh = (DH *)mc->pTmpKeys[SSL_TKPIDX_DH1024];
}
else {
/* It's because a sign-only certificate situation exists */
- dh = mc->pDHTmpParam1024;
+ dh = (DH *)mc->pTmpKeys[SSL_TKPIDX_DH1024];
}
return dh;
}
______________________________________________________________________
Apache Interface to OpenSSL (mod_ssl) www.modssl.org
User Support Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]
