From 31223caccd8ebaac84ff17227d5cca26efdb3ec8 Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Tue, 3 Apr 2018 02:41:24 -0400 Subject: [PATCH 1/3] Fix an issue with pending commands. This is a change from Stefano Rinaldi in ticket number 1862 --- scripts/policy/protocols/smb/smb2-main.bro | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/scripts/policy/protocols/smb/smb2-main.bro b/scripts/policy/protocols/smb/smb2-main.bro index 750a7ff1bc..52c8f8060c 100644 --- a/scripts/policy/protocols/smb/smb2-main.bro +++ b/scripts/policy/protocols/smb/smb2-main.bro @@ -68,6 +68,12 @@ event smb2_message(c: connection, hdr: SMB2::Header, is_orig: bool) &priority=-5 # Is this a response? if ( !is_orig ) { + # If the command that is being looked at right now was + # marked as PENDING, then we'll skip all of this and wait + # for a reply that isn't marked pending. + if ( c$smb_state$current_cmd$status == "PENDING" ) + return; + if ( SMB::write_cmd_log && c$smb_state$current_cmd$status !in SMB::ignored_command_statuses && c$smb_state$current_cmd$command !in SMB::deferred_logging_cmds ) From 9c85d3f3a90b198f1ca02ce58f188367acf923be Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Thu, 5 Apr 2018 17:12:33 -0400 Subject: [PATCH 2/3] On rare occasions the server doesn't return the tree id on read responses. This tracks the tree id given by the request This also addresses BIT-1862 with code submitted by Stefano Rinaldi and took some hints from his changes in other areas of the code. --- scripts/policy/protocols/smb/smb2-main.bro | 2 ++ src/analyzer/protocol/smb/smb-pipe.pac | 15 ++++----- src/analyzer/protocol/smb/smb2-com-read.pac | 15 ++++++--- src/analyzer/protocol/smb/smb2-protocol.pac | 34 +++++++++++++++++++-- 4 files changed, 51 insertions(+), 15 deletions(-) diff --git a/scripts/policy/protocols/smb/smb2-main.bro b/scripts/policy/protocols/smb/smb2-main.bro index 52c8f8060c..fa952eb5c3 100644 --- a/scripts/policy/protocols/smb/smb2-main.bro +++ b/scripts/policy/protocols/smb/smb2-main.bro @@ -72,7 +72,9 @@ event smb2_message(c: connection, hdr: SMB2::Header, is_orig: bool) &priority=-5 # marked as PENDING, then we'll skip all of this and wait # for a reply that isn't marked pending. if ( c$smb_state$current_cmd$status == "PENDING" ) + { return; + } if ( SMB::write_cmd_log && c$smb_state$current_cmd$status !in SMB::ignored_command_statuses && diff --git a/src/analyzer/protocol/smb/smb-pipe.pac b/src/analyzer/protocol/smb/smb-pipe.pac index 2407c63dd3..8ef07e0b0b 100644 --- a/src/analyzer/protocol/smb/smb-pipe.pac +++ b/src/analyzer/protocol/smb/smb-pipe.pac @@ -4,7 +4,7 @@ refine connection SMB_Conn += { %member{ - map tree_is_pipe_map; + map tree_is_pipe_map; map fid_to_analyzer_map; %} @@ -20,18 +20,18 @@ refine connection SMB_Conn += { } %} - function get_tree_is_pipe(tree_id: uint16): bool + function get_tree_is_pipe(tree_id: uint32): bool %{ - return ( tree_is_pipe_map.count(tree_id) > 0 ); + return ( tree_is_pipe_map.count(tree_id) > 0 && tree_is_pipe_map.at(tree_id) ); %} - function unset_tree_is_pipe(tree_id: uint16): bool + function unset_tree_is_pipe(tree_id: uint32): bool %{ tree_is_pipe_map.erase(tree_id); return true; %} - function set_tree_is_pipe(tree_id: uint16): bool + function set_tree_is_pipe(tree_id: uint32): bool %{ tree_is_pipe_map[tree_id] = true; return true; @@ -39,10 +39,11 @@ refine connection SMB_Conn += { function forward_dce_rpc(pipe_data: bytestring, fid: uint64, is_orig: bool): bool %{ - analyzer::dce_rpc::DCE_RPC_Analyzer *pipe_dcerpc; + analyzer::dce_rpc::DCE_RPC_Analyzer *pipe_dcerpc = nullptr; if ( fid_to_analyzer_map.count(fid) == 0 ) { - pipe_dcerpc = (analyzer::dce_rpc::DCE_RPC_Analyzer *)analyzer_mgr->InstantiateAnalyzer("DCE_RPC", bro_analyzer()->Conn()); + auto tmp_analyzer = analyzer_mgr->InstantiateAnalyzer("DCE_RPC", bro_analyzer()->Conn()); + pipe_dcerpc = static_cast(tmp_analyzer); if ( pipe_dcerpc ) { pipe_dcerpc->SetFileID(fid); diff --git a/src/analyzer/protocol/smb/smb2-com-read.pac b/src/analyzer/protocol/smb/smb2-com-read.pac index cf5d2ae065..849671488d 100644 --- a/src/analyzer/protocol/smb/smb2-com-read.pac +++ b/src/analyzer/protocol/smb/smb2-com-read.pac @@ -3,18 +3,19 @@ refine connection SMB_Conn += { %member{ // Track read offsets to provide correct // offsets for file manager. - std::map smb2_read_offsets; + std::map smb2_read_offsets; std::map smb2_read_fids; %} - function get_file_id(message_id: uint64): uint64 + function get_file_id(message_id: uint64, forget: bool): uint64 %{ if ( smb2_read_fids.count(message_id) == 0 ) return 0; else { uint64 fid = smb2_read_fids[message_id]; - smb2_read_fids.erase(message_id); + if ( forget ) + smb2_read_fids.erase(message_id); return fid; } %} @@ -40,7 +41,10 @@ refine connection SMB_Conn += { function proc_smb2_read_response(h: SMB2_Header, val: SMB2_read_response) : bool %{ uint64 offset = smb2_read_offsets[${h.message_id}]; - smb2_read_offsets.erase(${h.message_id}); + + // If a PENDING status was received, keep this around. + if ( ${h.status} != 0x00000103 ) + smb2_read_offsets.erase(${h.message_id}); if ( ! ${h.is_pipe} && ${val.data_len} > 0 ) { @@ -83,7 +87,8 @@ type SMB2_read_response(header: SMB2_Header) = record { pad : padding to data_offset - header.head_length; data : bytestring &length=data_len; } &let { - fid : uint64 = $context.connection.get_file_id(header.message_id); + # If a reply is has a pending status, let it remain. + fid : uint64 = $context.connection.get_file_id(header.message_id, header.status != 0x00000103); pipe_proc : bool = $context.connection.forward_dce_rpc(data, fid, false) &if(header.is_pipe); proc: bool = $context.connection.proc_smb2_read_response(header, this); diff --git a/src/analyzer/protocol/smb/smb2-protocol.pac b/src/analyzer/protocol/smb/smb2-protocol.pac index 1cad6e130e..5ca419eb2f 100644 --- a/src/analyzer/protocol/smb/smb2-protocol.pac +++ b/src/analyzer/protocol/smb/smb2-protocol.pac @@ -94,6 +94,12 @@ type SMB2_Message_Response(header: SMB2_Header) = case header.command of { refine connection SMB_Conn += { + %member{ + // Track tree_ids given in requests. Sometimes the server doesn't + // reply with the tree_id. Index is message_id, yield is tree_id + std::map smb2_request_tree_id; + %} + function BuildSMB2HeaderVal(hdr: SMB2_Header): BroVal %{ RecordVal* r = new RecordVal(BifType::Record::SMB2::Header); @@ -124,8 +130,20 @@ refine connection SMB_Conn += { function proc_smb2_message(h: SMB2_Header, is_orig: bool): bool %{ - //if ( ${h.command} == SMB2_READ ) - // printf("got a read %s command\n", is_orig ? "request" : "response"); + if ( is_orig ) + { + // Store the tree_id + smb2_request_tree_id[${h.message_id}] = ${h.tree_id}; + } + else + { + // Remove the stored tree_id unless the reply is pending. It will + // have already been used by the time this code is reached. + if ( ${h.status} != 0x00000103 ) + { + smb2_request_tree_id.erase(${h.message_id}); + } + } if ( smb2_message ) { @@ -135,6 +153,15 @@ refine connection SMB_Conn += { } return true; %} + + function get_request_tree_id(message_id: uint64): uint64 + %{ + // This is stored at the request and used at the reply. + if ( smb2_request_tree_id.count(message_id) > 0 ) + return smb2_request_tree_id[message_id]; + else + return 0; + %} }; function smb2_file_attrs_to_bro(val: SMB2_file_attributes): BroVal @@ -199,7 +226,8 @@ type SMB2_Header(is_orig: bool) = record { related = (flags >> 26) & 1; msigned = (flags >> 27) & 1; dfs = (flags) & 1; - is_pipe: bool = $context.connection.get_tree_is_pipe(tree_id); + request_tree_id = $context.connection.get_request_tree_id(message_id); + is_pipe: bool = $context.connection.get_tree_is_pipe(is_orig ? tree_id : request_tree_id); proc : bool = $context.connection.proc_smb2_message(this, is_orig); } &byteorder=littleendian; From a80131c06e22067228f87c1ce73ac462a8835ded Mon Sep 17 00:00:00 2001 From: Seth Hall Date: Thu, 5 Apr 2018 17:13:10 -0400 Subject: [PATCH 3/3] Updating the defined SMB2 dialects to match Microsofts current docs. --- scripts/base/protocols/smb/consts.bro | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/base/protocols/smb/consts.bro b/scripts/base/protocols/smb/consts.bro index 862a0ae693..f36d029be9 100644 --- a/scripts/base/protocols/smb/consts.bro +++ b/scripts/base/protocols/smb/consts.bro @@ -255,10 +255,12 @@ export { } &default=function(i: count): string { return fmt("unknown-%d", i); }; const dialects: table[count] of string = { - [0x0202] = "2.002", + [0x0202] = "2.0.2", [0x0210] = "2.1", [0x0300] = "3.0", - [0x0302] = "3.02", + [0x0302] = "3.0.2", + [0x0311] = "3.1.1", + [0x02FF] = "2.1+", } &default=function(i: count): string { return fmt("unknown-%d", i); }; const share_types: table[count] of string = {