Feature/http range/v23

.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

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

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 \

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
@@ -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,104 @@ 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);
+    }
+    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);
 }
 
 /**
@@ -292,6 +398,12 @@ int HTPFileStoreChunk(HtpState *s, const uint8_t *data, uint32_t data_len,
         goto end;
     }
 
+    if (s->file_range != NULL) {
+        if (HttpRangeAppendData(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");
@@ -304,6 +416,29 @@ 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 (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");
+        }
+        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);
+    }
+}
+
 /**
  *  \brief Close the file in the flow
  *
@@ -351,6 +486,12 @@ int HTPFileClose(HtpState *s, const uint8_t *data, uint32_t data_len,
         retval = -2;
     }
 
+    if (s->file_range != NULL) {
+        HTPFileCloseHandleRange(files, flags, s->file_range, data, data_len);
+        HttpRangeFreeBlock(s->file_range);
+        s->file_range = NULL;
+    }
+
 end:
     SCReturnInt(retval);
 }

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);