[RFC] Feature/smb/v18.0

[victorjulien]

Changes since #3260:

• rebase to master
• small update for der-parser 0.5.1

PRScript output (if applicable):

• PR victorjulien-pcap: https://buildbot.openinfosecfoundation.org/builders/victorjulien-pcap/builds/99
• PR victorjulien: https://buildbot.openinfosecfoundation.org/builders/victorjulien/builds/101

[norg]

docs look good

[jasonish]

Rather minor, but I just noticed that this, and nfs use "_" in filenames. Whereas the C code uses "-". Is there a specific reason for this?

[jasonish]

My first look at this was as a user and here are some things I noticed:

• My Linux to Linux CIFS shares use very long running sessions. So I had to remount those to get Suricata to pick anything up. This would be a good use case for per-protocol midstream pickup?

• I think we need a way, right from the start to filter out certain types of SMB logging. One example is to play a movie from an SMB mount and watch the flood of SMB2_COMMAND_READ messages go by. Is there a default set that would give us the following information: who mounted it, files created/deleted/read - without logging ever read block?

• The statux field looks like its the status-code. Can we call it "status_code" even though its logged as a hex string. The other option is to log it as an integer, but we do have some other cases where flags are logged as a hex string as well.

• Can the disposition field use a value in ALL_CAPS? Looks like the MSDN docs use FILE_OPEN, FILE_CREATE, etc.

• Can the access field also use ALL_CAPS values? Again the MSDN docs use strings like DELETE_ON_CLOSE.

• Rename "file" to "filename". It looks like NFS uses "filename", as well as fileinfo.

But overall this looks great! I think there's just too much information for the basic use case.

[jasonish]

We're missing an "smb" stub in the eve-log types.

[pevma]

I see a lot of these in verbose mode (though I have no debugging enabled but running with -vvv on the command line):

...
smb.rs:974) <Notice> (<rust>) -- check_gap_resync2: TX 2 is done post-GAP, mark all older ones complete
[32316] 8/3/2018 -- 00:56:06 - (debug.rs:27) <Notice> (<rust>) -- TXs 3 MIN 1 MAX 3
[32316] 8/3/2018 -- 00:56:06 - (debug.rs:28) <Notice> (<rust>) -- - OLD tx.id 1: SMBTransaction { id: 1, vercmd: SMBVerCmdStat { smb_ver: 1, smb1_cmd: 114, smb2_cmd: 0, status_set: false, status_is_dos_error: false, status: 0 }, hdr: SMBCommonHdr { ssn_id: 0, tree_id: 0, rec_type: 0, msg_id: 0 }, request_done: true, response_done: true, type_data: Some(NEGOTIATE(SMBTransactionNegotiate { smb_ver: 1, dialects: [[76, 65, 78, 77, 65, 78, 49, 46, 48], [76, 77, 49, 46, 50, 88, 48, 48, 50], [78, 84, 32, 76, 65, 78, 77, 65, 78, 32, 49, 46, 48], [78, 84, 32, 76, 77, 32, 48, 46, 49, 50]], dialects2: [] })), detect_flags_ts: 9223372036854775808, detect_flags_tc: 0, logged: LoggerFlags { flags: 0 }, de_state: Some(0x6040030add90), events: 0x0 }
[32316] 8/3/2018 -- 00:56:06 - (debug.rs:29) <Notice> (<rust>) -- - NEW tx.id 3: SMBTransaction { id: 3, vercmd: SMBVerCmdStat { smb_ver: 1, smb1_cmd: 117, smb2_cmd: 0, status_set: false, status_is_dos_error: false, status: 0 }, hdr: SMBCommonHdr { ssn_id: 30721, tree_id: 0, rec_type: 10, msg_id: 24261 }, request_done: true, response_done: false, type_data: Some(TREECONNECT(SMBTransactionTreeConnect { is_pipe: false, tree_id: 0, share_name: [92, 49, 48, 46, 51, 50, 46, 51, 52, 46, 56, 92, 73, 80, 67, 36, 63, 63, 63] })), detect_flags_ts: 0, detect_flags_tc: 0, logged: LoggerFlags { flags: 0 }, de_state: None, events: 0x0 }
[32316] 8/3/2018 -- 00:56:06 - (debug.rs:55) <Notice> (<rust>) -- idx 0 tx id 1 ver:1 cmd:114 progress true/true type_data Some(NEGOTIATE(SMBTransactionNegotiate { smb_ver: 1, dialects: [[76, 65, 78, 77, 65, 78, 49, 46, 48], [76, 77, 49, 46, 50, 88, 48, 48, 50], [78, 84, 32, 76, 65, 78, 77, 65, 78, 32, 49, 46, 48], [78, 84, 32, 76, 77, 32, 48, 46, 49, 50]], dialects2: [] })) tx SMBTransaction { id: 1, vercmd: SMBVerCmdStat { smb_ver: 1, smb1_cmd: 114, smb2_cmd: 0, status_set: false, status_is_dos_error: false, status: 0 }, hdr: SMBCommonHdr { ssn_id: 0, tree_id: 0, rec_type: 0, msg_id: 0 }, request_done: true, response_done: true, type_data: Some(NEGOTIATE(SMBTransactionNegotiate { smb_ver: 1, dialects: [[76, 65, 78, 77, 65, 78, 49, 46, 48], [76, 77, 49, 46, 50, 88, 48, 48, 50], [78, 84, 32, 76, 65, 78, 77, 65, 78, 32, 49, 46, 48], [78, 84, 32, 76, 77, 32, 48, 46, 49, 50]], dialects2: [] })), detect_flags_ts: 9223372036854775808, detect_flags_tc: 0, logged: LoggerFlags { flags: 0 }, de_state: Some(0x6040030add90), events: 0x0 }
....

was wondering if that is expected?

[inliniac]

On 07-03-18 18:41, Jason Ish wrote: # Can the disposition field use a value in ALL_CAPS? Looks like the MSDN docs use FILE_OPEN, FILE_CREATE, etc. # Can the access field also use ALL_CAPS values? Again the MSDN docs use strings like DELETE_ON_CLOSE.

Can you add a link?

[inliniac]

On 07-03-18 18:41, Jason Ish wrote: # Rename "file" to "filename". It looks like NFS uses "filename", as well as fileinfo.

I think it would be good to maybe have a common object in the root for this? For example I now have a few more fields: <pre> "smb": { "id": 143, "dialect": "2.10", "command": "SMB2_COMMAND_CREATE", "status": "STATUS_SUCCESS", "statux": "0x0", "session_id": 2332899971, "tree_id": 2531469078, "request_done": true, "response_done": true, "file": "sandnet.pcap", "disposition": "open", "access": "normal", "created": 1300461378, "accessed": 1505857809, "modified": 1300461378, "changed": 1300461378, "size": 435432725 } </pre> I guess it would make sense to have something like: <pre> "file": { "name": "sandnet.pcap", "disposition": "open", "access": "normal", "created": 1300461378, "accessed": 1505857809, "modified": 1300461378, "changed": 1300461378, "size": 435432725 } </pre> Although 'file' might be a misnomer as it can also be a dir or a pipe or a few other things.

[victorjulien]

On 06-03-18 20:05, Jason Ish wrote: Rather minor, but I just noticed that this, and nfs use "_" in filenames. Whereas the C code uses "-". Is there a specific reason for this?

No idea. We can change it.

[inliniac]

On 07-03-18 18:41, Jason Ish wrote: # My Linux to Linux CIFS shares use very long running sessions. So I had to remount those to get Suricata to pick anything up. This would be a good use case for per-protocol midstream pickup?

Yeah. The code is already quite good at post-GAP sync up. I was thinking that on midstream session pickup we can simply set the GAP flag so that the parser knows the start of the record might not align with the start of the data.

[inliniac]

On 07-03-18 18:41, Jason Ish wrote: # I think we need a way, right from the start to filter out certain types of SMB logging. One example is to play a movie from an SMB mount and watch the flood of SMB2_COMMAND_READ messages go by. Is there a default set that would give us the following information: who mounted it, files created/deleted/read - without logging ever read block?

Agreed. We have the same issue with NFS. There would be 2 rough approaches: 1. log filtering: only log for certain txs 2. tx filtering: only create tx for certain commands There is some basic code to do (2), but it's not configurable yet. I'm playing with logic like this: 1. for READ/WRITE/TRANS/TRANS2 don't create tx 2. *unless* there was an parser event Probably should extend it to a few more commands. However, I will be introducing some keywords to match on commands, status and some other things as well. So if I'd have a 'smb_command; content:"READ";' keyword, I do need a tx per command/status pair. So I'm also thinking about making it a bit more dynamic: 1. if smb logger wants to output each command: tx for each command 2. if rules match on command/status: tx for each command Then if logging is limited to 'important events' and detection is on, we'd need to do output filtering.

[inliniac]

On 07-03-18 18:41, Jason Ish wrote: # The statux field looks like its the status-code. Can we call it "status_code" even though its logged as a hex string. The other option is to log it as an integer, but we do have some other cases where flags are logged as a hex string as well.

I'm fine with renaming it. Field will have to be hex though, as that is how the codes are documented in the spec, and displayed by tools like wireshark.

[victorjulien]

In general I think I'd like to break out some of the sub-objects I created:

smb.dcerpc: dcerpc can be on it's own as well on the wire
smb.kerberos: same, plus it's used by other protocols too
smb.ntlmssp: same as kerberos
smb.filestuff: as discussed above

[victorjulien]

On 08-03-18 12:41, Peter Manev wrote: I see a lot of these in verbose mode (though I have no debugging enabled but running with |-vvv| on the command line):

...

was wondering if that is expected?

Right now, yes. I will refine it a bit but the goal right now is to find conditions in which the parser isn't behaving as it should. But for this PR you can ignore it.

[jasonish]

On 07-03-18 18:41, Jason Ish wrote:

Can the disposition field use a value in ALL_CAPS? Looks like the MSDN
docs use FILE_OPEN, FILE_CREATE, etc.

Can the access field also use ALL_CAPS values? Again the MSDN docs use
strings like DELETE_ON_CLOSE.
Can you add a link?

https://msdn.microsoft.com/en-us/library/windows/desktop/aa363858(v=vs.85).aspx - See FILE_FLAG_DELETE_ON_CLOSE. I think this is more of Windows programming guide, but its pretty closely tied to SMB.

[jasonish]

On 2018-03-09 03:15 AM, Victor Julien wrote: On 07-03-18 18:41, Jason Ish wrote: > # > > Rename "file" to "filename". It looks like NFS uses "filename", as well > as fileinfo. > I think it would be good to maybe have a common object in the root for this? For example I now have a few more fields: <pre> "smb": { "id": 143, "dialect": "2.10", "command": "SMB2_COMMAND_CREATE", "status": "STATUS_SUCCESS", "statux": "0x0", "session_id": 2332899971, "tree_id": 2531469078, "request_done": true, "response_done": true, "file": "sandnet.pcap", "disposition": "open", "access": "normal", "created": 1300461378, "accessed": 1505857809, "modified": 1300461378, "changed": 1300461378, "size": 435432725 } </pre> I guess it would make sense to have something like: <pre> "file": { "name": "sandnet.pcap", "disposition": "open", "access": "normal", "created": 1300461378, "accessed": 1505857809, "modified": 1300461378, "changed": 1300461378, "size": 435432725 } </pre> Although 'file' might be a misnomer as it can also be a dir or a pipe or a few other things.

I think we're all pretty used to the file as thing, not necessarily a plain file these days. But still I think the smb.file should be smb.filename, just for consistency.

[jasonish]

On 2018-03-09 03:18 AM, Victor Julien wrote: On 07-03-18 18:41, Jason Ish wrote: > # > > The statux field looks like its the status-code. Can we call it > "status_code" even though its logged as a hex string. The other option > is to log it as an integer, but we do have some other cases where flags > are logged as a hex string as well. > I'm fine with renaming it. Field will have to be hex though, as that is how the codes are documented in the spec, and displayed by tools like wireshark.

Yeah, for display that is fine. But if it all packs into an integer, thats should be the serialized format, and converted on display :) But like a said, we don't follow that strictly, so thats OK. I just don't think the fact that its hex has to be encoded into the name. So simplye "status-code", and hex as a string, starting with 0x. I think our other usage might not have the 0x. Jason

[victorjulien]

Replaced by #3281