Skip to content

Allow toggling the SACL in LDAP queries #20730

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+14 −3
Open

Allow toggling the SACL in LDAP queries #20730

Show file tree
Hide file tree
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
4 changes: 2 additions & 2 deletions lib/msf/core/exploit/remote/ldap/queries.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ def perform_ldap_query(ldap, filter, attributes, base, schema_dn, scope: nil)
results
end

def perform_ldap_query_streaming(ldap, filter, attributes, base, schema_dn, scope: nil)
def perform_ldap_query_streaming(ldap, filter, attributes, base, schema_dn, scope: nil, controls: [])
if attributes.nil? || schema_dn.nil?
attribute_properties = {}
else
Expand All @@ -96,7 +96,7 @@ def perform_ldap_query_streaming(ldap, filter, attributes, base, schema_dn, scop

scope ||= Net::LDAP::SearchScope_WholeSubtree
result_count = 0
ldap.search(base: base, filter: filter, attributes: attributes, scope: scope, return_result: false) do |result|
ldap.search(base: base, filter: filter, attributes: attributes, scope: scope, controls: controls, return_result: false) do |result|
result_count += 1
yield result, attribute_properties if block_given?
end
Expand Down
13 changes: 12 additions & 1 deletion modules/auxiliary/gather/ldap_query.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
class MetasploitModule < Msf::Auxiliary

include Msf::Exploit::Remote::LDAP
include Msf::Exploit::Remote::LDAP::ActiveDirectory
include Msf::Exploit::Remote::LDAP::Queries
include Msf::OptionalSession::LDAP
require 'json'
Expand Down Expand Up @@ -66,6 +67,10 @@ def initialize(info = {})
OptString.new('QUERY_FILTER', [false, 'Filter to send to the target LDAP server to perform the query'], conditions: %w[ACTION == RUN_SINGLE_QUERY]),
OptString.new('QUERY_ATTRIBUTES', [false, 'Comma separated list of attributes to retrieve from the server'], conditions: %w[ACTION == RUN_SINGLE_QUERY])
])

register_advanced_options([
OptBool.new('LDAP::QuerySacl', [true, 'Query the SACL field from security descriptors (requires privileges)', true])
Copy link
Contributor Author

@zeroSteiner zeroSteiner Nov 26, 2025

Choose a reason for hiding this comment

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

I would be in favor of turning this off by default but:

  1. I'd be concerned that non-Active Directory LDAP implementations might start failing queries if they don't support the control
  2. It'd technically change the old behavior

It could in thoery be made automatic where we'd fingerprint if the target LDAP server is AD and toggle it to false if so, or toggle it to false and re-issue a query if we noticed that we both requested a security descriptor and it was omitted in the results.

Copy link
Contributor

@skylerknecht skylerknecht Nov 29, 2025

Choose a reason for hiding this comment

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

While other offensive security tooling have their default set to not query for the SACL, I'd leave this one to false. Mostly because it is a general purpose LDAP query module. Leave the finger printing and/or re-issuing queries to operators so that we can avoid any unnecessary traffic from the module that might trip identity products. Some valid options I see is;

1.) Create a wrapper module for "Microsoft LDAP" where you have an interactive prompt to make LDAP queries with a single connection.
2.) If the ntSecurityDescriptor attribute was explicitly provided and no value was returned when there are results, throw a debug or information message stating "Failed to query ntSecurityDescriptor, for non-administrative users please set LDAP::QuerySACL to false".

])
end

def initialize_actions
Expand Down Expand Up @@ -185,7 +190,13 @@ def run
fail_with(Failure::BadConfig, "Could not compile the filter #{filter_string}. Error was #{e}")
end

result_count = perform_ldap_query_streaming(ldap, filter, attributes, query_base, schema_dn) do |result, attribute_properties|
controls = []
unless datastore['LDAP::QuerySacl']
# omit the control entirely if querying the SACL because that's the default behavior
controls = [adds_build_ldap_sd_control(sacl: false)]
end

result_count = perform_ldap_query_streaming(ldap, filter, attributes, query_base, schema_dn, controls: controls) do |result, attribute_properties|
show_output(normalize_entry(result, attribute_properties), datastore['OUTPUT_FORMAT'])
end

Expand Down
Loading