Re: svn commit: r630307 - in /httpd/httpd/trunk/modules/ssl: ssl_private.h ssl_scache.c ssl_scache_dbm.c ssl_scache_dc.c ssl_scache_memcache.c ssl_scache_shmcb.c

2008-02-25 Thread Joe Orton
On Sat, Feb 23, 2008 at 11:40:26AM +0100, Ruediger Pluem wrote:
 On 02/22/2008 08:58 PM, [EMAIL PROTECTED] wrote:
 Author: jorton
 Date: Fri Feb 22 11:58:39 2008
 New Revision: 630307

 URL: http://svn.apache.org/viewvc?rev=630307view=rev
...
  memcpy(expiry, dbmval.dptr, sizeof(time_t));
 +memcpy(dest, (char *)dbmval.dptr + sizeof(time_t), nData);

 Shouldn't we do

 *destlen = nData;

 here?

Fixed both of those cases in r630787.

 ==
 --- httpd/httpd/trunk/modules/ssl/ssl_scache_shmcb.c (original)
 +++ httpd/httpd/trunk/modules/ssl/ssl_scache_shmcb.c Fri Feb 22 11:58:39 2008
...
 +/* Only consider 'idx' if the id matches, and the removed
 + * flag isn't set; check the data length too to avoid a buffer
 + * overflow in case of corruption, which should be impossible,
 + * but it's cheap to be safe. */
 +if (idx-id_len == idlen  (idx-data_used - idx-id_len)  
 *destlen
 + shmcb_cyclic_memcmp(header-subcache_data_size,
 +   SHMCB_DATA(header, subcache),
 +   idx-data_pos, id, idx-id_len) == 0) {

 Where do you check for the removed flag?

And both of those cases in r630786.

Thanks a lot for the careful review!

joe


Re: svn commit: r630307 - in /httpd/httpd/trunk/modules/ssl: ssl_private.h ssl_scache.c ssl_scache_dbm.c ssl_scache_dc.c ssl_scache_memcache.c ssl_scache_shmcb.c

2008-02-23 Thread Ruediger Pluem



On 02/22/2008 08:58 PM, [EMAIL PROTECTED] wrote:

Author: jorton
Date: Fri Feb 22 11:58:39 2008
New Revision: 630307

URL: http://svn.apache.org/viewvc?rev=630307view=rev
Log:
Move SSL session data deserialization up out of the session cache
storage providers; includes a significant change to the shmcb storage
structure:

* modules/ssl/ssl_private.h (modssl_sesscache_provider): Change
  retrieve function to take dest/destlen output buffer, to take a
  constant id paramater, and to return a BOOL.

* modules/ssl/ssl_scache.c (ssl_scache_retrieve): Update accordingly,
  perform SSL deserialization here.

* modules/ssl/ssl_scache_dc.c (ssl_scache_dc_retrieve),
  modules/ssl/ssl_scache_dbm.c (ssl_scache_dbm_retrieve),
  modules/ssl/ssl_scache_memcache.c (ssl_scache_mc_retrieve):
  Update accordingly.

* modules/ssl/ssl_scache_shmcb.c: Store the whole ID in the cache
  before the data, so that each index can be compared against the
  requested ID without deserializing the data.  This requires approx
  20% extra storage per session in the common case, though should
  reduce CPU overhead in some retrieval paths.
  (SHMCBIndex): Replace s_id2 field with id_len.
  (shmcb_cyclic_memcmp): New function.
  (ssl_scache_shmcb_init): Change the heuristics to allow for increase
  in per-session storage requirement.
  (ssl_scache_shmcb_retrieve): Drop requirement on ID length.
  (shmcb_subcache_store): Store the ID in the cyclic buffer.
  (shmcb_subcache_retrieve, shmcb_subcache_remove): Compare against
  the stored ID rather than deserializing the data.
  (ssl_scache_shmcb_retrieve, ssl_scache_shmcb_store): Update
  accordingly.

Modified:
httpd/httpd/trunk/modules/ssl/ssl_private.h
httpd/httpd/trunk/modules/ssl/ssl_scache.c
httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c
httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c
httpd/httpd/trunk/modules/ssl/ssl_scache_memcache.c
httpd/httpd/trunk/modules/ssl/ssl_scache_shmcb.c




Modified: httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c?rev=630307r1=630306r2=630307view=diff
==
--- httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_scache_dbm.c Fri Feb 22 11:58:39 2008
@@ -188,16 +188,15 @@
 return TRUE;
 }
 
-static SSL_SESSION *ssl_scache_dbm_retrieve(server_rec *s, UCHAR *id, int idlen,

-apr_pool_t *p)
+static BOOL ssl_scache_dbm_retrieve(server_rec *s, const UCHAR *id, int idlen,
+unsigned char *dest, unsigned int *destlen,
+apr_pool_t *p)
 {
 SSLModConfigRec *mc = myModConfig(s);
 apr_dbm_t *dbm;
 apr_datum_t dbmkey;
 apr_datum_t dbmval;
-SSL_SESSION *sess = NULL;
-MODSSL_D2I_SSL_SESSION_CONST unsigned char *ucpData;
-int nData;
+unsigned int nData;
 time_t expiry;
 time_t now;
 apr_status_t rc;
@@ -221,32 +220,30 @@
  (fetch),
  mc-szSessionCacheDataFile);
 ssl_mutex_off(s);
-return NULL;
+return FALSE;
 }
 rc = apr_dbm_fetch(dbm, dbmkey, dbmval);
 if (rc != APR_SUCCESS) {
 apr_dbm_close(dbm);
 ssl_mutex_off(s);
-return NULL;
+return FALSE;
 }
 if (dbmval.dptr == NULL || dbmval.dsize = sizeof(time_t)) {
 apr_dbm_close(dbm);
 ssl_mutex_off(s);
-return NULL;
+return FALSE;
 }
 
 /* parse resulting data */

 nData = dbmval.dsize-sizeof(time_t);
-ucpData = malloc(nData);
-if (ucpData == NULL) {
+if (nData  *destlen) {
 apr_dbm_close(dbm);
 ssl_mutex_off(s);
-return NULL;
-}
-/* Cast needed, ucpData may be const */
-memcpy((unsigned char *)ucpData,
-   (char *)dbmval.dptr + sizeof(time_t), nData);
+return FALSE;
+}
+

 memcpy(expiry, dbmval.dptr, sizeof(time_t));
+memcpy(dest, (char *)dbmval.dptr + sizeof(time_t), nData);


Shouldn't we do

*destlen = nData;

here?


 
 apr_dbm_close(dbm);

 ssl_mutex_off(s);

Modified: httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c
URL: 
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c?rev=630307r1=630306r2=630307view=diff
==
--- httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c (original)
+++ httpd/httpd/trunk/modules/ssl/ssl_scache_dc.c Fri Feb 22 11:58:39 2008
@@ -116,32 +116,25 @@
 return TRUE;
 }
 
-static SSL_SESSION *ssl_scache_dc_retrieve(server_rec *s, UCHAR *id, int idlen, apr_pool_t *p)

+static BOOL ssl_scache_dc_retrieve(server_rec *s, const UCHAR *id, int idlen,
+   unsigned char *dest, unsigned int *destlen,
+   apr_pool_t *p)
 {
-unsigned