]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
source-pcap-file: Directory mode may miss files (bug #2394)
authorDanny Browning <danny.browning@protectwise.com>
Thu, 21 Dec 2017 21:09:08 +0000 (14:09 -0700)
committerVictor Julien <victor@inliniac.net>
Thu, 1 Mar 2018 13:38:43 +0000 (14:38 +0100)
https://redmine.openinfosecfoundation.org/issues/2394

Certain parameters of delay and poll interval could cause newly added
files in a directory to be missed. Cleaned up how time is handled for
files in a directory and fix which time is used for future directory
traversals. Add a mutex to make sure processing time is not optimized
away.

scripts/suricatasc/src/suricatasc.py
src/runmode-unix-socket.c
src/source-pcap-file-directory-helper.c
src/source-pcap-file-directory-helper.h
src/source-pcap-file.c
src/suricata.c
src/util-time.h

index 03483f37c2ce5d6dc23e067253cdfb42000a1541..22501afc2530e1ad1aea741e9ad64c90c8475bcf 100644 (file)
@@ -203,12 +203,13 @@ class SuricataSC:
                 tenant = None
                 if len(parts) > 3:
                     tenant = parts[3]
-                if cmd != "pcap-file-continuous":
+                if cmd != "pcap-file":
                     raise SuricataCommandException("Invalid command '%s'" % (command))
                 else:
                     arguments = {}
                     arguments["filename"] = filename
                     arguments["output-dir"] = output
+                    arguments["continuous"] = True
                     if tenant != None:
                         arguments["tenant"] = int(tenant)
             elif "iface-stat" in command:
index c28eee836fbe06f6e4ce839a1e4999a3982b490c..8702e9c1eb0044783973b6cca16ff7747c010bc8 100644 (file)
@@ -34,6 +34,7 @@
 #include "flow-timeout.h"
 #include "stream-tcp.h"
 #include "stream-tcp-reassemble.h"
+#include "source-pcap-file-directory-helper.h"
 #include "host.h"
 #include "defrag.h"
 #include "defrag-hash.h"
@@ -132,6 +133,7 @@ static int unix_manager_pcap_task_running = 0;
 static int unix_manager_pcap_task_failed = 0;
 static int unix_manager_pcap_task_interrupted = 0;
 static struct timespec unix_manager_pcap_last_processed;
+static SCCtrlMutex unix_manager_pcap_last_processed_mutex;
 
 /**
  * \brief return list of files in the queue
@@ -186,7 +188,7 @@ static TmEcode UnixSocketPcapCurrent(json_t *cmd, json_t* answer, void *data)
 {
     PcapCommand *this = (PcapCommand *) data;
 
-    if (this->current_file && this->current_file->filename) {
+    if (this->current_file != NULL && this->current_file->filename != NULL) {
         json_object_set_new(answer, "message",
                             json_string(this->current_file->filename));
     } else {
@@ -197,8 +199,11 @@ static TmEcode UnixSocketPcapCurrent(json_t *cmd, json_t* answer, void *data)
 
 static TmEcode UnixSocketPcapLastProcessed(json_t *cmd, json_t *answer, void *data)
 {
-    uint64_t epoch_millis = unix_manager_pcap_last_processed.tv_sec * 1000l +
-                        unix_manager_pcap_last_processed.tv_nsec / 100000l;
+    json_int_t epoch_millis;
+    SCCtrlMutexLock(&unix_manager_pcap_last_processed_mutex);
+    epoch_millis = SCTimespecAsEpochMillis(&unix_manager_pcap_last_processed);
+    SCCtrlMutexUnlock(&unix_manager_pcap_last_processed_mutex);
+
     json_object_set_new(answer, "message",
                         json_integer(epoch_millis));
 
@@ -422,7 +427,7 @@ static TmEcode UnixSocketAddPcapFile(json_t *cmd, json_t* answer, void *data)
 }
 
 /**
- * \brief Command to add a file to treatment list
+ * \brief Command to add a file to treatment list, forcing continuous mode
  *
  * \param cmd the content of command Arguments as a json_t object
  * \param answer the json_t object that has to be used to answer
@@ -467,6 +472,7 @@ static TmEcode UnixSocketPcapFilesCheck(void *data)
         }
         this->current_file = NULL;
     }
+
     if (TAILQ_EMPTY(&this->files)) {
         // nothing to do
         return TM_ECODE_OK;
@@ -573,8 +579,10 @@ void RunModeUnixSocketRegister(void)
 TmEcode UnixSocketPcapFile(TmEcode tm, struct timespec *last_processed)
 {
 #ifdef BUILD_UNIX_SOCKET
+    SCCtrlMutexLock(&unix_manager_pcap_last_processed_mutex);
     unix_manager_pcap_last_processed.tv_sec = last_processed->tv_sec;
     unix_manager_pcap_last_processed.tv_nsec = last_processed->tv_nsec;
+    SCCtrlMutexUnlock(&unix_manager_pcap_last_processed_mutex);
     switch (tm) {
         case TM_ECODE_DONE:
             SCLogInfo("Marking current task as done");
@@ -1399,6 +1407,10 @@ static int RunModeUnixSocketMaster(void)
     pcapcmd->running = 0;
     pcapcmd->current_file = NULL;
 
+    memset(&unix_manager_pcap_last_processed, 0, sizeof(struct timespec));
+
+    SCCtrlMutexInit(&unix_manager_pcap_last_processed_mutex, NULL);
+
     UnixManagerRegisterCommand("pcap-file", UnixSocketAddPcapFile, pcapcmd, UNIX_CMD_TAKE_ARGS);
     UnixManagerRegisterCommand("pcap-file-continuous", UnixSocketAddPcapFileContinuous, pcapcmd, UNIX_CMD_TAKE_ARGS);
     UnixManagerRegisterCommand("pcap-file-number", UnixSocketPcapFilesNumber, pcapcmd, 0);
index 7367442c964e0a6dbf318fef6f077c6f45b4d0fc..d026c1f5b7debca7aa5ee3e4281b4408bed3a2f5 100644 (file)
@@ -35,13 +35,10 @@ static TmEcode PcapDirectoryFailure(PcapFileDirectoryVars *ptv);
 static TmEcode PcapDirectoryDone(PcapFileDirectoryVars *ptv);
 static int PcapDirectoryGetModifiedTime(char const * file, struct timespec * out);
 static TmEcode PcapDirectoryInsertFile(PcapFileDirectoryVars *pv,
-                                       PendingFile *file_to_add,
-                                       struct timespec *file_to_add_modified_time);
+                                       PendingFile *file_to_add);
 static TmEcode PcapDirectoryPopulateBuffer(PcapFileDirectoryVars *ptv,
-                                           struct timespec * newer_than,
                                            struct timespec * older_than);
 static TmEcode PcapDirectoryDispatchForTimeRange(PcapFileDirectoryVars *pv,
-                                                 struct timespec *newer_than,
                                                  struct timespec *older_than);
 
 void GetTime(struct timespec *tm)
@@ -49,7 +46,7 @@ void GetTime(struct timespec *tm)
     struct timeval now;
     if(gettimeofday(&now, NULL) == 0) {
         tm->tv_sec  = now.tv_sec;
-        tm->tv_nsec = now.tv_usec * 1000;
+        tm->tv_nsec = now.tv_usec * 1000L;
     }
 }
 
@@ -248,22 +245,20 @@ int PcapDirectoryGetModifiedTime(char const *file, struct timespec *out)
 #endif
 
 #ifdef OS_DARWIN
-    *out = buf.st_mtimespec;
+    out->tv_sec = buf.st_mtimespec.tv_sec;
+    out->tv_nsec = buf.st_mtimespec.tv_nsec;
 #elif OS_WIN32
-    struct timespec ts;
-    memset(&ts, 0, sizeof(ts));
-    ts.tv_sec = buf.st_mtime;
-    *out = ts;
+    out->tv_sec = buf.st_mtime;
 #else
-    *out = buf.st_mtim;
+    out->tv_sec = buf.st_mtim.tv_sec;
+    out->tv_nsec = buf.st_mtim.tv_nsec;
 #endif
 
     return ret;
 }
 
 TmEcode PcapDirectoryInsertFile(PcapFileDirectoryVars *pv,
-                                PendingFile *file_to_add,
-                                struct timespec *file_to_add_modified_time
+                                PendingFile *file_to_add
 ) {
     PendingFile *file_to_compare = NULL;
     PendingFile *next_file_to_compare = NULL;
@@ -290,14 +285,7 @@ TmEcode PcapDirectoryInsertFile(PcapFileDirectoryVars *pv,
     } else {
         file_to_compare = TAILQ_FIRST(&pv->directory_content);
         while(file_to_compare != NULL) {
-            struct timespec modified_time;
-            memset(&modified_time, 0, sizeof(struct timespec));
-
-            if (PcapDirectoryGetModifiedTime(file_to_compare->filename,
-                                             &modified_time) == TM_ECODE_FAILED) {
-                SCReturnInt(TM_ECODE_FAILED);
-            }
-            if (CompareTimes(file_to_add_modified_time, &modified_time) < 0) {
+            if (CompareTimes(&file_to_add->modified_time, &file_to_compare->modified_time) < 0) {
                 TAILQ_INSERT_BEFORE(file_to_compare, file_to_add, next);
                 file_to_compare = NULL;
             } else {
@@ -315,7 +303,6 @@ TmEcode PcapDirectoryInsertFile(PcapFileDirectoryVars *pv,
 }
 
 TmEcode PcapDirectoryPopulateBuffer(PcapFileDirectoryVars *pv,
-                                    struct timespec *newer_than,
                                     struct timespec *older_than
 ) {
     if (unlikely(pv == NULL)) {
@@ -355,11 +342,14 @@ TmEcode PcapDirectoryPopulateBuffer(PcapFileDirectoryVars *pv,
             memset(&temp_time, 0, sizeof(struct timespec));
 
             if (PcapDirectoryGetModifiedTime(pathbuff, &temp_time) == 0) {
-                SCLogDebug("File %s time (%lu > %lu < %lu)", pathbuff,
-                           newer_than->tv_sec, temp_time.tv_sec, older_than->tv_sec);
+                SCLogDebug("%" PRIuMAX " < %" PRIuMAX "(%s) < %" PRIuMAX ")",
+                           (uintmax_t)SCTimespecAsEpochMillis(&pv->shared->last_processed),
+                           (uintmax_t)SCTimespecAsEpochMillis(&temp_time),
+                           pathbuff,
+                           (uintmax_t)SCTimespecAsEpochMillis(older_than));
 
                 // Skip files outside of our time range
-                if (CompareTimes(&temp_time, newer_than) < 0) {
+                if (CompareTimes(&temp_time, &pv->shared->last_processed) <= 0) {
                     SCLogDebug("Skipping old file %s", pathbuff);
                     continue;
                 }
@@ -387,9 +377,13 @@ TmEcode PcapDirectoryPopulateBuffer(PcapFileDirectoryVars *pv,
                 SCReturnInt(TM_ECODE_FAILED);
             }
 
-            SCLogDebug("Found \"%s\"", file_to_add->filename);
+            memset(&file_to_add->modified_time, 0, sizeof(struct timespec));
+            CopyTime(&temp_time, &file_to_add->modified_time);
 
-            if (PcapDirectoryInsertFile(pv, file_to_add, &temp_time) == TM_ECODE_FAILED) {
+            SCLogInfo("Found \"%s\" at %" PRIuMAX, file_to_add->filename,
+                       (uintmax_t)SCTimespecAsEpochMillis(&file_to_add->modified_time));
+
+            if (PcapDirectoryInsertFile(pv, file_to_add) == TM_ECODE_FAILED) {
                 SCLogError(SC_ERR_INVALID_ARGUMENT, "Failed to add file");
                 CleanupPendingFile(file_to_add);
 
@@ -403,10 +397,9 @@ TmEcode PcapDirectoryPopulateBuffer(PcapFileDirectoryVars *pv,
 
 
 TmEcode PcapDirectoryDispatchForTimeRange(PcapFileDirectoryVars *pv,
-                                          struct timespec *newer_than,
                                           struct timespec *older_than)
 {
-    if (PcapDirectoryPopulateBuffer(pv, newer_than, older_than) == TM_ECODE_FAILED) {
+    if (PcapDirectoryPopulateBuffer(pv, older_than) == TM_ECODE_FAILED) {
         SCLogError(SC_ERR_INVALID_ARGUMENT, "Failed to populate directory buffer");
         SCReturnInt(TM_ECODE_FAILED);
     }
@@ -414,7 +407,7 @@ TmEcode PcapDirectoryDispatchForTimeRange(PcapFileDirectoryVars *pv,
     TmEcode status = TM_ECODE_OK;
 
     if (TAILQ_EMPTY(&pv->directory_content)) {
-        SCLogInfo("Directory %s has no files to process", pv->filename);
+        SCLogDebug("Directory %s has no files to process", pv->filename);
         GetTime(older_than);
         older_than->tv_sec = older_than->tv_sec - pv->delay;
         rewinddir(pv->directory);
@@ -422,6 +415,9 @@ TmEcode PcapDirectoryDispatchForTimeRange(PcapFileDirectoryVars *pv,
     } else {
         PendingFile *current_file = NULL;
 
+        struct timespec last_time_seen;
+        memset(&last_time_seen, 0, sizeof(struct timespec));
+
         while (status == TM_ECODE_OK && !TAILQ_EMPTY(&pv->directory_content)) {
             current_file = TAILQ_FIRST(&pv->directory_content);
             TAILQ_REMOVE(&pv->directory_content, current_file, next);
@@ -467,16 +463,13 @@ TmEcode PcapDirectoryDispatchForTimeRange(PcapFileDirectoryVars *pv,
                         SCReturnInt(status);
                     }
 
-                    struct timespec temp_time;
-                    memset(&temp_time, 0, sizeof(struct timespec));
+                    SCLogInfo("Processed file %s, processed up to %" PRIuMAX,
+                               current_file->filename,
+                               (uintmax_t)SCTimespecAsEpochMillis(&current_file->modified_time));
 
-                    if (PcapDirectoryGetModifiedTime(current_file->filename,
-                                                     &temp_time) != 0) {
-                        CopyTime(&temp_time, newer_than);
+                    if(CompareTimes(&current_file->modified_time, &last_time_seen) > 0) {
+                        CopyTime(&current_file->modified_time, &last_time_seen);
                     }
-                    SCLogDebug("Processed file %s, processed up to %ld",
-                               current_file->filename, temp_time.tv_sec);
-                    CopyTime(&temp_time, &pv->shared->last_processed);
 
                     CleanupPendingFile(current_file);
                     pv->current_file = NULL;
@@ -486,7 +479,12 @@ TmEcode PcapDirectoryDispatchForTimeRange(PcapFileDirectoryVars *pv,
             }
         }
 
-        *newer_than = *older_than;
+        if(CompareTimes(&last_time_seen, &pv->shared->last_processed) > 0) {
+            SCLogInfo("Updating processed to %" PRIuMAX,
+                      (uintmax_t)SCTimespecAsEpochMillis(&last_time_seen));
+            CopyTime(&last_time_seen, &pv->shared->last_processed);
+            status = PcapRunStatus(pv);
+        }
     }
     GetTime(older_than);
     older_than->tv_sec = older_than->tv_sec - pv->delay;
@@ -500,8 +498,6 @@ TmEcode PcapDirectoryDispatch(PcapFileDirectoryVars *ptv)
 
     DIR *directory_check = NULL;
 
-    struct timespec newer_than;
-    memset(&newer_than, 0, sizeof(struct timespec));
     struct timespec older_than;
     memset(&older_than, 0, sizeof(struct timespec));
     older_than.tv_sec = LONG_MAX;
@@ -516,8 +512,9 @@ TmEcode PcapDirectoryDispatch(PcapFileDirectoryVars *ptv)
     while (status == TM_ECODE_OK) {
         //loop while directory is ok
         SCLogInfo("Processing pcaps directory %s, files must be newer than %" PRIuMAX " and older than %" PRIuMAX,
-                  ptv->filename, (uintmax_t)newer_than.tv_sec, (uintmax_t)older_than.tv_sec);
-        status = PcapDirectoryDispatchForTimeRange(ptv, &newer_than, &older_than);
+                  ptv->filename, (uintmax_t)SCTimespecAsEpochMillis(&ptv->shared->last_processed),
+                  (uintmax_t)SCTimespecAsEpochMillis(&older_than));
+        status = PcapDirectoryDispatchForTimeRange(ptv, &older_than);
         if (ptv->should_loop && status == TM_ECODE_OK) {
             sleep(poll_seconds);
             //update our status based on suricata control flags or unix command socket
index 496e83793b4adbf9c49d203642c8e517ff2e1e68..bc416db2bdc90522e881d128d856d3421d449c4d 100644 (file)
@@ -31,6 +31,7 @@
 typedef struct PendingFile_
 {
     char *filename;
+    struct timespec modified_time;
     TAILQ_ENTRY(PendingFile_) next;
 } PendingFile;
 /**
index 30c5483bdecc75ebc583b17d0da42a6e5e0a7420..c08a4d7492bfb00be2f34dabc2ad5a21f00d095f 100644 (file)
@@ -193,6 +193,7 @@ TmEcode ReceivePcapFileThreadInit(ThreadVars *tv, const void *initdata, void **d
     if (unlikely(ptv == NULL))
         SCReturnInt(TM_ECODE_FAILED);
     memset(ptv, 0, sizeof(PcapFileThreadVars));
+    memset(&ptv->shared.last_processed, 0, sizeof(struct timespec));
 
     intmax_t tenant = 0;
     if (ConfGetInt("pcap-file.tenant-id", &tenant) == 1) {
index a2d64eac7e435e44746e9be7f8b36913b99adb9a..8b39ea68f5bca260a27a6f194ee29c6bd5b0d93f 100644 (file)
@@ -2264,6 +2264,7 @@ void PostRunDeinit(const int runmode, struct timeval *start_time)
     HostCleanup();
     StreamTcpFreeConfig(STREAM_VERBOSE);
     DefragDestroy();
+
     TmqResetQueues();
 #ifdef PROFILING
     if (profiling_rules_enabled)
index 00c254b97b2b01d08de7c2f67c58a5e01c1e26c2..a7e53ca989c7cf40cbd036b36c410c8d2dcb9e3b 100644 (file)
@@ -63,5 +63,6 @@ int SCTimeToStringPattern (time_t epoch, const char *pattern, char *str,
 uint64_t SCParseTimeSizeString (const char *str);
 uint64_t SCGetSecondsUntil (const char *str, time_t epoch);
 uint64_t SCTimespecAsEpochMillis(const struct timespec *ts);
+
 #endif /* __UTIL_TIME_H__ */