Skip to content

Added signature verification support for SMB2 client#300

Open
cri-triovega wants to merge 1 commit intoTalAloni:masterfrom
TRIOVEGA:smb2-client-signature-verification
Open

Added signature verification support for SMB2 client#300
cri-triovega wants to merge 1 commit intoTalAloni:masterfrom
TRIOVEGA:smb2-client-signature-verification

Conversation

@cri-triovega
Copy link
Copy Markdown

@cri-triovega cri-triovega commented May 27, 2025

I added the signature verification for SMB2 messages on client side.

Adding the attribute m_sessionSetupResponseMessage to SMB2Client was a bit lazy, but the easiest method to implement the check without changing to much of the library code. Maybe you have a better approach in mind @TalAloni ?

Tests prove that the manipulation of the signature result in the desired behavior (either discarding the invalid message or disconnect from the server).

@cri-triovega cri-triovega marked this pull request as draft May 27, 2025 12:01
@cri-triovega cri-triovega reopened this May 28, 2025
@cri-triovega cri-triovega marked this pull request as ready for review May 28, 2025 14:10
@TalAloni
Copy link
Copy Markdown
Owner

First of all, thanks for the contribution!
My approach in software is "make it work" and I admit that the SMB2 client is very lax in terms of security.
and I admit that having the option to enable signature checks is in order.

With that said, I need to carefully check the implementation to make sure it does not introduce any regression,
and the selection of integration tests instead of unit tests is also questionable,
and I would probably move some of the signature verification code to a new method in SMB2Cryptography.

I will try to find time for a deeper review in the coming weeks. Thanks again.

@TalAloni TalAloni force-pushed the master branch 3 times, most recently from 093b856 to 5c5c764 Compare November 29, 2025 19:42
@TalAloni TalAloni force-pushed the master branch 2 times, most recently from d8327d6 to 4910252 Compare February 26, 2026 19:32
@TalAloni
Copy link
Copy Markdown
Owner

Apologies for taking such a long time to get to this. I started to look into it today and a deep dive into the specifications is in order to ensure that the code behaves correctly in all cases so I haven't been able to cover everything yet.

  1. I would probably prefer an SMB2Client opt-in constructor argument to activate signature requirement and verification, something like 'requestSigning'.
  2. I added SMB2Cryptography.VerifySignature method that should help with the verification.
  3. IIUC sending SecurityMode.SigningRequired | SecurityMode.SigningEnabled is incorrect, SigningEnabled is implied when SigningRequired is set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants