fix(ldap): resolve group members via 'member' fallback for missing 'm… - #1362
fix(ldap): resolve group members via 'member' fallback for missing 'm…#1362amikuss wants to merge 1 commit into
Conversation
…emberOf' backlinks
NeffIsBack
left a comment
There was a problem hiding this comment.
Thanks for the bug fix PR!
So far, the search preparation and display logic was separated because there were only small differences in both code branches. Since this is much more logic now, please just make a big if/else to separate between searching all groups and looking at a specific group.
| original_scope = self.scope | ||
| try: | ||
| self.scope = ldapasn1_impacket.Scope("baseObject") |
There was a problem hiding this comment.
What's the reason for the change in scope?
There was a problem hiding this comment.
It shouldn't be strictly necessary given that it is querying users/groups which do not have children, but I added it as a defensive precaution since I am assuming it will always only return the 1 desired DN on line 796.
| finally: | ||
| self.scope = original_scope |
There was a problem hiding this comment.
I don't think we need yet another try/except/finally block here if there isn't a specific reason (error) that should be caught.
| try: | ||
| member_resp = self.search("(objectClass=*)", member_attributes, baseDN=member_dn) | ||
| except Exception as e: | ||
| self.logger.debug(f"Failed to resolve member DN '{member_dn}': {e}") | ||
| member_resp = [] |
There was a problem hiding this comment.
Is there a reason for the try&except? LDAPSearchError are usually caught in the search itself. Did you notice any other?
There was a problem hiding this comment.
I did not realize they got caught in the search, try/except will be removed.
…emberOf' backlinks
Description
Added a fallback checking "members" attribute for the --groups "GROUP" argument.
If a user's "memberOf" backlink has not been populated, the current memberOf/primaryGroupID check does not return the user when running --groups "GROUP", even though --groups returns the correct number of group members.
I ran into this issue in a HTB CAPE environment, where a Foreign Security Principal's memberOf was not set, even though it was a member of a group. See screenshots below.
AI disclosure: Claude Sonnet 5 was used for some coding help.
Type of change
Insert an "x" inside the brackets for relevant items (do not delete options)
Setup guide for the review
I have been unable to reproduce this issue as the memberOf attribute is dynamically maintained, including FSPs.
I was unable to figure out why this was not the case in the CAPE lab environment and honestly unsure if this PR is necessary given that this seems like a very environment-specific quirk.
Screenshots (if appropriate):
Before:

After:

Checklist:
Insert an "x" inside the brackets for completed and relevant items (do not delete options)
poetry run ruff check ., use--fixto automatically fix what it can)tests/e2e_commands.txtfile if necessary (new modules or features are required to be added to the e2e tests)