From: Jeff Lucovsky <jeff@lucovsky.org>
Date: Tue, 7 May 2019 22:49:57 +0000 (-0700)
Subject: eve/ftp: Transaction support for unmatched requests
X-Git-Tag: suricata-5.0.0-rc1~195
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2149807bd62b02d0203887f8cd57b958268ccc4a;p=thirdparty%2Fsuricata.git

eve/ftp: Transaction support for unmatched requests

Modified transaction logic to create a new transaction with each
request; replies location transactions by using the oldest "open"
(unmatched) transaction or the last transaction if none are open.
---

diff --git a/doc/userguide/output/eve/eve-json-format.rst b/doc/userguide/output/eve/eve-json-format.rst
index 5eaf0240c7..338f9aadfa 100644
--- a/doc/userguide/output/eve/eve-json-format.rst
+++ b/doc/userguide/output/eve/eve-json-format.rst
@@ -545,6 +545,7 @@ Fields
 * "completion_code": The 3-digit completion code. The first digit indicates whether the response is good, bad or incomplete.
 * "dynamic_port": The dynamic port established for subsequent data transfers, when applicable, with a "PORT" or "EPRT" command.
 * "mode": The type of FTP connection. Most connections are "passive" but may be "active".
+* "reply_received": Indicates whether a response was matched to the command. In some non-typical cases, a command may lack a response.
 
 
 Examples
diff --git a/src/app-layer-ftp.c b/src/app-layer-ftp.c
index 86a6e822f6..176a804243 100644
--- a/src/app-layer-ftp.c
+++ b/src/app-layer-ftp.c
@@ -123,6 +123,8 @@ uint64_t ftp_config_memcap = 0;
 SC_ATOMIC_DECLARE(uint64_t, ftp_memuse);
 SC_ATOMIC_DECLARE(uint64_t, ftp_memcap);
 
+static void *FTPGetOldestTx(FtpState *);
+
 static void FTPParseMemcap(void)
 {
     const char *conf_val;
@@ -287,9 +289,6 @@ static void FTPTransactionFree(FTPTransaction *tx)
     if (tx->request) {
         FTPFree(tx->request, tx->request_length);
     }
-    if (tx->response) {
-        FTPFree(tx->response, tx->response_length);
-    }
 
     FTPString *str = NULL;
     while ((str = TAILQ_FIRST(&tx->response_list))) {
@@ -597,13 +596,10 @@ static int FTPParseRequest(Flow *f, void *ftp_state,
 
         state->command = cmd_descriptor->command;
 
-        FTPTransaction *tx = state->curr_tx;
-        if (tx == NULL || tx->done) {
-            tx = FTPTransactionCreate(state);
-            if (unlikely(tx == NULL))
-                return -1;
-            state->curr_tx = tx;
-        }
+        FTPTransaction *tx = FTPTransactionCreate(state);
+        if (unlikely(tx == NULL))
+            return -1;
+        state->curr_tx = tx;
 
         tx->command_descriptor = cmd_descriptor;
         tx->request_length = CopyCommandLine(&tx->request, state->current_line, state->current_line_len);
@@ -754,13 +750,14 @@ static int FTPParseResponse(Flow *f, void *ftp_state, AppLayerParserState *pstat
                             void *local_data, const uint8_t flags)
 {
     FtpState *state = (FtpState *)ftp_state;
-    FTPTransaction *tx = state->curr_tx;
     int retcode = 1;
 
     if (state->command == FTP_COMMAND_UNKNOWN) {
         return 1;
     }
 
+    FTPTransaction *tx = FTPGetOldestTx(state);
+    state->curr_tx = tx;
     if (state->command == FTP_COMMAND_AUTH_TLS) {
         if (input_len >= 4 && SCMemcmp("234 ", input, 4) == 0) {
             AppLayerRequestProtocolTLSUpgrade(f);
@@ -811,8 +808,7 @@ static int FTPParseResponse(Flow *f, void *ftp_state, AppLayerParserState *pstat
         FTPString *response = FTPStringAlloc();
         if (likely(response)) {
             response->len = CopyCommandLine(&response->str, input, input_len);
-            if (response->str)
-                TAILQ_INSERT_TAIL(&tx->response_list, response, next);
+            TAILQ_INSERT_TAIL(&tx->response_list, response, next);
         }
     }
 
@@ -860,6 +856,10 @@ static void FTPStateFree(void *s)
     FTPTransaction *tx = NULL;
     while ((tx = TAILQ_FIRST(&fstate->tx_list))) {
         TAILQ_REMOVE(&fstate->tx_list, tx, next);
+        SCLogDebug("[%s] state %p id %"PRIu64", Freeing %d bytes at %p",
+            tx->command_descriptor->command_name_upper,
+            s, tx->tx_id,
+            tx->request_length, tx->request);
         FTPTransactionFree(tx);
     }
 
@@ -879,6 +879,36 @@ static int FTPSetTxDetectState(void *vtx, DetectEngineState *de_state)
     return 0;
 }
 
+/**
+ * \brief This function returns the oldest open transaction; if none
+ * are open, then the oldest transaction is returned
+ * \param ftp_state the ftp state structure for the parser
+ *
+ * \retval transaction pointer when a transaction was found; NULL otherwise.
+ */
+static void *FTPGetOldestTx(FtpState *ftp_state)
+{
+    if (unlikely(!ftp_state)) {
+        SCLogDebug("NULL state object; no transactions available");
+        return NULL;
+    }
+    FTPTransaction *tx = NULL;
+    FTPTransaction *lasttx = NULL;
+    TAILQ_FOREACH(tx, &ftp_state->tx_list, next) {
+        /* Return oldest open tx */
+        if (!tx->done) {
+            SCLogDebug("Returning tx %p id %"PRIu64, tx, tx->tx_id);
+            return tx;
+        }
+        /* save for the end */
+        lasttx = tx;
+    }
+    /* All tx are closed; return last element */
+    if (lasttx)
+        SCLogDebug("Returning OLDEST tx %p id %"PRIu64, lasttx, lasttx->tx_id);
+    return lasttx;
+}
+
 static void *FTPGetTx(void *state, uint64_t tx_id)
 {
     FtpState *ftp_state = (FtpState *)state;
@@ -896,7 +926,6 @@ static void *FTPGetTx(void *state, uint64_t tx_id)
         }
     }
     return NULL;
-
 }
 
 static DetectEngineState *FTPGetTxDetectState(void *vtx)
diff --git a/src/app-layer-ftp.h b/src/app-layer-ftp.h
index 35e633d674..ec4a6ddd00 100644
--- a/src/app-layer-ftp.h
+++ b/src/app-layer-ftp.h
@@ -19,6 +19,7 @@
  * \file
  *
  * \author Pablo Rincon Crespo <pablo.rincon.crespo@gmail.com>
+ * \author Jeff Lucovsky <jeff@lucovsky.org>
  */
 
 #ifndef __APP_LAYER_FTP_H__
@@ -138,21 +139,22 @@ typedef struct FTPTransaction_  {
 
     /** indicates loggers done logging */
     uint32_t logged;
-    bool done;
 
+    /* for the request */
+    uint32_t request_length;
+    uint8_t *request;
+
+    /* for the command description */
     const FtpCommand *command_descriptor;
 
-    uint8_t direction;
-    uint16_t dyn_port;
-    bool active;
+    uint16_t dyn_port; /* dynamic port, if applicable */
+    bool done; /* transaction complete? */
+    bool active; /* active or passive mode */
 
-    uint8_t *request;
-    uint32_t request_length;
+    uint8_t direction;
 
     /* Handle multiple responses */
     TAILQ_HEAD(, FTPString_) response_list;
-    uint8_t *response;
-    uint32_t response_length;
 
     DetectEngineState *de_state;
 
diff --git a/src/output-json-ftp.c b/src/output-json-ftp.c
index 605654fa24..755692084f 100644
--- a/src/output-json-ftp.c
+++ b/src/output-json-ftp.c
@@ -109,6 +109,8 @@ static void JsonFTPLogJSON(json_t *tjs, Flow *f, FTPTransaction *tx)
                 json_object_set_new(cjs, "mode",
                         json_string((char*)(tx->active ? "active" : "passive")));
             }
+            json_object_set_new(cjs, "reply_received",
+                    json_string((char*)(tx->done ? "yes" : "no")));
         }
     }
 
diff --git a/suricata.yaml.in b/suricata.yaml.in
index 8d7c1ef17e..7cb4d74353 100644
--- a/suricata.yaml.in
+++ b/suricata.yaml.in
@@ -232,7 +232,7 @@ outputs:
             #md5: [body, subject]
 
         #- dnp3
-        #- ftp
+        - ftp
         - nfs
         - smb
         - tftp