Backports 5.0.x V2 Batched backports of several issues

.github/workflows/builds.yml

@@ -70,7 +70,7 @@ jobs:
                 texlive-upquote \
                 texlive-capt-of \
                 texlive-needspace \
-      - uses: actions/checkout@v1
+      - uses: actions/checkout@v2
       - name: Bundling libhtp
         run: git clone https://github.com/OISF/libhtp -b 0.5.x
       - name: Bundling suricata-update

ChangeLog

@@ -1,3 +1,16 @@
+5.0.2 -- 2020-02-13
+
+Bug #2993: Suricata 5.0.0beta1 memory allocation of 4294966034 bytes failed
+Bug #3380: Segfault when using multi-detect
+Bug #3400: smb: post-GAP file tx handling
+Bug #3424: nfs: post-GAP some transactions never close
+Bug #3425: nfs: post-GAP file tx handling
+Bug #3433: coverity: CID 1456679: Memory - corruptions (NEGATIVE_RETURNS)
+Bug #3434: coverity: CID 1456680: Incorrect expression (IDENTICAL_BRANCHES)
+Bug #3469: gcc10: compilation failure unless -fcommon is supplied (5.0.x)
+Bug #3473: Dropping privileges does not work with NFLOG (5.0.x)
+Documentation #3423: readthedocs shows title of documentation as "Suricata unknown documentation"
+
 5.0.1 -- 2019-12-13
 
 Bug #1871: intermittent abort()s at shutdown and in unix-socket

configure.ac

@@ -1,4 +1,4 @@
-    AC_INIT([suricata],[5.0.2-dev])
+    AC_INIT([suricata],[5.0.3-dev])
     m4_ifndef([AM_SILENT_RULES], [m4_define([AM_SILENT_RULES],[])])AM_SILENT_RULES([yes])
     AC_CONFIG_HEADERS([config.h])
     AC_CONFIG_SRCDIR([src/suricata.c])

doc/userguide/performance/tuning-considerations.rst

@@ -20,10 +20,10 @@ The memory is set up at start and the usage is as follows:
 mpm-algo: <ac|hs|ac-bs|ac-ks>
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-Controls the pattern matcher algorithm. AC (``Aho–Corasick ``) is the default. 
+Controls the pattern matcher algorithm. AC (``Aho–Corasick``) is the default.
 On supported platforms, :doc:`hyperscan` is the best option. On commodity 
 hardware if Hyperscan is not available the suggested setting is 
-``mpm-algo: ac-ks`` (``Aho–Corasick `` Ken Steele variant) as it performs better than 
+``mpm-algo: ac-ks`` (``Aho–Corasick`` Ken Steele variant) as it performs better than
 ``mpm-algo: ac``
 
 detect.profile: <low|medium|high|custom>

doc/userguide/rules/header-keywords.rst

@@ -111,6 +111,25 @@ The named variant of that example would be::
 
     ip_proto:PIM
 
+ipv4.hdr
+^^^^^^^^
+
+Sticky buffer to match on the whole IPv4 header.
+
+Example rule:
+
+.. container:: example-rule
+
+    alert ip any any -> any any (:example-rule-emphasis:`ipv4.hdr; content:"|3A|"; offset:9; depth:1;` sid:1234; rev:5;)
+
+This example looks if byte 9 of IPv4 header has value 3A.
+That means that the IPv4 protocol is ICMPv6.
+
+ipv6.hdr
+^^^^^^^^
+
+Sticky buffer to match on the whole IPv6 header.
+
 id
 ^^
 

doc/userguide/rules/payload-keywords.rst

@@ -304,7 +304,7 @@ The ``byte_test`` keyword extracts ``<num of bytes>`` and performs an operation
 Format::
 
   byte_test:<num of bytes>, [!]<operator>, <test value>, <offset> [,relative] \
-  [,<endian>][, string, <num type>][, dce][, bitmask <bitmask value>]; 
+  [,<endian>][, string, <num type>][, dce]; 
 
 
 +----------------+------------------------------------------------------------------------------+
@@ -337,8 +337,6 @@ Format::
 +----------------+------------------------------------------------------------------------------+
 | [dce]		 | Allow the DCE module determine the byte order 				|
 +----------------+------------------------------------------------------------------------------+
-| [bitmask]	 | Applies the AND operator on the bytes converted				|
-+----------------+------------------------------------------------------------------------------+
 
 
 Example::
@@ -376,7 +374,7 @@ Format::
 
   byte_jump:<num of bytes>, <offset> [, relative][, multiplier <mult_value>] \
 	[, <endian>][, string, <num_type>][, align][, from_beginning][, from_end] \ 
-        [, post_offset <value>][, dce][, bitmask <value>];
+        [, post_offset <value>][, dce];
 
 +-----------------------+-----------------------------------------------------------------------+
 | <num of bytes>	| The number of bytes selected from the packet to be converted		|
@@ -408,9 +406,6 @@ Format::
 +-----------------------+-----------------------------------------------------------------------+
 | [dce]			| Allow the DCE module determine the byte order				|
 +-----------------------+-----------------------------------------------------------------------+
-| [bitmask] <value>	| The AND operator will be applied by <value> and the			|
-|			| converted bytes, then jump operation is performed			|
-+-----------------------+-----------------------------------------------------------------------+
 
 Example::
 

rules/smtp-events.rules

@@ -29,4 +29,5 @@ alert smtp any any -> any any (msg:"SURICATA SMTP Mime boundary length exceeded"
 
 alert smtp any any -> any any (msg:"SURICATA SMTP duplicate fields"; flow:established,to_server; app-layer-event:smtp.duplicate_fields; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220018; rev:1;)
 alert smtp any any -> any any (msg:"SURICATA SMTP unparsable content"; flow:established,to_server; app-layer-event:smtp.unparsable_content; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220019; rev:1;)
-# next sid 2220020
+alert smtp any any -> any any (msg:"SURICATA SMTP filename truncated"; flow:established,to_server; app-layer-event:smtp.mime_long_filename; flowint:smtp.anomaly.count,+,1; classtype:protocol-command-decode; sid:2220020; rev:1;)
+# next sid 2220021

rust/src/core.rs

@@ -21,7 +21,6 @@ use std;
 use crate::filecontainer::*;
 
 /// Opaque C types.
-pub enum Flow {}
 pub enum DetectEngineState {}
 pub enum AppLayerDecoderEvents {}
 
@@ -183,3 +182,26 @@ pub fn sc_app_layer_decoder_events_free_events(
         }
     }
 }
+
+/// Opaque flow type (defined in C)
+pub enum Flow {}
+
+/// Extern functions operating on Flow.
+extern {
+    pub fn FlowGetLastTimeAsParts(flow: &Flow, secs: *mut u64, usecs: *mut u64);
+}
+
+/// Rust implementation of Flow.
+impl Flow {
+
+    /// Return the time of the last flow update as a `Duration`
+    /// since the epoch.
+    pub fn get_last_time(&mut self) -> std::time::Duration {
+        unsafe {
+            let mut secs: u64 = 0;
+            let mut usecs: u64 = 0;
+            FlowGetLastTimeAsParts(self, &mut secs, &mut usecs);
+            std::time::Duration::new(secs, usecs as u32 * 1000)
+        }
+    }
+}

rust/src/nfs/nfs.rs

@@ -119,6 +119,10 @@ pub struct NFSTransactionFile {
     /// last xid of this file transfer. Last READ or COMMIT normally.
     pub file_last_xid: u32,
 
+    /// after a gap, this will be set to a time in the future. If the file
+    /// receives no updates before that, it will be considered complete.
+    pub post_gap_ts: u64,
+
     /// file tracker for a single file. Boxed so that we don't use
     /// as much space if we're not a file tx.
     pub file_tracker: FileTransferTracker,
@@ -130,6 +134,7 @@ impl NFSTransactionFile {
             file_additional_procs: Vec::new(),
             chunk_count:0,
             file_last_xid: 0,
+            post_gap_ts: 0,
             file_tracker: FileTransferTracker::new(),
         }
     }
@@ -338,12 +343,20 @@ pub struct NFSState {
 
     is_udp: bool,
 
+    /// true as long as we have file txs that are in a post-gap
+    /// state. It means we'll do extra house keeping for those.
+    check_post_gap_file_txs: bool,
+
     pub nfs_version: u16,
 
     pub events: u16,
 
     /// tx counter for assigning incrementing id's to tx's
     tx_id: u64,
+
+    /// Timestamp in seconds of last update. This is packet time,
+    /// potentially coming from pcaps.
+    ts: u64,
 }
 
 impl NFSState {
@@ -366,9 +379,11 @@ impl NFSState {
             ts_gap:false,
             tc_gap:false,
             is_udp:false,
+            check_post_gap_file_txs:false,
             nfs_version:0,
             events:0,
             tx_id:0,
+            ts: 0,
         }
     }
     pub fn free(&mut self) {
@@ -483,6 +498,71 @@ impl NFSState {
         }
     }
 
+    fn post_gap_housekeeping_for_files(&mut self)
+    {
+        let mut post_gap_txs = false;
+        for tx in &mut self.transactions {
+            if let Some(NFSTransactionTypeData::FILE(ref f)) = tx.type_data {
+                if f.post_gap_ts > 0 {
+                    if self.ts > f.post_gap_ts {
+                        tx.request_done = true;
+                        tx.response_done = true;
+                    } else {
+                        post_gap_txs = true;
+                    }
+                }
+            }
+        }
+        self.check_post_gap_file_txs = post_gap_txs;
+    }
+
+    /* after a gap we will consider all transactions complete for our
+     * direction. File transfer transactions are an exception. Those
+     * can handle gaps. For the file transactions we set the current
+     * (flow) time and prune them in 60 seconds if no update for them
+     * was received. */
+    fn post_gap_housekeeping(&mut self, dir: u8)
+    {
+        if self.ts_ssn_gap && dir == STREAM_TOSERVER {
+            for tx in &mut self.transactions {
+                if tx.id >= self.tx_id {
+                    SCLogDebug!("post_gap_housekeeping: done");
+                    break;
+                }
+                if let Some(NFSTransactionTypeData::FILE(ref mut f)) = tx.type_data {
+                    // leaving FILE txs open as they can deal with gaps. We
+                    // remove them after 60 seconds of no activity though.
+                    if f.post_gap_ts == 0 {
+                        f.post_gap_ts = self.ts + 60;
+                        self.check_post_gap_file_txs = true;
+                    }
+                } else {
+                    SCLogDebug!("post_gap_housekeeping: tx {} marked as done TS", tx.id);
+                    tx.request_done = true;
+                }
+            }
+        } else if self.tc_ssn_gap && dir == STREAM_TOCLIENT {
+            for tx in &mut self.transactions {
+                if tx.id >= self.tx_id {
+                    SCLogDebug!("post_gap_housekeeping: done");
+                    break;
+                }
+                if let Some(NFSTransactionTypeData::FILE(ref mut f)) = tx.type_data {
+                    // leaving FILE txs open as they can deal with gaps. We
+                    // remove them after 60 seconds of no activity though.
+                    if f.post_gap_ts == 0 {
+                        f.post_gap_ts = self.ts + 60;
+                        self.check_post_gap_file_txs = true;
+                    }
+                } else {
+                    SCLogDebug!("post_gap_housekeeping: tx {} marked as done TC", tx.id);
+                    tx.request_done = true;
+                    tx.response_done = true;
+                }
+            }
+        }
+    }
+
     pub fn process_request_record_lookup<'b>(&mut self, r: &RpcPacket<'b>, xidmap: &mut NFSRequestXidMap) {
         match parse_nfs3_request_lookup(r.prog_data) {
             Ok((_, lookup)) => {
@@ -764,6 +844,11 @@ impl NFSState {
                         }
                     }
 
+                    // reset timestamp if we get called after a gap
+                    if tdf.post_gap_ts > 0 {
+                        tdf.post_gap_ts = 0;
+                    }
+
                     tdf.chunk_count += 1;
                     let cs = tdf.file_tracker.update(files, flags, data, gap_size);
                     /* see if we need to close the tx */
@@ -1113,6 +1198,12 @@ impl NFSState {
                 },
             }
         };
+
+        self.post_gap_housekeeping(STREAM_TOSERVER);
+        if self.check_post_gap_file_txs {
+            self.post_gap_housekeeping_for_files();
+        }
+
         status
     }
 
@@ -1271,6 +1362,10 @@ impl NFSState {
                 },
             }
         };
+        self.post_gap_housekeeping(STREAM_TOCLIENT);
+        if self.check_post_gap_file_txs {
+            self.post_gap_housekeeping_for_files();
+        }
         status
     }
     /// Parsing function
@@ -1357,7 +1452,7 @@ pub extern "C" fn rs_nfs_state_free(state: *mut std::os::raw::c_void) {
 
 /// C binding parse a NFS TCP request. Returns 1 on success, -1 on failure.
 #[no_mangle]
-pub extern "C" fn rs_nfs_parse_request(_flow: *mut Flow,
+pub extern "C" fn rs_nfs_parse_request(flow: &mut Flow,
                                        state: &mut NFSState,
                                        _pstate: *mut std::os::raw::c_void,
                                        input: *const u8,
@@ -1368,6 +1463,7 @@ pub extern "C" fn rs_nfs_parse_request(_flow: *mut Flow,
     let buf = unsafe{std::slice::from_raw_parts(input, input_len as usize)};
     SCLogDebug!("parsing {} bytes of request data", input_len);
 
+    state.ts = flow.get_last_time().as_secs();
     if state.parse_tcp_data_ts(buf) == 0 {
         1
     } else {
@@ -1388,7 +1484,7 @@ pub extern "C" fn rs_nfs_parse_request_tcp_gap(
 }
 
 #[no_mangle]
-pub extern "C" fn rs_nfs_parse_response(_flow: *mut Flow,
+pub extern "C" fn rs_nfs_parse_response(flow: &mut Flow,
                                         state: &mut NFSState,
                                         _pstate: *mut std::os::raw::c_void,
                                         input: *const u8,
@@ -1399,6 +1495,7 @@ pub extern "C" fn rs_nfs_parse_response(_flow: *mut Flow,
     SCLogDebug!("parsing {} bytes of response data", input_len);
     let buf = unsafe{std::slice::from_raw_parts(input, input_len as usize)};
 
+    state.ts = flow.get_last_time().as_secs();
     if state.parse_tcp_data_tc(buf) == 0 {
         1
     } else {

rust/src/nfs/rpc_records.rs

@@ -359,3 +359,23 @@ named!(pub parse_rpc_udp_reply<RpcReplyPacket>,
            }
    ))
 );
+
+#[cfg(test)]
+mod tests {
+    use crate::nfs::rpc_records::*;
+    use nom::Err::Incomplete;
+    use nom::Needed::Size;
+
+    #[test]
+    fn test_parse_input_too_short() {
+        let buf: &[u8] = &[
+            0x80, 0x0, 0x0, 0x9c, 0x8e, 0x28, 0x2, 0x7e
+        ];
+        let r = parse_rpc_request_partial(buf);
+        match r {
+            Err(Incomplete(e)) => { assert_eq!(e, Size(4)); },
+             _ => { panic!("failed {:?}",r); }
+       }
+    }
+}
+

rust/src/smb/files.rs

@@ -30,6 +30,9 @@ pub struct SMBTransactionFile {
     pub file_name: Vec<u8>,
     pub share_name: Vec<u8>,
     pub file_tracker: FileTransferTracker,
+    /// after a gap, this will be set to a time in the future. If the file
+    /// receives no updates before that, it will be considered complete.
+    pub post_gap_ts: u64,
 }
 
 impl SMBTransactionFile {
@@ -40,6 +43,7 @@ impl SMBTransactionFile {
             file_name: Vec::new(),
             share_name: Vec::new(),
             file_tracker: FileTransferTracker::new(),
+            post_gap_ts: 0,
         }
     }
 }
@@ -201,6 +205,11 @@ impl SMBState {
                         }
                     }
 
+                    // reset timestamp if we get called after a gap
+                    if tdf.post_gap_ts > 0 {
+                        tdf.post_gap_ts = 0;
+                    }
+
                     let file_data = &data[0..data_to_handle_len];
                     let cs = tdf.file_tracker.update(files, flags, file_data, gap_size);
                     cs