BloodHoundAD / SharpHoundCommon

Common library used by SharpHound.
GNU General Public License v3.0
73 stars 47 forks source link

LDAP Connection Logic Rewrite #127

Closed rvazarkar closed 2 months ago

rvazarkar commented 2 months ago

Description

Completely alters the way LDAP connections are made to be more stable

Motivation and Context

LDAP connections are currently brittle and prone to breaking. Additionally, the GetDomain call is very flaky and can cause failures where it shouldn't. With this change, we use the following workflow:

/// Step 1: If domain passed in is non-null, skip this step
        // - Call GetDomain with a null domain to get the user's current domain
        // Step 2: Take domain passed in to the function or resolved from step 1
        // - Try an ldap connection on SSL
        // - If ServerUnavailable - Try an ldap connection on non-SSL
        //     Step 3: Pass the domain to GetDomain to resolve to a better name (potentially)
        //     - If we get a better name, repeat step 2 with the new name
        //     Step 4:
        // - Use GetDomain to get a domain object along with a list of domain controllers
        // - Try the primary domain controller on both ssl/non-ssl
        // - Loop over domain controllers and try each on ssl/non-ssl

This allows several fallback steps when negotiation ldap connections, which should significantly improve the ability of SharpHound to successfully connect

How Has This Been Tested?

Local tests pass + testing in local environment. Additional tests incoming

Screenshots (if appropriate):

Types of changes

Checklist: