Add netlogon reg key #170
Add netlogon reg key #170
Conversation
JonasBK
added
enhancement
WalkthroughAdds population of a new DCRegistryData field, VulnerableNetlogonSecurityDescriptor, in ProcessDomainController by calling _dcRegistryProcessor.GetVulnerableNetlogonSecurityDescriptor(apiName); DCRegistryData now exposes this property publicly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant ObjectProcessors as ObjectProcessors
participant DCRegistryProcessor as DCRegistryProcessor
participant DCRegistryData as DCRegistryData
Caller->>ObjectProcessors: ProcessDomainController(apiName)
ObjectProcessors->>DCRegistryProcessor: GetVulnerableNetlogonSecurityDescriptor(apiName)
DCRegistryProcessor-->>ObjectProcessors: securityDescriptor
ObjectProcessors->>DCRegistryData: Initialize {..., VulnerableNetlogonSecurityDescriptor = securityDescriptor}
ObjectProcessors-->>Caller: DCRegistryData
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Runtime/ObjectProcessors.cs (2)
405-407: Batch DC registry reads to a single RPC session (optional).Three per-property calls may incur extra RemoteRegistry round-trips. Consider a DCRegistryProcessor method that opens the registry once and returns a populated DCRegistryData to minimize latency and handle transient errors atomically.
405-407: Expose a 'collected' indicator for parity (optional).If VulnerableNetlogonSecurityDescriptor uses a Collected pattern internally, consider surfacing a boolean (e.g., vnlsecdesccollected) alongside other DC flags for quick filtering—mirroring CARegistry collected flags elsewhere in this file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/Runtime/ObjectProcessors.cs(1 hunks)
🔇 Additional comments (1)
src/Runtime/ObjectProcessors.cs (1)
405-407: Verify SharpHoundCommon dependency and serialization
- csproj still pulls SharpHoundCommon 4.3.1 from NuGet; ensure that release (or a prerelease you target) includes the new
VulnerableNetlogonSecurityDescriptorproperty andGetVulnerableNetlogonSecurityDescriptor(string)method. If not, switch to the local project reference or bump to the PR-244 preview.- Confirm on
DCRegistryDatathatVulnerableNetlogonSecurityDescriptoris public and of typestring(SDDL), and that the processor method returns an SDDL string rather than raw bytes.
JonasBK
removed
the
blocked by SHC PR
Description
Collect the registry key that holds the security descriptor that determines who can perform the vulnerable version of netlogon
Motivation and Context
Ticket: BED-6429
Depends on this Commonlib PR: SpecterOps/SharpHoundCommon#244
How Has This Been Tested?
Locally in GOAD lab.
Screenshots (if appropriate):
From corresponding BloodHound PR:
Types of changes
Checklist:
Summary by CodeRabbit