Fix NTLM DomainName case mismatch breaking FQDN auth (#576)#577
Open
p0dalirius wants to merge 1 commit into
Open
Fix NTLM DomainName case mismatch breaking FQDN auth (#576)#577p0dalirius wants to merge 1 commit into
p0dalirius wants to merge 1 commit into
Conversation
CreateAuthenticateMessage upper-cased the AUTHENTICATE DomainName field while NTOWFv2 folded the domain in with its original case. NTLMv2 upper-cases only the username (MS-NLMP 3.3.2), so a mixed/lower-case domain (e.g. an FQDN like host.example.local) produced a DomainName field that disagreed with the domain used to build NTProofStr, and the server rejected it with STATUS_LOGON_FAILURE. It only worked when the domain was already upper-case or empty. Carry DomainName with its original case so it matches the NTOWFv2 input. Shared by smb_v10 and smb_v20. Fixes #576
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked Issue
Closes #576Root Cause
In
CreateAuthenticateMessage(crypto/spnego/ntlm/message/authenticate/authenticate.go), the AUTHENTICATEDomainNamefield was built withstrings.ToUpper(domain), but the NTLMv2 response a few lines below computes NTOWFv2 from the original-casedomain(ntlmv2.NewNTLMv2CtxWithPassword(domain, …)). Per MS-NLMP 3.3.2, NTOWFv2 upper-cases only the username, never the domain. The server validates by recomputing NTProofStr using the domain as carried in theDomainNamefield; because that field was upper-cased while the client's NTProofStr was built from the original case, the two NTProofStr values differed and the server returnedSTATUS_LOGON_FAILURE (0xc000006d). It only worked when the supplied domain was already all upper-case or empty (whereToUpperis a no-op).Fix Description
Carry the
DomainNamefield with its original case (dropstrings.ToUpperon the domain in both the Unicode and OEM branches) so it matches the domain folded into NTOWFv2. The username field was already case-preserving, and NTOWFv2 still upper-cases the username internally, so the change is limited to the domain. The workstation field is left untouched. The fix lives in sharedcrypto/spnego, so it benefits both the SMB1 (smb_v10) and SMB2 (smb_v20) clients.How Verified
domain="TMP-W-2016.local"(lower-case FQDN) →LOGON_FAILURE;domain="TMP-W-2016.LOCAL"(upper) and""→ success. After the fix, all ofTMP-W-2016.local,tmp-w-2016.local,TMP-W-2016.LOCAL,"", and.authenticate successfully with SMB signing active.DomainNamefield now equalsutf16.EncodeUTF16LE(domain), identical to the value used to build NTOWFv2, so client and server compute the same NTProofStr.go build ./..., plusgo test ./crypto/... ./network/smb/smb_v10/client/... ./network/smb/smb_v20/client/...all pass.Test Coverage
TestDomainNamePreservesCaseincrypto/spnego/ntlm/message/authenticate/authenticate_test.go— asserts theDomainNamefield of a generated AUTHENTICATE message equals the UTF-16LE encoding of the supplied mixed-case domain (fails against the pre-fix upper-casing).Scope of Change
crypto/spnego/ntlm/message/authenticate/authenticate.go,crypto/spnego/ntlm/message/authenticate/authenticate_test.go.Risk and Rollout
Low blast radius: the change only stops upper-casing the NTLM domain. Domains that previously worked were already upper-case or empty, for which the behavior is unchanged; mixed/lower-case domains that previously failed now work. Safe to merge without staged rollout.