More efficient collection persistance and deletion on retrieval. See #345 and #426.

This commit is contained in:
brectanus 2007-12-14 19:53:23 +00:00
parent 4c11791a94
commit 5065852dfe
4 changed files with 118 additions and 149 deletions

View File

@ -1,6 +1,8 @@
13 Dec 2007 - 2.5.0-rc1 14 Dec 2007 - 2.5.0-rc1
----------------------- -----------------------
* More efficient collection persistance.
* Fixed t:escapeSeqDecode to better follow ANSI C escapes. * Fixed t:escapeSeqDecode to better follow ANSI C escapes.
* Added t:jsDecode to decode JavScript escape sequences. * Added t:jsDecode to decode JavScript escape sequences.

View File

@ -46,7 +46,7 @@ static apr_table_t *collection_unpack(modsec_rec *msr, char *blob, unsigned int
blob_offset += var->value_len; blob_offset += var->value_len;
var->value_len--; var->value_len--;
if (log_vars) { if (log_vars && (msr->txcfg->debuglog_level >= 9)) {
msr_log(msr, 9, "Read variable: name \"%s\", value \"%s\".", msr_log(msr, 9, "Read variable: name \"%s\", value \"%s\".",
log_escape_ex(msr->mp, var->name, var->name_len), log_escape_ex(msr->mp, var->name, var->name_len),
log_escape_ex(msr->mp, var->value, var->value_len)); log_escape_ex(msr->mp, var->value, var->value_len));
@ -78,27 +78,30 @@ apr_table_t *collection_retrieve(modsec_rec *msr, const char *col_name,
if (msr->txcfg->data_dir == NULL) { if (msr->txcfg->data_dir == NULL) {
msr_log(msr, 1, "Unable to retrieve collection (name \"%s\", key \"%s\"). Use " msr_log(msr, 1, "Unable to retrieve collection (name \"%s\", key \"%s\"). Use "
"SecDataDir to define data directory first.", log_escape(msr->mp, col_name), "SecDataDir to define data directory first.", log_escape(msr->mp, col_name),
log_escape(msr->mp, col_key)); log_escape_ex(msr->mp, col_key, col_key_len));
return NULL; return NULL;
} }
dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", col_name, NULL); dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", col_name, NULL);
key.dptr = (char *)col_key;
key.dsize = col_key_len + 1;
rc = apr_sdbm_open(&dbm, dbm_filename, APR_READ | APR_SHARELOCK, rc = apr_sdbm_open(&dbm, dbm_filename, APR_READ | APR_SHARELOCK,
CREATEMODE, msr->mp); CREATEMODE, msr->mp);
if (rc != APR_SUCCESS) { if (rc != APR_SUCCESS) {
return NULL; return NULL;
} }
key.dptr = (char *)col_key;
key.dsize = col_key_len + 1;
value = (apr_sdbm_datum_t *)apr_pcalloc(msr->mp, sizeof(apr_sdbm_datum_t)); value = (apr_sdbm_datum_t *)apr_pcalloc(msr->mp, sizeof(apr_sdbm_datum_t));
rc = apr_sdbm_fetch(dbm, value, key); rc = apr_sdbm_fetch(dbm, value, key);
apr_sdbm_close(dbm);
if (rc != APR_SUCCESS) { if (rc != APR_SUCCESS) {
apr_sdbm_close(dbm);
msr_log(msr, 1, "Failed to read from DBM file \"%s\": %s", log_escape(msr->mp, msr_log(msr, 1, "Failed to read from DBM file \"%s\": %s", log_escape(msr->mp,
dbm_filename), get_apr_error(msr->mp, rc)); dbm_filename), get_apr_error(msr->mp, rc));
apr_sdbm_close(dbm);
return NULL; return NULL;
} }
@ -107,6 +110,11 @@ apr_table_t *collection_retrieve(modsec_rec *msr, const char *col_name,
return NULL; return NULL;
} }
/* ENH Need expiration (and perhaps other metadata) accessible in blob
* form so we can determine if we need to convert to a table. This will
* save some cycles.
*/
/* Transform raw data into a table. */ /* Transform raw data into a table. */
col = collection_unpack(msr, value->dptr, value->dsize, 1); col = collection_unpack(msr, value->dptr, value->dsize, 1);
if (col == NULL) return NULL; if (col == NULL) return NULL;
@ -121,38 +129,61 @@ apr_table_t *collection_retrieve(modsec_rec *msr, const char *col_name,
int expiry_time = atoi(var->value); int expiry_time = atoi(var->value);
if (expiry_time <= apr_time_sec(msr->request_time)) { if (expiry_time <= apr_time_sec(msr->request_time)) {
// TODO Why dup this? char *key_to_expire = te[i].key;
char *key_to_expire = apr_pstrdup(msr->mp, te[i].key);
/* Do not remove the record itself. */ /* Done early if the col expired */
if (strcmp(te[i].key, "__expire_KEY") == 0) { if (strcmp(key_to_expire, "__expire_KEY") == 0) {
expired = 1; expired = 1;
continue;
} }
if (msr->txcfg->debuglog_level >= 9) {
msr_log(msr, 9, "Removing key \"%s\" from collection.", key_to_expire + 9); msr_log(msr, 9, "Removing key \"%s\" from collection.", key_to_expire + 9);
msr_log(msr, 9, "Removing key \"%s\" from collection.", key_to_expire);
}
apr_table_unset(col, key_to_expire + 9); apr_table_unset(col, key_to_expire + 9);
msr_log(msr, 9, "Removing key \"%s\" from collection.", key_to_expire);
apr_table_unset(col, key_to_expire); apr_table_unset(col, key_to_expire);
msr_log(msr, 4, "Removed expired variable \"%s\".", key_to_expire + 9); if (msr->txcfg->debuglog_level >= 4) {
msr_log(msr, 4, "Removed expired variable \"%s\".", key_to_expire + 9);
}
break; break;
} }
} }
} }
} while(i != arr->nelts); } while(!expired && (i != arr->nelts));
/* Set IS_EXPIRED if expired */ /* Delete the collection if the variable "KEY" does not exist.
if (expired) { *
msc_string *var = (msc_string *)apr_table_get(col, "IS_EXPIRED"); * ENH It would probably be more efficient to hold the DBM
if (var == NULL) { * open until we determine if it needs deleted than to open a second
var = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string)); * time.
var->name = "IS_EXPIRED"; */
var->name_len = strlen(var->name); if (apr_table_get(col, "KEY") == NULL) {
rc = apr_sdbm_open(&dbm, dbm_filename, APR_CREATE | APR_WRITE | APR_SHARELOCK,
CREATEMODE, msr->mp);
if (rc != APR_SUCCESS) {
msr_log(msr, 1, "Failed to access DBM file \"%s\": %s",
log_escape(msr->mp, dbm_filename), get_apr_error(msr->mp, rc));
return NULL;
} }
if (var != NULL) {
var->value = "1"; rc = apr_sdbm_delete(dbm, key);
var->value_len = strlen(var->value);
apr_sdbm_close(dbm);
if (rc != APR_SUCCESS) {
msr_log(msr, 1, "Failed deleting collection (name \"%s\", "
"key \"%s\"): %s", log_escape(msr->mp, col_name),
log_escape_ex(msr->mp, col_key, col_key_len), get_apr_error(msr->mp, rc));
return NULL;
} }
if (expired && (msr->txcfg->debuglog_level >= 9)) {
msr_log(msr, 9, "Collection expired (name \"%s\", key \"%s\").", col_name, log_escape_ex(msr->mp, col_key, col_key_len));
}
if (msr->txcfg->debuglog_level >= 4) {
msr_log(msr, 4, "Deleted collection (name \"%s\", key \"%s\").",
log_escape(msr->mp, col_name), log_escape_ex(msr->mp, col_key, col_key_len));
}
return NULL;
} }
/* Update UPDATE_RATE */ /* Update UPDATE_RATE */
@ -171,13 +202,12 @@ apr_table_t *collection_retrieve(modsec_rec *msr, const char *col_name,
} else { } else {
apr_time_t td; apr_time_t td;
counter = atoi(var->value); counter = atoi(var->value);
var = (msc_string *)apr_table_get(col, "UPDATE_RATE");
if (var == NULL) { /* UPDATE_RATE is removed on store, so we add it back here */
var = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string)); var = (msc_string *)apr_pcalloc(msr->mp, sizeof(msc_string));
var->name = "UPDATE_RATE"; var->name = "UPDATE_RATE";
var->name_len = strlen(var->name); var->name_len = strlen(var->name);
apr_table_setn(col, var->name, (void *)var); apr_table_setn(col, var->name, (void *)var);
}
/* NOTE: No rate if there has been no time elapsed */ /* NOTE: No rate if there has been no time elapsed */
td = (apr_time_sec(apr_time_now()) - create_time); td = (apr_time_sec(apr_time_now()) - create_time);
@ -193,10 +223,10 @@ apr_table_t *collection_retrieve(modsec_rec *msr, const char *col_name,
} }
} }
apr_sdbm_close(dbm); if (msr->txcfg->debuglog_level >= 4) {
msr_log(msr, 4, "Retrieved collection (name \"%s\", key \"%s\").",
msr_log(msr, 4, "Retrieved collection (name \"%s\", key \"%s\", expired \"%d\").", log_escape(msr->mp, col_name), log_escape_ex(msr->mp, col_key, col_key_len));
log_escape(msr->mp, col_name), log_escape(msr->mp, col_key), expired); }
return col; return col;
} }
@ -230,84 +260,17 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
if (msr->txcfg->data_dir == NULL) { if (msr->txcfg->data_dir == NULL) {
msr_log(msr, 1, "Unable to store collection (name \"%s\", key \"%s\"). Use " msr_log(msr, 1, "Unable to store collection (name \"%s\", key \"%s\"). Use "
"SecDataDir to define data directory first.", "SecDataDir to define data directory first.",
log_escape(msr->mp, var_name->value), log_escape(msr->mp, var_key->value)); log_escape_ex(msr->mp, var_name->value, var_name->value_len), log_escape_ex(msr->mp, var_key->value, var_key->value_len));
return -1; return -1;
} }
dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", var_name->value, NULL); dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", var_name->value, NULL);
/* Remove expired variables. */ /* Delete IS_NEW on store. */
do { apr_table_unset(col, "IS_NEW");
arr = apr_table_elts(col);
te = (apr_table_entry_t *)arr->elts;
for (i = 0; i < arr->nelts; i++) {
if (strncmp(te[i].key, "__expire_", 9) == 0) {
msc_string *var = (msc_string *)te[i].val;
int expiry_time = atoi(var->value);
/* Do not remove the record itself. */ /* Delete UPDATE_RATE on store to save space as it is calculated */
if (strcmp(te[i].key, "__expire_KEY") == 0) continue; apr_table_unset(col, "UPDATE_RATE");
if (expiry_time <= apr_time_sec(msr->request_time)) {
char *key_to_expire = apr_pstrdup(msr->mp, te[i].key);
msr_log(msr, 9, "Removing key \"%s\" from collection.", key_to_expire + 9);
apr_table_unset(col, key_to_expire + 9);
msr_log(msr, 9, "Removing key \"%s\" from collection.", key_to_expire);
apr_table_unset(col, key_to_expire);
msr_log(msr, 4, "Removed expired variable \"%s\".", key_to_expire + 9);
break;
}
}
}
} while(i != arr->nelts);
/* Delete the collection if the variable "KEY" does not exist. */
if (apr_table_get(col, "KEY") == NULL) {
rc = apr_sdbm_open(&dbm, dbm_filename, APR_CREATE | APR_WRITE | APR_SHARELOCK,
CREATEMODE, msr->mp);
if (rc != APR_SUCCESS) {
msr_log(msr, 1, "Failed to access DBM file \"%s\": %s",
log_escape(msr->mp, dbm_filename), get_apr_error(msr->mp, rc));
return -1;
}
key.dptr = var_key->value;
key.dsize = var_key->value_len + 1;
rc = apr_sdbm_delete(dbm, key);
if (rc != APR_SUCCESS) {
msr_log(msr, 1, "Failed deleting collection (name \"%s\", "
"key \"%s\"): %s", log_escape(msr->mp, var_name->value),
log_escape(msr->mp, var_key->value), get_apr_error(msr->mp, rc));
apr_sdbm_close(dbm);
return -1;
}
msr_log(msr, 4, "Deleted collection (name \"%s\", key \"%s\").",
log_escape(msr->mp, var_name->value), log_escape(msr->mp, var_key->value));
apr_sdbm_close(dbm);
return 1;
}
/* Set IS_NEW to 0 on store. */
{
msc_string *var = (msc_string *)apr_table_get(col, "IS_NEW");
if (var != NULL) {
var->value = "0";
var->value_len = strlen(var->value);
}
}
/* Set IS_EXPIRED to 0 on store. */
{
msc_string *var = (msc_string *)apr_table_get(col, "IS_EXPIRED");
if (var != NULL) {
var->value = "0";
var->value_len = strlen(var->value);
}
}
/* Update the timeout value. */ /* Update the timeout value. */
{ {
@ -351,6 +314,11 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
var->value_len = strlen(var->value); var->value_len = strlen(var->value);
} }
/* ENH Make the expiration timestamp accessible in blob form so that
* it is easier/faster to determine expiration without having to
* convert back to table form
*/
/* Calculate the size first. */ /* Calculate the size first. */
blob_size = 3 + 2; blob_size = 3 + 2;
arr = apr_table_elts(col); arr = apr_table_elts(col);
@ -403,9 +371,11 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
blob[blob_offset + 2 + len - 1] = '\0'; blob[blob_offset + 2 + len - 1] = '\0';
blob_offset += 2 + len; blob_offset += 2 + len;
msr_log(msr, 9, "Wrote variable: name \"%s\", value \"%s\".", if (msr->txcfg->debuglog_level >= 9) {
log_escape_ex(msr->mp, var->name, var->name_len), msr_log(msr, 9, "Wrote variable: name \"%s\", value \"%s\".",
log_escape_ex(msr->mp, var->value, var->value_len)); log_escape_ex(msr->mp, var->name, var->name_len),
log_escape_ex(msr->mp, var->value, var->value_len));
}
} }
blob[blob_offset] = 0; blob[blob_offset] = 0;
@ -414,6 +384,12 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
/* And, finally, store it. */ /* And, finally, store it. */
dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", var_name->value, NULL); dbm_filename = apr_pstrcat(msr->mp, msr->txcfg->data_dir, "/", var_name->value, NULL);
key.dptr = var_key->value;
key.dsize = var_key->value_len + 1;
value.dptr = (char *)blob;
value.dsize = blob_size;
rc = apr_sdbm_open(&dbm, dbm_filename, APR_CREATE | APR_WRITE | APR_SHARELOCK, rc = apr_sdbm_open(&dbm, dbm_filename, APR_CREATE | APR_WRITE | APR_SHARELOCK,
CREATEMODE, msr->mp); CREATEMODE, msr->mp);
if (rc != APR_SUCCESS) { if (rc != APR_SUCCESS) {
@ -422,24 +398,20 @@ int collection_store(modsec_rec *msr, apr_table_t *col) {
return -1; return -1;
} }
key.dptr = var_key->value;
key.dsize = var_key->value_len + 1;
value.dptr = (char *)blob;
value.dsize = blob_size;
rc = apr_sdbm_store(dbm, key, value, APR_SDBM_REPLACE); rc = apr_sdbm_store(dbm, key, value, APR_SDBM_REPLACE);
apr_sdbm_close(dbm);
if (rc != APR_SUCCESS) { if (rc != APR_SUCCESS) {
msr_log(msr, 1, "Failed to write to DBM file \"%s\": %s", dbm_filename, msr_log(msr, 1, "Failed to write to DBM file \"%s\": %s", dbm_filename,
get_apr_error(msr->mp, rc)); get_apr_error(msr->mp, rc));
apr_sdbm_close(dbm);
return -1; return -1;
} }
msr_log(msr, 4, "Persisted collection (name \"%s\", key \"%s\").", if (msr->txcfg->debuglog_level >= 4) {
log_escape(msr->mp, var_name->value), log_escape(msr->mp, var_key->value)); msr_log(msr, 4, "Persisted collection (name \"%s\", key \"%s\").",
log_escape_ex(msr->mp, var_name->value, var_name->value_len), log_escape_ex(msr->mp, var_key->value, var_key->value_len));
apr_sdbm_close(dbm); }
return 0; return 0;
} }
@ -490,14 +462,16 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
*/ */
rc = apr_sdbm_firstkey(dbm, &key); rc = apr_sdbm_firstkey(dbm, &key);
while(rc == APR_SUCCESS) { while(rc == APR_SUCCESS) {
char *s = apr_pstrmemdup(msr->mp, key.dptr, key.dsize); char *s = apr_pstrmemdup(msr->mp, key.dptr, key.dsize - 1);
*(char **)apr_array_push(keys_arr) = s; *(char **)apr_array_push(keys_arr) = s;
rc = apr_sdbm_nextkey(dbm, &key); rc = apr_sdbm_nextkey(dbm, &key);
} }
apr_sdbm_unlock(dbm); apr_sdbm_unlock(dbm);
msr_log(msr, 9, "Found %d record(s) in file \"%s\".", keys_arr->nelts, if (msr->txcfg->debuglog_level >= 9) {
log_escape(msr->mp, dbm_filename)); msr_log(msr, 9, "Found %d record(s) in file \"%s\".", keys_arr->nelts,
log_escape(msr->mp, dbm_filename));
}
/* Now retrieve the entires one by one. */ /* Now retrieve the entires one by one. */
keys = (char **)keys_arr->elts; keys = (char **)keys_arr->elts;
@ -519,6 +493,7 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
col = collection_unpack(msr, value.dptr, value.dsize, 0); col = collection_unpack(msr, value.dptr, value.dsize, 0);
if (col == NULL) { if (col == NULL) {
apr_sdbm_close(dbm);
return -1; return -1;
} }
@ -526,25 +501,30 @@ int collections_remove_stale(modsec_rec *msr, const char *col_name) {
if (var == NULL) { if (var == NULL) {
msr_log(msr, 1, "Collection cleanup discovered entry with no " msr_log(msr, 1, "Collection cleanup discovered entry with no "
"__expire_KEY (name \"%s\", key \"%s\").", "__expire_KEY (name \"%s\", key \"%s\").",
log_escape(msr->mp, col_name), log_escape(msr->mp, key.dptr)); log_escape(msr->mp, col_name), log_escape_ex(msr->mp, key.dptr, key.dsize - 1));
} else { } else {
unsigned int expiry_time = atoi(var->value); unsigned int expiry_time = atoi(var->value);
msr_log(msr, 9, "Record (name \"%s\", key \"%s\") set to expire in %" APR_TIME_T_FMT " seconds.", if (msr->txcfg->debuglog_level >= 9) {
log_escape(msr->mp, col_name), log_escape(msr->mp, key.dptr), msr_log(msr, 9, "Record (name \"%s\", key \"%s\") set to expire in %" APR_TIME_T_FMT " seconds.",
expiry_time - now); log_escape(msr->mp, col_name), log_escape_ex(msr->mp, key.dptr, key.dsize - 1),
expiry_time - now);
}
if (expiry_time <= now) { if (expiry_time <= now) {
rc = apr_sdbm_delete(dbm, key); rc = apr_sdbm_delete(dbm, key);
if (rc != APR_SUCCESS) { if (rc != APR_SUCCESS) {
msr_log(msr, 1, "Failed deleting collection (name \"%s\", " msr_log(msr, 1, "Failed deleting collection (name \"%s\", "
"key \"%s\"): %s", log_escape(msr->mp, col_name), "key \"%s\"): %s", log_escape(msr->mp, col_name),
log_escape(msr->mp, key.dptr), get_apr_error(msr->mp, rc)); log_escape_ex(msr->mp, key.dptr, key.dsize - 1), get_apr_error(msr->mp, rc));
apr_sdbm_close(dbm);
return -1; return -1;
} }
msr_log(msr, 4, "Removed stale collection (name \"%s\", " if (msr->txcfg->debuglog_level >= 4) {
"key \"%s\").", log_escape(msr->mp, col_name), msr_log(msr, 4, "Removed stale collection (name \"%s\", "
log_escape(msr->mp, key.dptr)); "key \"%s\").", log_escape(msr->mp, col_name),
log_escape_ex(msr->mp, key.dptr, key.dsize - 1));
}
} }
} }
} else { } else {

View File

@ -1330,14 +1330,6 @@ static apr_status_t init_collection(modsec_rec *msr, const char *real_col_name,
var->value = "1"; var->value = "1";
var->value_len = strlen(var->value); var->value_len = strlen(var->value);
apr_table_setn(table, var->name, (void *)var); apr_table_setn(table, var->name, (void *)var);
/* It has not yet expired. */
var = apr_pcalloc(msr->mp, sizeof(msc_string));
var->name = "IS_EXPIRED";
var->name_len = strlen(var->name);
var->value = "0";
var->value_len = strlen(var->value);
apr_table_setn(table, var->name, (void *)var);
} }
/* Add the collection to the list. */ /* Add the collection to the list. */

View File

@ -3,7 +3,7 @@
<title>ModSecurity Reference Manual</title> <title>ModSecurity Reference Manual</title>
<articleinfo> <articleinfo>
<releaseinfo>Version 2.5.0-rc1/ (December 13, 2007)</releaseinfo> <releaseinfo>Version 2.5.0-rc1/ (December 14, 2007)</releaseinfo>
<copyright> <copyright>
<year>2004-2007</year> <year>2004-2007</year>
@ -3943,11 +3943,6 @@ SecRule REQUEST_URI "^/cgi-bin/script\.pl" \
the creation of the collection.</para> the creation of the collection.</para>
</listitem> </listitem>
<listitem>
<para><literal moreinfo="none">IS_EXPIRED</literal> - set to 1 if
the collection is expired otherwise set to 0.</para>
</listitem>
<listitem> <listitem>
<para><literal moreinfo="none">IS_NEW</literal> - set to 1 if the <para><literal moreinfo="none">IS_NEW</literal> - set to 1 if the
collection is new (not yet persisted) otherwise set to 0.</para> collection is new (not yet persisted) otherwise set to 0.</para>