fix: Align managed-service-account DNS requirements in ADCS compositions BED-9336

[JonasBK]

Description

This PR aligns ADCS edge creation and composition when a certificate template requires a DNS or domain-DNS subject alternative name.

gMSAs and sMSAs are represented as User nodes, but they possess the DNS identity required by these templates. Creation already preserved these accounts in some paths, while several composition implementations treated them as ordinary users and discarded the supporting template path.

This change:

• Adds a shared isManagedServiceAccount helper for the gmsa and msa properties.
• Reuses the helper in DNS filtering and computer-derived target classification.
• Preserves computers, gMSAs, and sMSAs for DNS-required certificate templates.
• Continues rejecting ordinary users when the template requires DNS.
• Treats absent managed-account properties as false and propagates malformed-property errors.
• Applies equivalent composition handling to ESC3, ESC6, existing ESC9a edges, ESC10a, and ESC13.
• Adds creation and composition coverage for ordinary users, computers, gMSAs, and sMSAs.

Stack

• Base: main
• Head: BED-9336-adcs-managed-account-dns
• This is PR 1 of 4.
• Merge the stack in order.
• After this PR merges, retarget BED-9336-adcs-trust-paths to main.

Commit review order

1. refactor: centralize managed service account detection BED-9336

   • Introduces and unit-tests isManagedServiceAccount.
   • Removes duplicate gMSA/sMSA property handling.

2. fix: align ESC3 managed-account DNS requirements BED-9336

   • Applies the shared decision to ESC3 composition.
   • Adds an ESC3 harness covering both DNS template flags and ordinary-user rejection.

3. fix: align managed-account DNS in ADCS compositions BED-9336

   • Applies the same rule to ESC6, existing ESC9a, ESC10a, and ESC13.
   • Adds end-to-end creation and composition assertions.

Motivation and Context

Resolves BED-9336

Creation and composition must evaluate certificate-template requirements consistently. Previously, a managed service account could receive a valid post-processed edge because it has a DNS identity, but composition could reject the same path because the account is ingested as a User.

That produced edges with empty or incomplete composition graphs. Centralizing the account classification removes the inconsistency without relaxing DNS requirements for ordinary users.

How Has This Been Tested?

Validated with:

• go test ./packages/go/analysis/ad -count=1
• just prepare-for-codereview

Coverage includes:

• gMSA and sMSA detection.
• Missing, false, and malformed managed-account properties.
• ESC3 templates using SubjectAltRequireDNS and SubjectAltRequireDomainDNS.
• ESC6, existing ESC9a, ESC10a, and ESC13 composition.
• Ordinary-user negative cases and computer positive cases.

Screenshots (optional):

N/A — backend graph analysis only.

Types of changes

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• I have met the contributing prerequisites
  • Assigned myself to this PR
  • Added the appropriate labels
  • Associated an issue: Issues and Pull Requests README #672
  • Read the Contributing guide: https://github.com/SpecterOps/BloodHound/wiki/Contributing
• I have ensured that related documentation is up-to-date
  • Open API docs
  • Code comments (GoDocs / JSDocs)
• I have followed proper test practices
  • Added/updated tests to cover my changes
  • All new and existing tests passed

Summary by CodeRabbit

• New Features
  • Improved certificate enrollment path analysis for managed service accounts, including gMSA and sMSA scenarios.
  • Added support for DNS and domain-DNS certificate requirements in relevant security analyses.
• Bug Fixes
  • Regular users are now excluded from paths requiring managed service account capabilities.
  • Improved handling of invalid certificate templates and property errors.
• Tests
  • Added comprehensive integration coverage for ESC3, ESC6, ESC9, ESC10, and ESC13 managed service account scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 0d63baa0-a9db-4598-8508-9ba6b3971767

📥 Commits

Reviewing files that changed from the base of the PR and between 7d00dbf and 5628cff.

📒 Files selected for processing (2)

• packages/go/analysis/ad/ad_internal_test.go
• packages/go/analysis/ad/owns.go

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds managed service account detection and applies it to ESC3, ESC6, ESC9, ESC10, and ESC13 path composition. It adds DNS-focused graph fixtures and integration tests for GMSA, MSA, and regular-user behavior.

Changes

Managed service account AD CS support

Layer / File(s)	Summary
Managed service account detection packages/go/analysis/ad/ad.go, packages/go/analysis/ad/owns.go, packages/go/analysis/ad/ad_internal_test.go	The shared classifier checks GMSA and MSA properties, propagates property errors, and supports computer-derived target classification. Unit tests cover valid, absent, false, and invalid properties.
AD CS path filtering packages/go/analysis/ad/esc3.go, packages/go/analysis/ad/esc6.go, packages/go/analysis/ad/esc9.go, packages/go/analysis/ad/esc10.go, packages/go/analysis/ad/esc13.go, packages/go/analysis/ad/esc_shared.go	ESC paths retain managed service account victims when certificate templates require DNS properties. Non-managed users are excluded, and classification errors stop traversal. ESC13 also stops later traversal when path 1 finds no enterprise CAs.
ESC3 DNS fixture and validation cmd/api/src/test/integration/harnesses.go, packages/go/analysis/ad/adcs_integration_test.go	The new harness builds DNS and domain-DNS ESC3 graph scenarios. Integration tests validate four managed service account relationships and the absence of regular-user relationships.
Cross-path integration validation packages/go/analysis/ad/ad_parallel_integration_test.go	Integration coverage validates managed service account compositions for ESC6a, ESC9a, ESC10a, and ESC13 using DNS-required and DNS-free templates.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 5628c

The PR aligns DNS-template handling for managed service accounts while continuing to reject ordinary users, with targeted test coverage. No actionable merge-blocking risk remains beyond normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant ADCSGraph
  participant ESCPathComposition
  participant isManagedServiceAccount
  participant IntegrationTest
  ADCSGraph->>ESCPathComposition: provide principals and certificate templates
  ESCPathComposition->>isManagedServiceAccount: classify user victim
  isManagedServiceAccount-->>ESCPathComposition: managed status or property error
  ESCPathComposition->>IntegrationTest: return retained or excluded compositions
  IntegrationTest->>ESCPathComposition: validate ESC3, ESC6a, ESC9a, ESC10a, and ESC13 results

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the managed-service-account DNS composition fix and includes the associated ticket.
Description check	✅ Passed	The description covers the required change, motivation, ticket, testing, change type, and checklist. It provides detailed test coverage and explains the stack context.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch BED-9336-adcs-managed-account-dns

---

_{Comment @coderabbitai help to get the list of available commands.}

[kpowderly]

Thanks for the extra information on this ❤️ it was extremely helpful to understand the problem statement! Great job!

[JonasBK]

Hey @kpowderly, thanks a lot for the review! :)
I believe I have fixed the stuff you pointed out in the latest commit