Fix ldap auto authentication #18935
Merged
Fix ldap auto authentication #18935
Conversation
This file contains 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
When the auth method is set to auto and a domain is specific, use NTLM.
zeroSteiner
commented
Mar 7, 2024
Comment on lines
+37
to
+38
| if opts[:username].present? && opts[:domain].present? | ||
| connect_opts.merge!(ldap_auth_opts_ntlm(opts)) |
There was a problem hiding this comment.
This is the core of the fix right here. When the username and domain are both specified, use NTLM instead of plaintext.
The other code was moved around so the NTLM code didn't have to be duplicated. Since the NTLM code was placed into a method, and the simple code was already duplicated, it seemed like a good time to refactor everything into their own methods.
|
Changes look great, thanks for fixing this @zeroSteiner! ✅ Set the USERNAME datastore option to a UPN, see the module works ✅ Set the USERNAME datastore option to a normal username and the DOMAIN option to the domain, see the module works |
jheysel-r7
closed this pull request by merging all changes
into
rapid7:master
in
0cd2bc5
Release NotesThis PR fixes a common user mistake when authenticating with LDAP modules. Now users can specify either the USERNAME (user) and DOMAIN (domain.local) datastore options or the original format of just the USERNAME in the UPN format ([email protected]). This fix updates the LDAP library. |
18 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This fixes a common user mistake when authenticating with LDAP modules. Most modules, when set to the default authentication mechanism of
autouse NTLM. LDAP however uses the simple plaintext mechanism in this case. If the user is expecting to authenticate to a domain controller, the username will need to be in the UPN format ([email protected]), however, through muscle memory they might specific the DOMAIN datastore option. In this case, their authentication attempt will fail.This updates the LDAP library to check if a domain was specified, and if it was, it'll use NTLM as the authentication mechanism instead of LDAP. Now users can set the DOMAIN datastore option, along with the USERNAME and PASSWORD and things should just work.
Verification
msfconsoleuse auxiliary/gather/ldap_queryset LDAP::Auth auto(the default)