mod_ssl was "leaking" on restart since mc->tTmpKeys table entries

were allocated using apr_palloc out of s->process->pool and pushed
into an apr_array_header_t.
solve the problem by moving from apr_array_header_t's to an apr_hash_t.
also add ssl_asn1_table_{set,unset} wrappers to use malloc/free so we
do not "leak" from s->process->pool.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@93612 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/modules/ssl/mod_ssl.h b/modules/ssl/mod_ssl.h
index 462693d8f4011b38e01331ef2c8cb6ef9975ae20..7871e8666525d1a7a3d2589ce56a53f2ec8066bb 100644 (file)
--- a/modules/ssl/mod_ssl.h
+++ b/modules/ssl/mod_ssl.h
@@ -516,7 +516,7 @@ typedef struct {
     apr_lock_t     *pMutex;
     apr_array_header_t   *aRandSeed;
     int             nScoreboardSize; /* used for builtin random seed */
-    ssl_ds_table   *tTmpKeys;
+    apr_hash_t     *tTmpKeys;
     void           *pTmpKeys[SSL_TKPIDX_MAX];
     ssl_ds_table   *tPublicCert;
     ssl_ds_table   *tPrivateKey;
@@ -741,6 +741,13 @@ void         *ssl_ds_table_get(ssl_ds_table *, char *);
 void          ssl_ds_table_wipeout(ssl_ds_table *);
 void          ssl_ds_table_kill(ssl_ds_table *);
 
+unsigned char *ssl_asn1_table_set(apr_hash_t *table,
+                                  const void *key,
+                                  long int length);
+
+void ssl_asn1_table_unset(apr_hash_t *table,
+                          const void *key);
+
 /*  Mutex Support  */
 int          ssl_mutex_init(server_rec *, apr_pool_t *);
 int          ssl_mutex_reinit(server_rec *, apr_pool_t *);

diff --git a/modules/ssl/ssl_engine_config.c b/modules/ssl/ssl_engine_config.c
index c5613da86f9b5b395178cdd0349c7cf65e1b8260..2160c4e6c8c828442a55c0fee67dfd22fe748de5 100644 (file)
--- a/modules/ssl/ssl_engine_config.c
+++ b/modules/ssl/ssl_engine_config.c
@@ -103,7 +103,7 @@ SSLModConfigRec *ssl_config_global_create(server_rec *s)
         mc->aRandSeed              = apr_array_make(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));
+        mc->tTmpKeys               = apr_hash_make(pPool);
 #ifdef SSL_EXPERIMENTAL_ENGINE
         mc->szCryptoDevice         = NULL;
 #endif

diff --git a/modules/ssl/ssl_engine_ds.c b/modules/ssl/ssl_engine_ds.c
index 72bb442b6122897d9b959e0b12a39d4b8e33b87d..2702d20a8f8598a008c3413ce75834db33453cf8 100644 (file)
--- a/modules/ssl/ssl_engine_ds.c
+++ b/modules/ssl/ssl_engine_ds.c
@@ -193,3 +193,58 @@ void ssl_ds_table_kill(ssl_ds_table *t)
     return;
 }
 
+/*
+ * certain key and cert data needs to survive restarts,
+ * which are stored in the user data table of s->process->pool.
+ * to prevent "leaking" of this data, we use malloc/free
+ * rather than apr_palloc and these wrappers to help make sure
+ * we do not leak the malloc-ed data.
+ */
+unsigned char *ssl_asn1_table_set(apr_hash_t *table,
+                                  const void *key,
+                                  long int length)
+{
+    apr_ssize_t klen = strlen(key);
+    ssl_asn1_t *asn1 = apr_hash_get(table, key, klen);
+
+    /*
+     * if a value for this key already exists,
+     * reuse as much of the already malloc-ed data
+     * as possible.
+     */
+    if (asn1 && (asn1->nData != length)) {
+        free(asn1->cpData); /* XXX: realloc? */
+        asn1->cpData = NULL;
+    }
+    else {
+        asn1 = malloc(sizeof(*asn1));
+        asn1->cpData = NULL;
+    }
+
+    asn1->nData = length;
+    if (!asn1->cpData) {
+        asn1->cpData = malloc(length);
+    }
+
+    apr_hash_set(table, key, klen, asn1);
+
+    return asn1->cpData; /* caller will assign a value to this */
+}
+
+void ssl_asn1_table_unset(apr_hash_t *table,
+                          const void *key)
+{
+    apr_ssize_t klen = strlen(key);
+    ssl_asn1_t *asn1 = apr_hash_get(table, key, klen);
+
+    if (!asn1) {
+        return;
+    }
+
+    if (asn1->cpData) {
+        free(asn1->cpData);
+    }
+    free(asn1);
+
+    apr_hash_set(table, key, klen, NULL);
+}

diff --git a/modules/ssl/ssl_engine_init.c b/modules/ssl/ssl_engine_init.c
index d511486f7c5b858da21b4b6dd734daa40dabd673..4876502ec4de58e93f96106973c23df5032762a2 100644 (file)
--- a/modules/ssl/ssl_engine_init.c
+++ b/modules/ssl/ssl_engine_init.c
@@ -272,6 +272,7 @@ void ssl_init_TmpKeysHandle(int action, server_rec *s, apr_pool_t *p)
     SSLModConfigRec *mc = myModConfig(s);
     ssl_asn1_t *asn1;
     unsigned char *ucp;
+    long int length;
     RSA *rsa;
     DH *dh;
 
@@ -288,10 +289,10 @@ void ssl_init_TmpKeysHandle(int action, server_rec *s, apr_pool_t *p)
                     "Init: Failed to generate temporary 512 bit RSA private key");
             ssl_die();
         }
-        asn1 = (ssl_asn1_t *)ssl_ds_table_push(mc->tTmpKeys, "RSA:512");
-        asn1->nData  = i2d_RSAPrivateKey(rsa, NULL);
-        asn1->cpData = apr_palloc(mc->pPool, asn1->nData);
-        ucp = asn1->cpData; i2d_RSAPrivateKey(rsa, &ucp); /* 2nd arg increments */
+
+        length = i2d_RSAPrivateKey(rsa, NULL);
+        ucp = ssl_asn1_table_set(mc->tTmpKeys, "RSA:512", length);
+        (void)i2d_RSAPrivateKey(rsa, &ucp); /* 2nd arg increments */
         RSA_free(rsa);
 
         /* generate 1024 bit RSA key */
@@ -300,10 +301,10 @@ void ssl_init_TmpKeysHandle(int action, server_rec *s, apr_pool_t *p)
                     "Init: Failed to generate temporary 1024 bit RSA private key");
             ssl_die();
         }
-        asn1 = (ssl_asn1_t *)ssl_ds_table_push(mc->tTmpKeys, "RSA:1024");
-        asn1->nData  = i2d_RSAPrivateKey(rsa, NULL);
-        asn1->cpData = apr_palloc(mc->pPool, asn1->nData);
-        ucp = asn1->cpData; i2d_RSAPrivateKey(rsa, &ucp); /* 2nd arg increments */
+
+        length = i2d_RSAPrivateKey(rsa, NULL);
+        ucp = ssl_asn1_table_set(mc->tTmpKeys, "RSA:1024", length);
+        (void)i2d_RSAPrivateKey(rsa, &ucp); /* 2nd arg increments */
         RSA_free(rsa);
 
         ssl_log(s, SSL_LOG_INFO, "Init: Configuring temporary DH parameters (512/1024 bits)");
@@ -313,10 +314,10 @@ void ssl_init_TmpKeysHandle(int action, server_rec *s, apr_pool_t *p)
             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, "DH:512");
-        asn1->nData  = i2d_DHparams(dh, NULL);
-        asn1->cpData = apr_palloc(mc->pPool, asn1->nData);
-        ucp = asn1->cpData; i2d_DHparams(dh, &ucp); /* 2nd arg increments */
+
+        length = i2d_DHparams(dh, NULL);
+        ucp = ssl_asn1_table_set(mc->tTmpKeys, "DH:512", length);
+        (void)i2d_DHparams(dh, &ucp); /* 2nd arg increments */
         /* no need to free dh, it's static */
 
         /* import 1024 bit DH param */
@@ -324,10 +325,10 @@ void ssl_init_TmpKeysHandle(int action, server_rec *s, apr_pool_t *p)
             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, "DH:1024");
-        asn1->nData  = i2d_DHparams(dh, NULL);
-        asn1->cpData = apr_palloc(mc->pPool, asn1->nData);
-        ucp = asn1->cpData; i2d_DHparams(dh, &ucp); /* 2nd arg increments */
+
+        length = i2d_DHparams(dh, NULL);
+        ucp = ssl_asn1_table_set(mc->tTmpKeys, "DH:1024", length);
+        (void)i2d_DHparams(dh, &ucp); /* 2nd arg increments */
         /* no need to free dh, it's static */
     }
 
@@ -337,7 +338,7 @@ void ssl_init_TmpKeysHandle(int action, server_rec *s, apr_pool_t *p)
         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) {
+        if ((asn1 = (ssl_asn1_t *)apr_hash_get(mc->tTmpKeys, "RSA:512", APR_HASH_KEY_STRING)) != NULL) {
             ucp = asn1->cpData;
             if ((mc->pTmpKeys[SSL_TKPIDX_RSA512] = 
                  (void *)d2i_RSAPrivateKey(NULL, SSL_UCP_CAST(&ucp), asn1->nData)) == NULL) {
@@ -347,7 +348,7 @@ void ssl_init_TmpKeysHandle(int action, server_rec *s, apr_pool_t *p)
         }
 
         /* allocate 1024 bit RSA key */
-        if ((asn1 = (ssl_asn1_t *)ssl_ds_table_get(mc->tTmpKeys, "RSA:1024")) != NULL) {
+        if ((asn1 = (ssl_asn1_t *)apr_hash_get(mc->tTmpKeys, "RSA:1024", APR_HASH_KEY_STRING)) != NULL) {
             ucp = asn1->cpData;
             if ((mc->pTmpKeys[SSL_TKPIDX_RSA1024] = 
                  (void *)d2i_RSAPrivateKey(NULL, SSL_UCP_CAST(&ucp), asn1->nData)) == NULL) {
@@ -359,7 +360,7 @@ void ssl_init_TmpKeysHandle(int action, server_rec *s, apr_pool_t *p)
         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) {
+        if ((asn1 = (ssl_asn1_t *)apr_hash_get(mc->tTmpKeys, "DH:512", APR_HASH_KEY_STRING)) != NULL) {
             ucp = asn1->cpData;
             if ((mc->pTmpKeys[SSL_TKPIDX_DH512] = 
                  (void *)d2i_DHparams(NULL, SSL_UCP_CAST(&ucp), asn1->nData)) == NULL) {
@@ -369,7 +370,7 @@ void ssl_init_TmpKeysHandle(int action, server_rec *s, apr_pool_t *p)
         }
 
         /* allocate 1024 bit DH param */
-        if ((asn1 = (ssl_asn1_t *)ssl_ds_table_get(mc->tTmpKeys, "DH:1024")) != NULL) {
+        if ((asn1 = (ssl_asn1_t *)apr_hash_get(mc->tTmpKeys, "DH:512", APR_HASH_KEY_STRING)) != NULL) {
             ucp = asn1->cpData;
             if ((mc->pTmpKeys[SSL_TKPIDX_DH1024] = 
                  (void *)d2i_DHparams(NULL, SSL_UCP_CAST(&ucp), asn1->nData)) == NULL) {