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 2001/08/24 06:08:04 UTC

cvs commit: httpd-2.0/modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c

dougm       01/08/23 21:08:04

  Modified:    .        CHANGES
               modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c
  Log:
  Implement CRYPTO_set_locking_callback() for mod_ssl
  Submitted by:	Madhusudan Mathihalli <ma...@hp.com>
  Reviewed by:	dougm
  
  Revision  Changes    Path
  1.325     +4 -0      httpd-2.0/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/CHANGES,v
  retrieving revision 1.324
  retrieving revision 1.325
  diff -u -r1.324 -r1.325
  --- CHANGES	2001/08/23 15:45:49	1.324
  +++ CHANGES	2001/08/24 04:08:04	1.325
  @@ -1,4 +1,8 @@
   Changes with Apache 2.0.25-dev
  +  *)  Implement CRYPTO_set_locking_callback() in terms of apr_lock
  +      for mod_ssl
  +     [Madhusudan Mathihalli <ma...@hp.com>]
  +
     *) Fix for mod_include. Ryan's patch to check error
        codes put a return in the wrong place. Also, the
        include handler return code wasn't being checked.
  
  
  
  1.26      +0 -1      httpd-2.0/modules/ssl/README
  
  Index: README
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/README,v
  retrieving revision 1.25
  retrieving revision 1.26
  diff -u -r1.25 -r1.26
  --- README	2001/08/22 21:37:15	1.25
  +++ README	2001/08/24 04:08:04	1.26
  @@ -174,7 +174,6 @@
    o Whether to unregister and how to unregister?
      ssl_var_unregister();
      ssl_ext_unregister();
  - o We certainly need CRYPTO_set_locking_callback() now also under Unix!
    o Do we need SSL_set_read_ahead()?
    o Enable use of MM, SHMCB and SHMHT.
    o Enable SSL extensions (ssl_engine_ext.c)
  
  
  
  1.32      +1 -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.31
  retrieving revision 1.32
  diff -u -r1.31 -r1.32
  --- mod_ssl.h	2001/08/24 00:09:30	1.31
  +++ mod_ssl.h	2001/08/24 04:08:04	1.32
  @@ -728,7 +728,7 @@
   ssl_algo_t   ssl_util_algotypeof(X509 *, EVP_PKEY *); 
   char        *ssl_util_algotypestr(ssl_algo_t);
   char        *ssl_util_ptxtsub(apr_pool_t *, const char *, const char *, char *);
  -void         ssl_util_thread_setup(void);
  +void         ssl_util_thread_setup(server_rec *, apr_pool_t *);
   apr_status_t     ssl_util_setmodconfig(server_rec *, const char *, SSLModConfigRec *);
   SSLModConfigRec *ssl_util_getmodconfig(server_rec *, const char *);
   SSLModConfigRec *ssl_util_getmodconfig_ssl(SSL *, const char *);
  
  
  
  1.11      +1 -0      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.10
  retrieving revision 1.11
  diff -u -r1.10 -r1.11
  --- ssl_engine_init.c	2001/08/23 19:42:44	1.10
  +++ ssl_engine_init.c	2001/08/24 04:08:04	1.11
  @@ -185,6 +185,7 @@
           ssl_init_SSLLibrary();
       }
   #endif
  +    ssl_util_thread_setup(s, p);
       if (mc->nInitCount == 1) {
           ssl_pphrase_Handle(s, p);
           ssl_init_TmpKeysHandle(SSL_TKP_GEN, s, p);
  
  
  
  1.12      +46 -0     httpd-2.0/modules/ssl/ssl_util.c
  
  Index: ssl_util.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_util.c,v
  retrieving revision 1.11
  retrieving revision 1.12
  diff -u -r1.11 -r1.12
  --- ssl_util.c	2001/08/23 19:35:23	1.11
  +++ ssl_util.c	2001/08/24 04:08:04	1.12
  @@ -328,3 +328,49 @@
       return mc;
   }
   
  +/*
  + * To ensure thread-safetyness in OpenSSL - work in progress
  + */
  +
  +static apr_lock_t *lock_cs[CRYPTO_NUM_LOCKS];
  +static long        lock_count[CRYPTO_NUM_LOCKS];
  +
  +void ssl_util_thread_locking_callback(int mode, int type, char *file, int line)
  +{
  +    if (mode & CRYPTO_LOCK) {
  +        apr_lock_acquire(lock_cs[type]);
  +        lock_count[type]++;
  +    }
  +    else {
  +        apr_lock_release(lock_cs[type]);
  +    }
  +}
  +
  +apr_status_t ssl_util_thread_cleanup(void *data)
  +{
  +    int i;
  +
  +    CRYPTO_set_locking_callback(NULL);
  +    for (i = 0; i < CRYPTO_NUM_LOCKS; i++)
  +        apr_lock_destroy(lock_cs[i]);
  +    return APR_SUCCESS;
  +}
  +
  +void ssl_util_thread_setup(server_rec *s, apr_pool_t *p)
  +{
  +    int i;
  +    SSLModConfigRec *mc = myModConfig(s);
  +
  +    *lock_cs = apr_palloc(p, CRYPTO_NUM_LOCKS);
  +    for (i = 0; i < CRYPTO_NUM_LOCKS; i++)
  +    {
  +        lock_count[i]=0;
  +        apr_lock_create(&(lock_cs[i]), APR_MUTEX, APR_LOCKALL,
  +                                                mc->szMutexFile, p);
  +    }
  +
  +    CRYPTO_set_locking_callback((void (*)())ssl_util_thread_locking_callback);
  +    apr_pool_cleanup_register(p, NULL,
  +                ssl_util_thread_cleanup, apr_pool_cleanup_null);
  +
  +}
  
  
  

Re: cvs commit: httpd-2.0/modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c

Posted by Doug MacEachern <do...@covalent.net>.
On Thu, 23 Aug 2001, Justin Erenkrantz wrote:
 
> If you created the locks as INTRAPROCESS before fork()ing new children, 
> would each children's copy of the lock be isolated?  Maybe.  If so, 
> then the post_config hook works, but I think child_init makes a bit 
> more sense.  But, that's me.  -- justin

there must be other intraprocess locks created at startup time, i'd be
surprised if there's an issue.  one problem with using child init is that
not all mpms support it, notice theres a few missing:
% find . -name "*.c" | xargs grep ap_run_child_init 
./server/mpm/perchild/perchild.c:    ap_run_child_init(pchild, ap_server_conf);
./server/mpm/prefork/prefork.c:    ap_run_child_init(pchild, ap_server_conf);
./server/mpm/spmt_os2/spmt_os2.c:    ap_run_child_init(pchild, ap_server_conf);
./server/mpm/threaded/threaded.c:    ap_run_child_init(pchild, ap_server_conf);
./server/mpm/worker/worker.c:    ap_run_child_init(pchild, ap_server_conf);


Re: cvs commit: httpd-2.0/modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Thu, Aug 23, 2001 at 10:42:16PM -0700, Doug MacEachern wrote:
> > IMHO, the call to ssl_util_thread_setup should be moved to 
> > ssl_init_Child and even potentially surrounded by 
> > #if APR_HAS_THREADS.  -- justin
> 
> why child init rather than module init?
> i did turn off the locking unless the mpm is threaded.  isn't it possible
> that APR_HAS_THREADS is true even though the mpm is not threaded?

Yes, you are correct here.  I didn't think about the mpm query
facilities.  =)

It should be in child init rather than module init so that the locks are
created per child not globally (via post_config hook).  That is the 
tradeoff by having APR_INTRAPROCESS.  

If you created the locks as INTRAPROCESS before fork()ing new children, 
would each children's copy of the lock be isolated?  Maybe.  If so, 
then the post_config hook works, but I think child_init makes a bit 
more sense.  But, that's me.  -- justin


Re: cvs commit: httpd-2.0/modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c

Posted by Doug MacEachern <do...@covalent.net>.
On Thu, 23 Aug 2001, Justin Erenkrantz wrote:
 
> I meant to review this patch, but I accidentally hit the delete key on 
> the original message.  Anyway, that APR_LOCKALL can be APR_INTRAPROCESS.
> We're only concerned about safety within our process not across 
> multiple processes - there is no need to needlessly lock across
> processes here.

right, thanks, that change is in.
 
> IMHO, the call to ssl_util_thread_setup should be moved to 
> ssl_init_Child and even potentially surrounded by 
> #if APR_HAS_THREADS.  -- justin

why child init rather than module init?
i did turn off the locking unless the mpm is threaded.  isn't it possible
that APR_HAS_THREADS is true even though the mpm is not threaded?



Re: cvs commit: httpd-2.0/modules/ssl README mod_ssl.h ssl_engine_init.c ssl_util.c

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Fri, Aug 24, 2001 at 04:08:04AM -0000, dougm@apache.org wrote:
>   +    *lock_cs = apr_palloc(p, CRYPTO_NUM_LOCKS);
>   +    for (i = 0; i < CRYPTO_NUM_LOCKS; i++)
>   +    {
>   +        lock_count[i]=0;
>   +        apr_lock_create(&(lock_cs[i]), APR_MUTEX, APR_LOCKALL,
>   +                                                mc->szMutexFile, p);
>   +    }

I meant to review this patch, but I accidentally hit the delete key on 
the original message.  Anyway, that APR_LOCKALL can be APR_INTRAPROCESS.
We're only concerned about safety within our process not across 
multiple processes - there is no need to needlessly lock across
processes here.

IMHO, the call to ssl_util_thread_setup should be moved to 
ssl_init_Child and even potentially surrounded by 
#if APR_HAS_THREADS.  -- justin