You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by do...@apache.org on 2002/02/28 01:01:57 UTC

cvs commit: httpd-2.0/modules/ssl mod_ssl.h ssl_engine_config.c ssl_engine_ds.c ssl_engine_init.c

dougm       02/02/27 16:01:57

  Modified:    modules/ssl mod_ssl.h ssl_engine_config.c ssl_engine_ds.c
                        ssl_engine_init.c
  Log:
  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.
  
  Revision  Changes    Path
  1.60      +8 -1      httpd-2.0/modules/ssl/mod_ssl.h
  
  Index: mod_ssl.h
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/mod_ssl.h,v
  retrieving revision 1.59
  retrieving revision 1.60
  diff -u -r1.59 -r1.60
  --- mod_ssl.h	27 Feb 2002 19:51:33 -0000	1.59
  +++ mod_ssl.h	28 Feb 2002 00:01:57 -0000	1.60
  @@ -516,7 +516,7 @@
       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;
  @@ -740,6 +740,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 *);
  
  
  
  1.23      +1 -1      httpd-2.0/modules/ssl/ssl_engine_config.c
  
  Index: ssl_engine_config.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_config.c,v
  retrieving revision 1.22
  retrieving revision 1.23
  diff -u -r1.22 -r1.23
  --- ssl_engine_config.c	27 Feb 2002 19:51:33 -0000	1.22
  +++ ssl_engine_config.c	28 Feb 2002 00:01:57 -0000	1.23
  @@ -103,7 +103,7 @@
           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
  
  
  
  1.7       +55 -0     httpd-2.0/modules/ssl/ssl_engine_ds.c
  
  Index: ssl_engine_ds.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_ds.c,v
  retrieving revision 1.6
  retrieving revision 1.7
  diff -u -r1.6 -r1.7
  --- ssl_engine_ds.c	2 Aug 2001 05:25:53 -0000	1.6
  +++ ssl_engine_ds.c	28 Feb 2002 00:01:57 -0000	1.7
  @@ -193,3 +193,58 @@
       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);
  +}
  
  
  
  1.28      +21 -20    httpd-2.0/modules/ssl/ssl_engine_init.c
  
  Index: ssl_engine_init.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_init.c,v
  retrieving revision 1.27
  retrieving revision 1.28
  diff -u -r1.27 -r1.28
  --- ssl_engine_init.c	27 Feb 2002 03:21:09 -0000	1.27
  +++ ssl_engine_init.c	28 Feb 2002 00:01:57 -0000	1.28
  @@ -272,6 +272,7 @@
       SSLModConfigRec *mc = myModConfig(s);
       ssl_asn1_t *asn1;
       unsigned char *ucp;
  +    long int length;
       RSA *rsa;
       DH *dh;
   
  @@ -288,10 +289,10 @@
                       "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 @@
                       "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 @@
               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 @@
               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 @@
           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 @@
           }
   
           /* 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 @@
           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 @@
           }
   
           /* 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) {
  
  
  

Re: cvs commit: httpd-2.0/modules/ssl mod_ssl.h ssl_engine_config.c ssl_engine_ds.c ssl_engine_init.c

Posted by Greg Stein <gs...@lyra.org>.
On Thu, Feb 28, 2002 at 12:01:57AM -0000, dougm@apache.org wrote:
>...
>   +++ mod_ssl.h	28 Feb 2002 00:01:57 -0000	1.60
>   @@ -516,7 +516,7 @@
>        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;
>   @@ -740,6 +740,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);

Those keys are strings, so the signature should be "const char *". That type
is compatible with the hash table key type.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/