Skip to content

Commit

Permalink
smb: do not use tree id to match create request and response
Browse files Browse the repository at this point in the history
As an SMB2 async response does not have a tree id, even if
the request has it.

Per spec, MessageId should be enough to identifiy a message request
and response uniquely across all messages that are sent on the same
SMB2 Protocol transport connection.
So, the tree id is redundant anyways.

Ticket: OISF#5508
  • Loading branch information
catenacyber authored and benignbala committed Nov 12, 2022
1 parent 09762f1 commit ad1b7e3
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 3 deletions.
26 changes: 26 additions & 0 deletions etc/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -3191,6 +3191,19 @@
"named_pipe": {
"type": "string"
},
"rename": {
"type": "object",
"optional": true,
"properties": {
"from": {
"type": "string"
},
"to": {
"type": "string"
}
},
"additionalProperties": false
},
"request_done": {
"type": "boolean"
},
Expand All @@ -3203,6 +3216,19 @@
"session_id": {
"type": "integer"
},
"set_info": {
"type": "object",
"optional": true,
"properties": {
"class": {
"type": "string"
},
"info_level": {
"type": "string"
}
},
"additionalProperties": false
},
"share": {
"type": "string"
},
Expand Down
18 changes: 18 additions & 0 deletions rust/src/smb/smb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,24 @@ impl SMBCommonHdr {
}

}
pub fn from2_notree(r: &Smb2Record, rec_type: u32) -> SMBCommonHdr {
// async responses do not have a tree id (even if the request has it)
// making thus the match between the two impossible.
// Per spec, MessageId should be enough to identifiy a message request and response uniquely
// across all messages that are sent on the same SMB2 Protocol transport connection.
// cf https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/ea4560b7-90da-4803-82b5-344754b92a79
let msg_id = match rec_type {
SMBHDR_TYPE_TRANS_FRAG | SMBHDR_TYPE_SHARE => { 0 },
_ => { r.message_id as u64 },
};

SMBCommonHdr {
rec_type : rec_type,
ssn_id : r.session_id,
tree_id : 0,
msg_id : msg_id,
}
}
pub fn from1(r: &SmbRecord, rec_type: u32) -> SMBCommonHdr {
let tree_id = match rec_type {
SMBHDR_TYPE_TREE => { 0 },
Expand Down
6 changes: 3 additions & 3 deletions rust/src/smb/smb2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ pub fn smb2_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
Some(n) => { n.to_vec() },
None => {
// try to find latest created file in case of chained commands
let mut guid_key = SMBCommonHdr::from2(r, SMBHDR_TYPE_FILENAME);
let mut guid_key = SMBCommonHdr::from2_notree(r, SMBHDR_TYPE_FILENAME);
if guid_key.msg_id == 0 {
b"<unknown>".to_vec()
} else {
Expand Down Expand Up @@ -563,7 +563,7 @@ pub fn smb2_request_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)

SCLogDebug!("create_options {:08x}", cr.create_options);

let name_key = SMBCommonHdr::from2(r, SMBHDR_TYPE_FILENAME);
let name_key = SMBCommonHdr::from2_notree(r, SMBHDR_TYPE_FILENAME);
state.ssn2vec_map.insert(name_key, cr.data.to_vec());

let tx_hdr = SMBCommonHdr::from2(r, SMBHDR_TYPE_GENERICTX);
Expand Down Expand Up @@ -728,7 +728,7 @@ pub fn smb2_response_record<'b>(state: &mut SMBState, r: &Smb2Record<'b>)
Ok((_, cr)) => {
SCLogDebug!("SMBv2: Create response => {:?}", cr);

let guid_key = SMBCommonHdr::from2(r, SMBHDR_TYPE_FILENAME);
let guid_key = SMBCommonHdr::from2_notree(r, SMBHDR_TYPE_FILENAME);
if let Some(mut p) = state.ssn2vec_map.remove(&guid_key) {
p.retain(|&i|i != 0x00);
state.guid2name_map.insert(cr.guid.to_vec(), p);
Expand Down

0 comments on commit ad1b7e3

Please sign in to comment.