Project

General

Profile

Actions

Bug #3109

closed

dcerpc engine not generating alerts

Added by Travis Green over 4 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
High
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

Rules using dce* keywords do not generate an alert despite matching packet contents. For example, given these two rules:

alert tcp any any -> $HOME_NET any (msg:"TGI LATERAL DCERPC ATSVC v1.0 Bind UUID 1ff70682-0a51-30e8-076d-740be8cee98b"; flow:established; dce_iface:1ff70682-0a51-30e8-076d-740be8cee98b,any_frag; reference:url,401trg.com/an-introduction-to-smb-for-network-security-analysts/; classtype:attempted-admin; sid:2610115; rev:1; metadata:notworking;)

alert tcp any any -> $HOME_NET any (msg:"TGI LATERAL DCERPC ATSVC v1.0 Bind UUID"; flow:established; content:"|82 06 f7 1f 51 0a e8 30 07 6d 74 0b e8 ce e9 8b|"; reference:url,401trg.com/an-introduction-to-smb-for-network-security-analysts/; classtype:attempted-admin; sid:2610113; rev:1;)

and this packet: https://imgur.com/a/necQQvy

An alert is only generated for the rule that does not use dce_iface. Pcap attached for repro.


Files

20171220_smb_at_schedule.pcap (3.53 KB) 20171220_smb_at_schedule.pcap Travis Green, 08/09/2019 11:01 PM

Related issues 2 (0 open2 closed)

Related to Suricata - Bug #4769: dcerpc dce_iface just match a packetClosedEloy PérezActions
Related to Suricata - Bug #4767: Rule error in SMB dce_iface and dce_opnum keywordsClosedEloy PérezActions
Actions #1

Updated by Travis Green over 4 years ago

Eric Leblond had this deeper analysis to offer:

regit 4:39 PM
ok so here we are:
Diff
diff --git a/rust/src/smb/detect.rs b/rust/src/smb/detect.rs
index c3b890eef..1ab8f3f4d 100644
--- a/rust/src/smb/detect.rs
+++ b/rust/src/smb/detect.rs
@@ -182,13 +182,13 @@ pub extern "C" fn rs_smb_tx_get_dce_iface(state: &mut SMBState,
                                             ver_check: u16)
                                             -> u8
 {
-    let is_dcerpc_request = match tx.type_data {
+/*    let is_dcerpc_request = match tx.type_data {
         Some(SMBTransactionTypeData::DCERPC(ref x)) => { x.req_cmd == 1 },
         _ => { false },
     };
     if !is_dcerpc_request {
         return 0;
-    }
+    } */
     let ifaces = match state.dcerpc_ifaces {
         Some(ref x) => x,
         _ => {
@@ -197,12 +197,13 @@ pub extern "C" fn rs_smb_tx_get_dce_iface(state: &mut SMBState,
     };

     let uuid = unsafe{std::slice::from_raw_parts(uuid_ptr, uuid_len as usize)};
-    SCLogDebug!("looking for UUID {:?}", uuid);
+    SCLogNotice!("looking for UUID {:?}", uuid);

     for i in ifaces {
-        SCLogDebug!("stored UUID {:?} acked {} ack_result {}", i, i.acked, i.ack_result);
+        SCLogNotice!("stored UUID {:?} acked {} ack_result {}", i, i.acked, i.ack_result);

-        if i.acked && i.ack_result == 0 && i.uuid == uuid {
+        //if i.acked && i.ack_result == 0 && i.uuid == uuid {
+        if i.uuid == uuid {
             if match_version(ver_op as u8, ver_check as u16, i.ver) {
                 return 1;
             }

First code is working only on request
second if is forcing check on fact the iface is acked and the ack_result is 0
First point: this contredict a previous commit https://github.com/OISF/suricata/commit/c4b56ca28917eb460ea9eb223b9bc98fbb9ee1d8
so we have a bug
second point: I don't know enough the protocol but for me, if we specify a match on iface and we have iface we should have a match any way
@tgreen with this patch I got 2 alerts
on signature that was failing
tgreen profile image    
regit 4:55 PM
4.1.x with rust and without is not matching too

Actions #2

Updated by Eric Leblond over 4 years ago

Side note: the pcap needs stream.midstream to true to get properly analysed but this does not alert correctly after that.

Actions #3

Updated by Andreas Herz over 4 years ago

  • Assignee set to OISF Dev
  • Target version set to TBD
Actions #4

Updated by Victor Julien over 4 years ago

Can you create (a) SV test case(s)?

Actions #5

Updated by Victor Julien over 4 years ago

Can you test against: Suricata 4.1.5 (no rust), 4.1.5 (with rust) and git master?

Actions #6

Updated by Victor Julien over 4 years ago

  • Status changed from New to Feedback
  • Assignee changed from OISF Dev to Travis Green
  • Target version changed from TBD to 70
Actions #7

Updated by Travis Green over 4 years ago

Submitted PR for suricata-verify test https://github.com/OISF/suricata-verify/pull/139

Actions #8

Updated by Travis Green over 4 years ago

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  PATCHED  -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
$ cat /dev/null > ./fast.log && sudo /opt/suricata-git.latest/src/suricata -c /etc/suricata/suricata.testsuri4.yaml -l . -S ~/rules/lateral-rules/lateral.rules -k none -r ./merged.pcap && cat ./fast.log && wc ./fast.log
[29956] 21/10/2019 -- 12:36:03 - (suricata.c:1072) <Notice> (LogVersion) -- This is Suricata version 5.0.0-dev (412ae11ba 2019-10-12) running in USER mode
<snip>
   56  1331 14696 ./fast.log

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  UNPATCHED  -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
$ cat /dev/null > ./fast.log && sudo /opt/suricata-git.debug/src/suricata -c /etc/suricata/suricata.testsuri4.yaml -l . -S ~/rules/lateral-rules/lateral.rules -k none -r ./merged.pcap && cat ./fast.log && wc ./fast.log
[30428] 21/10/2019 -- 12:41:15 - (suricata.c:1076) <Notice> (LogVersion) -- This is Suricata version 5.0.0-dev (494617bb3 2019-09-12) running in USER mode
<snip>
   42   974 10832 ./fast.log

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  UNPATCHED 4.1.5 w/rust  -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
$ cat /dev/null > ./fast.log && sudo /opt/suricata-4.1.5.rust/src/suricata -c /etc/suricata/suricata.testsuri4.yaml -l . -S ~/rules/lateral-rules/lateral.rules -k none -r ./merged.pcap && cat ./fast.log && wc ./fast.log
21/10/2019 -- 12:36:49 - <Notice> - This is Suricata version 4.1.5 RELEASE
<snip>
   42   974 10832 ./fast.log

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  UNPATCHED 4.1.5 w/o rust  -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
cat /dev/null > ./fast.log && sudo /opt/suricata-4.1.5.norust/src/suricata -c /etc/suricata/suricata.testsuri4.yaml -l . -S ~/rules/lateral-rules/lateral.rules -k none -r ./merged.pcap && cat ./fast.log && wc ./fast.log
21/10/2019 -- 12:36:54 - <Notice> - This is Suricata version 4.1.5 RELEASE
   46  1076 11936 ./fast.log

details @ https://gist.github.com/travisbgreen/e3b34e848efbe2fe0dc37183786ce9be

Actions #9

Updated by Victor Julien about 4 years ago

  • Assignee changed from Travis Green to OISF Dev
Actions #10

Updated by Victor Julien over 3 years ago

  • Target version changed from 70 to TBD

I think this works as intended: bind followed by a request. IIRC this was done for Snort compatibility.

I agree it would be nicer to be able to just match on the bind, but I don't want to break existing rules either.

Perhaps we need a new set of keywords where compatibility doesn't have to be taken into account.

Actions #11

Updated by Philippe Antoine about 3 years ago

  • Assignee changed from OISF Dev to Shivani Bhardwaj
Actions #12

Updated by Victor Julien over 2 years ago

  • Related to Bug #4769: dcerpc dce_iface just match a packet added
Actions #13

Updated by Victor Julien over 2 years ago

  • Related to Bug #4767: Rule error in SMB dce_iface and dce_opnum keywords added
Actions #14

Updated by Shivani Bhardwaj over 2 years ago

  • Status changed from Feedback to Assigned
  • Priority changed from Normal to High
Actions #15

Updated by Shivani Bhardwaj over 2 years ago

Victor Julien wrote in #note-10:

I think this works as intended: bind followed by a request. IIRC this was done for Snort compatibility.

I agree it would be nicer to be able to just match on the bind, but I don't want to break existing rules either.

Perhaps we need a new set of keywords where compatibility doesn't have to be taken into account.

I researched a bit and found that Victor's analysis is correct. We do not alert here because there is no request following the bind request. This way iface match works even in combination with opnum and stub_data (as a part of the non association calls).

So, do we need to implement any new keywords or close this issue?

Actions #16

Updated by Victor Julien over 2 years ago

If the BIND, BIND_ACK and REQUEST are all part of the same TX, could we relax this requirement? Then a dce.iface only rule could work, but a mix of dce.iface and dce.opnum as well.

Actions #17

Updated by Victor Julien almost 2 years ago

What needs to be done on this now that #4767 and #4769 are done?

Actions #18

Updated by Shivani Bhardwaj almost 2 years ago

Asked some questions on https://github.com/OISF/suricata-verify/pull/799
Seems to be working with a slight rule change now in master and master-6.0.x so there was perhaps an accidental fix.

Actions #19

Updated by Shivani Bhardwaj over 1 year ago

  • Status changed from Assigned to Closed
  • Target version changed from TBD to 7.0.0-beta1

It seems to no longer be an issue in 6.0+ as demonstrated in https://github.com/OISF/suricata-verify/pull/845

Actions

Also available in: Atom PDF