Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use ldap_connect in place of ldap_new to trigger the bind early #18145

Closed
wants to merge 1 commit into from
Closed

Use ldap_connect in place of ldap_new to trigger the bind early #18145

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/auxiliary/gather/ldap_hashdump.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def run_host(ip)
entries_returned = 0

print_status("#{peer} Connecting...")
ldap_new do |ldap|
Copy link
Contributor

@adfoster-r7 adfoster-r7 Jun 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this now skips the monkey patching logic that was explicitly put in to catch some bind errors:

metasploit-framework/lib/msf/core/exploit/remote/ldap.rb

Lines 223 to 255 in dfbd14e

def ldap_new(opts = {})
ldap = Net::LDAP.new(get_connect_opts.merge(opts))
# NASTY, but required
# monkey patch ldap object in order to ignore bind errors
# Some servers (e.g. OpenLDAP) return result even after a bind
# has failed, e.g. with LDAP_INAPPROPRIATE_AUTH - anonymous bind disallowed.
# See: https://www.openldap.org/doc/admin23/security.html#Authentication%20Methods
# "Note that disabling the anonymous bind mechanism does not prevent anonymous
# access to the directory."
# Bug created for Net:LDAP at https://github.com/ruby-ldap/ruby-net-ldap/issues/375
#
# @yieldparam conn [Net::LDAP] The LDAP connection handle to use for connecting to
# the target LDAP server.
# @param args [Hash] A hash containing options for the ldap connection
def ldap.use_connection(args)
if @open_connection
yield @open_connection
else
begin
conn = new_connection
conn.bind(args[:auth] || @auth)
# Commented out vs. original
# result = conn.bind(args[:auth] || @auth)
# return result unless result.result_code == Net::LDAP::ResultCodeSuccess
yield conn
ensure
conn.close if conn
end
end
end
yield ldap
end

Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I didn't spot that, not intentional no, I'll take another pass at this, would even just reverting this change and removing the if ldap.get_operation_result.code == 0 section be viable?

ldap_connect do |ldap|
if ldap.get_operation_result.code == 0
vprint_status("#{peer} LDAP connection established")
else
Expand Down