]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
smb: improve request/response mapping 3593/head
authorVictor Julien <victor@inliniac.net>
Thu, 20 Dec 2018 08:11:21 +0000 (09:11 +0100)
committerVictor Julien <victor@inliniac.net>
Thu, 20 Dec 2018 09:32:48 +0000 (10:32 +0100)
Only use ssn_id and msg_id for mapping a response to a request.

By not using the tree_id it can always be included in the tx.hdr which
means it can be logged properly in case of IOCTL and DCERPC.

rust/src/smb/dcerpc.rs
rust/src/smb/session.rs
rust/src/smb/smb.rs
rust/src/smb/smb2_ioctl.rs

index 9b71f13cbdfe093e1e76b321e987e2210df05bd0..9d9536d2fb939612fc9561970ffdf07834d7f7da 100644 (file)
@@ -232,7 +232,7 @@ impl SMBState {
 
         SCLogDebug!("looking for {:?}", dce_hdr);
         for tx in &mut self.transactions {
-            let found = dce_hdr == tx.hdr.to_dcerpc(vercmd) &&
+            let found = dce_hdr.compare(&tx.hdr.to_dcerpc(vercmd)) &&
                 match tx.type_data {
                 Some(SMBTransactionTypeData::DCERPC(ref x)) => {
                     x.call_id == call_id
index 741a0d63d005fa99584d86c1da87520a0ab1cf10..9f35c6d7c7ff991e537d101a6e71803393e89a9e 100644 (file)
@@ -62,7 +62,7 @@ impl SMBState {
         -> Option<&mut SMBTransaction>
     {
         for tx in &mut self.transactions {
-            let hit = tx.hdr == hdr && match tx.type_data {
+            let hit = tx.hdr.compare(&hdr) && match tx.type_data {
                 Some(SMBTransactionTypeData::SESSIONSETUP(_)) => { true },
                 _ => { false },
             };
index 9f2f1d42e4f26ab2b49c36142319f2d8528b0344..82d2d5ea831a442f3706f5755ecfce74d40c95ca 100644 (file)
@@ -701,6 +701,12 @@ impl SMBCommonHdr {
             msg_id : msg_id,
         }
     }
+
+    // don't include tree id
+    pub fn compare(&self, hdr: &SMBCommonHdr) -> bool {
+        (self.rec_type == hdr.rec_type && self.ssn_id == hdr.ssn_id &&
+         self.msg_id == hdr.msg_id)
+    }
 }
 
 #[derive(Hash, Eq, PartialEq, Debug)]
@@ -973,10 +979,10 @@ impl SMBState {
             let found = if tx.vercmd.get_version() == smb_ver {
                 if smb_ver == 1 {
                     let (_, cmd) = tx.vercmd.get_smb1_cmd();
-                    cmd as u16 == smb_cmd && tx.hdr == *key
+                    cmd as u16 == smb_cmd && tx.hdr.compare(key)
                 } else if smb_ver == 2 {
                     let (_, cmd) = tx.vercmd.get_smb2_cmd();
-                    cmd == smb_cmd && tx.hdr == *key
+                    cmd == smb_cmd && tx.hdr.compare(key)
                 } else {
                     false
                 }
@@ -1054,7 +1060,7 @@ impl SMBState {
         -> Option<&mut SMBTransaction>
     {
         for tx in &mut self.transactions {
-            let hit = tx.hdr == hdr && match tx.type_data {
+            let hit = tx.hdr.compare(&hdr) && match tx.type_data {
                 Some(SMBTransactionTypeData::TREECONNECT(_)) => { true },
                 _ => { false },
             };
@@ -1090,7 +1096,7 @@ impl SMBState {
         for tx in &mut self.transactions {
             let found = match tx.type_data {
                 Some(SMBTransactionTypeData::CREATE(ref _d)) => {
-                    *hdr == tx.hdr
+                    tx.hdr.compare(&hdr)
                 },
                 _ => { false },
             };
index f06941648db3a1b767a2fe7bde82e612b359754c..43ce24270ef852cd4424de67c2721ac2cfebf56c 100644 (file)
@@ -59,14 +59,13 @@ impl SMBState {
 // IOCTL responses ASYNC don't set the tree id
 pub fn smb2_ioctl_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
 {
+    let hdr = SMBCommonHdr::from2(r, SMBHDR_TYPE_HEADER);
     match parse_smb2_request_ioctl(r.data) {
         IResult::Done(_, rd) => {
             SCLogDebug!("IOCTL request data: {:?}", rd);
             let is_dcerpc = rd.is_pipe && match state.get_service_for_guid(&rd.guid) {
                 (_, x) => x,
             };
-            let hdr = SMBCommonHdr::new(SMBHDR_TYPE_HEADER,
-                    r.session_id, 0, r.message_id);
             if is_dcerpc {
                 SCLogDebug!("IOCTL request data is_pipe. Calling smb_write_dcerpc_record");
                 let vercmd = SMBVerCmdStat::new2(SMB2_COMMAND_IOCTL);
@@ -78,8 +77,6 @@ pub fn smb2_ioctl_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
             }
         },
         _ => {
-            let hdr = SMBCommonHdr::new(SMBHDR_TYPE_HEADER,
-                    r.session_id, 0, r.message_id);
             let tx = state.new_generic_tx(2, r.command, hdr);
             tx.set_event(SMBEvent::MalformedData);
         },
@@ -89,6 +86,7 @@ pub fn smb2_ioctl_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
 // IOCTL responses ASYNC don't set the tree id
 pub fn smb2_ioctl_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
 {
+    let hdr = SMBCommonHdr::from2(r, SMBHDR_TYPE_HEADER);
     match parse_smb2_response_ioctl(r.data) {
         IResult::Done(_, rd) => {
             SCLogDebug!("IOCTL response data: {:?}", rd);
@@ -98,16 +96,12 @@ pub fn smb2_ioctl_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
             };
             if is_dcerpc {
                 SCLogDebug!("IOCTL response data is_pipe. Calling smb_read_dcerpc_record");
-                let hdr = SMBCommonHdr::new(SMBHDR_TYPE_HEADER,
-                        r.session_id, 0, r.message_id);
                 let vercmd = SMBVerCmdStat::new2_with_ntstatus(SMB2_COMMAND_IOCTL, r.nt_status);
                 SCLogDebug!("TODO passing empty GUID");
                 smb_read_dcerpc_record(state, vercmd, hdr, &[],rd.data);
             } else {
-                let tx_key = SMBCommonHdr::new(SMBHDR_TYPE_HEADER,
-                        r.session_id, 0, r.message_id);
-                SCLogDebug!("SMB2_COMMAND_IOCTL/SMB_NTSTATUS_PENDING looking for {:?}", tx_key);
-                match state.get_generic_tx(2, SMB2_COMMAND_IOCTL, &tx_key) {
+                SCLogDebug!("SMB2_COMMAND_IOCTL/SMB_NTSTATUS_PENDING looking for {:?}", hdr);
+                match state.get_generic_tx(2, SMB2_COMMAND_IOCTL, &hdr) {
                     Some(tx) => {
                         tx.set_status(r.nt_status, false);
                         if r.nt_status != SMB_NTSTATUS_PENDING {
@@ -119,10 +113,8 @@ pub fn smb2_ioctl_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
             }
         },
         _ => {
-            let tx_key = SMBCommonHdr::new(SMBHDR_TYPE_HEADER,
-                    r.session_id, 0, r.message_id);
-            SCLogDebug!("SMB2_COMMAND_IOCTL/SMB_NTSTATUS_PENDING looking for {:?}", tx_key);
-            match state.get_generic_tx(2, SMB2_COMMAND_IOCTL, &tx_key) {
+            SCLogDebug!("SMB2_COMMAND_IOCTL/SMB_NTSTATUS_PENDING looking for {:?}", hdr);
+            match state.get_generic_tx(2, SMB2_COMMAND_IOCTL, &hdr) {
                 Some(tx) => {
                     SCLogDebug!("updated status of tx {}", tx.id);
                     tx.set_status(r.nt_status, false);