Skip to content

Conversation

@cri-triovega
Copy link

@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
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.

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