Skip to content

NTDS.dit Plugin - #1347

Merged
Schamper merged 26 commits into
fox-it:mainfrom
B0TAxy:feautre/ntds_dit_parsing
Feb 11, 2026
Merged

Schamper merged 26 commits into
fox-it:mainfrom
B0TAxy:feautre/ntds_dit_parsing

Conversation

@B0TAxy

@B0TAxy B0TAxy commented Sep 30, 2025 •

Copy link
Copy Markdown
Contributor

Description

This merge request introduces an initial implementation of an NTDS.dit Plugin.
The parser can parse and decrypt secret records (e.g., user hashes, supplemental credentials) from an Active Directory NTDS database.

At this stage, the parser is in a late development preview:

Core functionality for parsing and decrypting secret records is available.

Support for additional record types (e.g., groups, ACLs, domain data) is not yet implemented.

Output support for BloodHound-compatible JSON is planned.

Automated tests have not been added yet — these will be included in a follow-up update.

The goal of creating this MR now is to gather early feedback and design review before finalizing the remaining features.

Checklist

  • Implementation (core)
  • Tests
  • BloodHound export
  • Full record support

@B0TAxy B0TAxy changed the title NTDS.dit Pkugin NTDS.dit Plugin Sep 30, 2025
@B0TAxy
B0TAxy force-pushed the feautre/ntds_dit_parsing branch from 5f6c452 to 0be940c Compare October 3, 2025 13:54
@joost-j joost-j mentioned this pull request Oct 31, 2025
3 of 5 tasks
@joost-j

joost-j commented Nov 6, 2025 •

Copy link
Copy Markdown
Contributor

Regarding this PR, I've managed to get it to work with my own PR (fox-it/dissect.database#8) by changing a few things:

In the __init__():

ntds_path_key = target.registry.value(
    key="HKLM\\SYSTEM\\CurrentControlSet\\Services\\NTDS\\Parameters", 
    value="DSA Database file"
)
self.ntds = NTDS(target.fs.open(ntds_path_key.value))

Changing all the record[NAME_TO_INTERNAL["XXX"]] references to record.XXX where XXX refers to the official ldapDisplayName, since these names are already automatically converted. Lastly, by changing the database data collection part:

def _collect_pek_and_user_records(self) -> tuple[bytes, Generator[User]]:
    return next(self.ntds.lookup(objectCategory="domainDNS")).pekList, self.ntds.users()

and some other minor adjustments. Even on a small dataset that already seemed to result in a performance boost, since the NTDS class uses indexes to look for the pekList and User records.

Note: not all accounts seem to have entries for dBCSPwd, ntPwdHistory, lmPwdHistory and even unicodePwd, so that probably needs some try/exception wrapping.

Please let me know what you think of this and if you need any help with refactoring & editing the code. You can also DM me on the Dissect discord

@B0TAxy

B0TAxy commented Nov 13, 2025

Copy link
Copy Markdown
Contributor Author

Hi @joost-j ,
Thanks for the suggestion! I’ve converted the PR to use your approach, and it’s working well with the changes you mentioned. I’ve also wrapped the optional attributes (dBCSPwd, ntPwdHistory, lmPwdHistory, unicodePwd etc) as you noted. Really appreciate the guidance — the adjustments to record.XXX and the NTDS data collection logic make the code cleaner and improve performance.

@joost-j

joost-j commented Nov 27, 2025 •

Copy link
Copy Markdown
Contributor

Cool! Good to hear - a quick check from my side also confirms that it works well. When running your ntds.user_accounts plugin I can see the NTDS User class attributes, as well as the hashes, really nice! Hoping to wrap up the NTDS parser this year, so that this PR can be finished as well. Just a small remark; I did need to change the from Cryptodome.Cipher on line 9 to from Crypto.Cipher.

@B0TAxy

B0TAxy commented Nov 28, 2025

Copy link
Copy Markdown
Contributor Author

Cool! Good to hear - a quick check from my side also confirms that it works well. When running your ntds.user_accounts plugin I can see the NTDS User class attributes, as well as the hashes, really nice! Hoping to wrap up the NTDS parser this year, so that this PR can be finished as well. Just a small remark; I did need to change the from Cryptodome.Cipher on line 9 to from Crypto.Cipher.

Hi @joost-j , Thanks for the insight! I’ve fixed that import as well.

By the way, do you plan to test your database parser against any older domain controllers? The extraction seems to be a bit different — especially the hash extraction — and I currently don’t have good test data from older DC versions to compare with.

@Schamper

Copy link
Copy Markdown
Contributor

By the way, do you plan to test your database parser against any older domain controllers? The extraction seems to be a bit different — especially the hash extraction — and I currently don’t have good test data from older DC versions to compare with.

That is the goal, but we will incrementally improve the parser.

@codspeed

codspeed Bot commented Dec 18, 2025 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 11 untouched benchmarks


Comparing B0TAxy:feautre/ntds_dit_parsing (0c80899) with main (d9a1481)

Open in CodSpeed

@B0TAxy

B0TAxy commented Dec 24, 2025

Copy link
Copy Markdown
Contributor Author

By the way, do you plan to test your database parser against any older domain controllers? The extraction seems to be a bit different — especially the hash extraction — and I currently don’t have good test data from older DC versions to compare with.

That is the goal, but we will incrementally improve the parser.

Makes sense. Once you feel the parser is ready for those older versions, could you let me know? I’d be happy to run a few tests on my end and give you some feedback on how it handles the extraction.

@joost-j

joost-j commented Jan 7, 2026

Copy link
Copy Markdown
Contributor

PR fox-it/dissect.database#8 is now ready to be reviewed, and probably will be reviewed somewhere in the upcoming weeks. The PR now also includes most of the decryption routines for PEK lists etc. Once that PR is merged, you can edit your plugin once more to focus mainly on the plugin functionality and record publishing parts, leaving out most of the internals because you can now mostly wrap around those functions from dissect.database. I'll ping you once the PR has been merged!

@Schamper

Schamper commented Jan 9, 2026

Copy link
Copy Markdown
Contributor

I’d be happy to run a few tests on my end and give you some feedback on how it handles the extraction.

If you have any test sets of old domains that are suitable for open-source sharing, that would be greatly appreciated!

@B0TAxy

B0TAxy commented Jan 9, 2026

Copy link
Copy Markdown
Contributor Author

PR fox-it/dissect.database#8 is now ready to be reviewed, and probably will be reviewed somewhere in the upcoming weeks. The PR now also includes most of the decryption routines for PEK lists etc. Once that PR is merged, you can edit your plugin once more to focus mainly on the plugin functionality and record publishing parts, leaving out most of the internals because you can now mostly wrap around those functions from dissect.database. I'll ping you once the PR has been merged!

Thanks for the heads-up! I’ve already converted the plugin to use the new functions from your PR, and the code is definitely much cleaner now.

However, I’ve run into a couple of issues with the new routines. First, the hash decryption doesn't seem to be working—it’s returning the same buffer instead of the decrypted result. Second, the tool is now crashing when I try to decrypt the hash for the krbtgt account. Upon early investigation, the buffer I'm getting from unicodePwd for that account is significantly smaller than it used to be.

I'll keep digging into it, but let me know if there are any specific nuances in the new dissect.database functions I should be aware of regarding these cases!

@B0TAxy

B0TAxy commented Jan 9, 2026

Copy link
Copy Markdown
Contributor Author

I’d be happy to run a few tests on my end and give you some feedback on how it handles the extraction.

If you have any test sets of old domains that are suitable for open-source sharing, that would be greatly appreciated!

Unfortunately, I don't currently have any test data from old domains that would be suitable for open-source sharing.

@Schamper

Schamper commented Jan 9, 2026 •

Copy link
Copy Markdown
Contributor

@B0TAxy you need to “unlock” the PEK first now (ntds.pek.unlock(syskey)). Second, it only does the RC4/AES decryption. I meant for the DES decryption to be in dissect.target and re-used from the SAM plugin (it’s currently embedded inside decrypt_single_hash, but it should be separated to a des_decrypt).

If after that you still have some issues, please let me know and if possible provide some test data (PEK, syskey and encrypted blobs).

@B0TAxy

B0TAxy commented Jan 9, 2026

Copy link
Copy Markdown
Contributor Author

@B0TAxy you need to “unlock” the PEK first now (ntds.pek.unlock(syskey)). Second, it only does the RC4/AES decryption. I meant for the DES decryption to be in dissect.target and re-used from the SAM plugin (it’s currently embedded inside decrypt_single_hash, but it should be separated to a des_decrypt).

If after that you still have some issues, please let me know and if possible provide some test data (PEK, syskey and encrypted blobs).

Yes, I'm aware of the issue—I had some trouble with Git and it didn't push the new code as expected. That’s fixed now and the latest version is pushed. However, I’m still running into the problem with the krbtgt decryption.

@Schamper

Copy link
Copy Markdown
Contributor

I can take a look at the fixtures and hash length issue. What exactly do you mean with ground truth data? What would you need?

As for the exception, probably this is because I currently chose to throw an exception when trying to list the groups/members of objects that have been replicated from another DC (phantom objects), as that NTDS won't have that information available. My reasoning was that returning an empty list is not really accurate, since they probably do have groups/members, we just can't know about it. I wanted to experiment with how that would work out in dissect.target, if the exception is annoying (vs a boolean check on is_phantom), we can change that behavior.

@B0TAxy

B0TAxy commented Jan 17, 2026

Copy link
Copy Markdown
Contributor Author

I can take a look at the fixtures and hash length issue. What exactly do you mean with ground truth data? What would you need?

As for the exception, probably this is because I currently chose to throw an exception when trying to list the groups/members of objects that have been replicated from another DC (phantom objects), as that NTDS won't have that information available. My reasoning was that returning an empty list is not really accurate, since they probably do have groups/members, we just can't know about it. I wanted to experiment with how that would work out in dissect.target, if the exception is annoying (vs a boolean check on is_phantom), we can change that behavior.

Thanks for looking into the fixtures and hash length!
Regarding ground truth data: I meant that I need the expected cleartext passwords (or target hashes) to verify my parsing logic. For example, @joost-j provided these credentials:

  • henk.devries: Winter2025!
  • beau.terham: Zomer2027!

Knowing these allows me to confirm that I am extracting the correct hash.
As for the exception: The logic regarding phantom objects makes sense and isn't annoying. However, the error I encountered is actually different. I checked, and it seems to be an IndexError (among others) originating in the backlinks function.

@Schamper

Copy link
Copy Markdown
Contributor

Except for the changed passwords (which I unfortunately don't remember what I changed them too :)) all data is the same as here: https://github.com/Orange-Cyberdefense/GOAD/blob/main/ad/GOAD/data/config.json

@Schamper

Copy link
Copy Markdown
Contributor

However, the error I encountered is actually different. I checked, and it seems to be an IndexError (among others) originating in the backlinks function.

I looked into this, and I believe this is a side-effect of the large NTDS.dit probably being "dirty" (can you confirm @joost-j?). Probably this could be fixed if we implement transaction files: fox-it/dissect.database#18

@Schamper

Copy link
Copy Markdown
Contributor

I looked into this, and I believe this is a side-effect of the large NTDS.dit probably being "dirty" (can you confirm @joost-j?). Probably this could be fixed if we implement transaction files: fox-it/dissect.database#18

We confirmed this is the case and updated the test data in dissect.database. It's now merged too! 🥳 If you could update the file in this PR that'd be appreciated.

@Schamper

Schamper commented Feb 6, 2026

Copy link
Copy Markdown
Contributor

I've made some changes to simplify the plugin further. Depends on fox-it/dissect.database#27 though.

@B0TAxy
B0TAxy force-pushed the feautre/ntds_dit_parsing branch from 7a9caae to a6b4e50 Compare February 6, 2026 14:51
Comment thread tests/_data/plugins/os/windows/ad/ntds/large/ntds.dit.gz Outdated
@B0TAxy
B0TAxy force-pushed the feautre/ntds_dit_parsing branch from a6b4e50 to a1e6f2d Compare February 6, 2026 16:02
@B0TAxy

B0TAxy commented Feb 6, 2026

Copy link
Copy Markdown
Contributor Author

I’ve updated the branch to include more comprehensive tests for the NTDS parsing logic. However, I’ve hit a snag with test_users.

It's currently throwing ValueError: data must be 16 bytes long

Apart from that, the tests should be good now

@Schamper

Copy link
Copy Markdown
Contributor

It's currently throwing ValueError: data must be 16 bytes long

I've created fox-it/dissect.database#29 to address this inside of the NTDS parser itself. Turns out that it's just a big blob that's stored, we need to split it up into 16 byte chunks ourselves.

@B0TAxy

B0TAxy commented Feb 10, 2026

Copy link
Copy Markdown
Contributor Author

It's currently throwing ValueError: data must be 16 bytes long

I've created fox-it/dissect.database#29 to address this inside of the NTDS parser itself. Turns out that it's just a big blob that's stored, we need to split it up into 16 byte chunks ourselves.

Glad to hear you found the fix. Let me know once the PR is merged!

@codecov

codecov Bot commented Feb 11, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.58824% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.93%. Comparing base (d9a1481) to head (0c80899).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
dissect/target/plugins/os/windows/ad/ntds.py 96.36% 2 Missing ⚠️
...issect/target/plugins/os/windows/credential/sam.py 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1347      +/-   ##
==========================================
+ Coverage   80.91%   80.93%   +0.01%     
==========================================
  Files         396      397       +1     
  Lines       34827    34892      +65     
==========================================
+ Hits        28180    28239      +59     
- Misses       6647     6653       +6     
Flag Coverage Δ
unittests 80.93% <95.58%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Schamper
Schamper merged commit 4ec38bf into fox-it:main Feb 11, 2026
20 of 24 checks passed
@B0TAxy
B0TAxy deleted the feautre/ntds_dit_parsing branch February 11, 2026 14:09
Matthijsy pushed a commit to Matthijsy/dissect.target that referenced this pull request Mar 2, 2026
Co-authored-by: Schamper <1254028+Schamper@users.noreply.github.com>
twiggler pushed a commit to twiggler/dissect-monorepo-test that referenced this pull request May 26, 2026
Co-authored-by: Schamper <1254028+Schamper@users.noreply.github.com>
twiggler pushed a commit to twiggler/dissect-monorepo-test that referenced this pull request Jun 4, 2026
Co-authored-by: Schamper <1254028+Schamper@users.noreply.github.com>
twiggler pushed a commit to twiggler/dissect-monorepo-test that referenced this pull request Jun 5, 2026
Co-authored-by: Schamper <1254028+Schamper@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants