Skip to content

Fix SMB signing detection reporting "required" when only enabled#14

Open
ndog7 wants to merge 1 commit into
depthsecurity:masterfrom
ndog7:SMB-Detect-Bug-Fix
Open

Fix SMB signing detection reporting "required" when only enabled#14
ndog7 wants to merge 1 commit into
depthsecurity:masterfrom
ndog7:SMB-Detect-Bug-Fix

Conversation

@ndog7

@ndog7 ndog7 commented Jul 22, 2026

Copy link
Copy Markdown

The SMB detector read signing state from impacket's RequireSigning flag (and isSigningRequired(), which returns the same value). impacket force-sets RequireSigning=True for any SMB 3.1.1 session regardless of the server's actual security mode because 3.1.1 uses pre-auth integrity ("Always Sign"). Since Windows 10 / Server 2016+ negotiate 3.1.1 by default, hosts with signing merely enabled (a valid relay target) were misclassified as "signing required" and dropped as protected.

Fix: derive signing from the authoritative signal the SMB2_NEGOTIATE_SIGNING_REQUIRED bit in the server's negotiate-response SecurityMode via a new _get_signing_required() helper:

  • SMB1 -> isSigningRequired() (accurate on the SMB1 path)

  • SMB >= 3.0 -> test ServerSecurityMode & SMB2_NEGOTIATE_SIGNING_REQUIRED directly, bypassing impacket's 3.1.1 override

  • SMB 2.0/2.1 -> RequireSigning (accurate; override doesn't apply and ServerSecurityMode isn't populated below 3.0)

    Changes (protocols/smb_detector.py):

  • Add _get_signing_required(conn) helper encapsulating the logic above

  • Route all four signing read sites through the helper:

    • main detection path
    • Kerberos-auth-failed fallback
    • logon-failure (STATUS_LOGON_FAILURE) fallback
    • access-denied (STATUS_ACCESS_DENIED) fallback
  • Remove the now-unused per-site smbv1 = (dialect == SMB_DIALECT) locals

… signing is merely enabled (not required). Since a server that only enables signing is a valid NTLM relay target, this caused relayable hosts to be misclassified as protected.
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.

1 participant