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.

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

diff --git a/modules/ssl/ssl_private.h b/modules/ssl/ssl_private.h
index 04b30b5e1adb38f6993412892832012cdeb46690..190d3b4eb5d576d43ede60340974e725eaf7cd0a 100644 (file)
--- a/modules/ssl/ssl_private.h
+++ b/modules/ssl/ssl_private.h
@@ -371,8 +371,14 @@ typedef struct {
     BOOL (*store)(server_rec *s, UCHAR *id, int idlen,
                   time_t expiry, 
                   unsigned char *data, unsigned int datalen);
-    SSL_SESSION *(*retrieve)(server_rec *s, UCHAR *id, int idlen,
-                             apr_pool_t *pool);
+    /* Retrieve cached data with key ID of length IDLEN,
+     * returning TRUE on success or FALSE otherwise.  If
+     * TRUE, the data must be placed in DEST, which has length
+     * on entry of *DESTLEN.  *DESTLEN must be updated to 
+     * equal the length of data written on exit. */
+    BOOL (*retrieve)(server_rec *s, const UCHAR *id, int idlen,
+                     unsigned char *dest, unsigned int *destlen,
+                     apr_pool_t *pool);
     void (*delete)(server_rec *s, UCHAR *id, int idlen, apr_pool_t *pool);
     void (*status)(request_rec *r, int flags, apr_pool_t *pool);
 } modssl_sesscache_provider;

diff --git a/modules/ssl/ssl_scache.c b/modules/ssl/ssl_scache.c
index 0bdd025af50a8ea95330b75325171b629b53d8a1..11c5f3defed1d4a8fcd070cd69030c8203a65854 100644 (file)
--- a/modules/ssl/ssl_scache.c
+++ b/modules/ssl/ssl_scache.c
@@ -88,8 +88,17 @@ SSL_SESSION *ssl_scache_retrieve(server_rec *s, UCHAR *id, int idlen,
                                  apr_pool_t *p)
 {
     SSLModConfigRec *mc = myModConfig(s);
+    unsigned char dest[SSL_SESSION_MAX_DER];
+    unsigned int destlen = SSL_SESSION_MAX_DER;
+    MODSSL_D2I_SSL_SESSION_CONST unsigned char *ptr;
+    
+    if (mc->sesscache->retrieve(s, id, idlen, dest, &destlen, p) == FALSE) {
+        return NULL;
+    }
+
+    ptr = dest;
 
-    return mc->sesscache->retrieve(s, id, idlen, p);
+    return d2i_SSL_SESSION(NULL, &ptr, destlen);
 }
 
 void ssl_scache_remove(server_rec *s, UCHAR *id, int idlen,

diff --git a/modules/ssl/ssl_scache_dbm.c b/modules/ssl/ssl_scache_dbm.c
index 755083dda6ad18b68e7195f553381a50fa229ed9..c5009d8c07751aa8c6d423bd3e664ae674cc36f9 100644 (file)
--- a/modules/ssl/ssl_scache_dbm.c
+++ b/modules/ssl/ssl_scache_dbm.c
@@ -188,16 +188,15 @@ static BOOL ssl_scache_dbm_store(server_rec *s, UCHAR *id, int idlen,
     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 @@ static SSL_SESSION *ssl_scache_dbm_retrieve(server_rec *s, UCHAR *id, int idlen,
                      "(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);
 
     apr_dbm_close(dbm);
     ssl_mutex_off(s);
@@ -254,14 +251,11 @@ static SSL_SESSION *ssl_scache_dbm_retrieve(server_rec *s, UCHAR *id, int idlen,
     /* make sure the stuff is still not expired */
     now = time(NULL);
     if (expiry <= now) {
-        ssl_scache_dbm_remove(s, id, idlen, p);
-        return NULL;
+        ssl_scache_dbm_remove(s, (UCHAR *)id, idlen, p);
+        return FALSE;
     }
 
-    /* unstreamed SSL_SESSION */
-    sess = d2i_SSL_SESSION(NULL, &ucpData, nData);
-
-    return sess;
+    return TRUE;
 }
 
 static void ssl_scache_dbm_remove(server_rec *s, UCHAR *id, int idlen,

diff --git a/modules/ssl/ssl_scache_dc.c b/modules/ssl/ssl_scache_dc.c
index a08a7dbb7f3fa14b84b116443b147a8658513086..dda21a09ec2b5a9c75e6ea1b8ff0a47a236d4930 100644 (file)
--- a/modules/ssl/ssl_scache_dc.c
+++ b/modules/ssl/ssl_scache_dc.c
@@ -116,32 +116,25 @@ static BOOL ssl_scache_dc_store(server_rec *s, UCHAR *id, int idlen,
     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 char der[SSL_SESSION_MAX_DER];
-    unsigned int der_len;
-    SSL_SESSION *pSession;
-    MODSSL_D2I_SSL_SESSION_CONST unsigned char *pder = der;
+    unsigned int data_len;
     SSLModConfigRec *mc = myModConfig(s);
     DC_CTX *ctx = mc->tSessionCacheDataTable;
 
     /* Retrieve any corresponding session from the distributed cache context */
-    if (!DC_CTX_get_session(ctx, id, idlen, der, SSL_SESSION_MAX_DER,
-                            &der_len)) {
+    if (!DC_CTX_get_session(ctx, id, idlen, dest, *destlen, &data_len)) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "distributed scache 'get_session' MISS");
-        return NULL;
+        return FALSE;
     }
-    if (der_len > SSL_SESSION_MAX_DER) {
+    if (data_len > *destlen) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "distributed scache 'get_session' OVERFLOW");
-        return NULL;
-    }
-    pSession = d2i_SSL_SESSION(NULL, &pder, der_len);
-    if (!pSession) {
-        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "distributed scache 'get_session' CORRUPT");
-        return NULL;
+        return FALSE;
     }
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, "distributed scache 'get_session' HIT");
-    return pSession;
+    return TRUE;
 }
 
 static void ssl_scache_dc_remove(server_rec *s, UCHAR *id, int idlen, apr_pool_t *p)

diff --git a/modules/ssl/ssl_scache_memcache.c b/modules/ssl/ssl_scache_memcache.c
index 65b72585486e2a53cbe52396fd49e15e948ddd10..c59c5b3879e5efbe635878354deab74a924eabf5 100644 (file)
--- a/modules/ssl/ssl_scache_memcache.c
+++ b/modules/ssl/ssl_scache_memcache.c
@@ -163,8 +163,8 @@ static void ssl_scache_mc_kill(server_rec *s)
 
 }
 
-static char *mc_session_id2sz(unsigned char *id, int idlen,
-                               char *str, int strsize)
+static char *mc_session_id2sz(const unsigned char *id, int idlen,
+                              char *str, int strsize)
 {
     char *cp;
     int n;
@@ -207,57 +207,45 @@ static BOOL ssl_scache_mc_store(server_rec *s, UCHAR *id, int idlen,
     return TRUE;
 }
 
-static SSL_SESSION *ssl_scache_mc_retrieve(server_rec *s, UCHAR *id, int idlen,
-                                           apr_pool_t *p)
+static BOOL ssl_scache_mc_retrieve(server_rec *s, const UCHAR *id, int idlen,
+                                   unsigned char *dest, unsigned int *destlen,
+                                   apr_pool_t *p)
 {
-    SSL_SESSION *pSession;
-    MODSSL_D2I_SSL_SESSION_CONST unsigned char *pder;
     apr_size_t der_len;
-    char buf[MC_KEY_LEN];
+    char buf[MC_KEY_LEN], *der;
     char* strkey = NULL;
     apr_status_t rv;
 
     strkey = mc_session_id2sz(id, idlen, buf, sizeof(buf));
 
-    if(!strkey) {
+    if (!strkey) {
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                      "scache_mc: Key generation borked.");
-        return NULL;
+        return FALSE;
     }
 
-    rv = apr_memcache_getp(memctxt,  p, strkey,
-                           (char**)&pder, &der_len, NULL);
-
-    if (rv == APR_NOTFOUND) {
-        /* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                       "scache_mc: 'get_session' MISS"); */
-        return NULL;
-    }
+    /* ### this could do with a subpool, but _getp looks like it will
+     * eat memory like it's going out of fashion anyway. */
 
-    if (rv != APR_SUCCESS) {
-        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
-                     "scache_mc: 'get_session' FAIL");
-        return NULL;
+    rv = apr_memcache_getp(memctxt, p, strkey,
+                           &der, &der_len, NULL);
+    if (rv) {
+        if (rv != APR_NOTFOUND) {
+            ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
+                         "scache_mc: 'get_session' FAIL");
+        }
+        return FALSE;
     }
-
-    if (der_len > SSL_SESSION_MAX_DER) {
-        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
+    else if (der_len > *destlen) {
+        ap_log_error(APLOG_MARK, APLOG_ERR, rv, s,
                      "scache_mc: 'get_session' OVERFLOW");
-        return NULL;
-    }
-
-    pSession = d2i_SSL_SESSION(NULL, &pder, der_len);
-
-    if (!pSession) {
-        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
-                     "scache_mc: 'get_session' CORRUPT");
-        return NULL;
-    }
+        return FALSE;
+    }    
 
-    /* ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-        "scache_mc: 'get_session' HIT"); */
+    memcpy(dest, der, der_len);
+    *destlen = der_len;
 
-    return pSession;
+    return TRUE;
 }
 
 static void ssl_scache_mc_remove(server_rec *s, UCHAR *id, int idlen, apr_pool_t *p)

diff --git a/modules/ssl/ssl_scache_shmcb.c b/modules/ssl/ssl_scache_shmcb.c
index ec46cedd304281164651e1c0272f185fcff7f77b..b70ddcdb6b1af8da967a9dbbbaeea1b38b6881c8 100644 (file)
--- a/modules/ssl/ssl_scache_shmcb.c
+++ b/modules/ssl/ssl_scache_shmcb.c
@@ -80,8 +80,8 @@ typedef struct {
     unsigned int data_pos;
     /* size (most logic ignores this, we keep it only to minimise memcpy) */
     unsigned int data_used;
-    /* Optimisation to prevent ASN decoding unless a match is likely */
-    unsigned char s_id2;
+    /* length of the used data which contains the id */
+    unsigned int id_len;
     /* Used to mark explicitly-removed sessions */
     unsigned char removed;
 } SHMCBIndex;
@@ -115,6 +115,18 @@ typedef struct {
  * use.  Both ->idx_* values have a range of [0, header->index_num)
  *
  * Each in-use SHMCBIndex structure represents a single SSL session.
+ * The ID and data segment are stored consecutively in the subcache's
+ * cyclic data buffer.  The "Data" segment can thus be seen to 
+ * look like this, for example
+ *
+ * offset:  [ 0     1     2     3     4     5     6    ...
+ * contents:[ ID1   Data1       ID2   Data2       ID3  ...
+ *
+ * where the corresponding indices would look like:
+ *
+ * idx1 = { data_pos = 0, data_used = 3, id_len = 1, ...}
+ * idx2 = { data_pos = 3, data_used = 3, id_len = 1, ...}
+ * ...
  */
 
 /* This macro takes a pointer to the header and a zero-based index and returns
@@ -190,12 +202,42 @@ static void shmcb_cyclic_cton_memcpy(unsigned int buf_size, unsigned char *dest,
     }
 }
 
+/* A memcmp against a cyclic data buffer.  Compares SRC of length
+ * SRC_LEN data which begins at offset DEST_OFFSET in cyclic buffer
+ * DATA which is of size BUF_SIZE.  Got that?  Good. */
+static int shmcb_cyclic_memcmp(unsigned int buf_size, unsigned char *data,
+                               unsigned int dest_offset, 
+                               const unsigned char *src,
+                               unsigned int src_len)
+{
+    if (dest_offset + src_len < buf_size)
+        /* It be compared all in one go */
+        return memcmp(data + dest_offset, src, src_len);
+    else {
+        /* Compare the two splits */
+        int diff;
+        
+        diff = memcmp(data + dest_offset, src, buf_size - dest_offset);
+        if (diff) {
+            return diff;
+        }
+        return memcmp(data, src + buf_size - dest_offset,
+                      src_len + dest_offset - buf_size);
+    }
+}
+
+
 /* Prototypes for low-level subcache operations */
 static void shmcb_subcache_expire(server_rec *, SHMCBHeader *, SHMCBSubcache *);
-static BOOL shmcb_subcache_store(server_rec *, SHMCBHeader *, SHMCBSubcache *,
-                                 UCHAR *, unsigned int, UCHAR *, time_t);
-static SSL_SESSION *shmcb_subcache_retrieve(server_rec *, SHMCBHeader *, SHMCBSubcache *,
-                                            UCHAR *, unsigned int);
+static BOOL shmcb_subcache_store(server_rec *s, SHMCBHeader *header,
+                                 SHMCBSubcache *subcache, 
+                                 UCHAR *data, unsigned int data_len,
+                                 UCHAR *id, unsigned int id_len,
+                                 time_t expiry);
+static BOOL shmcb_subcache_retrieve(server_rec *, SHMCBHeader *, SHMCBSubcache *,
+                                    const UCHAR *id, unsigned int idlen,
+                                    UCHAR *data, unsigned int *datalen);
+                                    
 static BOOL shmcb_subcache_remove(server_rec *, SHMCBHeader *, SHMCBSubcache *,
                                  UCHAR *, unsigned int);
 
@@ -271,12 +313,13 @@ static void ssl_scache_shmcb_init(server_rec *s, apr_pool_t *p)
     /* Select the number of subcaches to create and how many indexes each
      * should contain based on the size of the memory (the header has already
      * been subtracted). Typical non-client-auth sslv3/tlsv1 sessions are
-     * around 150 bytes, so erring to division by 120 helps ensure we would
-     * exhaust data storage before index storage (except sslv2, where it's
-     * *slightly* the other way). From there, we select the number of subcaches
-     * to be a power of two, such that the number of indexes per subcache at
-     * least twice the number of subcaches. */
-    num_idx = (shm_segsize) / 120;
+     * around 180 bytes (148 bytes data and 32 bytes for the id), so
+     * erring to division by 150 helps ensure we would exhaust data
+     * storage before index storage (except sslv2, where it's
+     * *slightly* the other way). From there, we select the number of
+     * subcaches to be a power of two, such that the number of indexes
+     * per subcache at least twice the number of subcaches. */
+    num_idx = (shm_segsize) / 150;
     num_subcache = 256;
     while ((num_idx / num_subcache) < (2 * num_subcache))
         num_subcache /= 2;
@@ -370,7 +413,7 @@ static BOOL ssl_scache_shmcb_store(server_rec *s, UCHAR *id, int idlen,
         goto done;
     }
     if (!shmcb_subcache_store(s, header, subcache, encoded,
-                              len_encoded, id, timeout)) {
+                              len_encoded, id, idlen, timeout)) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
                      "can't store a session!");
         goto done;
@@ -384,35 +427,34 @@ done:
     return to_return;
 }
 
-static SSL_SESSION *ssl_scache_shmcb_retrieve(server_rec *s, UCHAR *id, int idlen,
-                                              apr_pool_t *p)
+static BOOL ssl_scache_shmcb_retrieve(server_rec *s, 
+                                      const UCHAR *id, int idlen,
+                                      unsigned char *dest, unsigned int *destlen,
+                                      apr_pool_t *p)
 {
     SSLModConfigRec *mc = myModConfig(s);
-    SSL_SESSION *pSession = NULL;
     SHMCBHeader *header = mc->tSessionCacheDataTable;
     SHMCBSubcache *subcache = SHMCB_MASK(header, id);
+    BOOL rv;
 
     ssl_mutex_on(s);
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                  "ssl_scache_shmcb_retrieve (0x%02x -> subcache %d)",
                  SHMCB_MASK_DBG(header, id));
-    if (idlen < 4) {
-        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s, "unusably short session_id provided "
-                "(%u bytes)", idlen);
-        goto done;
-    }
+
     /* Get the session corresponding to the session_id or NULL if it doesn't
      * exist (or is flagged as "removed"). */
-    pSession = shmcb_subcache_retrieve(s, header, subcache, id, idlen);
-    if (pSession)
+    rv = shmcb_subcache_retrieve(s, header, subcache, id, idlen,
+                                 dest, destlen);
+    if (rv)
         header->stat_retrieves_hit++;
     else
         header->stat_retrieves_miss++;
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                  "leaving ssl_scache_shmcb_retrieve successfully");
-done:
+
     ssl_mutex_off(s);
-    return pSession;
+    return rv;
 }
 
 static void ssl_scache_shmcb_remove(server_rec *s, UCHAR *id, int idlen, apr_pool_t *p)
@@ -564,16 +606,18 @@ static void shmcb_subcache_expire(server_rec *s, SHMCBHeader *header,
 static BOOL shmcb_subcache_store(server_rec *s, SHMCBHeader *header,
                                  SHMCBSubcache *subcache, 
                                  UCHAR *data, unsigned int data_len,
-                                 UCHAR *id, time_t expiry)
+                                 UCHAR *id, unsigned int id_len,
+                                 time_t expiry)
 {
-    unsigned int new_offset, new_idx;
+    unsigned int data_offset, new_idx, id_offset;
     SHMCBIndex *idx;
+    unsigned int total_len = id_len + data_len;
 
     /* Sanity check the input */
-    if ((data_len > header->subcache_data_size) || (data_len > SSL_SESSION_MAX_DER)) {
+    if (total_len > header->subcache_data_size) {
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
                      "inserting session larger (%d) than subcache data area (%d)",
-                     data_len, header->subcache_data_size);
+                     total_len, header->subcache_data_size);
         return FALSE;
     }
 
@@ -581,7 +625,7 @@ static BOOL shmcb_subcache_store(server_rec *s, SHMCBHeader *header,
     shmcb_subcache_expire(s, header, subcache);
 
     /* Loop until there is enough space to insert */
-    if (header->subcache_data_size - subcache->data_used < data_len
+    if (header->subcache_data_size - subcache->data_used < total_len
         || subcache->idx_used == header->index_num) {
         unsigned int loop = 0;
 
@@ -611,7 +655,7 @@ static BOOL shmcb_subcache_store(server_rec *s, SHMCBHeader *header,
             /* Loop admin */
             idx = idx2;
             loop++;
-        } while (header->subcache_data_size - subcache->data_used < data_len);
+        } while (header->subcache_data_size - subcache->data_used < total_len);
 
         ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                      "finished force-expire, subcache: idx_used=%d, "
@@ -628,11 +672,18 @@ static BOOL shmcb_subcache_store(server_rec *s, SHMCBHeader *header,
      * would make this stuff *MUCH* more efficient. Mind you, it's very
      * efficient right now because I'm ignoring this problem!!!
      */
+    /* Insert the id */
+    id_offset = SHMCB_CYCLIC_INCREMENT(subcache->data_pos, subcache->data_used,
+                                       header->subcache_data_size);
+    shmcb_cyclic_ntoc_memcpy(header->subcache_data_size,
+                             SHMCB_DATA(header, subcache), id_offset,
+                             id, id_len);
+    subcache->data_used += id_len;
     /* Insert the data */
-    new_offset = SHMCB_CYCLIC_INCREMENT(subcache->data_pos, subcache->data_used,
-                                        header->subcache_data_size);
+    data_offset = SHMCB_CYCLIC_INCREMENT(subcache->data_pos, subcache->data_used,
+                                         header->subcache_data_size);
     shmcb_cyclic_ntoc_memcpy(header->subcache_data_size,
-                             SHMCB_DATA(header, subcache), new_offset,
+                             SHMCB_DATA(header, subcache), data_offset,
                              data, data_len);
     subcache->data_used += data_len;
     /* Insert the index */
@@ -640,13 +691,14 @@ static BOOL shmcb_subcache_store(server_rec *s, SHMCBHeader *header,
                                      header->index_num);
     idx = SHMCB_INDEX(subcache, new_idx);
     idx->expires = expiry;
-    idx->data_pos = new_offset;
-    idx->data_used = data_len;
-    idx->s_id2 = id[1];
+    idx->data_pos = id_offset;
+    idx->data_used = total_len;
+    idx->id_len = id_len;
     idx->removed = 0;
     subcache->idx_used++;
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                 "insert happened at idx=%d, data=%d", new_idx, new_offset);
+                 "insert happened at idx=%d, data=(%u:%u)", new_idx, 
+                 id_offset, data_offset);
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                  "finished insert, subcache: idx_pos/idx_used=%d/%d, "
                  "data_pos/data_used=%d/%d",
@@ -655,9 +707,10 @@ static BOOL shmcb_subcache_store(server_rec *s, SHMCBHeader *header,
     return TRUE;
 }
 
-static SSL_SESSION *shmcb_subcache_retrieve(server_rec *s, SHMCBHeader *header,
-                                            SHMCBSubcache *subcache, UCHAR *id,
-                                            unsigned int idlen)
+static BOOL shmcb_subcache_retrieve(server_rec *s, SHMCBHeader *header,
+                                    SHMCBSubcache *subcache, 
+                                    const UCHAR *id, unsigned int idlen,
+                                    UCHAR *dest, unsigned int *destlen)
 {
     unsigned int pos;
     unsigned int loop = 0;
@@ -669,39 +722,31 @@ static SSL_SESSION *shmcb_subcache_retrieve(server_rec *s, SHMCBHeader *header,
     while (loop < subcache->idx_used) {
         SHMCBIndex *idx = SHMCB_INDEX(subcache, pos);
 
-        /* Only consider 'idx' if;
-         * (a) the s_id2 byte matches
-         * (b) the "removed" flag isn't set.
-         */
-        if ((idx->s_id2 == id[1]) && !idx->removed) {
-            SSL_SESSION *pSession;
-            unsigned char *s_id;
-            unsigned int s_idlen;
-            unsigned char tempasn[SSL_SESSION_MAX_DER];
-            MODSSL_D2I_SSL_SESSION_CONST unsigned char *ptr = tempasn;
-
+        /* 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) {
             ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                         "possible match at idx=%d, data=%d", pos, idx->data_pos);
-            /* Copy the data */
+                         "match at idx=%d, data=%d", pos, idx->data_pos);
+            unsigned int data_offset;
+
+            /* Find the offset of the data segment, after the id */
+            data_offset = SHMCB_CYCLIC_INCREMENT(idx->data_pos, 
+                                                 idx->id_len,
+                                                 header->subcache_data_size);
+
+            *destlen = idx->data_used - idx->id_len;
+
+            /* Copy out the data */
             shmcb_cyclic_cton_memcpy(header->subcache_data_size,
-                                     tempasn, SHMCB_DATA(header, subcache),
-                                     idx->data_pos, idx->data_used);
-            /* Decode the session */
-            pSession = d2i_SSL_SESSION(NULL, &ptr, idx->data_used);
-            if (!pSession) {
-                ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
-                             "shmcb_subcache_retrieve internal error");
-                return NULL;
-            }
-            s_id = SSL_SESSION_get_session_id(pSession);
-            s_idlen = SSL_SESSION_get_session_id_length(pSession);
-            if (s_idlen == idlen && memcmp(s_id, id, idlen) == 0) {
-                /* Found the matching session */
-                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
-                             "shmcb_subcache_retrieve returning matching session");
-                return pSession;
-            }
-            SSL_SESSION_free(pSession);
+                                     dest, SHMCB_DATA(header, subcache),
+                                     data_offset, *destlen);
+
+            return TRUE;
         }
         /* Increment */
         loop++;
@@ -710,7 +755,7 @@ static SSL_SESSION *shmcb_subcache_retrieve(server_rec *s, SHMCBHeader *header,
 
     ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                  "shmcb_subcache_retrieve found no match");
-    return NULL;
+    return FALSE;
 }
 
 static BOOL shmcb_subcache_remove(server_rec *s, SHMCBHeader *header,
@@ -730,38 +775,20 @@ static BOOL shmcb_subcache_remove(server_rec *s, SHMCBHeader *header,
     pos = subcache->idx_pos;
     while (!to_return && (loop < subcache->idx_used)) {
         SHMCBIndex *idx = SHMCB_INDEX(subcache, pos);
-        /* Only consider 'idx' if the s_id2 byte matches and it's not already
-         * removed - easiest way to avoid costly ASN decodings. */
-        if ((idx->s_id2 == id[1]) && !idx->removed) {
-            SSL_SESSION *pSession;
-            unsigned char *s_id;
-            unsigned int s_idlen;
-            unsigned char tempasn[SSL_SESSION_MAX_DER];
-            MODSSL_D2I_SSL_SESSION_CONST unsigned char *ptr = tempasn;
 
+        /* Only consider 'idx' if the id matches, and the "removed"
+         * flag isn't set. */
+        if (idx->id_len == idlen
+            && shmcb_cyclic_memcmp(header->subcache_data_size,
+                                   SHMCB_DATA(header, subcache),
+                                   idx->data_pos, id, idx->id_len) == 0) {
             ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                          "possible match at idx=%d, data=%d", pos, idx->data_pos);
-            /* Copy the data */
-            shmcb_cyclic_cton_memcpy(header->subcache_data_size,
-                                     tempasn, SHMCB_DATA(header, subcache),
-                                     idx->data_pos, idx->data_used);
-            /* Decode the session */
-            pSession = d2i_SSL_SESSION(NULL, &ptr, idx->data_used);
-            if (!pSession) {
-                ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
-                             "shmcb_subcache_remove internal error");
-                return FALSE;
-            }
-            s_id = SSL_SESSION_get_session_id(pSession);
-            s_idlen = SSL_SESSION_get_session_id_length(pSession);
-            if (s_idlen == idlen && memcmp(s_id, id, idlen) == 0) {
-                /* Found the matching session, remove it quietly. */
-                idx->removed = 1;
-                to_return = TRUE;
-                ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
+            /* Found the matching session, remove it quietly. */
+            idx->removed = 1;
+            to_return = TRUE;
+            ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
                              "shmcb_subcache_remove removing matching session");
-            }
-            SSL_SESSION_free(pSession);
         }
         /* Increment */
         loop++;