Skip to content

NTDS: Enhanced records#1599

Open
B0TAxy wants to merge 5 commits intofox-it:mainfrom
B0TAxy:feat/enhanced_ntds_records
Open

NTDS: Enhanced records#1599
B0TAxy wants to merge 5 commits intofox-it:mainfrom
B0TAxy:feat/enhanced_ntds_records

Conversation

@B0TAxy
Copy link
Contributor

@B0TAxy B0TAxy commented Mar 3, 2026

This PR expands the parser's coverage by adding support for several new record types, ensuring a more comprehensive map of the target environment.
Also, added better handling in case the pek cannot be unlocked

@B0TAxy B0TAxy changed the title NTDS: enhanced records NTDS: Enhanced records Mar 3, 2026
@B0TAxy
Copy link
Contributor Author

B0TAxy commented Mar 3, 2026

@Schamper, as far as I know, this is complete. I would love for this to be merged as quickly as possible so I can work on the BloodHound PR more comfortably

@B0TAxy
Copy link
Contributor Author

B0TAxy commented Mar 4, 2026

After further testing on the Bloodhound plugin, I’ve noticed a few missing fields. While these need to be addressed, I’d prefer to merge the current PR now and handle those specific fixes directly within the Bloodhound PR.

@Schamper
Copy link
Member

Schamper commented Mar 4, 2026

After further testing on the Bloodhound plugin, I’ve noticed a few missing fields. While these need to be addressed, I’d prefer to merge the current PR now and handle those specific fixes directly within the Bloodhound PR.

What would those be?

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 4, 2026

Merging this PR will not alter performance

✅ 12 untouched benchmarks


Comparing B0TAxy:feat/enhanced_ntds_records (91db389) with main (0f31fe5)

Open in CodSpeed

@B0TAxy
Copy link
Contributor Author

B0TAxy commented Mar 4, 2026

After further testing on the Bloodhound plugin, I’ve noticed a few missing fields. While these need to be addressed, I’d prefer to merge the current PR now and handle those specific fixes directly within the Bloodhound PR.

What would those be?

Adding some fields in the records that I discover as look at each bloodhound Json

@Schamper
Copy link
Member

Schamper commented Mar 4, 2026

Wouldn't it be nicer to just add those in this PR? Especially if it's just a few fields.

@B0TAxy
Copy link
Contributor Author

B0TAxy commented Mar 4, 2026

Wouldn't it be nicer to just add those in this PR? Especially if it's just a few fields.

I totally see your point, and normally I’d agree. The main issue is that bouncing between these two PRs is getting a bit messy since they depend on each other. That said, if you’d prefer to have everything polished in one go before merging, I’m happy to hunker down and finish the fields here.

@Schamper
Copy link
Member

I don't see any changes to the existing records in #1598, so all of the proposed improvements are already in this PR?

@B0TAxy
Copy link
Contributor Author

B0TAxy commented Mar 16, 2026

I don't see any changes to the existing records in #1598, so all of the proposed improvements are already in this PR?

I've actually moved the changes regarding the record enhancement from #1598 into this PR. I'm currently adding a few more updates on top of those, and I'll push everything once it's ready for review.

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.

2 participants