Merge remote-tracking branch 'origin/topic/seth/smb-pending-fix'

* origin/topic/seth/smb-pending-fix:
  Updating the defined SMB2 dialects to match Microsofts current docs.
  On rare occasions the server doesn't return the tree id on read responses.
  Fix an issue with pending commands.

BIT-1862 #merged

CHANGES

@@ -1,4 +1,18 @@
 
+2.5-513 | 2018-04-18 10:38:41 -0500
+
+  * Updating the defined SMB2 dialects to match Microsofts current docs.
+    (Corelight)
+
+  * BIT-1862: Improve handling SMB pending commands and read response tree id.
+    (Stefano Rinaldi, Corelight)
+
+    On rare occasions, SMB server doesn't return the tree id on read responses.
+
+  * Better reporter for Brostring with embedded NUL (Philippe Antoine)
+
+  * Fix config input reader on systems with gcc 4.8 (Daniel Thayer)
+
 2.5-504 | 2018-04-06 10:51:19 -0700
 
   * Trim the Travis CI build log output. (Daniel Thayer)

VERSION

@@ -1 +1 @@
-2.5-504
+2.5-513

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 = {

scripts/policy/protocols/smb/smb2-main.bro

@@ -68,6 +68,14 @@ 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 )

src/analyzer/protocol/smb/smb-pipe.pac

@@ -4,7 +4,7 @@
 
 refine connection SMB_Conn += {
 	%member{
-		map<uint16,bool> tree_is_pipe_map;
+		map<uint32,bool> tree_is_pipe_map;
 		map<uint64,analyzer::dce_rpc::DCE_RPC_Analyzer*> 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<analyzer::dce_rpc::DCE_RPC_Analyzer *>(tmp_analyzer);
 			if ( pipe_dcerpc )
 				{
 				pipe_dcerpc->SetFileID(fid);

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<uint16,uint64> smb2_read_offsets;
+		std::map<uint64,uint64> smb2_read_offsets;
 		std::map<uint64,uint64> 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);

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<uint64,uint64> 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;