From f3bcab31baf7c7ab2b825fe27721a10c21dd5696 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Thu, 12 Aug 2021 16:23:11 +0200 Subject: [PATCH 01/12] detect: suppress scan-build warning --- src/detect-engine.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/detect-engine.c b/src/detect-engine.c index 032731ef3680..58e039a6fe78 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -1078,7 +1078,9 @@ void InspectionBufferInit(InspectionBuffer *buffer, uint32_t initial_size) void InspectionBufferSetupMulti(InspectionBuffer *buffer, const DetectEngineTransforms *transforms, const uint8_t *data, const uint32_t data_len) { +#ifdef DEBUG_VALIDATION DEBUG_VALIDATE_BUG_ON(!buffer->multi); +#endif buffer->inspect = buffer->orig = data; buffer->inspect_len = buffer->orig_len = data_len; buffer->len = 0; @@ -1090,7 +1092,9 @@ void InspectionBufferSetupMulti(InspectionBuffer *buffer, const DetectEngineTran void InspectionBufferSetup(DetectEngineThreadCtx *det_ctx, const int list_id, InspectionBuffer *buffer, const uint8_t *data, const uint32_t data_len) { +#ifdef DEBUG_VALIDATION DEBUG_VALIDATE_BUG_ON(buffer->multi); +#endif if (buffer->inspect == NULL) { #ifdef UNITTESTS if (det_ctx && list_id != -1) From d6c6a918a88fa78016c62fe42fe1a9182dbcf391 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 18 Feb 2020 15:01:03 +0100 Subject: [PATCH 02/12] util: adds util function SCBufferCmp Compares two buffers with their two sizes --- src/util-memcmp.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/util-memcmp.h b/src/util-memcmp.h index f1e1fdd63b49..61eeca053e59 100644 --- a/src/util-memcmp.h +++ b/src/util-memcmp.h @@ -377,5 +377,15 @@ static inline int SCMemcmpLowercase(const void *s1, const void *s2, size_t len) #endif /* SIMD */ +static inline int SCBufferCmp(const void *s1, size_t len1, const void *s2, size_t len2) +{ + if (len1 == len2) { + return SCMemcmp(s1, s2, len1); + } else if (len1 < len2) { + return -1; + } + return 1; +} + #endif /* __UTIL_MEMCMP_H__ */ From 470241c1d480a47552a085b05c82f6addee940de Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 18 Feb 2021 11:44:24 +0100 Subject: [PATCH 03/12] util: export Djb2 hash string function --- src/util-hash-string.c | 17 ++++++++++------- src/util-hash-string.h | 1 + 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/util-hash-string.c b/src/util-hash-string.c index ceb1b301a303..923cdde85caa 100644 --- a/src/util-hash-string.c +++ b/src/util-hash-string.c @@ -19,19 +19,22 @@ #include "util-hash-string.h" /* djb2 string hashing */ -uint32_t StringHashFunc(HashTable *ht, void *data, uint16_t datalen) +uint32_t StringHashDjb2(uint8_t *data, uint32_t datalen) { uint32_t hash = 5381; - int c; - - while ((c = *(char *)data++)) + for (uint32_t i = 0; i < datalen; i++) { + uint32_t c = data[i]; hash = ((hash << 5) + hash) + c; /* hash * 33 + c */ - - hash = hash % ht->array_size; - + } return hash; } +/* djb2 string hashing */ +uint32_t StringHashFunc(HashTable *ht, void *data, uint16_t datalen) +{ + return StringHashDjb2(data, datalen) % ht->array_size; +} + char StringHashCompareFunc(void *data1, uint16_t datalen1, void *data2, uint16_t datalen2) { diff --git a/src/util-hash-string.h b/src/util-hash-string.h index 263b738d7529..a08874b19fd8 100644 --- a/src/util-hash-string.h +++ b/src/util-hash-string.h @@ -18,6 +18,7 @@ #ifndef __UTIL_HASH_STRING_H__ #define __UTIL_HASH_STRING_H__ +uint32_t StringHashDjb2(uint8_t *data, uint32_t datalen); uint32_t StringHashFunc(HashTable *ht, void *data, uint16_t datalen); char StringHashCompareFunc(void *data1, uint16_t datalen1, void *data2, uint16_t datalen2); From e957ca8ca14bb129d69a5b5d29e44c79626f2296 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 18 Feb 2021 15:03:36 +0100 Subject: [PATCH 04/12] http/range: reassemble files from different flows with range adds a container, ie a thread safe hash table whose key is the filename keep a tree of unordered ranges, up to a memcap limit adds HTPFileOpenWithRange to handle like HTPFileOpen if there is a range : open 2 files, one for the whole reassembled, and one only for the current range --- rules/http-events.rules | 4 +- src/Makefile.am | 2 + src/app-layer-htp-file.c | 130 +++++++++-- src/app-layer-htp-file.h | 3 +- src/app-layer-htp-range.c | 453 ++++++++++++++++++++++++++++++++++++++ src/app-layer-htp-range.h | 97 ++++++++ src/app-layer-htp.c | 165 ++++++-------- src/app-layer-htp.h | 4 + src/flow-manager.c | 2 + src/suricata.c | 3 + src/util-thash.c | 2 +- src/util-thash.h | 1 + suricata.yaml.in | 7 + 13 files changed, 756 insertions(+), 117 deletions(-) create mode 100644 src/app-layer-htp-range.c create mode 100644 src/app-layer-htp-range.h diff --git a/rules/http-events.rules b/rules/http-events.rules index 023728c9aa9d..90e08e7d3628 100644 --- a/rules/http-events.rules +++ b/rules/http-events.rules @@ -83,4 +83,6 @@ alert http any any -> any any (msg:"SURICATA HTTP compression bomb"; flow:establ alert http any any -> any any (msg:"SURICATA HTTP too many warnings"; flow:established; app-layer-event:http.too_many_warnings; flowint:http.anomaly.count,+,1; classtype:protocol-command-decode; sid:2221050; rev:1;) -# next sid 2221051 +alert http any any -> any any (msg:"SURICATA HTTP invalid Range header value"; flow:established; app-layer-event:http.range_invalid; flowint:http.anomaly.count,+,1; classtype:protocol-command-decode; sid:2221051; rev:1;) + +# next sid 2221052 diff --git a/src/Makefile.am b/src/Makefile.am index 4965809dd58b..bdde102a9854 100755 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -30,6 +30,7 @@ noinst_HEADERS = \ app-layer-htp.h \ app-layer-htp-libhtp.h \ app-layer-htp-mem.h \ + app-layer-htp-range.h \ app-layer-htp-xff.h \ app-layer-http2.h \ app-layer-ike.h \ @@ -611,6 +612,7 @@ libsuricata_c_a_SOURCES = \ app-layer-htp-file.c \ app-layer-htp-libhtp.c \ app-layer-htp-mem.c \ + app-layer-htp-range.c \ app-layer-htp-xff.c \ app-layer-http2.c \ app-layer-ike.c \ diff --git a/src/app-layer-htp-file.c b/src/app-layer-htp-file.c index b84df0e30f6d..c47afae5526f 100644 --- a/src/app-layer-htp-file.c +++ b/src/app-layer-htp-file.c @@ -27,6 +27,7 @@ #include "suricata.h" #include "suricata-common.h" #include "debug.h" +#include "util-validate.h" #include "decode.h" #include "threads.h" @@ -114,6 +115,8 @@ int HTPFileOpen(HtpState *s, const uint8_t *filename, uint16_t filename_len, sbcfg = &s->cfg->response.sbcfg; + // we shall not open a new file if there is a current one + DEBUG_VALIDATE_BUG_ON(s->file_range != NULL); } else { if (s->files_ts == NULL) { s->files_ts = FileContainerAlloc(); @@ -214,6 +217,39 @@ int HTPParseContentRange(bstr * rawvalue, HtpContentRange *range) return 0; } +/** + * Performs parsing + checking of the content-range value + * + * @param[in] rawvalue + * @param[out] range + * + * @return HTP_OK on success, HTP_ERROR, -2, -3 on failure. + */ +static int HTPParseAndCheckContentRange( + bstr *rawvalue, HtpContentRange *range, HtpState *s, HtpTxUserData *htud) +{ + int r = HTPParseContentRange(rawvalue, range); + if (r != 0) { + AppLayerDecoderEventsSetEventRaw(&htud->decoder_events, HTTP_DECODER_EVENT_RANGE_INVALID); + s->events++; + SCLogDebug("parsing range failed, going back to normal file"); + return r; + } + /* crparsed.end <= 0 means a range with only size + * this is the answer to an unsatisfied range with the whole file + * crparsed.size <= 0 means an unknown size, so we do not know + * when to close it... + */ + if (range->end <= 0 || range->size <= 0) { + SCLogDebug("range without all information"); + return -2; + } else if (range->end == range->size - 1 && range->start == 0) { + SCLogDebug("range without all information"); + return -3; + } + return r; +} + /** * \brief Sets range for a file * @@ -225,34 +261,86 @@ int HTPParseContentRange(bstr * rawvalue, HtpContentRange *range) * \retval -2 error parsing * \retval -3 error negative end in range */ -int HTPFileSetRange(HtpState *s, bstr *rawvalue) +int HTPFileOpenWithRange(HtpState *s, const uint8_t *filename, uint16_t filename_len, + const uint8_t *data, uint32_t data_len, uint64_t txid, bstr *rawvalue, HtpTxUserData *htud) { SCEnter(); + uint16_t flags; if (s == NULL) { SCReturnInt(-1); } + // This function is only called STREAM_TOCLIENT from HtpResponseBodyHandle + HtpContentRange crparsed; + if (HTPParseAndCheckContentRange(rawvalue, &crparsed, s, htud) != 0) { + return HTPFileOpen( + s, filename, (uint32_t)filename_len, data, data_len, txid, STREAM_TOCLIENT); + } + flags = FileFlowToFlags(s->f, STREAM_TOCLIENT); + if ((s->flags & HTP_FLAG_STORE_FILES_TS) || + ((s->flags & HTP_FLAG_STORE_FILES_TX_TS) && txid == s->store_tx_id)) { + flags |= FILE_STORE; + flags &= ~FILE_NOSTORE; + } else if (!(flags & FILE_STORE) && (s->f->file_flags & FLOWFILE_NO_STORE_TC)) { + flags |= FILE_NOSTORE; + } + FileContainer * files = s->files_tc; if (files == NULL) { - SCLogDebug("no files in state"); + s->files_tc = FileContainerAlloc(); + if (s->files_tc == NULL) { + SCReturnInt(-1); + } + files = s->files_tc; + } + + if (FileOpenFileWithId(files, &s->cfg->response.sbcfg, s->file_track_id++, filename, + filename_len, data, data_len, flags) != 0) { SCReturnInt(-1); } + FileSetTx(files->tail, txid); - HtpContentRange crparsed; - if (HTPParseContentRange(rawvalue, &crparsed) != 0) { - SCLogDebug("parsing range failed"); - SCReturnInt(-2); + if (FileSetRange(files, crparsed.start, crparsed.end) < 0) { + SCLogDebug("set range failed"); } - if (crparsed.end <= 0) { - SCLogDebug("negative end in range"); - SCReturnInt(-3); + htp_tx_t *tx = htp_list_get(s->conn->transactions, txid); + if (!tx) { + SCReturnInt(-1); } - int retval = FileSetRange(files, crparsed.start, crparsed.end); - if (retval == -1) { - SCLogDebug("set range failed"); + uint8_t *keyurl; + size_t keylen; + if (tx->request_hostname != NULL) { + keylen = bstr_len(tx->request_hostname) + filename_len + 1; + keyurl = SCMalloc(keylen); + if (keyurl == NULL) { + SCReturnInt(-1); + } + memcpy(keyurl, bstr_ptr(tx->request_hostname), bstr_len(tx->request_hostname)); + memcpy(keyurl + bstr_len(tx->request_hostname), filename, filename_len); + keyurl[keylen - 1] = 0; + } else { + // do not reassemble file without host info + return HTPFileOpen( + s, filename, (uint32_t)filename_len, data, data_len, txid, STREAM_TOCLIENT); } - SCReturnInt(retval); + HttpRangeContainerFile *file_range_container = + HttpRangeContainerUrlGet(keyurl, keylen, &s->f->lastts); + SCFree(keyurl); + if (file_range_container == NULL) { + // probably reached memcap + return HTPFileOpen( + s, filename, (uint32_t)filename_len, data, data_len, txid, STREAM_TOCLIENT); + } + s->file_range = ContainerUrlRangeOpenFile(file_range_container, crparsed.start, crparsed.end, + crparsed.size, &s->cfg->response.sbcfg, filename, filename_len, flags, data, data_len); + if (s->file_range == NULL) { + // probably reached memcap + return HTPFileOpen( + s, filename, (uint32_t)filename_len, data, data_len, txid, STREAM_TOCLIENT); + } + + SCReturnInt(0); } /** @@ -292,6 +380,11 @@ int HTPFileStoreChunk(HtpState *s, const uint8_t *data, uint32_t data_len, goto end; } + if (s->file_range != NULL) { + if (ContainerUrlRangeAppendData(s->file_range, data, data_len) < 0) { + SCLogDebug("Failed to append data"); + } + } result = FileAppendData(files, data, data_len); if (result == -1) { SCLogDebug("appending data failed"); @@ -350,6 +443,17 @@ int HTPFileClose(HtpState *s, const uint8_t *data, uint32_t data_len, } else if (result == -2) { retval = -2; } + if (s->file_range != NULL) { + if (ContainerUrlRangeAppendData(s->file_range, data, data_len) < 0) { + SCLogDebug("Failed to append data"); + } + File *ranged = ContainerUrlRangeClose(s->file_range, flags); + if (ranged) { + FileContainerAdd(files, ranged); + } + SCFree(s->file_range); + s->file_range = NULL; + } end: SCReturnInt(retval); diff --git a/src/app-layer-htp-file.h b/src/app-layer-htp-file.h index 3e6bdc1fb923..1bdf4578c606 100644 --- a/src/app-layer-htp-file.h +++ b/src/app-layer-htp-file.h @@ -33,7 +33,8 @@ typedef struct HtpContentRange_ { int HTPFileOpen(HtpState *, const uint8_t *, uint16_t, const uint8_t *, uint32_t, uint64_t, uint8_t); int HTPParseContentRange(bstr * rawvalue, HtpContentRange *range); -int HTPFileSetRange(HtpState *, bstr *rawvalue); +int HTPFileOpenWithRange(HtpState *, const uint8_t *, uint16_t, const uint8_t *, uint32_t, uint64_t, + bstr *rawvalue, HtpTxUserData *htud); int HTPFileStoreChunk(HtpState *, const uint8_t *, uint32_t, uint8_t); int HTPFileClose(HtpState *, const uint8_t *, uint32_t, uint8_t, uint8_t); diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c new file mode 100644 index 000000000000..c2447dda105d --- /dev/null +++ b/src/app-layer-htp-range.c @@ -0,0 +1,453 @@ +/* Copyright (C) 2021 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +/** + * \file + * + * \author Philippe Antoine + */ + +#include "suricata-common.h" +#include "app-layer-htp-range.h" +#include "util-misc.h" //ParseSizeStringU64 +#include "util-thash.h" //HashTable +#include "util-memcmp.h" //SCBufferCmp +#include "util-hash-string.h" //StringHashDjb2 +#include "util-validate.h" //DEBUG_VALIDATE_BUG_ON +#include "util-byte.h" //StringParseUint32 + +typedef struct ContainerTHashTable { + THashTableContext *ht; + uint32_t timeout; +} ContainerTHashTable; + +// globals +ContainerTHashTable ContainerUrlRangeList; + +#define CONTAINER_URLRANGE_HASH_SIZE 256 + +int HttpRangeContainerBufferCompare(HttpRangeContainerBuffer *a, HttpRangeContainerBuffer *b) +{ + // lexical order : start, buflen, offset + if (a->start > b->start) + return 1; + if (a->start < b->start) + return -1; + if (a->buflen > b->buflen) + return 1; + if (a->buflen < b->buflen) + return -1; + if (a->offset > b->offset) + return 1; + if (a->offset < b->offset) + return -1; + return 0; +} + +RB_GENERATE(HTTP_RANGES, HttpRangeContainerBuffer, rb, HttpRangeContainerBufferCompare); + +static int ContainerUrlRangeSet(void *dst, void *src) +{ + HttpRangeContainerFile *src_s = src; + HttpRangeContainerFile *dst_s = dst; + dst_s->len = src_s->len; + dst_s->key = SCMalloc(dst_s->len); + BUG_ON(dst_s->key == NULL); + memcpy(dst_s->key, src_s->key, dst_s->len); + dst_s->files = FileContainerAlloc(); + BUG_ON(dst_s->files == NULL); + RB_INIT(&dst_s->fragment_tree); + dst_s->flags = 0; + dst_s->totalsize = 0; + SCMutexInit(&dst_s->mutex, NULL); + dst_s->hdata = NULL; + + return 0; +} + +static bool ContainerUrlRangeCompare(void *a, void *b) +{ + const HttpRangeContainerFile *as = a; + const HttpRangeContainerFile *bs = b; + if (SCBufferCmp(as->key, as->len, bs->key, bs->len) == 0) { + return true; + } + return false; +} + +static uint32_t ContainerUrlRangeHash(void *s) +{ + HttpRangeContainerFile *cur = s; + uint32_t h = StringHashDjb2(cur->key, cur->len); + return h; +} + +// base data stays in hash +static void ContainerUrlRangeFree(void *s) +{ + HttpRangeContainerBuffer *range, *tmp; + + HttpRangeContainerFile *cu = s; + SCFree(cu->key); + cu->key = NULL; + FileContainerFree(cu->files); + cu->files = NULL; + RB_FOREACH_SAFE (range, HTTP_RANGES, &cu->fragment_tree, tmp) { + RB_REMOVE(HTTP_RANGES, &cu->fragment_tree, range); + SCFree(range->buffer); + (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, range->buflen); + SCFree(range); + } + SCMutexDestroy(&cu->mutex); +} + +static bool ContainerValueRangeTimeout(HttpRangeContainerFile *cu, struct timeval *ts) +{ + // we only timeout if we have no flow referencing us + SCMutexLock(&cu->mutex); + bool r = ((uint32_t)ts->tv_sec > cu->expire && SC_ATOMIC_GET(cu->hdata->use_cnt) == 0); + SCMutexUnlock(&cu->mutex); + return r; +} + +static void ContainerUrlRangeUpdate(HttpRangeContainerFile *cu, uint32_t expire) +{ + cu->expire = expire; +} + +#define HTTP_RANGE_DEFAULT_TIMEOUT 60 +#define HTTP_RANGE_DEFAULT_MEMCAP 100 * 1024 * 1024 + +void HttpRangeContainersInit(void) +{ + SCLogDebug("containers start"); + const char *str = NULL; + uint64_t memcap = HTTP_RANGE_DEFAULT_MEMCAP; + uint32_t timeout = HTTP_RANGE_DEFAULT_TIMEOUT; + if (ConfGetValue("app-layer.protocols.http.urlrange.memcap", &str) == 1) { + if (ParseSizeStringU64(str, &memcap) < 0) { + SCLogWarning(SC_ERR_INVALID_VALUE, + "memcap value cannot be deduced: %s," + " resetting to default", + str); + memcap = 0; + } + } + if (ConfGetValue("app-layer.protocols.http.urlrange.timeout", &str) == 1) { + if (StringParseUint32(&timeout, 10, strlen(str), str) <= 0) { + SCLogWarning(SC_ERR_INVALID_VALUE, + "timeout value cannot be deduced: %s," + " resetting to default", + str); + timeout = 0; + } + } + + ContainerUrlRangeList.ht = + THashInit("app-layer.protocols.http.urlrange", sizeof(HttpRangeContainerFile), + ContainerUrlRangeSet, ContainerUrlRangeFree, ContainerUrlRangeHash, + ContainerUrlRangeCompare, false, memcap, CONTAINER_URLRANGE_HASH_SIZE); + ContainerUrlRangeList.timeout = timeout; + + SCLogDebug("containers started"); +} + +void HttpRangeContainersDestroy(void) +{ + THashShutdown(ContainerUrlRangeList.ht); +} + +uint32_t HttpRangeContainersTimeoutHash(struct timeval *ts) +{ + uint32_t cnt = 0; + + for (size_t i = 0; i < ContainerUrlRangeList.ht->config.hash_size; i++) { + THashHashRow *hb = &ContainerUrlRangeList.ht->array[i]; + + if (HRLOCK_TRYLOCK(hb) != 0) + continue; + /* hash bucket is now locked */ + THashData *h = hb->head; + while (h) { + THashData *n = h->next; + if (ContainerValueRangeTimeout(h->data, ts)) { + /* remove from the hash */ + if (h->prev != NULL) + h->prev->next = h->next; + if (h->next != NULL) + h->next->prev = h->prev; + if (hb->head == h) + hb->head = h->next; + if (hb->tail == h) + hb->tail = h->prev; + h->next = NULL; + h->prev = NULL; + // we should log the timed out file somehow... + // but it does not belong to any flow... + ContainerUrlRangeFree(h->data); + THashDataMoveToSpare(ContainerUrlRangeList.ht, h); + } + h = n; + } + HRLOCK_UNLOCK(hb); + } + + return cnt; +} + +void *HttpRangeContainerUrlGet(const uint8_t *key, size_t keylen, struct timeval *ts) +{ + HttpRangeContainerFile lookup; + // cast so as not to have const in the structure + lookup.key = (uint8_t *)key; + lookup.len = keylen; + struct THashDataGetResult res = THashGetFromHash(ContainerUrlRangeList.ht, &lookup); + if (res.data) { + // nothing more to do if (res.is_new) + ContainerUrlRangeUpdate(res.data->data, ts->tv_sec + ContainerUrlRangeList.timeout); + HttpRangeContainerFile *c = res.data->data; + c->hdata = res.data; + THashDataUnlock(res.data); + return res.data->data; + } + return NULL; +} + +static HttpRangeContainerBlock *ContainerUrlRangeOpenFileAux(HttpRangeContainerFile *c, + uint64_t start, uint64_t end, uint64_t total, const StreamingBufferConfig *sbcfg, + const uint8_t *name, uint16_t name_len, uint16_t flags) +{ + SCMutexLock(&c->mutex); + if (c->files->tail == NULL) { + if (FileOpenFileWithId(c->files, sbcfg, 0, name, name_len, NULL, 0, flags) != 0) { + SCLogDebug("open file for range failed"); + THashDecrUsecnt(c->hdata); + SCMutexUnlock(&c->mutex); + return NULL; + } + } + HttpRangeContainerBlock *curf = SCCalloc(1, sizeof(HttpRangeContainerBlock)); + if (curf == NULL) { + THashDecrUsecnt(c->hdata); + SCMutexUnlock(&c->mutex); + return NULL; + } + if (total > c->totalsize) { + // TODOask add checks about totalsize remaining the same + c->totalsize = total; + } + uint64_t buflen = end - start + 1; + if (start == c->files->tail->size && !c->appending) { + // easy case : append to current file + curf->container = c; + c->appending = true; + SCMutexUnlock(&c->mutex); + return curf; + } else if (start < c->files->tail->size && c->files->tail->size - start >= buflen) { + // only overlap + THashDecrUsecnt(c->hdata); + // redundant to be explicit that this block is independent + curf->container = NULL; + curf->toskip = buflen; + SCMutexUnlock(&c->mutex); + return curf; + } else if (start < c->files->tail->size && c->files->tail->size - start < buflen && + !c->appending) { + // skip first overlap, then append + curf->toskip = c->files->tail->size - start; + c->appending = true; + curf->container = c; + SCMutexUnlock(&c->mutex); + return curf; + } + // else { + // block/range to be inserted in ordered linked list + if (!(THASH_CHECK_MEMCAP(ContainerUrlRangeList.ht, buflen))) { + // TODOask release memory for other ranges cf RangeContainerFree(c); + // skips this range + curf->toskip = buflen; + curf->container = NULL; + THashDecrUsecnt(c->hdata); + SCMutexUnlock(&c->mutex); + return curf; + } + curf->container = c; + (void)SC_ATOMIC_ADD(ContainerUrlRangeList.ht->memuse, buflen); + HttpRangeContainerBuffer *range = SCCalloc(1, sizeof(HttpRangeContainerBuffer)); + BUG_ON(range == NULL); + range->buffer = SCMalloc(buflen); + BUG_ON(range->buffer == NULL); + range->buflen = buflen; + range->start = start; + + curf->current = range; + SCMutexUnlock(&c->mutex); + return curf; +} + +HttpRangeContainerBlock *ContainerUrlRangeOpenFile(HttpRangeContainerFile *c, uint64_t start, + uint64_t end, uint64_t total, const StreamingBufferConfig *sbcfg, const uint8_t *name, + uint16_t name_len, uint16_t flags, const uint8_t *data, size_t len) +{ + HttpRangeContainerBlock *r = + ContainerUrlRangeOpenFileAux(c, start, end, total, sbcfg, name, name_len, flags); + if (ContainerUrlRangeAppendData(r, data, len) < 0) { + SCLogDebug("Failed to append data while openeing"); + } + return r; +} + +int ContainerUrlRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, size_t len) +{ + if (len == 0) { + return 0; + } + // first check if we have a current allocated buffer to copy to + // in the case of an unordered range being handled + if (c->current) { + if (data == NULL) { + // just feed the gap in the current position, instead of its right one + return FileAppendData(c->container->files, data, len); + } else if (c->current->offset + len <= c->current->buflen) { + memcpy(c->current->buffer + c->current->offset, data, len); + c->current->offset += len; + } else { + memcpy(c->current->buffer + c->current->offset, data, + c->current->buflen - c->current->offset); + c->current->offset = c->current->buflen; + } + return 0; + // then check if we are skipping + } else if (c->toskip > 0) { + if (c->toskip >= len) { + c->toskip -= len; + return 0; + } // else + DEBUG_VALIDATE_BUG_ON(c->container->files == NULL); + int r; + if (data == NULL) { + // gap overlaping already known data + r = FileAppendData(c->container->files, NULL, len - c->toskip); + } else { + r = FileAppendData(c->container->files, data + c->toskip, len - c->toskip); + } + c->toskip = 0; + return r; + } // else { + // last we are ordered, simply append + DEBUG_VALIDATE_BUG_ON(c->container->files == NULL); + return FileAppendData(c->container->files, data, len); +} + +static void ContainerUrlRangeFileClose(HttpRangeContainerFile *c, uint16_t flags) +{ + DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(c->hdata->use_cnt) == 0); + THashDecrUsecnt(c->hdata); + // move ownership of file c->files->head to caller + FileCloseFile(c->files, NULL, 0, c->flags | flags); + c->files->head = NULL; + c->files->tail = NULL; + if (SC_ATOMIC_GET(c->hdata->use_cnt) == 0) { + THashRemoveFromHash(ContainerUrlRangeList.ht, c); + } + // otherwise, the hash entry will be used for another read of the file +} + +File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags) +{ + if (c->container == NULL) { + // everything was just skipped : nothing to do + return NULL; + } + + SCMutexLock(&c->container->mutex); + + if (c->current) { + // some out-or-order range is finished + if (c->container->files->tail && + c->container->files->tail->size >= c->current->start + c->current->offset) { + // if the range has become obsolete because we received the data already + // we just free it + (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, c->current->buflen); + SCFree(c->current->buffer); + SCFree(c->current); + } else { + // otherwise insert in red and black tree + HTTP_RANGES_RB_INSERT(&c->container->fragment_tree, c->current); + } + THashDecrUsecnt(c->container->hdata); + SCMutexUnlock(&c->container->mutex); + return NULL; + } + + // else { + if (c->toskip > 0) { + // was only an overlapping range, truncated before new bytes + THashDecrUsecnt(c->container->hdata); + SCMutexUnlock(&c->container->mutex); + return NULL; + } + + // else { + // we just finished an in-order block + c->container->appending = false; + DEBUG_VALIDATE_BUG_ON(c->container->files->tail == NULL); + File *f = c->container->files->tail; + + // have we reached a saved range ? + HttpRangeContainerBuffer *range; + RB_FOREACH(range, HTTP_RANGES, &c->container->fragment_tree) + { + if (f->size < range->start) { + break; + } + if (f->size == range->start) { + // a new range just begins where we ended, append it + if (FileAppendData(c->container->files, range->buffer, range->offset) != 0) { + ContainerUrlRangeFileClose(c->container, flags); + SCMutexUnlock(&c->container->mutex); + return f; + } + } else { + // the range starts before where we ended + uint64_t overlap = f->size - range->start; + if (overlap < range->offset) { + // And the range ends beyond where we ended + // in this case of overlap, only add the extra data + if (FileAppendData(c->container->files, range->buffer + overlap, + range->offset - overlap) != 0) { + ContainerUrlRangeFileClose(c->container, flags); + SCMutexUnlock(&c->container->mutex); + return f; + } + } + } + // anyways, remove this range from the linked list, as we are now beyond it + RB_REMOVE(HTTP_RANGES, &c->container->fragment_tree, range); + } + + if (f->size >= c->container->totalsize) { + // we finished the whole file + ContainerUrlRangeFileClose(c->container, flags); + } else { + // we are expecting more ranges + THashDecrUsecnt(c->container->hdata); + f = NULL; + } + SCMutexUnlock(&c->container->mutex); + return f; +} diff --git a/src/app-layer-htp-range.h b/src/app-layer-htp-range.h new file mode 100644 index 000000000000..83907d6a4d0b --- /dev/null +++ b/src/app-layer-htp-range.h @@ -0,0 +1,97 @@ +/* Copyright (C) 2021 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +#ifndef __APP_LAYER_HTP_RANGE_H__ +#define __APP_LAYER_HTP_RANGE_H__ + +#include "util-thash.h" + +void HttpRangeContainersInit(void); +void HttpRangeContainersDestroy(void); +uint32_t HttpRangeContainersTimeoutHash(struct timeval *ts); + +void *HttpRangeContainerUrlGet(const uint8_t *key, size_t keylen, struct timeval *ts); + +// linked list of ranges : buffer with offset +typedef struct HttpRangeContainerBuffer { + /** red and black tree */ + RB_ENTRY(HttpRangeContainerBuffer) rb; + /** allocated buffer */ + uint8_t *buffer; + /** length of buffer */ + uint64_t buflen; + /** the start of the range (offset relative to the absolute beginning of the file) */ + uint64_t start; + /** offset of bytes written in buffer (relative to the start of the range) */ + uint64_t offset; +} HttpRangeContainerBuffer; + +int HttpRangeContainerBufferCompare(HttpRangeContainerBuffer *a, HttpRangeContainerBuffer *b); + +RB_HEAD(HTTP_RANGES, HttpRangeContainerBuffer); +RB_PROTOTYPE(HTTP_RANGES, HttpRangeContainerBuffer, rb, HttpRangeContainerBufferCompare); + +/** Item in hash table for a file in multiple ranges + * Thread-safety is ensured by the thread-safe hash table + * The number of use is increased for each flow opening a new HttpRangeContainerBlock + * until it closes this HttpRangeContainerBlock + */ +typedef struct HttpRangeContainerFile { + /** key for hashtable */ + uint8_t *key; + /** key length */ + uint32_t len; + /** pointer to hashtable data, for use count */ + THashData *hdata; + /** expire time in epoch */ + uint32_t expire; + /** total epxected size of the file in ranges */ + uint64_t totalsize; + /** file flags */ + uint16_t flags; + /** file container, with only one file */ + FileContainer *files; + /** red and black tree list of ranges which came out of order */ + struct HTTP_RANGES fragment_tree; + /** wether a range file is currently appending */ + bool appending; + /** mutex */ + SCMutex mutex; +} HttpRangeContainerFile; + +/** A structure representing a single range request : + * either skipping, buffering, or appending + * As this belongs to a flow, appending data to it is ensured to be thread-safe + * Only one block per file has the pointer to the container + */ +typedef struct HttpRangeContainerBlock { + /** state where we skip content */ + uint64_t toskip; + /** current out of order range to write into */ + HttpRangeContainerBuffer *current; + /** pointer to the main file container, where to directly append data */ + HttpRangeContainerFile *container; +} HttpRangeContainerBlock; + +int ContainerUrlRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, size_t len); +File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags); + +HttpRangeContainerBlock *ContainerUrlRangeOpenFile(HttpRangeContainerFile *c, uint64_t start, + uint64_t end, uint64_t total, const StreamingBufferConfig *sbcfg, const uint8_t *name, + uint16_t name_len, uint16_t flags, const uint8_t *data, size_t len); + +#endif /* __APP_LAYER_HTP_RANGE_H__ */ diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index 49225a813629..0afd74cdfe23 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -99,113 +99,74 @@ static uint64_t htp_state_memuse = 0; static uint64_t htp_state_memcnt = 0; #endif -SCEnumCharMap http_decoder_event_table[ ] = { - { "UNKNOWN_ERROR", - HTTP_DECODER_EVENT_UNKNOWN_ERROR}, - { "GZIP_DECOMPRESSION_FAILED", - HTTP_DECODER_EVENT_GZIP_DECOMPRESSION_FAILED}, - { "REQUEST_FIELD_MISSING_COLON", - HTTP_DECODER_EVENT_REQUEST_FIELD_MISSING_COLON}, - { "RESPONSE_FIELD_MISSING_COLON", - HTTP_DECODER_EVENT_RESPONSE_FIELD_MISSING_COLON}, - { "INVALID_REQUEST_CHUNK_LEN", - HTTP_DECODER_EVENT_INVALID_REQUEST_CHUNK_LEN}, - { "INVALID_RESPONSE_CHUNK_LEN", - HTTP_DECODER_EVENT_INVALID_RESPONSE_CHUNK_LEN}, +SCEnumCharMap http_decoder_event_table[] = { + { "UNKNOWN_ERROR", HTTP_DECODER_EVENT_UNKNOWN_ERROR }, + { "GZIP_DECOMPRESSION_FAILED", HTTP_DECODER_EVENT_GZIP_DECOMPRESSION_FAILED }, + { "REQUEST_FIELD_MISSING_COLON", HTTP_DECODER_EVENT_REQUEST_FIELD_MISSING_COLON }, + { "RESPONSE_FIELD_MISSING_COLON", HTTP_DECODER_EVENT_RESPONSE_FIELD_MISSING_COLON }, + { "INVALID_REQUEST_CHUNK_LEN", HTTP_DECODER_EVENT_INVALID_REQUEST_CHUNK_LEN }, + { "INVALID_RESPONSE_CHUNK_LEN", HTTP_DECODER_EVENT_INVALID_RESPONSE_CHUNK_LEN }, { "INVALID_TRANSFER_ENCODING_VALUE_IN_REQUEST", - HTTP_DECODER_EVENT_INVALID_TRANSFER_ENCODING_VALUE_IN_REQUEST}, + HTTP_DECODER_EVENT_INVALID_TRANSFER_ENCODING_VALUE_IN_REQUEST }, { "INVALID_TRANSFER_ENCODING_VALUE_IN_RESPONSE", - HTTP_DECODER_EVENT_INVALID_TRANSFER_ENCODING_VALUE_IN_RESPONSE}, + HTTP_DECODER_EVENT_INVALID_TRANSFER_ENCODING_VALUE_IN_RESPONSE }, { "INVALID_CONTENT_LENGTH_FIELD_IN_REQUEST", - HTTP_DECODER_EVENT_INVALID_CONTENT_LENGTH_FIELD_IN_REQUEST}, + HTTP_DECODER_EVENT_INVALID_CONTENT_LENGTH_FIELD_IN_REQUEST }, { "INVALID_CONTENT_LENGTH_FIELD_IN_RESPONSE", - HTTP_DECODER_EVENT_INVALID_CONTENT_LENGTH_FIELD_IN_RESPONSE}, + HTTP_DECODER_EVENT_INVALID_CONTENT_LENGTH_FIELD_IN_RESPONSE }, { "DUPLICATE_CONTENT_LENGTH_FIELD_IN_REQUEST", - HTTP_DECODER_EVENT_DUPLICATE_CONTENT_LENGTH_FIELD_IN_REQUEST}, + HTTP_DECODER_EVENT_DUPLICATE_CONTENT_LENGTH_FIELD_IN_REQUEST }, { "DUPLICATE_CONTENT_LENGTH_FIELD_IN_RESPONSE", - HTTP_DECODER_EVENT_DUPLICATE_CONTENT_LENGTH_FIELD_IN_RESPONSE}, - { "100_CONTINUE_ALREADY_SEEN", - HTTP_DECODER_EVENT_100_CONTINUE_ALREADY_SEEN}, + HTTP_DECODER_EVENT_DUPLICATE_CONTENT_LENGTH_FIELD_IN_RESPONSE }, + { "100_CONTINUE_ALREADY_SEEN", HTTP_DECODER_EVENT_100_CONTINUE_ALREADY_SEEN }, { "UNABLE_TO_MATCH_RESPONSE_TO_REQUEST", - HTTP_DECODER_EVENT_UNABLE_TO_MATCH_RESPONSE_TO_REQUEST}, - { "INVALID_SERVER_PORT_IN_REQUEST", - HTTP_DECODER_EVENT_INVALID_SERVER_PORT_IN_REQUEST}, - { "INVALID_AUTHORITY_PORT", - HTTP_DECODER_EVENT_INVALID_AUTHORITY_PORT}, - { "REQUEST_HEADER_INVALID", - HTTP_DECODER_EVENT_REQUEST_HEADER_INVALID}, - { "RESPONSE_HEADER_INVALID", - HTTP_DECODER_EVENT_RESPONSE_HEADER_INVALID}, - { "MISSING_HOST_HEADER", - HTTP_DECODER_EVENT_MISSING_HOST_HEADER}, - { "HOST_HEADER_AMBIGUOUS", - HTTP_DECODER_EVENT_HOST_HEADER_AMBIGUOUS}, - { "INVALID_REQUEST_FIELD_FOLDING", - HTTP_DECODER_EVENT_INVALID_REQUEST_FIELD_FOLDING}, - { "INVALID_RESPONSE_FIELD_FOLDING", - HTTP_DECODER_EVENT_INVALID_RESPONSE_FIELD_FOLDING}, - { "REQUEST_FIELD_TOO_LONG", - HTTP_DECODER_EVENT_REQUEST_FIELD_TOO_LONG}, - { "RESPONSE_FIELD_TOO_LONG", - HTTP_DECODER_EVENT_RESPONSE_FIELD_TOO_LONG}, - { "REQUEST_LINE_INVALID", - HTTP_DECODER_EVENT_REQUEST_LINE_INVALID}, - { "REQUEST_BODY_UNEXPECTED", - HTTP_DECODER_EVENT_REQUEST_BODY_UNEXPECTED}, + HTTP_DECODER_EVENT_UNABLE_TO_MATCH_RESPONSE_TO_REQUEST }, + { "INVALID_SERVER_PORT_IN_REQUEST", HTTP_DECODER_EVENT_INVALID_SERVER_PORT_IN_REQUEST }, + { "INVALID_AUTHORITY_PORT", HTTP_DECODER_EVENT_INVALID_AUTHORITY_PORT }, + { "REQUEST_HEADER_INVALID", HTTP_DECODER_EVENT_REQUEST_HEADER_INVALID }, + { "RESPONSE_HEADER_INVALID", HTTP_DECODER_EVENT_RESPONSE_HEADER_INVALID }, + { "MISSING_HOST_HEADER", HTTP_DECODER_EVENT_MISSING_HOST_HEADER }, + { "HOST_HEADER_AMBIGUOUS", HTTP_DECODER_EVENT_HOST_HEADER_AMBIGUOUS }, + { "INVALID_REQUEST_FIELD_FOLDING", HTTP_DECODER_EVENT_INVALID_REQUEST_FIELD_FOLDING }, + { "INVALID_RESPONSE_FIELD_FOLDING", HTTP_DECODER_EVENT_INVALID_RESPONSE_FIELD_FOLDING }, + { "REQUEST_FIELD_TOO_LONG", HTTP_DECODER_EVENT_REQUEST_FIELD_TOO_LONG }, + { "RESPONSE_FIELD_TOO_LONG", HTTP_DECODER_EVENT_RESPONSE_FIELD_TOO_LONG }, + { "REQUEST_LINE_INVALID", HTTP_DECODER_EVENT_REQUEST_LINE_INVALID }, + { "REQUEST_BODY_UNEXPECTED", HTTP_DECODER_EVENT_REQUEST_BODY_UNEXPECTED }, { "REQUEST_SERVER_PORT_TCP_PORT_MISMATCH", - HTTP_DECODER_EVENT_REQUEST_SERVER_PORT_TCP_PORT_MISMATCH}, - { "REQUEST_URI_HOST_INVALID", - HTTP_DECODER_EVENT_URI_HOST_INVALID}, - { "REQUEST_HEADER_HOST_INVALID", - HTTP_DECODER_EVENT_HEADER_HOST_INVALID}, - { "REQUEST_AUTH_UNRECOGNIZED", - HTTP_DECODER_EVENT_AUTH_UNRECOGNIZED}, - { "REQUEST_HEADER_REPETITION", - HTTP_DECODER_EVENT_REQUEST_HEADER_REPETITION}, - { "RESPONSE_HEADER_REPETITION", - HTTP_DECODER_EVENT_RESPONSE_HEADER_REPETITION}, - { "DOUBLE_ENCODED_URI", - HTTP_DECODER_EVENT_DOUBLE_ENCODED_URI}, - { "URI_DELIM_NON_COMPLIANT", - HTTP_DECODER_EVENT_URI_DELIM_NON_COMPLIANT}, - { "METHOD_DELIM_NON_COMPLIANT", - HTTP_DECODER_EVENT_METHOD_DELIM_NON_COMPLIANT}, - { "REQUEST_LINE_LEADING_WHITESPACE", - HTTP_DECODER_EVENT_REQUEST_LINE_LEADING_WHITESPACE}, - { "TOO_MANY_ENCODING_LAYERS", - HTTP_DECODER_EVENT_TOO_MANY_ENCODING_LAYERS}, - { "ABNORMAL_CE_HEADER", - HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER}, - { "RESPONSE_MULTIPART_BYTERANGES", - HTTP_DECODER_EVENT_RESPONSE_MULTIPART_BYTERANGES}, + HTTP_DECODER_EVENT_REQUEST_SERVER_PORT_TCP_PORT_MISMATCH }, + { "REQUEST_URI_HOST_INVALID", HTTP_DECODER_EVENT_URI_HOST_INVALID }, + { "REQUEST_HEADER_HOST_INVALID", HTTP_DECODER_EVENT_HEADER_HOST_INVALID }, + { "REQUEST_AUTH_UNRECOGNIZED", HTTP_DECODER_EVENT_AUTH_UNRECOGNIZED }, + { "REQUEST_HEADER_REPETITION", HTTP_DECODER_EVENT_REQUEST_HEADER_REPETITION }, + { "RESPONSE_HEADER_REPETITION", HTTP_DECODER_EVENT_RESPONSE_HEADER_REPETITION }, + { "DOUBLE_ENCODED_URI", HTTP_DECODER_EVENT_DOUBLE_ENCODED_URI }, + { "URI_DELIM_NON_COMPLIANT", HTTP_DECODER_EVENT_URI_DELIM_NON_COMPLIANT }, + { "METHOD_DELIM_NON_COMPLIANT", HTTP_DECODER_EVENT_METHOD_DELIM_NON_COMPLIANT }, + { "REQUEST_LINE_LEADING_WHITESPACE", HTTP_DECODER_EVENT_REQUEST_LINE_LEADING_WHITESPACE }, + { "TOO_MANY_ENCODING_LAYERS", HTTP_DECODER_EVENT_TOO_MANY_ENCODING_LAYERS }, + { "ABNORMAL_CE_HEADER", HTTP_DECODER_EVENT_ABNORMAL_CE_HEADER }, + { "RESPONSE_MULTIPART_BYTERANGES", HTTP_DECODER_EVENT_RESPONSE_MULTIPART_BYTERANGES }, { "RESPONSE_ABNORMAL_TRANSFER_ENCODING", - HTTP_DECODER_EVENT_RESPONSE_ABNORMAL_TRANSFER_ENCODING}, - { "RESPONSE_CHUNKED_OLD_PROTO", - HTTP_DECODER_EVENT_RESPONSE_CHUNKED_OLD_PROTO}, - { "RESPONSE_INVALID_PROTOCOL", - HTTP_DECODER_EVENT_RESPONSE_INVALID_PROTOCOL}, - { "RESPONSE_INVALID_STATUS", - HTTP_DECODER_EVENT_RESPONSE_INVALID_STATUS}, - { "REQUEST_LINE_INCOMPLETE", - HTTP_DECODER_EVENT_REQUEST_LINE_INCOMPLETE}, - - { "LZMA_MEMLIMIT_REACHED", - HTTP_DECODER_EVENT_LZMA_MEMLIMIT_REACHED}, - { "COMPRESSION_BOMB", - HTTP_DECODER_EVENT_COMPRESSION_BOMB}, + HTTP_DECODER_EVENT_RESPONSE_ABNORMAL_TRANSFER_ENCODING }, + { "RESPONSE_CHUNKED_OLD_PROTO", HTTP_DECODER_EVENT_RESPONSE_CHUNKED_OLD_PROTO }, + { "RESPONSE_INVALID_PROTOCOL", HTTP_DECODER_EVENT_RESPONSE_INVALID_PROTOCOL }, + { "RESPONSE_INVALID_STATUS", HTTP_DECODER_EVENT_RESPONSE_INVALID_STATUS }, + { "REQUEST_LINE_INCOMPLETE", HTTP_DECODER_EVENT_REQUEST_LINE_INCOMPLETE }, + + { "LZMA_MEMLIMIT_REACHED", HTTP_DECODER_EVENT_LZMA_MEMLIMIT_REACHED }, + { "COMPRESSION_BOMB", HTTP_DECODER_EVENT_COMPRESSION_BOMB }, + + { "RANGE_INVALID", HTTP_DECODER_EVENT_RANGE_INVALID }, /* suricata warnings/errors */ - { "MULTIPART_GENERIC_ERROR", - HTTP_DECODER_EVENT_MULTIPART_GENERIC_ERROR}, - { "MULTIPART_NO_FILEDATA", - HTTP_DECODER_EVENT_MULTIPART_NO_FILEDATA}, - { "MULTIPART_INVALID_HEADER", - HTTP_DECODER_EVENT_MULTIPART_INVALID_HEADER}, + { "MULTIPART_GENERIC_ERROR", HTTP_DECODER_EVENT_MULTIPART_GENERIC_ERROR }, + { "MULTIPART_NO_FILEDATA", HTTP_DECODER_EVENT_MULTIPART_NO_FILEDATA }, + { "MULTIPART_INVALID_HEADER", HTTP_DECODER_EVENT_MULTIPART_INVALID_HEADER }, - { "TOO_MANY_WARNINGS", - HTTP_DECODER_EVENT_TOO_MANY_WARNINGS}, + { "TOO_MANY_WARNINGS", HTTP_DECODER_EVENT_TOO_MANY_WARNINGS }, - { NULL, -1 }, + { NULL, -1 }, }; static void *HTPStateGetTx(void *alstate, uint64_t tx_id); @@ -1749,8 +1710,15 @@ static int HtpResponseBodyHandle(HtpState *hstate, HtpTxUserData *htud, } if (filename != NULL) { - result = HTPFileOpen(hstate, filename, (uint32_t)filename_len, - data, data_len, HtpGetActiveResponseTxID(hstate), STREAM_TOCLIENT); + // set range if present + htp_header_t *h_content_range = htp_table_get_c(tx->response_headers, "content-range"); + if (h_content_range != NULL) { + result = HTPFileOpenWithRange(hstate, filename, (uint32_t)filename_len, data, + data_len, HtpGetActiveResponseTxID(hstate), h_content_range->value, htud); + } else { + result = HTPFileOpen(hstate, filename, (uint32_t)filename_len, data, data_len, + HtpGetActiveResponseTxID(hstate), STREAM_TOCLIENT); + } SCLogDebug("result %d", result); if (result == -1) { goto end; @@ -1761,11 +1729,6 @@ static int HtpResponseBodyHandle(HtpState *hstate, HtpTxUserData *htud, htud->tcflags |= HTP_FILENAME_SET; htud->tcflags &= ~HTP_DONTSTORE; } - //set range if present - htp_header_t *h_content_range = htp_table_get_c(tx->response_headers, "content-range"); - if (h_content_range != NULL) { - HTPFileSetRange(hstate, h_content_range->value); - } } } else if (tx->response_line != NULL || tx->is_protocol_0_9) diff --git a/src/app-layer-htp.h b/src/app-layer-htp.h index d6fa4f00954a..66c52a359e77 100644 --- a/src/app-layer-htp.h +++ b/src/app-layer-htp.h @@ -38,6 +38,7 @@ #include "app-layer-htp-mem.h" #include "detect-engine-state.h" #include "util-streaming-buffer.h" +#include "app-layer-htp-range.h" #include "rust.h" #include @@ -132,6 +133,8 @@ enum { HTTP_DECODER_EVENT_LZMA_MEMLIMIT_REACHED, HTTP_DECODER_EVENT_COMPRESSION_BOMB, + HTTP_DECODER_EVENT_RANGE_INVALID, + /* suricata errors/warnings */ HTTP_DECODER_EVENT_MULTIPART_GENERIC_ERROR, HTTP_DECODER_EVENT_MULTIPART_NO_FILEDATA, @@ -257,6 +260,7 @@ typedef struct HtpState_ { uint16_t events; uint16_t htp_messages_offset; /**< offset into conn->messages list */ uint32_t file_track_id; /**< used to assign file track ids to files */ + HttpRangeContainerBlock *file_range; /**< used to assign track ids to range file */ uint64_t last_request_data_stamp; uint64_t last_response_data_stamp; } HtpState; diff --git a/src/flow-manager.c b/src/flow-manager.c index 1d4306a90e10..aecdada37ac8 100644 --- a/src/flow-manager.c +++ b/src/flow-manager.c @@ -66,6 +66,7 @@ #include "host-timeout.h" #include "defrag-timeout.h" #include "ippair-timeout.h" +#include "app-layer-htp-range.h" #include "output-flow.h" #include "util-validate.h" @@ -987,6 +988,7 @@ static TmEcode FlowManager(ThreadVars *th_v, void *thread_data) //uint32_t hosts_pruned = HostTimeoutHash(&ts); IPPairTimeoutHash(&ts); + HttpRangeContainersTimeoutHash(&ts); other_last_sec = (uint32_t)ts.tv_sec; } diff --git a/src/suricata.c b/src/suricata.c index eb9a3a258099..3c8f58161104 100644 --- a/src/suricata.c +++ b/src/suricata.c @@ -68,6 +68,7 @@ #include "conf.h" #include "conf-yaml-loader.h" +#include "app-layer-htp-range.h" #include "datasets.h" #include "stream-tcp.h" @@ -341,6 +342,7 @@ static void GlobalsDestroy(SCInstance *suri) AppLayerDeSetup(); DatasetsSave(); DatasetsDestroy(); + HttpRangeContainersDestroy(); TagDestroyCtx(); LiveDeviceListClean(); @@ -1999,6 +2001,7 @@ void PreRunInit(const int runmode) { /* Initialize Datasets to be able to use them with unix socket */ DatasetsInit(); + HttpRangeContainersInit(); if (runmode == RUNMODE_UNIX_SOCKET) return; diff --git a/src/util-thash.c b/src/util-thash.c index 4087ae239528..9dba6cdbca0c 100644 --- a/src/util-thash.c +++ b/src/util-thash.c @@ -37,7 +37,7 @@ static THashData *THashGetUsed(THashTableContext *ctx); static void THashDataEnqueue (THashDataQueue *q, THashData *h); -static void THashDataMoveToSpare(THashTableContext *ctx, THashData *h) +void THashDataMoveToSpare(THashTableContext *ctx, THashData *h) { THashDataEnqueue(&ctx->spare_q, h); (void) SC_ATOMIC_SUB(ctx->counter, 1); diff --git a/src/util-thash.h b/src/util-thash.h index fe6544a0d72f..34fa9c20a44b 100644 --- a/src/util-thash.h +++ b/src/util-thash.h @@ -214,5 +214,6 @@ void THashCleanup(THashTableContext *ctx); int THashWalk(THashTableContext *, THashFormatFunc, THashOutputFunc, void *); int THashRemoveFromHash (THashTableContext *ctx, void *data); void THashConsolidateMemcap(THashTableContext *ctx); +void THashDataMoveToSpare(THashTableContext *ctx, THashData *h); #endif /* __THASH_H__ */ diff --git a/suricata.yaml.in b/suricata.yaml.in index 3c188adf454d..0f652a6a598e 100644 --- a/suricata.yaml.in +++ b/suricata.yaml.in @@ -823,6 +823,13 @@ app-layer: dp: 53 http: enabled: yes + + # Range Containers default settings + # urlrange: + # memcap: 100mb + # timeout: 60 + + # memcap: Maximum memory capacity for HTTP # Default is unlimited, values can be 64mb, e.g. From 92c3ce9a959bb60a69b434fd206b802cf4873f99 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Mon, 2 Aug 2021 12:39:21 +0200 Subject: [PATCH 05/12] http/range: fix memory leak on out of order ranges --- src/app-layer-htp-range.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c index c2447dda105d..b81e9a5d3a06 100644 --- a/src/app-layer-htp-range.c +++ b/src/app-layer-htp-range.c @@ -409,9 +409,8 @@ File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags) File *f = c->container->files->tail; // have we reached a saved range ? - HttpRangeContainerBuffer *range; - RB_FOREACH(range, HTTP_RANGES, &c->container->fragment_tree) - { + HttpRangeContainerBuffer *range, *safe = NULL; + RB_FOREACH_SAFE (range, HTTP_RANGES, &c->container->fragment_tree, safe) { if (f->size < range->start) { break; } @@ -437,7 +436,10 @@ File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags) } } // anyways, remove this range from the linked list, as we are now beyond it - RB_REMOVE(HTTP_RANGES, &c->container->fragment_tree, range); + HTTP_RANGES_RB_REMOVE(&c->container->fragment_tree, range); + (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, range->buflen); + SCFree(range->buffer); + SCFree(range); } if (f->size >= c->container->totalsize) { From c705fe8402d890d8bd514211d757a57f3ee9bebd Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 3 Aug 2021 12:21:31 +0200 Subject: [PATCH 06/12] http/range: optimize struct layout --- src/app-layer-htp-range.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/app-layer-htp-range.h b/src/app-layer-htp-range.h index 83907d6a4d0b..039a72745b36 100644 --- a/src/app-layer-htp-range.h +++ b/src/app-layer-htp-range.h @@ -55,18 +55,18 @@ typedef struct HttpRangeContainerFile { uint8_t *key; /** key length */ uint32_t len; - /** pointer to hashtable data, for use count */ - THashData *hdata; /** expire time in epoch */ uint32_t expire; + /** pointer to hashtable data, for use count */ + THashData *hdata; /** total epxected size of the file in ranges */ uint64_t totalsize; - /** file flags */ - uint16_t flags; /** file container, with only one file */ FileContainer *files; /** red and black tree list of ranges which came out of order */ struct HTTP_RANGES fragment_tree; + /** file flags */ + uint16_t flags; /** wether a range file is currently appending */ bool appending; /** mutex */ From 879c76ac8e1dc44488fab7760dde5fe947436477 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 3 Aug 2021 11:51:26 +0200 Subject: [PATCH 07/12] http/range: cleanup and simplification Simplify locking by using the THashData lock instead of a separate range lock. Avoid size_t in function arguments. Clean up file handling functions. Implement handling of alloc errors. Rename yaml entry to byterange Unify public api naming --- src/app-layer-htp-file.c | 83 +++++++++++-- src/app-layer-htp-range.c | 254 ++++++++++++++++++++++++-------------- src/app-layer-htp-range.h | 19 +-- src/app-layer-htp.c | 2 + suricata.yaml.in | 5 +- 5 files changed, 249 insertions(+), 114 deletions(-) diff --git a/src/app-layer-htp-file.c b/src/app-layer-htp-file.c index c47afae5526f..7ac9746165db 100644 --- a/src/app-layer-htp-file.c +++ b/src/app-layer-htp-file.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2007-2011 Open Information Security Foundation +/* Copyright (C) 2007-2021 Open Information Security Foundation * * You can copy, redistribute or modify this Program under the terms of * the GNU General Public License version 2 as published by the Free @@ -332,17 +332,53 @@ int HTPFileOpenWithRange(HtpState *s, const uint8_t *filename, uint16_t filename return HTPFileOpen( s, filename, (uint32_t)filename_len, data, data_len, txid, STREAM_TOCLIENT); } - s->file_range = ContainerUrlRangeOpenFile(file_range_container, crparsed.start, crparsed.end, + DEBUG_VALIDATE_BUG_ON(s->file_range); + s->file_range = HttpRangeOpenFile(file_range_container, crparsed.start, crparsed.end, crparsed.size, &s->cfg->response.sbcfg, filename, filename_len, flags, data, data_len); + SCLogDebug("s->file_range == %p", s->file_range); if (s->file_range == NULL) { + SCLogDebug("s->file_range == NULL"); + THashDecrUsecnt(file_range_container->hdata); + DEBUG_VALIDATE_BUG_ON( + SC_ATOMIC_GET(file_range_container->hdata->use_cnt) > (uint32_t)INT_MAX); + THashDataUnlock(file_range_container->hdata); + // probably reached memcap return HTPFileOpen( s, filename, (uint32_t)filename_len, data, data_len, txid, STREAM_TOCLIENT); + /* in some cases we don't take a reference, so decr use cnt */ + } else if (s->file_range->container == NULL) { + THashDecrUsecnt(file_range_container->hdata); + } else { + SCLogDebug("container %p use_cnt %u", s->file_range, + SC_ATOMIC_GET(s->file_range->container->hdata->use_cnt)); + DEBUG_VALIDATE_BUG_ON( + SC_ATOMIC_GET(s->file_range->container->hdata->use_cnt) > (uint32_t)INT_MAX); } + /* we're done, so unlock. But since we have a reference in s->file_range keep use_cnt. */ + THashDataUnlock(file_range_container->hdata); SCReturnInt(0); } +static void HTPFileStoreChunkHandleRange( + HttpRangeContainerBlock *c, const uint8_t *data, uint32_t data_len) +{ + if (c->container) { + THashDataLock(c->container->hdata); + DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(c->container->hdata->use_cnt) == 0); + if (HttpRangeAppendData(c, data, data_len) < 0) { + SCLogDebug("Failed to append data"); + } + DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(c->container->hdata->use_cnt) > (uint32_t)INT_MAX); + THashDataUnlock(c->container->hdata); + } else if (c->toskip > 0) { + if (HttpRangeProcessSkip(c, data, data_len) < 0) { + SCLogDebug("Failed to append data"); + } + } +} + /** * \brief Store a chunk of data in the flow * @@ -381,10 +417,9 @@ int HTPFileStoreChunk(HtpState *s, const uint8_t *data, uint32_t data_len, } if (s->file_range != NULL) { - if (ContainerUrlRangeAppendData(s->file_range, data, data_len) < 0) { - SCLogDebug("Failed to append data"); - } + HTPFileStoreChunkHandleRange(s->file_range, data, data_len); } + result = FileAppendData(files, data, data_len); if (result == -1) { SCLogDebug("appending data failed"); @@ -397,6 +432,33 @@ int HTPFileStoreChunk(HtpState *s, const uint8_t *data, uint32_t data_len, SCReturnInt(retval); } +static void HTPFileCloseHandleRange(FileContainer *files, const uint8_t flags, + HttpRangeContainerBlock *c, const uint8_t *data, uint32_t data_len) +{ + if (c->container) { + THashDataLock(c->container->hdata); + if (c->container->error) { + SCLogDebug("range in ERROR state"); + } + if (HttpRangeAppendData(c, data, data_len) < 0) { + SCLogDebug("Failed to append data"); + } + File *ranged = HttpRangeClose(c, flags); + if (ranged) { + /* HtpState owns the constructed file now */ + FileContainerAdd(files, ranged); + } + SCLogDebug("c->container->files->tail %p", c->container->files->tail); + THashDataUnlock(c->container->hdata); + } else { + if (c->toskip > 0) { + if (HttpRangeProcessSkip(c, data, data_len) < 0) { + SCLogDebug("Failed to append data"); + } + } + } +} + /** * \brief Close the file in the flow * @@ -443,15 +505,10 @@ int HTPFileClose(HtpState *s, const uint8_t *data, uint32_t data_len, } else if (result == -2) { retval = -2; } + if (s->file_range != NULL) { - if (ContainerUrlRangeAppendData(s->file_range, data, data_len) < 0) { - SCLogDebug("Failed to append data"); - } - File *ranged = ContainerUrlRangeClose(s->file_range, flags); - if (ranged) { - FileContainerAdd(files, ranged); - } - SCFree(s->file_range); + HTPFileCloseHandleRange(files, flags, s->file_range, data, data_len); + HttpRangeFreeBlock(s->file_range); s->file_range = NULL; } diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c index b81e9a5d3a06..4a4c222754ca 100644 --- a/src/app-layer-htp-range.c +++ b/src/app-layer-htp-range.c @@ -38,6 +38,8 @@ typedef struct ContainerTHashTable { // globals ContainerTHashTable ContainerUrlRangeList; +static void HttpRangeBlockDerefContainer(HttpRangeContainerBlock *b); + #define CONTAINER_URLRANGE_HASH_SIZE 256 int HttpRangeContainerBufferCompare(HttpRangeContainerBuffer *a, HttpRangeContainerBuffer *b) @@ -66,16 +68,19 @@ static int ContainerUrlRangeSet(void *dst, void *src) HttpRangeContainerFile *dst_s = dst; dst_s->len = src_s->len; dst_s->key = SCMalloc(dst_s->len); - BUG_ON(dst_s->key == NULL); + if (dst_s->key == NULL) + return -1; memcpy(dst_s->key, src_s->key, dst_s->len); dst_s->files = FileContainerAlloc(); - BUG_ON(dst_s->files == NULL); + if (dst_s->files == NULL) { + SCFree(dst_s->key); + return -1; + } RB_INIT(&dst_s->fragment_tree); dst_s->flags = 0; dst_s->totalsize = 0; - SCMutexInit(&dst_s->mutex, NULL); dst_s->hdata = NULL; - + dst_s->error = false; return 0; } @@ -83,6 +88,13 @@ static bool ContainerUrlRangeCompare(void *a, void *b) { const HttpRangeContainerFile *as = a; const HttpRangeContainerFile *bs = b; + + /* ranges in the error state should not be found so they can + * be evicted */ + if (as->error || bs->error) { + return false; + } + if (SCBufferCmp(as->key, as->len, bs->key, bs->len) == 0) { return true; } @@ -99,7 +111,7 @@ static uint32_t ContainerUrlRangeHash(void *s) // base data stays in hash static void ContainerUrlRangeFree(void *s) { - HttpRangeContainerBuffer *range, *tmp; + HttpRangeContainerBuffer *range = NULL, *tmp = NULL; HttpRangeContainerFile *cu = s; SCFree(cu->key); @@ -112,16 +124,17 @@ static void ContainerUrlRangeFree(void *s) (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, range->buflen); SCFree(range); } - SCMutexDestroy(&cu->mutex); } -static bool ContainerValueRangeTimeout(HttpRangeContainerFile *cu, struct timeval *ts) +static inline bool ContainerValueRangeTimeout(HttpRangeContainerFile *cu, struct timeval *ts) { // we only timeout if we have no flow referencing us - SCMutexLock(&cu->mutex); - bool r = ((uint32_t)ts->tv_sec > cu->expire && SC_ATOMIC_GET(cu->hdata->use_cnt) == 0); - SCMutexUnlock(&cu->mutex); - return r; + if ((uint32_t)ts->tv_sec > cu->expire || cu->error) { + if (SC_ATOMIC_GET(cu->hdata->use_cnt) == 0) { + return true; + } + } + return false; } static void ContainerUrlRangeUpdate(HttpRangeContainerFile *cu, uint32_t expire) @@ -138,7 +151,7 @@ void HttpRangeContainersInit(void) const char *str = NULL; uint64_t memcap = HTTP_RANGE_DEFAULT_MEMCAP; uint32_t timeout = HTTP_RANGE_DEFAULT_TIMEOUT; - if (ConfGetValue("app-layer.protocols.http.urlrange.memcap", &str) == 1) { + if (ConfGetValue("app-layer.protocols.http.byterange.memcap", &str) == 1) { if (ParseSizeStringU64(str, &memcap) < 0) { SCLogWarning(SC_ERR_INVALID_VALUE, "memcap value cannot be deduced: %s," @@ -147,7 +160,7 @@ void HttpRangeContainersInit(void) memcap = 0; } } - if (ConfGetValue("app-layer.protocols.http.urlrange.timeout", &str) == 1) { + if (ConfGetValue("app-layer.protocols.http.byterange.timeout", &str) == 1) { if (StringParseUint32(&timeout, 10, strlen(str), str) <= 0) { SCLogWarning(SC_ERR_INVALID_VALUE, "timeout value cannot be deduced: %s," @@ -158,7 +171,7 @@ void HttpRangeContainersInit(void) } ContainerUrlRangeList.ht = - THashInit("app-layer.protocols.http.urlrange", sizeof(HttpRangeContainerFile), + THashInit("app-layer.protocols.http.byterange", sizeof(HttpRangeContainerFile), ContainerUrlRangeSet, ContainerUrlRangeFree, ContainerUrlRangeHash, ContainerUrlRangeCompare, false, memcap, CONTAINER_URLRANGE_HASH_SIZE); ContainerUrlRangeList.timeout = timeout; @@ -173,9 +186,10 @@ void HttpRangeContainersDestroy(void) uint32_t HttpRangeContainersTimeoutHash(struct timeval *ts) { + SCLogDebug("timeout: starting"); uint32_t cnt = 0; - for (size_t i = 0; i < ContainerUrlRangeList.ht->config.hash_size; i++) { + for (uint32_t i = 0; i < ContainerUrlRangeList.ht->config.hash_size; i++) { THashHashRow *hb = &ContainerUrlRangeList.ht->array[i]; if (HRLOCK_TRYLOCK(hb) != 0) @@ -183,7 +197,9 @@ uint32_t HttpRangeContainersTimeoutHash(struct timeval *ts) /* hash bucket is now locked */ THashData *h = hb->head; while (h) { + DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(h->use_cnt) > (uint32_t)INT_MAX); THashData *n = h->next; + THashDataLock(h); if (ContainerValueRangeTimeout(h->data, ts)) { /* remove from the hash */ if (h->prev != NULL) @@ -198,20 +214,30 @@ uint32_t HttpRangeContainersTimeoutHash(struct timeval *ts) h->prev = NULL; // we should log the timed out file somehow... // but it does not belong to any flow... - ContainerUrlRangeFree(h->data); + SCLogDebug("timeout: removing range %p", h); + ContainerUrlRangeFree(h->data); // TODO do we need a "RECYCLE" func? + DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(h->use_cnt) > (uint32_t)INT_MAX); + THashDataUnlock(h); THashDataMoveToSpare(ContainerUrlRangeList.ht, h); + } else { + THashDataUnlock(h); } h = n; } HRLOCK_UNLOCK(hb); } + SCLogDebug("timeout: ending"); return cnt; } +/** + * \returns locked data + */ void *HttpRangeContainerUrlGet(const uint8_t *key, size_t keylen, struct timeval *ts) { HttpRangeContainerFile lookup; + memset(&lookup, 0, sizeof(lookup)); // cast so as not to have const in the structure lookup.key = (uint8_t *)key; lookup.len = keylen; @@ -221,49 +247,43 @@ void *HttpRangeContainerUrlGet(const uint8_t *key, size_t keylen, struct timeval ContainerUrlRangeUpdate(res.data->data, ts->tv_sec + ContainerUrlRangeList.timeout); HttpRangeContainerFile *c = res.data->data; c->hdata = res.data; - THashDataUnlock(res.data); + SCLogDebug("c %p", c); return res.data->data; } return NULL; } -static HttpRangeContainerBlock *ContainerUrlRangeOpenFileAux(HttpRangeContainerFile *c, - uint64_t start, uint64_t end, uint64_t total, const StreamingBufferConfig *sbcfg, - const uint8_t *name, uint16_t name_len, uint16_t flags) +static HttpRangeContainerBlock *HttpRangeOpenFileAux(HttpRangeContainerFile *c, uint64_t start, + uint64_t end, uint64_t total, const StreamingBufferConfig *sbcfg, const uint8_t *name, + uint16_t name_len, uint16_t flags) { - SCMutexLock(&c->mutex); + DEBUG_VALIDATE_BUG_ON(c->files == NULL); + if (c->files->tail == NULL) { if (FileOpenFileWithId(c->files, sbcfg, 0, name, name_len, NULL, 0, flags) != 0) { SCLogDebug("open file for range failed"); - THashDecrUsecnt(c->hdata); - SCMutexUnlock(&c->mutex); return NULL; } } HttpRangeContainerBlock *curf = SCCalloc(1, sizeof(HttpRangeContainerBlock)); if (curf == NULL) { - THashDecrUsecnt(c->hdata); - SCMutexUnlock(&c->mutex); + c->error = true; return NULL; } if (total > c->totalsize) { // TODOask add checks about totalsize remaining the same c->totalsize = total; } - uint64_t buflen = end - start + 1; + const uint64_t buflen = end - start + 1; if (start == c->files->tail->size && !c->appending) { // easy case : append to current file curf->container = c; c->appending = true; - SCMutexUnlock(&c->mutex); return curf; } else if (start < c->files->tail->size && c->files->tail->size - start >= buflen) { // only overlap - THashDecrUsecnt(c->hdata); // redundant to be explicit that this block is independent - curf->container = NULL; curf->toskip = buflen; - SCMutexUnlock(&c->mutex); return curf; } else if (start < c->files->tail->size && c->files->tail->size - start < buflen && !c->appending) { @@ -271,47 +291,74 @@ static HttpRangeContainerBlock *ContainerUrlRangeOpenFileAux(HttpRangeContainerF curf->toskip = c->files->tail->size - start; c->appending = true; curf->container = c; - SCMutexUnlock(&c->mutex); return curf; } - // else { // block/range to be inserted in ordered linked list if (!(THASH_CHECK_MEMCAP(ContainerUrlRangeList.ht, buflen))) { - // TODOask release memory for other ranges cf RangeContainerFree(c); // skips this range curf->toskip = buflen; - curf->container = NULL; - THashDecrUsecnt(c->hdata); - SCMutexUnlock(&c->mutex); return curf; } curf->container = c; - (void)SC_ATOMIC_ADD(ContainerUrlRangeList.ht->memuse, buflen); + HttpRangeContainerBuffer *range = SCCalloc(1, sizeof(HttpRangeContainerBuffer)); - BUG_ON(range == NULL); + if (range == NULL) { + c->error = true; + SCFree(curf); + return NULL; + } + + (void)SC_ATOMIC_ADD(ContainerUrlRangeList.ht->memuse, buflen); range->buffer = SCMalloc(buflen); - BUG_ON(range->buffer == NULL); + if (range->buffer == NULL) { + c->error = true; + SCFree(curf); + SCFree(range); + (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, buflen); + return NULL; + } range->buflen = buflen; range->start = start; - curf->current = range; - SCMutexUnlock(&c->mutex); return curf; } -HttpRangeContainerBlock *ContainerUrlRangeOpenFile(HttpRangeContainerFile *c, uint64_t start, - uint64_t end, uint64_t total, const StreamingBufferConfig *sbcfg, const uint8_t *name, - uint16_t name_len, uint16_t flags, const uint8_t *data, size_t len) +HttpRangeContainerBlock *HttpRangeOpenFile(HttpRangeContainerFile *c, uint64_t start, uint64_t end, + uint64_t total, const StreamingBufferConfig *sbcfg, const uint8_t *name, uint16_t name_len, + uint16_t flags, const uint8_t *data, uint32_t len) { HttpRangeContainerBlock *r = - ContainerUrlRangeOpenFileAux(c, start, end, total, sbcfg, name, name_len, flags); - if (ContainerUrlRangeAppendData(r, data, len) < 0) { + HttpRangeOpenFileAux(c, start, end, total, sbcfg, name, name_len, flags); + if (HttpRangeAppendData(r, data, len) < 0) { SCLogDebug("Failed to append data while openeing"); } return r; } -int ContainerUrlRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, size_t len) +/** + * \note if we are called with a non-null c->container, it is locked + */ +int HttpRangeProcessSkip(HttpRangeContainerBlock *c, const uint8_t *data, const uint32_t len) +{ + SCLogDebug("update toskip: adding %u bytes to block %p", (uint32_t)len, c); + if (c->toskip >= len) { + c->toskip -= len; + return 0; + } + int r = 0; + if (c->container) { + if (data == NULL) { + // gap overlaping already known data + r = FileAppendData(c->container->files, NULL, len - c->toskip); + } else { + r = FileAppendData(c->container->files, data + c->toskip, len - c->toskip); + } + } + c->toskip = 0; + return r; +} + +int HttpRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, uint32_t len) { if (len == 0) { return 0; @@ -319,12 +366,20 @@ int ContainerUrlRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, // first check if we have a current allocated buffer to copy to // in the case of an unordered range being handled if (c->current) { + SCLogDebug("update current: adding %u bytes to block %p", len, c); + // GAP "data" if (data == NULL) { // just feed the gap in the current position, instead of its right one - return FileAppendData(c->container->files, data, len); - } else if (c->current->offset + len <= c->current->buflen) { + return FileAppendData(c->container->files, NULL, len); + // data, but we're not yet complete + } else if (c->current->offset + len < c->current->buflen) { memcpy(c->current->buffer + c->current->offset, data, len); c->current->offset += len; + // data, we're complete + } else if (c->current->offset + len == c->current->buflen) { + memcpy(c->current->buffer + c->current->offset, data, len); + c->current->offset += len; + // data, more than expected } else { memcpy(c->current->buffer + c->current->offset, data, c->current->buflen - c->current->offset); @@ -333,50 +388,39 @@ int ContainerUrlRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, return 0; // then check if we are skipping } else if (c->toskip > 0) { - if (c->toskip >= len) { - c->toskip -= len; - return 0; - } // else - DEBUG_VALIDATE_BUG_ON(c->container->files == NULL); - int r; - if (data == NULL) { - // gap overlaping already known data - r = FileAppendData(c->container->files, NULL, len - c->toskip); - } else { - r = FileAppendData(c->container->files, data + c->toskip, len - c->toskip); - } - c->toskip = 0; - return r; - } // else { + return HttpRangeProcessSkip(c, data, len); + } // last we are ordered, simply append DEBUG_VALIDATE_BUG_ON(c->container->files == NULL); + SCLogDebug("update files (FileAppendData)"); return FileAppendData(c->container->files, data, len); } -static void ContainerUrlRangeFileClose(HttpRangeContainerFile *c, uint16_t flags) +static void HttpRangeFileClose(HttpRangeContainerFile *c, uint16_t flags) { + SCLogDebug("closing range %p flags %04x", c, flags); DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(c->hdata->use_cnt) == 0); - THashDecrUsecnt(c->hdata); // move ownership of file c->files->head to caller FileCloseFile(c->files, NULL, 0, c->flags | flags); c->files->head = NULL; c->files->tail = NULL; - if (SC_ATOMIC_GET(c->hdata->use_cnt) == 0) { - THashRemoveFromHash(ContainerUrlRangeList.ht, c); - } - // otherwise, the hash entry will be used for another read of the file } -File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags) +/** + * \note if `f` is non-NULL, the ownership of the file is transfered to the caller. + */ +File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) { + SCLogDebug("c %p c->container %p c->current %p", c, c->container, c->current); + if (c->container == NULL) { // everything was just skipped : nothing to do return NULL; } - SCMutexLock(&c->container->mutex); - + /* we're processing an OOO chunk, won't be able to get us a full file just yet */ if (c->current) { + SCLogDebug("processing ooo chunk as c->current is set %p", c->current); // some out-or-order range is finished if (c->container->files->tail && c->container->files->tail->size >= c->current->start + c->current->offset) { @@ -385,30 +429,37 @@ File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags) (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, c->current->buflen); SCFree(c->current->buffer); SCFree(c->current); + c->current = NULL; + SCLogDebug("c->current was obsolete"); } else { - // otherwise insert in red and black tree - HTTP_RANGES_RB_INSERT(&c->container->fragment_tree, c->current); + /* otherwise insert in red and black tree. If res != NULL, the insert + failed because its a dup. */ + HttpRangeContainerBuffer *res = + HTTP_RANGES_RB_INSERT(&c->container->fragment_tree, c->current); + if (res) { + SCLogDebug("duplicate range fragment"); + SCFree(c->current->buffer); + SCFree(c->current); + } + SCLogDebug("inserted range fragment"); + c->current = NULL; } - THashDecrUsecnt(c->container->hdata); - SCMutexUnlock(&c->container->mutex); + SCLogDebug("c->current was set, file incomplete so return NULL"); return NULL; } - // else { if (c->toskip > 0) { // was only an overlapping range, truncated before new bytes - THashDecrUsecnt(c->container->hdata); - SCMutexUnlock(&c->container->mutex); + SCLogDebug("c->toskip %" PRIu64, c->toskip); return NULL; } - // else { // we just finished an in-order block c->container->appending = false; DEBUG_VALIDATE_BUG_ON(c->container->files->tail == NULL); File *f = c->container->files->tail; - // have we reached a saved range ? + /* See if we can use our stored fragments to (partly) reconstruct the file */ HttpRangeContainerBuffer *range, *safe = NULL; RB_FOREACH_SAFE (range, HTTP_RANGES, &c->container->fragment_tree, safe) { if (f->size < range->start) { @@ -417,8 +468,8 @@ File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags) if (f->size == range->start) { // a new range just begins where we ended, append it if (FileAppendData(c->container->files, range->buffer, range->offset) != 0) { - ContainerUrlRangeFileClose(c->container, flags); - SCMutexUnlock(&c->container->mutex); + HttpRangeFileClose(c->container, flags | FILE_TRUNCATED); + c->container->error = true; return f; } } else { @@ -429,13 +480,13 @@ File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags) // in this case of overlap, only add the extra data if (FileAppendData(c->container->files, range->buffer + overlap, range->offset - overlap) != 0) { - ContainerUrlRangeFileClose(c->container, flags); - SCMutexUnlock(&c->container->mutex); + HttpRangeFileClose(c->container, flags | FILE_TRUNCATED); + c->container->error = true; return f; } } } - // anyways, remove this range from the linked list, as we are now beyond it + /* Remove this range from the tree */ HTTP_RANGES_RB_REMOVE(&c->container->fragment_tree, range); (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, range->buflen); SCFree(range->buffer); @@ -444,12 +495,35 @@ File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags) if (f->size >= c->container->totalsize) { // we finished the whole file - ContainerUrlRangeFileClose(c->container, flags); + HttpRangeFileClose(c->container, flags); } else { // we are expecting more ranges - THashDecrUsecnt(c->container->hdata); f = NULL; + SCLogDebug("expecting more use_cnt %u", SC_ATOMIC_GET(c->container->hdata->use_cnt)); } - SCMutexUnlock(&c->container->mutex); + SCLogDebug("returning f %p (c:%p container:%p)", f, c, c->container); return f; } + +static void HttpRangeBlockDerefContainer(HttpRangeContainerBlock *b) +{ + if (b && b->container) { + DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(b->container->hdata->use_cnt) == 0); + THashDecrUsecnt(b->container->hdata); + b->container = NULL; + } +} + +void HttpRangeFreeBlock(HttpRangeContainerBlock *b) +{ + if (b) { + HttpRangeBlockDerefContainer(b); + + if (b->current) { + (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, b->current->buflen); + SCFree(b->current->buffer); + SCFree(b->current); + } + SCFree(b); + } +} diff --git a/src/app-layer-htp-range.h b/src/app-layer-htp-range.h index 039a72745b36..2d3fcc3bcc68 100644 --- a/src/app-layer-htp-range.h +++ b/src/app-layer-htp-range.h @@ -57,7 +57,7 @@ typedef struct HttpRangeContainerFile { uint32_t len; /** expire time in epoch */ uint32_t expire; - /** pointer to hashtable data, for use count */ + /** pointer to hashtable data, for locking and use count */ THashData *hdata; /** total epxected size of the file in ranges */ uint64_t totalsize; @@ -69,8 +69,8 @@ typedef struct HttpRangeContainerFile { uint16_t flags; /** wether a range file is currently appending */ bool appending; - /** mutex */ - SCMutex mutex; + /** error condition for this range. Its up to timeout handling to cleanup */ + bool error; } HttpRangeContainerFile; /** A structure representing a single range request : @@ -87,11 +87,14 @@ typedef struct HttpRangeContainerBlock { HttpRangeContainerFile *container; } HttpRangeContainerBlock; -int ContainerUrlRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, size_t len); -File *ContainerUrlRangeClose(HttpRangeContainerBlock *c, uint16_t flags); +int HttpRangeProcessSkip(HttpRangeContainerBlock *c, const uint8_t *data, const uint32_t len); +int HttpRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, uint32_t len); +File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags); -HttpRangeContainerBlock *ContainerUrlRangeOpenFile(HttpRangeContainerFile *c, uint64_t start, - uint64_t end, uint64_t total, const StreamingBufferConfig *sbcfg, const uint8_t *name, - uint16_t name_len, uint16_t flags, const uint8_t *data, size_t len); +HttpRangeContainerBlock *HttpRangeOpenFile(HttpRangeContainerFile *c, uint64_t start, uint64_t end, + uint64_t total, const StreamingBufferConfig *sbcfg, const uint8_t *name, uint16_t name_len, + uint16_t flags, const uint8_t *data, uint32_t len); + +void HttpRangeFreeBlock(HttpRangeContainerBlock *b); #endif /* __APP_LAYER_HTP_RANGE_H__ */ diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index 0afd74cdfe23..2c22b28e445a 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -374,6 +374,8 @@ void HTPStateFree(void *state) htp_connp_destroy_all(s->connp); } + HttpRangeFreeBlock(s->file_range); + FileContainerFree(s->files_ts); FileContainerFree(s->files_tc); HTPFree(s, sizeof(HtpState)); diff --git a/suricata.yaml.in b/suricata.yaml.in index 0f652a6a598e..d7804545d910 100644 --- a/suricata.yaml.in +++ b/suricata.yaml.in @@ -824,12 +824,11 @@ app-layer: http: enabled: yes - # Range Containers default settings - # urlrange: + # Byte Range Containers default settings + # byterange: # memcap: 100mb # timeout: 60 - # memcap: Maximum memory capacity for HTTP # Default is unlimited, values can be 64mb, e.g. From 96806500af7e81c68e17e0091e06643079c05053 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 17 Aug 2021 14:07:17 +0200 Subject: [PATCH 08/12] thash: add debug validation check for use_cnt --- src/util-thash.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util-thash.c b/src/util-thash.c index 9dba6cdbca0c..a46a0e3f8e2d 100644 --- a/src/util-thash.c +++ b/src/util-thash.c @@ -33,6 +33,7 @@ #include "util-byte.h" #include "util-hash-lookup3.h" +#include "util-validate.h" static THashData *THashGetUsed(THashTableContext *ctx); static void THashDataEnqueue (THashDataQueue *q, THashData *h); @@ -186,6 +187,8 @@ static THashData *THashDataAlloc(THashTableContext *ctx) static void THashDataFree(THashTableContext *ctx, THashData *h) { if (h != NULL) { + DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(h->use_cnt) != 0); + if (h->data != NULL) { ctx->config.DataFree(h->data); } From 75ce5e600af31f7ece7c32414c7f1f93536bce64 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 27 Aug 2021 17:11:23 +0200 Subject: [PATCH 09/12] http: avoid one lock for range append data Better structure design to ensure that one flow maximum is owning and appending into the file, adding fileOwning field. Adds also a gap field in a range buffer, so that we can feed the gap on closing, when we are protected from concurrency by a lock, (lock which got removed in the append path) Fixes memcap when encountering a duplicate while inserting in red and black tree Adds many comments --- src/app-layer-htp-file.c | 36 +++-------- src/app-layer-htp-range.c | 131 +++++++++++++++++++++++++------------- src/app-layer-htp-range.h | 20 ++++-- 3 files changed, 109 insertions(+), 78 deletions(-) diff --git a/src/app-layer-htp-file.c b/src/app-layer-htp-file.c index 7ac9746165db..9728079fa350 100644 --- a/src/app-layer-htp-file.c +++ b/src/app-layer-htp-file.c @@ -361,24 +361,6 @@ int HTPFileOpenWithRange(HtpState *s, const uint8_t *filename, uint16_t filename SCReturnInt(0); } -static void HTPFileStoreChunkHandleRange( - HttpRangeContainerBlock *c, const uint8_t *data, uint32_t data_len) -{ - if (c->container) { - THashDataLock(c->container->hdata); - DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(c->container->hdata->use_cnt) == 0); - if (HttpRangeAppendData(c, data, data_len) < 0) { - SCLogDebug("Failed to append data"); - } - DEBUG_VALIDATE_BUG_ON(SC_ATOMIC_GET(c->container->hdata->use_cnt) > (uint32_t)INT_MAX); - THashDataUnlock(c->container->hdata); - } else if (c->toskip > 0) { - if (HttpRangeProcessSkip(c, data, data_len) < 0) { - SCLogDebug("Failed to append data"); - } - } -} - /** * \brief Store a chunk of data in the flow * @@ -417,7 +399,9 @@ int HTPFileStoreChunk(HtpState *s, const uint8_t *data, uint32_t data_len, } if (s->file_range != NULL) { - HTPFileStoreChunkHandleRange(s->file_range, data, data_len); + if (HttpRangeAppendData(s->file_range, data, data_len) < 0) { + SCLogDebug("Failed to append data"); + } } result = FileAppendData(files, data, data_len); @@ -435,14 +419,16 @@ int HTPFileStoreChunk(HtpState *s, const uint8_t *data, uint32_t data_len, static void HTPFileCloseHandleRange(FileContainer *files, const uint8_t flags, HttpRangeContainerBlock *c, const uint8_t *data, uint32_t data_len) { + if (HttpRangeAppendData(c, data, data_len) < 0) { + SCLogDebug("Failed to append data"); + } if (c->container) { + // we only call HttpRangeClose if we may some new data + // ie we do not call it if we skipped all this range request THashDataLock(c->container->hdata); if (c->container->error) { SCLogDebug("range in ERROR state"); } - if (HttpRangeAppendData(c, data, data_len) < 0) { - SCLogDebug("Failed to append data"); - } File *ranged = HttpRangeClose(c, flags); if (ranged) { /* HtpState owns the constructed file now */ @@ -450,12 +436,6 @@ static void HTPFileCloseHandleRange(FileContainer *files, const uint8_t flags, } SCLogDebug("c->container->files->tail %p", c->container->files->tail); THashDataUnlock(c->container->hdata); - } else { - if (c->toskip > 0) { - if (HttpRangeProcessSkip(c, data, data_len) < 0) { - SCLogDebug("Failed to append data"); - } - } } } diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c index 4a4c222754ca..bc15acbaa0b0 100644 --- a/src/app-layer-htp-range.c +++ b/src/app-layer-htp-range.c @@ -78,6 +78,7 @@ static int ContainerUrlRangeSet(void *dst, void *src) } RB_INIT(&dst_s->fragment_tree); dst_s->flags = 0; + dst_s->lastsize = 0; dst_s->totalsize = 0; dst_s->hdata = NULL; dst_s->error = false; @@ -131,6 +132,7 @@ static inline bool ContainerValueRangeTimeout(HttpRangeContainerFile *cu, struct // we only timeout if we have no flow referencing us if ((uint32_t)ts->tv_sec > cu->expire || cu->error) { if (SC_ATOMIC_GET(cu->hdata->use_cnt) == 0) { + DEBUG_VALIDATE_BUG_ON(cu->fileOwned); return true; } } @@ -260,6 +262,11 @@ static HttpRangeContainerBlock *HttpRangeOpenFileAux(HttpRangeContainerFile *c, DEBUG_VALIDATE_BUG_ON(c->files == NULL); if (c->files->tail == NULL) { + DEBUG_VALIDATE_BUG_ON(c->fileOwned); + /* this is the first request, we open a single file in the file container + * this could be part of ContainerUrlRangeSet if we could have + * all the arguments there + */ if (FileOpenFileWithId(c->files, sbcfg, 0, name, name_len, NULL, 0, flags) != 0) { SCLogDebug("open file for range failed"); return NULL; @@ -270,29 +277,45 @@ static HttpRangeContainerBlock *HttpRangeOpenFileAux(HttpRangeContainerFile *c, c->error = true; return NULL; } + curf->fileOwning = false; if (total > c->totalsize) { - // TODOask add checks about totalsize remaining the same + // we grow to the maximum size indicated by different range requests + // we could add some warning/app-layer event in this case where + // different range requests indicate different total sizes c->totalsize = total; } const uint64_t buflen = end - start + 1; - if (start == c->files->tail->size && !c->appending) { + + /* The big part of this function is now to decide which kind of HttpRangeContainerBlock + * we will return : + * - skipping already processed data + * - storing out of order data for later use + * - directly appending to the file if we are at the right offset + */ + if (start == c->lastsize && !c->fileOwned) { // easy case : append to current file curf->container = c; - c->appending = true; + curf->fileOwning = true; + // If we see 2 duplicate range requests with the same range, + // the first one takes the ownership with fileOwned + // protected by the lock from caller HTPFileOpenWithRange + c->fileOwned = true; return curf; - } else if (start < c->files->tail->size && c->files->tail->size - start >= buflen) { + } else if (start < c->lastsize && c->lastsize - start >= buflen) { // only overlap // redundant to be explicit that this block is independent curf->toskip = buflen; return curf; - } else if (start < c->files->tail->size && c->files->tail->size - start < buflen && - !c->appending) { - // skip first overlap, then append - curf->toskip = c->files->tail->size - start; - c->appending = true; + } else if (start < c->lastsize && c->lastsize - start < buflen && !c->fileOwned) { + // some overlap, then some data to append to the file + curf->toskip = c->lastsize - start; + curf->fileOwning = true; + c->fileOwned = true; curf->container = c; return curf; } + // Because we are not in the previous cases, we will store the data for later use + // block/range to be inserted in ordered linked list if (!(THASH_CHECK_MEMCAP(ContainerUrlRangeList.ht, buflen))) { // skips this range @@ -319,6 +342,8 @@ static HttpRangeContainerBlock *HttpRangeOpenFileAux(HttpRangeContainerFile *c, } range->buflen = buflen; range->start = start; + range->offset = 0; + range->gap = 0; curf->current = range; return curf; } @@ -335,42 +360,47 @@ HttpRangeContainerBlock *HttpRangeOpenFile(HttpRangeContainerFile *c, uint64_t s return r; } -/** - * \note if we are called with a non-null c->container, it is locked - */ -int HttpRangeProcessSkip(HttpRangeContainerBlock *c, const uint8_t *data, const uint32_t len) +int HttpRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, uint32_t len) { - SCLogDebug("update toskip: adding %u bytes to block %p", (uint32_t)len, c); + if (len == 0) { + return 0; + } + // first check if we need to skip all bytes if (c->toskip >= len) { c->toskip -= len; return 0; } - int r = 0; - if (c->container) { - if (data == NULL) { - // gap overlaping already known data - r = FileAppendData(c->container->files, NULL, len - c->toskip); - } else { - r = FileAppendData(c->container->files, data + c->toskip, len - c->toskip); + // then if we need to skip only some bytes + if (c->toskip > 0) { + int r; + if (c->fileOwning) { + DEBUG_VALIDATE_BUG_ON(c->container->files == NULL); + if (data == NULL) { + // gap overlaping already known data + r = FileAppendData(c->container->files, NULL, len - c->toskip); + } else { + r = FileAppendData(c->container->files, data + c->toskip, len - c->toskip); + } } + c->toskip = 0; + return r; } - c->toskip = 0; - return r; -} - -int HttpRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, uint32_t len) -{ - if (len == 0) { - return 0; + // If we are owning the file to append to it, let's do it + if (c->fileOwning) { + DEBUG_VALIDATE_BUG_ON(c->container->files == NULL); + SCLogDebug("update files (FileAppendData)"); + return FileAppendData(c->container->files, data, len); } - // first check if we have a current allocated buffer to copy to + // If we are not in the previous cases, only one case remains + DEBUG_VALIDATE_BUG_ON(c->current == NULL); + // So we have a current allocated buffer to copy to // in the case of an unordered range being handled if (c->current) { SCLogDebug("update current: adding %u bytes to block %p", len, c); // GAP "data" if (data == NULL) { - // just feed the gap in the current position, instead of its right one - return FileAppendData(c->container->files, NULL, len); + // just save the length of the gap + c->current->gap += len; // data, but we're not yet complete } else if (c->current->offset + len < c->current->buflen) { memcpy(c->current->buffer + c->current->offset, data, len); @@ -386,14 +416,7 @@ int HttpRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, uint32_ c->current->offset = c->current->buflen; } return 0; - // then check if we are skipping - } else if (c->toskip > 0) { - return HttpRangeProcessSkip(c, data, len); } - // last we are ordered, simply append - DEBUG_VALIDATE_BUG_ON(c->container->files == NULL); - SCLogDebug("update files (FileAppendData)"); - return FileAppendData(c->container->files, data, len); } static void HttpRangeFileClose(HttpRangeContainerFile *c, uint16_t flags) @@ -413,17 +436,14 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) { SCLogDebug("c %p c->container %p c->current %p", c, c->container, c->current); - if (c->container == NULL) { - // everything was just skipped : nothing to do - return NULL; - } + DEBUG_VALIDATE_BUG_ON(c->container == NULL); /* we're processing an OOO chunk, won't be able to get us a full file just yet */ if (c->current) { SCLogDebug("processing ooo chunk as c->current is set %p", c->current); // some out-or-order range is finished if (c->container->files->tail && - c->container->files->tail->size >= c->current->start + c->current->offset) { + c->container->lastsize >= c->current->start + c->current->offset) { // if the range has become obsolete because we received the data already // we just free it (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, c->current->buflen); @@ -438,6 +458,7 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) HTTP_RANGES_RB_INSERT(&c->container->fragment_tree, c->current); if (res) { SCLogDebug("duplicate range fragment"); + (void)SC_ATOMIC_SUB(ContainerUrlRangeList.ht->memuse, c->current->buflen); SCFree(c->current->buffer); SCFree(c->current); } @@ -455,18 +476,30 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) } // we just finished an in-order block - c->container->appending = false; + DEBUG_VALIDATE_BUG_ON(c->container->fileOwned == false); + c->container->fileOwned = false; DEBUG_VALIDATE_BUG_ON(c->container->files->tail == NULL); + // we update the file size now that we own it again + c->container->lastsize = c->container->files->tail->size; File *f = c->container->files->tail; /* See if we can use our stored fragments to (partly) reconstruct the file */ HttpRangeContainerBuffer *range, *safe = NULL; RB_FOREACH_SAFE (range, HTTP_RANGES, &c->container->fragment_tree, safe) { if (f->size < range->start) { + // this next range is not reached yet break; } if (f->size == range->start) { // a new range just begins where we ended, append it + if (range->gap > 0) { + // if the range had a gap, begin by it + if (FileAppendData(c->container->files, NULL, range->gap) != 0) { + HttpRangeFileClose(c->container, flags | FILE_TRUNCATED); + c->container->error = true; + return f; + } + } if (FileAppendData(c->container->files, range->buffer, range->offset) != 0) { HttpRangeFileClose(c->container, flags | FILE_TRUNCATED); c->container->error = true; @@ -476,6 +509,14 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) // the range starts before where we ended uint64_t overlap = f->size - range->start; if (overlap < range->offset) { + if (range->gap > 0) { + // if the range had a gap, begin by it + if (FileAppendData(c->container->files, NULL, range->gap) != 0) { + HttpRangeFileClose(c->container, flags | FILE_TRUNCATED); + c->container->error = true; + return f; + } + } // And the range ends beyond where we ended // in this case of overlap, only add the extra data if (FileAppendData(c->container->files, range->buffer + overlap, diff --git a/src/app-layer-htp-range.h b/src/app-layer-htp-range.h index 2d3fcc3bcc68..835750cb7730 100644 --- a/src/app-layer-htp-range.h +++ b/src/app-layer-htp-range.h @@ -38,6 +38,8 @@ typedef struct HttpRangeContainerBuffer { uint64_t start; /** offset of bytes written in buffer (relative to the start of the range) */ uint64_t offset; + /** number of gaped bytes */ + uint64_t gap; } HttpRangeContainerBuffer; int HttpRangeContainerBufferCompare(HttpRangeContainerBuffer *a, HttpRangeContainerBuffer *b); @@ -46,9 +48,13 @@ RB_HEAD(HTTP_RANGES, HttpRangeContainerBuffer); RB_PROTOTYPE(HTTP_RANGES, HttpRangeContainerBuffer, rb, HttpRangeContainerBufferCompare); /** Item in hash table for a file in multiple ranges - * Thread-safety is ensured by the thread-safe hash table + * Thread-safety is ensured with the thread-safe hash table cf THashData * The number of use is increased for each flow opening a new HttpRangeContainerBlock * until it closes this HttpRangeContainerBlock + * The design goal is to have concurrency only on opening and closing a range request + * and have a lock-free data structure belonging to one Flow + * (see HttpRangeContainerBlock below) + * for every append in between (we suppose we have many appends per range request) */ typedef struct HttpRangeContainerFile { /** key for hashtable */ @@ -59,16 +65,19 @@ typedef struct HttpRangeContainerFile { uint32_t expire; /** pointer to hashtable data, for locking and use count */ THashData *hdata; - /** total epxected size of the file in ranges */ + /** total expected size of the file in ranges */ uint64_t totalsize; + /** size of the file after last sync */ + uint64_t lastsize; /** file container, with only one file */ FileContainer *files; /** red and black tree list of ranges which came out of order */ struct HTTP_RANGES fragment_tree; /** file flags */ uint16_t flags; - /** wether a range file is currently appending */ - bool appending; + /** wether a HttpRangeContainerBlock is currently + owning the FileContainer in order to append to the file */ + bool fileOwned; /** error condition for this range. Its up to timeout handling to cleanup */ bool error; } HttpRangeContainerFile; @@ -85,9 +94,10 @@ typedef struct HttpRangeContainerBlock { HttpRangeContainerBuffer *current; /** pointer to the main file container, where to directly append data */ HttpRangeContainerFile *container; + /** we own the container's file for now */ + bool fileOwning; } HttpRangeContainerBlock; -int HttpRangeProcessSkip(HttpRangeContainerBlock *c, const uint8_t *data, const uint32_t len); int HttpRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, uint32_t len); File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags); From 0c96b5f4e56b266e9c3374a1a60a2d4d7e1f0615 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 27 Aug 2021 17:36:50 +0200 Subject: [PATCH 10/12] ci: dummy git configuration for rebase --- .github/workflows/builds.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index a624357976a1..6c0c7a62e62f 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -102,6 +102,8 @@ jobs: cd suricata-verify git fetch origin pull/${sv_pr}/head:prep git checkout prep + git config --global user.email you@example.com + git config --global user.name You git rebase ${DEFAULT_SV_BRANCH} cd .. fi From ef29258869de4f67f93be2fc2d28aab911034974 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 27 Aug 2021 17:49:01 +0200 Subject: [PATCH 11/12] fixup warnings --- src/app-layer-htp-range.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c index bc15acbaa0b0..2d62ba727091 100644 --- a/src/app-layer-htp-range.c +++ b/src/app-layer-htp-range.c @@ -372,7 +372,7 @@ int HttpRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, uint32_ } // then if we need to skip only some bytes if (c->toskip > 0) { - int r; + int r = 0; if (c->fileOwning) { DEBUG_VALIDATE_BUG_ON(c->container->files == NULL); if (data == NULL) { @@ -415,8 +415,8 @@ int HttpRangeAppendData(HttpRangeContainerBlock *c, const uint8_t *data, uint32_ c->current->buflen - c->current->offset); c->current->offset = c->current->buflen; } - return 0; } + return 0; } static void HttpRangeFileClose(HttpRangeContainerFile *c, uint16_t flags) From 08eca7e490df3b08378503b7d1a83ed6b4c4dc53 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 27 Aug 2021 19:51:00 +0200 Subject: [PATCH 12/12] fixup SV --- src/app-layer-htp-range.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/app-layer-htp-range.c b/src/app-layer-htp-range.c index 2d62ba727091..4ffa085493c0 100644 --- a/src/app-layer-htp-range.c +++ b/src/app-layer-htp-range.c @@ -480,7 +480,6 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) c->container->fileOwned = false; DEBUG_VALIDATE_BUG_ON(c->container->files->tail == NULL); // we update the file size now that we own it again - c->container->lastsize = c->container->files->tail->size; File *f = c->container->files->tail; /* See if we can use our stored fragments to (partly) reconstruct the file */ @@ -495,12 +494,14 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) if (range->gap > 0) { // if the range had a gap, begin by it if (FileAppendData(c->container->files, NULL, range->gap) != 0) { + c->container->lastsize = f->size; HttpRangeFileClose(c->container, flags | FILE_TRUNCATED); c->container->error = true; return f; } } if (FileAppendData(c->container->files, range->buffer, range->offset) != 0) { + c->container->lastsize = f->size; HttpRangeFileClose(c->container, flags | FILE_TRUNCATED); c->container->error = true; return f; @@ -512,6 +513,7 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) if (range->gap > 0) { // if the range had a gap, begin by it if (FileAppendData(c->container->files, NULL, range->gap) != 0) { + c->container->lastsize = f->size; HttpRangeFileClose(c->container, flags | FILE_TRUNCATED); c->container->error = true; return f; @@ -521,6 +523,7 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) // in this case of overlap, only add the extra data if (FileAppendData(c->container->files, range->buffer + overlap, range->offset - overlap) != 0) { + c->container->lastsize = f->size; HttpRangeFileClose(c->container, flags | FILE_TRUNCATED); c->container->error = true; return f; @@ -533,6 +536,8 @@ File *HttpRangeClose(HttpRangeContainerBlock *c, uint16_t flags) SCFree(range->buffer); SCFree(range); } + // wait until we merged all the buffers to update known size + c->container->lastsize = f->size; if (f->size >= c->container->totalsize) { // we finished the whole file