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

Fix TARGET_DN object query #500

Merged
merged 1 commit into from
Dec 11, 2024
Merged

Fix TARGET_DN object query #500

merged 1 commit into from
Dec 11, 2024

Conversation

MaxToffy
Copy link
Contributor

@MaxToffy MaxToffy commented Dec 4, 2024

Summary

This PR fix the same problem as in dacledit.py : fortra/impacket#1855

@NeffIsBack
Copy link
Contributor

Thanks for the bug fix!
If i understood the example on impacket correct, searching for RootDNSServers would result in not being found? Or what query exactly would break before?

@NeffIsBack NeffIsBack added the bug-fix This Pull Request fixes a bug label Dec 4, 2024
@MaxToffy
Copy link
Contributor Author

MaxToffy commented Dec 4, 2024 •
edited
Loading

Yes, searching for the DN DC=RootDNSServers,CN=MicrosoftDNS,DC=DomainDnsZones,DC=DOMAIN,DC=TLD would fail finding the object because the search base is by default on DC=DOMAIN,DC=TLD.

With this fix, instead of searching with the base DC=DOMAIN,DC=TLD, we directly search from the target DN (DC=RootDNSServers,CN=MicrosoftDNS,DC=DomainDnsZones,DC=DOMAIN,DC=TLD) provided by the users witch fix the issue on finding the object.

NeffIsBack
NeffIsBack approved these changes Dec 10, 2024
View reviewed changes
Copy link
Contributor

@NeffIsBack NeffIsBack left a comment

Choose a reason for hiding this comment

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

LGTM:
image

NeffIsBack added a commit that referenced this pull request Dec 10, 2024
@NeffIsBack
Add base-dn options for ldap to fix stuff like #500
c4671a2
@NeffIsBack NeffIsBack mentioned this pull request Dec 10, 2024
Merged
2 tasks
@NeffIsBack NeffIsBack merged commit dcb3bbd into Pennyw0rth:main Dec 11, 2024
3 checks passed
@MaxToffy MaxToffy deleted the fix-daclread branch December 11, 2024 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NeffIsBack NeffIsBack NeffIsBack approved these changes

@zblurx zblurx Awaiting requested review from zblurx zblurx is a code owner

@Marshall-Hallenbeck Marshall-Hallenbeck Awaiting requested review from Marshall-Hallenbeck Marshall-Hallenbeck is a code owner

@mpgn mpgn Awaiting requested review from mpgn mpgn is a code owner

Assignees
No one assigned
Labels
bug-fix This Pull Request fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MaxToffy @NeffIsBack