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

Huge refactoring: poetry support, crawl the trusted domains, handle -no-pass beautifuly and so on. #177

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Huge refactoring: poetry support, crawl the trusted domains, handle -no-pass beautifuly and so on. #177

wants to merge 6 commits into from
+2,663 −1,099

Conversation

n3rada
Copy link

@n3rada n3rada commented Apr 29, 2024 •
edited
Loading

Hello @dirkjanm 👋

Overview of Changes:

  • Kerberos Authentication:

    • Refined the handling of the -no-pass parameter to ensure correct behavior when Kerberos authentication is used without a password.
    • Added functionality to automatically retrieve the username from the TGT when not provided, enhancing usability.

  • Poetry Integration:

    • Transitioned the project to use poetry for dependency management and packaging, aligning with modern Python project standards.
    • Removed the bloodhound.py file and restructured project directories as needed to fit with poetry conventions.

Details

Like this issue mentioned it already, it is currently not respecting the -no-pass parameter. So I took the time to make the necessary corrections. It will close #168 tho'.

The args.no_pass option is intended to skip password prompting (useful when using Kerberos authentication without a password), but there is indeed an issue with how it's handled in the logic for authentication setup. The problem arises in the condition checks where the authentication object (ADAuthentication) is created.

In fact, all these checks are unnecessary. We can let the ADAuthentication class handle the parameters as it already does when initialized.

    # Initialize variables for LM and NT hashes
    lm, nt = "", ""

    # Only attempt to split hashes if they are provided
    if args.hashes:
        try:
            lm, nt = args.hashes.split(":")
        except ValueError:
            logger.error(
                "Hashes provided in an incorrect format. Expected format: LM:NT"
            )
            sys.exit(1)

    auth = ADAuthentication(
        username=args.username,
        password=args.password,
        domain=args.domain,
        auth_method=args.auth_method,
        lm_hash=lm,
        nt_hash=nt,
        aeskey=args.aesKey,
    )

    ad = AD(
        auth=auth,
        domain=args.domain,
        nameserver=args.nameserver,
        dns_tcp=args.dns_tcp,
        dns_timeout=args.dns_timeout,
        use_ldaps=args.use_ldaps,
    )

I've also enabled the script to run even if the user doesn't provide a username. Because it can easily retrieve the TGT username.

Now it works well:

bloodhound-python -k -ns '172.16.190.168' -d "home.com" -c "all,LoggedOn" --zip --dns-tcp
INFO: Found AD domain: home.com
INFO: Using TGT from cache
INFO: Extracted the username from TGT: john
INFO: Connecting to LDAP server: dc.home.com
INFO: Found 1 domains
INFO: Found 1 domains in the forest
INFO: Found 2 computers
INFO: Connecting to LDAP server: dc.home.com
INFO: Found 7 users
INFO: Found 52 groups
INFO: Found 2 gpos
INFO: Found 5 ous
INFO: Found 22 containers
INFO: Found 1 trusts
INFO: Starting computer enumeration with 10 workers
INFO: Querying computer: web05
INFO: Querying computer: dc.home.com
WARNING: Could not resolve: web05: All nameservers failed to answer the query web05. IN A: Server 172.16.190.168 TCP port 53 answered SERVFAIL
INFO: User with SID S-1-5-21-1416213050-106196312-571527550-1103 is logged in on dc.home.com
INFO: Done in 00M 03S
INFO: Compressing output into 20240427231049_bloodhound.zip

I also added a better resolution for computers too.

Before it was:

INFO: Querying computer: web05
INFO: Querying computer: dc.home.com
WARNING: Could not resolve: web05: All nameservers failed to answer the query web05. IN A: Server Do53:172.16.186.168@53 answered SERVFAIL

Now it is:

INFO: Querying computer: web05
INFO: Querying computer: dc.home.com
INFO: Resolved: dc.home.com -> 172.16.186.168
INFO: Resolved using FQDN: web05.home.com -> 172.16.186.164
INFO: Resolved: web05 -> 172.16.186.164

I hope you find it all to your liking. For my part, it allows me to continue using your wonderful tool!

Btw, I also adapt the project in order to make-it compatible with poetry, which is the modern way to work inside a python3 project. I deleted the bloodhound.py file as requested by modern projects:
image

@n3rada
Copy link
Author

n3rada commented May 1, 2024

I just enhanced the logging output that allow to directly see the trusts and that built a zip file with the targeted domain name:

INFO: Found AD domain: dev.com
INFO: Attempting to get Kerberos TGT for user administrator
INFO: Successfully retrieved initial TGT for user domain: home.com.
INFO: Detected inter-realm trust scenario.
INFO: Retrieved initial referral TGS to access dev.com domain services.
INFO: Successfully obtained final TGS, enabling access to the target domain's services.
INFO: Completed TGT acquisition process.
INFO: Connecting to LDAP server: rdc02.dev.com
INFO: Found 1 domains
INFO: Found 2 domains in the forest: ops.dev.com, dev.com
INFO: Found 1 computers
INFO: Connecting to LDAP server: rdc02.dev.com
INFO: Found 5 users
INFO: Found 52 groups
INFO: Found 2 gpos
INFO: Found 4 ous
INFO: Found 19 containers
INFO: Found 2 trusts: home.com, ops.dev.com
INFO: Starting computer enumeration with 10 workers
INFO: Querying computer: rdc02.dev.com
INFO: Done in 00M 14S
INFO: Successfully created and filled dev.com_2024-05-01T14-08-55_bloodhound_data.zip

Date format is now in ISO 8601 style.

@n3rada n3rada changed the title Improve No-Password Kerberos Auth and Add Poetry Support Huge refactoring: poetry support, crawl the trusted domains, handle -no-pass beautifuly and so on. May 1, 2024
@n3rada
Copy link
Author

n3rada commented May 1, 2024 •
edited
Loading

And finally, it is now possible to add --crawl to crawl through all the trusted domains, which will close #176!

@n3rada
Copy link
Author

n3rada commented Jul 19, 2024

@dirkjanm, any news?

@dirkjanm
Copy link
Owner

I'll try to find some time to review this next week

@n3rada
Copy link
Author

n3rada commented Jul 20, 2024 •
edited
Loading

I hope you find it good enough. Don't hesitate to ask me to correct it here if necessary.

@dirkjanm
Copy link
Owner

hey, I've taken a look at the code, first of all I'd like to say that i appreciate you spending time on adding these features and improvements.

I would like to merge the features and improvements into the main branch, but there seem to be way more changes than what you indicated. 90% of the lines that were rewritten were done so with no functional changes but only stylistic preferences. Clean code is of course nice, but reviewing all those changes and ensuring the intended functionality is still the same is going to take me hours. Furthermore, all that rewriting would make it really hard to track down bugs in the future because of all the moving around and changing lines.

Would you be open to re-submit this with only the functional improvements and fixes rather than with the whole refactoring of the code?

@n3rada
Copy link
Author

n3rada commented Jul 27, 2024

I completely understand your concerns! However, I need some clarification on what you mean by "improvements and fixes." My main goal was to integrate poetry for easier installation using a utility like pipx, which necessitated some refactoring and file reorganization.

For instance, I created a cli.py to house a crucial ingest method, facilitating the chaining of relationship trusts.

Upon reviewing my changes, I noticed that many of the alterations were minor corrections, such as changing single quotes to double quotes, which do not affect the tool's functionality. I assure you that I haven’t made any major changes to your already excellent tool. 🥂

My focus was on cli.py and core.py. I would greatly appreciate it if you could review each file individually to see that most changes are stylistic, largely due to the use of utilities like black. If there are specific files you prefer I not alter, please let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NTLM needs domain\username and a password when -no-pass is set
2 participants
@n3rada @dirkjanm