You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jw...@apache.org on 2002/02/25 05:23:04 UTC

cvs commit: httpd-2.0/modules/ssl mod_ssl.h ssl_engine_dh.c ssl_engine_init.c ssl_engine_kernel.c ssl_engine_rand.c ssl_scache_dbm.c ssl_scache_shmcb.c ssl_scache_shmht.c

jwoolley    02/02/24 20:23:04

  Modified:    .        CHANGES
               modules/ssl mod_ssl.h ssl_engine_dh.c ssl_engine_init.c
                        ssl_engine_kernel.c ssl_engine_rand.c
                        ssl_scache_dbm.c ssl_scache_shmcb.c
                        ssl_scache_shmht.c
  Log:
  Forward port of changes in mod_ssl for Apache 1.3 up through mod_ssl
  version 2.8.7-1.3.23.
  
  Revision  Changes    Path
  1.607     +4 -0      httpd-2.0/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/CHANGES,v
  retrieving revision 1.606
  retrieving revision 1.607
  diff -u -d -u -r1.606 -r1.607
  --- CHANGES	23 Feb 2002 20:56:35 -0000	1.606
  +++ CHANGES	25 Feb 2002 04:23:03 -0000	1.607
  @@ -1,4 +1,8 @@
   Changes with Apache 2.0.33-dev
  +
  +  *) Merged in changes to mod_ssl up through 2.8.7-1.3.23.
  +     [Ralf S. Engelschall, Cliff Woolley]
  +
     *) mod-include: make it handle flush'es and fix the 'false-alarm'
        [Justin Everkrantz, Brian Pane, Ian Holsman]
   
  
  
  
  1.56      +5 -0      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.55
  retrieving revision 1.56
  diff -u -d -u -r1.55 -r1.56
  --- mod_ssl.h	18 Jan 2002 23:26:46 -0000	1.55
  +++ mod_ssl.h	25 Feb 2002 04:23:03 -0000	1.56
  @@ -513,6 +513,7 @@
       char           *szMutexFile;
       apr_lock_t     *pMutex;
       apr_array_header_t   *aRandSeed;
  +    int             nScoreboardSize; /* used for builtin random seed */
       ssl_ds_table   *tTmpKeys;
       void           *pTmpKeys[SSL_TKPIDX_MAX];
       ssl_ds_table   *tPublicCert;
  @@ -675,7 +676,11 @@
   int          ssl_callback_NewSessionCacheEntry(SSL *, SSL_SESSION *);
   SSL_SESSION *ssl_callback_GetSessionCacheEntry(SSL *, unsigned char *, int, int *);
   void         ssl_callback_DelSessionCacheEntry(SSL_CTX *, SSL_SESSION *);
  +#if SSL_LIBRARY_VERSION >= 0x00907000
  +void         ssl_callback_LogTracingState(const SSL *, int, int);
  +#else
   void         ssl_callback_LogTracingState(SSL *, int, int);
  +#endif
   
   /*  Session Cache Support  */
   void         ssl_scache_init(server_rec *, apr_pool_t *);
  
  
  
  1.6       +2 -2      httpd-2.0/modules/ssl/ssl_engine_dh.c
  
  Index: ssl_engine_dh.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_dh.c,v
  retrieving revision 1.5
  retrieving revision 1.6
  diff -u -d -u -r1.5 -r1.6
  --- ssl_engine_dh.c	7 Aug 2001 16:19:03 -0000	1.5
  +++ ssl_engine_dh.c	25 Feb 2002 04:23:03 -0000	1.6
  @@ -225,10 +225,10 @@
   
   #   generate C source from DH params
   my $dhsource = '';
  -open(FP, "openssl dh -noout -C -in dh512.pem | indent | expand -8 |") || die;
  +open(FP, "openssl dh -noout -C -in dh512.pem | indent | expand |") || die;
   $dhsource .= $_ while (<FP>);
   close(FP);
  -open(FP, "openssl dh -noout -C -in dh1024.pem | indent | expand -8 |") || die;
  +open(FP, "openssl dh -noout -C -in dh1024.pem | indent | expand |") || die;
   $dhsource .= $_ while (<FP>);
   close(FP);
   $dhsource =~ s|(DH\s+\*get_dh)|static $1|sg;
  
  
  
  1.26      +6 -1      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.25
  retrieving revision 1.26
  diff -u -d -u -r1.25 -r1.26
  --- ssl_engine_init.c	16 Feb 2002 18:35:21 -0000	1.25
  +++ ssl_engine_init.c	25 Feb 2002 04:23:03 -0000	1.26
  @@ -221,7 +221,11 @@
   
       /*
        * Seed the Pseudo Random Number Generator (PRNG)
  +     *
  +     * Note: scoreboard size must be fetched at init time because
  +     * ap_calc_scoreboard_size() is not threadsafe
        */
  +    mc->nScoreboardSize = ap_calc_scoreboard_size();
       ssl_rand_seed(s, p, SSL_RSCTX_STARTUP, "Init: ");
   
       /*
  @@ -713,7 +717,8 @@
               }
               if (SSL_X509_getCN(p, sc->pPublicCert[i], &cp)) {
                   if (apr_is_fnmatch(cp) &&
  -                    !apr_fnmatch(cp, s->server_hostname, FNM_PERIOD|FNM_CASE_BLIND)) {
  +                    apr_fnmatch(cp, s->server_hostname,
  +                                FNM_PERIOD|FNM_CASE_BLIND) == FNM_NOMATCH) {
                       ssl_log(s, SSL_LOG_WARN,
                           "Init: (%s) %s server certificate wildcard CommonName (CN) `%s' "
                           "does NOT match server name!?", cpVHostID, 
  
  
  
  1.42      +5 -1      httpd-2.0/modules/ssl/ssl_engine_kernel.c
  
  Index: ssl_engine_kernel.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_kernel.c,v
  retrieving revision 1.41
  retrieving revision 1.42
  diff -u -d -u -r1.41 -r1.42
  --- ssl_engine_kernel.c	3 Feb 2002 01:50:58 -0000	1.41
  +++ ssl_engine_kernel.c	25 Feb 2002 04:23:03 -0000	1.42
  @@ -1587,7 +1587,11 @@
    * SSL handshake and does SSL record layer stuff. We use it to
    * trace OpenSSL's processing in out SSL logfile.
    */
  +#if SSL_LIBRARY_VERSION >= 0x00907000
  +void ssl_callback_LogTracingState(const SSL *ssl, int where, int rc)
  +#else
   void ssl_callback_LogTracingState(SSL *ssl, int where, int rc)
  +#endif
   {
       conn_rec *c;
       server_rec *s;
  @@ -1597,7 +1601,7 @@
       /*
        * find corresponding server
        */
  -    if ((c = (conn_rec *)SSL_get_app_data(ssl)) == NULL)
  +    if ((c = (conn_rec *)SSL_get_app_data((SSL *)ssl)) == NULL)
           return;
       s = c->base_server;
       if ((sc = mySrvConfig(s)) == NULL)
  
  
  
  1.12      +13 -9     httpd-2.0/modules/ssl/ssl_engine_rand.c
  
  Index: ssl_engine_rand.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_engine_rand.c,v
  retrieving revision 1.11
  retrieving revision 1.12
  diff -u -d -u -r1.11 -r1.12
  --- ssl_engine_rand.c	10 Jan 2002 00:28:00 -0000	1.11
  +++ ssl_engine_rand.c	25 Feb 2002 04:23:03 -0000	1.12
  @@ -81,6 +81,7 @@
       int nReq, nDone;
       apr_file_t *fp;
       int i, n, l;
  +    int m;
   
       mc = myModConfig(s);
       nReq  = 0;
  @@ -154,18 +155,21 @@
                   RAND_seed(stackdata+n, 128);
                   nDone += 128;
   
  -#if XXX_SBENTROPY_SOLVED
                   /*
  -                 * XXX: This is entirely borked, sizeof(scoreboard) < 1024
  +                 * seed in data extracted from the current scoreboard
                    *
  -                 * seed in an 1KB extract of the current scoreboard
  +                 * XXX: this assumes that the entire scoreboard is
  +                 * allocated in one big block of memory that begins at
  +                 * the location pointed to by ap_scoreboard_image->global
                    */
  -                if (ap_scoreboard_image != NULL) {
  -                    n = ssl_rand_choosenum(0,ap_calc_scoreboard_size()-1024-1);
  -                    RAND_seed(((unsigned char *)ap_scoreboard_image)+n, 1024);
  -                    nDone += 1024;
  +                if (ap_scoreboard_image != NULL && mc->nScoreboardSize > 16)
  +                {
  +                    m = ((mc->nScoreboardSize / 2) - 1);
  +                    n = ssl_rand_choosenum(0, m);
  +                    RAND_seed(
  +                        ((unsigned char *)ap_scoreboard_image->global)+n, m);
  +                    nDone += m;
                   }
  -#endif
               }
           }
       }
  
  
  
  1.11      +3 -1      httpd-2.0/modules/ssl/ssl_scache_dbm.c
  
  Index: ssl_scache_dbm.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_scache_dbm.c,v
  retrieving revision 1.10
  retrieving revision 1.11
  diff -u -d -u -r1.10 -r1.11
  --- ssl_scache_dbm.c	7 Nov 2001 14:09:36 -0000	1.10
  +++ ssl_scache_dbm.c	25 Feb 2002 04:23:03 -0000	1.11
  @@ -142,8 +142,10 @@
       UCHAR *ucp;
   
       /* streamline session data */
  +    if ((nData = i2d_SSL_SESSION(sess, NULL)) > sizeof(ucaData))
  +        return FALSE;
       ucp = ucaData;
  -    nData = i2d_SSL_SESSION(sess, &ucp);
  +    i2d_SSL_SESSION(sess, &ucp);
   
       /* be careful: do not try to store too much bytes in a DBM file! */
   #ifdef PAIRMAX
  
  
  
  1.4       +15 -13    httpd-2.0/modules/ssl/ssl_scache_shmcb.c
  
  Index: ssl_scache_shmcb.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_scache_shmcb.c,v
  retrieving revision 1.3
  retrieving revision 1.4
  diff -u -d -u -r1.3 -r1.4
  --- ssl_scache_shmcb.c	5 May 2001 10:12:07 -0000	1.3
  +++ ssl_scache_shmcb.c	25 Feb 2002 04:23:03 -0000	1.4
  @@ -183,9 +183,9 @@
       unsigned int division_offset;
       unsigned int division_size;
       unsigned int queue_size;
  -    unsigned char index_num;
  -    unsigned char index_offset;
  -    unsigned char index_size;
  +    unsigned int index_num;
  +    unsigned int index_offset;
  +    unsigned int index_size;
       unsigned int cache_data_offset;
       unsigned int cache_data_size;
       unsigned long num_stores;
  @@ -208,10 +208,10 @@
       unsigned int queue_size;
       unsigned int cache_data_offset;
       unsigned int cache_data_size;
  +    unsigned int index_num;
  +    unsigned int index_offset;
  +    unsigned int index_size;
       unsigned char division_mask;
  -    unsigned char index_num;
  -    unsigned char index_offset;
  -    unsigned char index_size;
   #endif
   } SHMCBHeader;
   
  @@ -456,7 +456,7 @@
       return;
   }
   
  -BOOL ssl_scache_shmcb_store(server_rec *s, UCHAR * id, int idlen,
  +BOOL ssl_scache_shmcb_store(server_rec *s, UCHAR *id, int idlen,
                              time_t timeout, SSL_SESSION * pSession)
   {
       SSLModConfigRec *mc = myModConfig();
  @@ -478,7 +478,7 @@
       return to_return;
   }
   
  -SSL_SESSION *ssl_scache_shmcb_retrieve(server_rec *s, UCHAR * id, int idlen)
  +SSL_SESSION *ssl_scache_shmcb_retrieve(server_rec *s, UCHAR *id, int idlen)
   {
       SSLModConfigRec *mc = myModConfig();
       void *shm_segment;
  @@ -499,14 +499,16 @@
       return pSession;
   }
   
  -void ssl_scache_shmcb_remove(server_rec *s, UCHAR * id, int idlen)
  +void ssl_scache_shmcb_remove(server_rec *s, UCHAR *id, int idlen)
   {
       SSLModConfigRec *mc = myModConfig();
       void *shm_segment;
   
       /* We've kludged our pointer into the other cache's member variable. */
       shm_segment = (void *) mc->tSessionCacheDataTable;
  +    ssl_mutex_on(s);
       shmcb_remove_session(s, shm_segment, id, idlen);
  +    ssl_mutex_off(s);
   }
   
   void ssl_scache_shmcb_expire(server_rec *s)
  @@ -705,7 +707,7 @@
   }
   
   static BOOL shmcb_store_session(
  -    server_rec *s, void *shm_segment, UCHAR * id,
  +    server_rec *s, void *shm_segment, UCHAR *id,
       int idlen, SSL_SESSION * pSession,
       time_t timeout)
   {
  @@ -755,7 +757,7 @@
   
   static SSL_SESSION *shmcb_retrieve_session(
       server_rec *s, void *shm_segment,
  -    UCHAR * id, int idlen)
  +    UCHAR *id, int idlen)
   {
       SHMCBHeader *header;
       SHMCBQueue queue;
  @@ -795,7 +797,7 @@
   
   static BOOL shmcb_remove_session(
       server_rec *s, void *shm_segment,
  -    UCHAR * id, int idlen)
  +    UCHAR *id, int idlen)
   {
       SHMCBHeader *header;
       SHMCBQueue queue;
  @@ -992,7 +994,7 @@
       const SHMCBQueue *queue, unsigned int idx)
   {
       /* bounds check */
  -    if (idx > (unsigned int) queue->header->index_num)
  +    if (idx > queue->header->index_num)
           return NULL;
   
       /* Return a pointer to the index. NB: I am being horribly pendantic
  
  
  
  1.4       +3 -1      httpd-2.0/modules/ssl/ssl_scache_shmht.c
  
  Index: ssl_scache_shmht.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/modules/ssl/ssl_scache_shmht.c,v
  retrieving revision 1.3
  retrieving revision 1.4
  diff -u -d -u -r1.3 -r1.4
  --- ssl_scache_shmht.c	5 May 2001 10:12:07 -0000	1.3
  +++ ssl_scache_shmht.c	25 Feb 2002 04:23:03 -0000	1.4
  @@ -175,8 +175,10 @@
       UCHAR *ucp;
   
       /* streamline session data */
  +    if ((nData = i2d_SSL_SESSION(sess, NULL)) > sizeof(ucaData))
  +        return FALSE;
       ucp = ucaData;
  -    nData = i2d_SSL_SESSION(sess, &ucp);
  +    i2d_SSL_SESSION(sess, &ucp);
   
       ssl_mutex_on(s);
       if (table_insert_kd(mc->tSessionCacheDataTable, 
  
  
  

Re: cvs commit: httpd-2.0/modules/ssl mod_ssl.h ssl_engine_dh.c ssl_engine_init.c ssl_engine_kernel.c ssl_engine_rand.c ssl_scache_dbm.c ssl_scache_shmcb.c ssl_scache_shmht.c

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Mon, Feb 25, 2002 at 05:14:34PM -0500, Cliff Woolley wrote:
> If that's the agreement, then fine, I'll nuke it.  It certainly caused me
> quite a bit of headache to get it to work in the first place, and I was
> definitely uneasy about the assumptions it was making about the layout of
> the scoreboard.  I wouldn't mind hearing Ralf's input, of course...

I don't believe using the scoreboard is a valid source of entropy.
It would be too easy to effect it.  So, let's just remove it.  We're
so conservative about our random sources that I just don't think
using the scoreboard is a benefit.

I'd say it should be removed in mod_ssl for 1.3, but that's not our
call.  -- justin


Re: cvs commit: httpd-2.0/modules/ssl mod_ssl.h ssl_engine_dh.c ssl_engine_init.c ssl_engine_kernel.c ssl_engine_rand.c ssl_scache_dbm.c ssl_scache_shmcb.c ssl_scache_shmht.c

Posted by Doug MacEachern <do...@covalent.net>.
On Mon, 25 Feb 2002, Cliff Woolley wrote:
 
> ssl_rand_seed() runs on every request if you configure it that way.

this is true, when 'SSLRandomSeed connect builtin' is configured, which is 
the default.  not sure how much the scoreboard image changes between 
requests.  though somewhat related, i still have on my ssl performance 
todo-list, optimizing 'SSLRandomSeed connect builtin'.  first problem is 
that RAND_seed() mutex locks in a threaded MPM.  and there's three calls 
to it at connect time:

1st - adds pid (already happened at startup) and time() (which RAND_seed 
already does everytime you call it).  i'm no random number expert, but 
would be surprised if seed with the same values is useful.

2nd - stackdata (from unsigned char stackdata[256]), no idea how random 
that'll be.

3rd - scoreboard data

better sources can be configured, but require reading from a file, running 
an external program or talking to an EGD.  i think builtin could be 
improved.  how about if threads are available, spawn a low priority thread 
to gather entropy using apr_generate_random_bytes() which mod_ssl can grab as 
needed without blocking?


Re: cvs commit: httpd-2.0/modules/ssl mod_ssl.h ssl_engine_dh.c ssl_engine_init.c ssl_engine_kernel.c ssl_engine_rand.c ssl_scache_dbm.c ssl_scache_shmcb.c ssl_scache_shmht.c

Posted by Cliff Woolley <jw...@apache.org>.
On Mon, 25 Feb 2002, William A. Rowe, Jr. wrote:

> At 01:55 PM 2/25/2002, Justin Erenkrantz wrote:
> >Why was the XXX_SBENTROPY_SOLVED define removed?  I believe we wanted
> >to avoid using scoreboard as an entropy source because it isn't very
> >random.  Therefore, I think we should just remove this code
> >altogether.  Or, am I missing something?  -- justin

Best I could tell from the commit logs and the comments in the code, it
was only disabled because it was causing segfaults (because it was
incompatible with the new scoreboard layout).  Ralf's patch touched that
block of code, and it's still used in mod_ssl for Apache 1.3.  I used his
patch as a basis for fixing the corresponding block in 2.0, assuming that
it was only off because it was broken; why else would it still be in 1.3?
But...

> +1 on removing this old, misleading code altogether.

If that's the agreement, then fine, I'll nuke it.  It certainly caused me
quite a bit of headache to get it to work in the first place, and I was
definitely uneasy about the assumptions it was making about the layout of
the scoreboard.  I wouldn't mind hearing Ralf's input, of course...

> You missed nothing.  This patch is borked.  Although in truth is does
> allow the process to add the score as entropy, it gains us little, since
> the scoreboard is initialized to nothing but a few bytes of data and a
> big block 'o nulls.

ssl_rand_seed() runs on every request if you configure it that way.


--Cliff


--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA


Re: cvs commit: httpd-2.0/modules/ssl mod_ssl.h ssl_engine_dh.c ssl_engine_init.c ssl_engine_kernel.c ssl_engine_rand.c ssl_scache_dbm.c ssl_scache_shmcb.c ssl_scache_shmht.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 01:55 PM 2/25/2002, Justin Erenkrantz wrote:
>Why was the XXX_SBENTROPY_SOLVED define removed?  I believe we wanted
>to avoid using scoreboard as an entropy source because it isn't very
>random.  Therefore, I think we should just remove this code
>altogether.  Or, am I missing something?  -- justin

You missed nothing.  This patch is borked.  Although in truth is does allow the
process to add the score as entropy, it gains us little, since the 
scoreboard is
initialized to nothing but a few bytes of data and a big block 'o nulls.

+1 on removing this old, misleading code altogether.

Bill


Re: cvs commit: httpd-2.0/modules/ssl mod_ssl.h ssl_engine_dh.c ssl_engine_init.c ssl_engine_kernel.c ssl_engine_rand.c ssl_scache_dbm.c ssl_scache_shmcb.c ssl_scache_shmht.c

Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Mon, Feb 25, 2002 at 04:23:04AM -0000, jwoolley@apache.org wrote:
>   @@ -154,18 +155,21 @@
>                    RAND_seed(stackdata+n, 128);
>                    nDone += 128;
>    
>   -#if XXX_SBENTROPY_SOLVED
>                    /*
>   -                 * XXX: This is entirely borked, sizeof(scoreboard) < 1024
>   +                 * seed in data extracted from the current scoreboard
>                     *
>   -                 * seed in an 1KB extract of the current scoreboard
>   +                 * XXX: this assumes that the entire scoreboard is
>   +                 * allocated in one big block of memory that begins at
>   +                 * the location pointed to by ap_scoreboard_image->global
>                     */
>   -                if (ap_scoreboard_image != NULL) {
>   -                    n = ssl_rand_choosenum(0,ap_calc_scoreboard_size()-1024-1);
>   -                    RAND_seed(((unsigned char *)ap_scoreboard_image)+n, 1024);
>   -                    nDone += 1024;
>   +                if (ap_scoreboard_image != NULL && mc->nScoreboardSize > 16)
>   +                {
>   +                    m = ((mc->nScoreboardSize / 2) - 1);
>   +                    n = ssl_rand_choosenum(0, m);
>   +                    RAND_seed(
>   +                        ((unsigned char *)ap_scoreboard_image->global)+n, m);
>   +                    nDone += m;
>                    }
>   -#endif
>                }
>            }
>        }
>   
>   
>   

Why was the XXX_SBENTROPY_SOLVED define removed?  I believe we wanted
to avoid using scoreboard as an entropy source because it isn't very
random.  Therefore, I think we should just remove this code
altogether.  Or, am I missing something?  -- justin