From 8adf172ab896f54ddb97ef7faac444f9db7ea2ae Mon Sep 17 00:00:00 2001
From: Philippe Antoine <contact@catenacyber.fr>
Date: Thu, 3 Feb 2022 21:55:09 +0100
Subject: [PATCH] nfs: limits the number of active transactions per flow

Ticket: 4530
---
 doc/userguide/configuration/suricata-yaml.rst | 18 +++++----------
 rules/nfs-events.rules                        |  1 +
 rust/src/nfs/nfs.rs                           | 22 +++++++++++++++++++
 suricata.yaml.in                              |  1 +
 4 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/doc/userguide/configuration/suricata-yaml.rst b/doc/userguide/configuration/suricata-yaml.rst
index 8cb069d2d7..67b839f8d3 100644
--- a/doc/userguide/configuration/suricata-yaml.rst
+++ b/doc/userguide/configuration/suricata-yaml.rst
@@ -1388,20 +1388,12 @@ Its default value is 4096 bytes, but it can be set to any uint32 by a flow.
 `http2.max-streams` refers to `SETTINGS_MAX_CONCURRENT_STREAMS` from rfc 7540 section 6.5.2.
 Its default value is unlimited.
 
-Configure MQTT
-~~~~~~~~~~~~~~
-
-MQTT has one parameter that can be customized.
-`mqtt.max-tx` refers to the maximum number of live transactions for each flow.
-The app-layer event `mqtt.too_many_transactions` is triggered when this value is reached.
-The point of this parameter is to find a balance between the completeness of analysis
-and the resource consumption.
-
-Configure FTP
-~~~~~~~~~~~~~
+Maximum transactions
+~~~~~~~~~~~~~~~~~~~~
 
-FTP has one parameter that can be customized.
-`ftp.max-tx` refers to the maximum number of live transactions for each flow.
+MQTT, FTP, and NFS have each a `max-tx` parameter that can be customized.
+`max-tx` refers to the maximum number of live transactions for each flow.
+An app-layer event `protocol.too_many_transactions` is triggered when this value is reached.
 The point of this parameter is to find a balance between the completeness of analysis
 and the resource consumption.
 
diff --git a/rules/nfs-events.rules b/rules/nfs-events.rules
index 33adc3b343..0f86893ad4 100644
--- a/rules/nfs-events.rules
+++ b/rules/nfs-events.rules
@@ -6,3 +6,4 @@
 #
 alert nfs any any -> any any (msg:"SURICATA NFS malformed request data"; flow:to_server; app-layer-event:nfs.malformed_data; classtype:protocol-command-decode; sid:2223000; rev:1;)
 alert nfs any any -> any any (msg:"SURICATA NFS malformed response data"; flow:to_client; app-layer-event:nfs.malformed_data; classtype:protocol-command-decode; sid:2223001; rev:1;)
+alert nfs any any -> any any (msg:"SURICATA NFS too many transactions"; app-layer-event:nfs.too_many_transactions; classtype:protocol-command-decode; sid:2223002; rev:1;)
diff --git a/rust/src/nfs/nfs.rs b/rust/src/nfs/nfs.rs
index 0781dcc228..3706a9d072 100644
--- a/rust/src/nfs/nfs.rs
+++ b/rust/src/nfs/nfs.rs
@@ -41,6 +41,8 @@ pub static mut SURICATA_NFS_FILE_CONFIG: Option<&'static SuricataFileContext> =
 
 pub const NFS_MIN_FRAME_LEN: u16 = 32;
 
+static mut NFS_MAX_TX: usize = 1024;
+
 static mut ALPROTO_NFS: AppProto = ALPROTO_UNKNOWN;
 /*
  * Record parsing.
@@ -90,6 +92,7 @@ pub enum NFSEvent {
     MalformedData = 0,
     NonExistingVersion = 1,
     UnsupportedVersion = 2,
+    TooManyTransactions = 3,
 }
 
 #[derive(Debug)]
@@ -344,6 +347,18 @@ impl NFSState {
         let mut tx = NFSTransaction::new();
         self.tx_id += 1;
         tx.id = self.tx_id;
+        if self.transactions.len() > unsafe { NFS_MAX_TX } {
+            // set at least one another transaction to the drop state
+            for tx_old in &mut self.transactions {
+                if !tx_old.request_done || !tx_old.response_done {
+                    tx_old.request_done = true;
+                    tx_old.response_done = true;
+                    tx_old.is_file_closed = true;
+                    tx_old.tx_data.set_event(NFSEvent::TooManyTransactions as u8);
+                    break;
+                }
+            }
+        }
         return tx;
     }
 
@@ -1912,6 +1927,13 @@ pub unsafe extern "C" fn rs_nfs_udp_register_parser() {
         {
             let _ = AppLayerRegisterParser(&parser, alproto);
         }
+        if let Some(val) = conf_get("app-layer.protocols.nfs.max-tx") {
+            if let Ok(v) = val.parse::<usize>() {
+                NFS_MAX_TX = v;
+            } else {
+                SCLogError!("Invalid value for nfs.max-tx");
+            }
+        }
         SCLogDebug!("Rust nfs parser registered.");
     } else {
         SCLogDebug!("Protocol detector and parser disabled for nfs.");
diff --git a/suricata.yaml.in b/suricata.yaml.in
index 49a6a43bff..66e7d8b43d 100644
--- a/suricata.yaml.in
+++ b/suricata.yaml.in
@@ -886,6 +886,7 @@ app-layer:
 
     nfs:
       enabled: yes
+      # max-tx: 1024
     tftp:
       enabled: yes
     dns: