jborean93 / smbprotocol

Python SMBv2 and v3 Client
MIT License
323 stars 73 forks source link

Compounded related requests: should TreeId and SessionId be set to 0xff..ff? #260

Closed ondrahb closed 10 months ago

ondrahb commented 10 months ago

Hello, we've hit a STATUS_NETWORK_NAME_DELETED error while testing this library against a smb server implementation that didn't follow this section of the specification:

3.3.5.2.7.2 Handling Compounded Related Requests The server MUST handle each individual operation that is described in the chain in order. For the first operation, the identifiers for FileId, SessionId, and TreeId MUST be taken from the received operation. For every subsequent operation, the values used for FileId, SessionId, and TreeId MUST be the ones used in processing the previous operation or generated for the previous resulting response.

It looked for treeconnect with id 0xff..ff and didn't find one. While that is a fault of the server, it looks to me that the current behavior of the smbprotocol library might also not follow the specification:

3.2.4.1.4 Sending Compounded Requests The client MUST construct the subsequent request as it would do normally. For any subsequent requests the client MUST set SMB2_FLAGS_RELATED_OPERATIONS in the Flags field of the SMB2 header to indicate that it is using the SessionId, TreeId, and FileId supplied in the previous request (or generated by the server in processing that request). For an operation compounded with an SMB2 CREATE request, the FileId field SHOULD be set to { 0xFFFFFFFFFFFFFFFF, 0xFFFFFFFFFFFFFFFF }.

I understand "as it would do normally" as that it should also set the SessionId/TreeId fields as what the section for SMB2_CLOSE for instance prescribes:

3.2.4.5 Application Requests Closing a File or Named Pipe The SessionId field is set to Open.TreeConnect.Session.SessionId. / The TreeId field is set to Open.TreeConnect.TreeConnectId.

Is there some section of the specification from which the 0xff..ff setting that is done in smbprotocol/connection.py around 1254 follows? Should/can these two lines just be safely deleted? It seems both Windows and Linux CIFS client just keep whatever was sent in the previous requests.

jborean93 commented 10 months ago

Hmm I think you are right, I must have interpreted the FileId set to -1 as representing the session and tree connect id. I'll have to do some testing to ensure that removing those lines won't break anything.

jborean93 commented 10 months ago

Thanks for the report, the PR https://github.com/jborean93/smbprotocol/pull/266 removes the setting of these fields so it should now be part of the spec.

ondrahb commented 10 months ago

Thanks!