smbj
smbj copied to clipboard
Performance improvement for unsigned packets (#817 fix)
The Session.send() will check if the server has set RequireSecuritySignature in the Negotiate Protocol Response. If server and client do not wish to sign packets, an unsigned packet will be sent.
This should address https://github.com/hierynomus/smbj/issues/817
It's always nice that the guys at MS write a spec (MS-SMB2) and then a tech-note, which to some extend contradict eachother...
From section 3.2.4.1.1 Signing the Message
The client MUST sign the message if one of the following conditions is TRUE:
- If Connection.Dialect is equal to "2.0.2" or "2.1", the message being sent contains a nonzero
value in the SessionId field and the session identified by the SessionId has
Session.SigningRequired equal to TRUE.
- If Connection.Dialect belongs to 3.x dialect family, the message being sent contains a nonzero
value in the SessionId field and one of the following conditions is TRUE:
- The session identified by SessionId has Session.EncryptData equal to FALSE.
- The tree connection identified by the TreeId field has TreeConnect.EncryptData equal to
FALSE.
If Session.SigningRequired is FALSE, the client MAY<99> sign the request.
That means that your implementation is too simple unfortunately. It doesn't take into account the SMB3 requirements which are now implicitly covered because we just "always sign if we can"...
Also it feels like "sane" behaviour to sign when we can, and make it explicit in the client to not sign. Implicitly offering up security over performance is rarely a good idea.
Thanks for the feedback. It basically means for SMB 3.x, either you encrypt or sign. I've incorporated the changes.
Is this PR not complete?
Please complete this PR ASAP as I need the exact same feature
I believe that If the server does not expect client packets to be signed, it would not validate the integrity even if we sign it. Based on this assumption, I've made these changes.
Please complete this PR ASAP as I need the exact same feature
💨
@hierynomus Can we assist somehow in getting this released?