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=630307&view=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=630307&r1=630306&r2=630307&view=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=630307&r1=630306&r2=630307&view=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 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;
Shouldn't we do
*destlen = data_len;
here?
+ return TRUE;
}
static void ssl_scache_dc_remove(server_rec *s, UCHAR *id, int idlen, apr_pool_t *p)
Modified: httpd/httpd/trunk/modules/ssl/ssl_scache_shmcb.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/ssl/ssl_scache_shmcb.c?rev=630307&r1=630306&r2=630307&view=diff
==============================================================================
--- 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
@@ -669,39 +722,31 @@
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) {
Where do you check for the removed flag?
@@ -730,38 +775,20 @@
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) {
Where do you check for the removed flag?
Regards
Rüdiger
