-
Notifications
You must be signed in to change notification settings - Fork 197
ADCS ESC11 coverage #1679
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.
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
base: main
Are you sure you want to change the base?
ADCS ESC11 coverage #1679
Conversation
WalkthroughThis update introduces support for the new relationship kind Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant API
participant Analysis
participant GraphDB
User->>UI: Initiates search or views edge info for ESC11/NTLM relay to ADCS RPC
UI->>API: Requests edge composition or relay targets for CoerceAndRelayNTLMToADCSRPC
API->>Analysis: Analyzes graph for vulnerable Enterprise CAs (RPCEncryptionEnforced = false)
Analysis->>GraphDB: Queries nodes and relationships with new properties/kind
GraphDB-->>Analysis: Returns matching nodes/edges
Analysis-->>API: Returns composition/targets
API-->>UI: Returns data
UI-->>User: Displays help texts, relay targets, and composition info
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/WindowsAbuse.tsx (1)
24-27
: Minor grammatical improvement needed.Consider improving the wording for better clarity:
- 1. Start the Relay Server The NTLM relay can be executed with tools like Inveigh or ntlmrelayx.py, + 1. Start the Relay Server NTLM relay attacks can be executed with tools like Inveigh or ntlmrelayx.py,packages/go/analysis/ad/ntlm.go (2)
268-274
: Consider consistent variable naming across traversal blocks.The variable
enterpriseCANode
is declared in both traversal blocks (lines 243 and 268). While functionally correct due to different scopes, consider extracting the enterprise CA node retrieval logic into a helper function for better maintainability.
295-391
: Consider reducing code duplication between edge composition functions.The functions
GetCoerceAndRelayNTLMtoADCSEdgeComposition
andGetCoerceAndRelayNTLMtoADCSRPCEdgeComposition
are nearly identical, differing only in theisRPC
parameter passed tocollectADCSCandidateSegments
. Consider consolidating them into a single function.Create a single parameterized function:
-func GetCoerceAndRelayNTLMtoADCSEdgeComposition(ctx context.Context, db graph.Database, edge *graph.Relationship) (graph.PathSet, error) { +func getCoerceAndRelayNTLMtoADCSEdgeCompositionBase(ctx context.Context, db graph.Database, edge *graph.Relationship, isRPC bool) (graph.PathSet, error) {Then create thin wrappers:
func GetCoerceAndRelayNTLMtoADCSEdgeComposition(ctx context.Context, db graph.Database, edge *graph.Relationship) (graph.PathSet, error) { return getCoerceAndRelayNTLMtoADCSEdgeCompositionBase(ctx, db, edge, false) } func GetCoerceAndRelayNTLMtoADCSRPCEdgeComposition(ctx context.Context, db graph.Database, edge *graph.Relationship) (graph.PathSet, error) { return getCoerceAndRelayNTLMtoADCSEdgeCompositionBase(ctx, db, edge, true) }packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCS/WindowsAbuse.tsx (1)
51-53
: Consider improving code formatting consistency.The inline code uses HTML
<code>
tags while the example command uses Typography with 'pre' variant. For better consistency, consider using a consistent approach for code formatting.- Connection String format: <code>\\SERVER_NETBIOS@PORT/PATH/TO/FILE</code>. Example: + Connection String format: <Typography component='code'>\\SERVER_NETBIOS@PORT/PATH/TO/FILE</Typography>. Example:Alternatively, you could wrap both in a consistent code formatting component or use Material-UI's
<Box>
with monospace font styling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
cmd/api/src/test/integration/harnesses/CoerceAndRelayNTLMToADCSRPC.svg
is excluded by!**/*.svg
📒 Files selected for processing (26)
cmd/api/src/analysis/ad/ntlm_integration_test.go
(1 hunks)cmd/api/src/test/integration/harnesses.go
(2 hunks)cmd/api/src/test/integration/harnesses/CoerceAndRelayNTLMToADCSRPC.json
(1 hunks)packages/cue/bh/ad/ad.cue
(6 hunks)packages/go/analysis/ad/ad.go
(2 hunks)packages/go/analysis/ad/ntlm.go
(8 hunks)packages/go/analysis/ad/post.go
(1 hunks)packages/go/ein/ad.go
(1 hunks)packages/go/ein/incoming_models.go
(1 hunks)packages/go/graphschema/ad/ad.go
(6 hunks)packages/go/graphschema/common/common.go
(1 hunks)packages/javascript/bh-shared-ui/src/commonSearchesAGI.ts
(2 hunks)packages/javascript/bh-shared-ui/src/commonSearchesAGT.ts
(2 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCS/LinuxAbuse.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCS/WindowsAbuse.tsx
(2 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/CoerceAndRelayNTLMToADCSRPC.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/Composition.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/General.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/LinuxAbuse.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/Opsec.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/References.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/RelayTargets.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/WindowsAbuse.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx
(2 hunks)packages/javascript/bh-shared-ui/src/edgeTypes.tsx
(1 hunks)packages/javascript/bh-shared-ui/src/graphSchema.ts
(6 hunks)
🧰 Additional context used
🧠 Learnings (10)
packages/go/graphschema/common/common.go (1)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
cmd/api/src/test/integration/harnesses/CoerceAndRelayNTLMToADCSRPC.json (2)
Learnt from: mvlipka
PR: SpecterOps/BloodHound#1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
packages/cue/bh/ad/ad.cue (2)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: definitelynotagoblin
PR: SpecterOps/BloodHound#1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the `isProtectedComputer` function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
packages/go/analysis/ad/ad.go (2)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
packages/javascript/bh-shared-ui/src/commonSearchesAGT.ts (1)
Learnt from: benwaples
PR: SpecterOps/BloodHound#1515
File: packages/javascript/bh-shared-ui/src/views/TierManagement/Save/SelectorForm/SelectorForm.tsx:202-211
Timestamp: 2025-05-30T16:39:53.440Z
Learning: In BloodHound, Cypher injection vulnerabilities are only a concern when `enable_cypher_mutations` is enabled, in which case users already have direct access to execute Cypher via the editor or API endpoints, making string interpolation in queries a non-security issue.
packages/javascript/bh-shared-ui/src/commonSearchesAGI.ts (1)
Learnt from: benwaples
PR: SpecterOps/BloodHound#1515
File: packages/javascript/bh-shared-ui/src/views/TierManagement/Save/SelectorForm/SelectorForm.tsx:202-211
Timestamp: 2025-05-30T16:39:53.440Z
Learning: In BloodHound, Cypher injection vulnerabilities are only a concern when `enable_cypher_mutations` is enabled, in which case users already have direct access to execute Cypher via the editor or API endpoints, making string interpolation in queries a non-security issue.
packages/go/graphschema/ad/ad.go (1)
Learnt from: elikmiller
PR: SpecterOps/BloodHound#1563
File: packages/go/graphschema/azure/azure.go:24-24
Timestamp: 2025-06-06T23:12:14.181Z
Learning: In BloodHound, files in packages/go/graphschema/*/`*.go` are generated from CUE schemas. When `just prepare-for-codereview` is run, it triggers code generation that may automatically add import aliases or other formatting changes. These changes are legitimate outputs of the generation process, not manual edits that would be overwritten.
cmd/api/src/test/integration/harnesses.go (2)
undefined
<retrieved_learning>
Learnt from: mvlipka
PR: #1615
File: cmd/api/src/test/integration/harnesses/AZPIMRolesHarness.json:190-194
Timestamp: 2025-06-26T20:50:03.695Z
Learning: The JSON files in cmd/api/src/test/integration/harnesses/ directory (like AZPIMRolesHarness.json) are used as reference documents for writing Go test harnesses, not as actual data files that get parsed by the application code. Minor formatting issues like trailing spaces in these reference files don't affect functionality.
</retrieved_learning>
<retrieved_learning>
Learnt from: definitelynotagoblin
PR: #1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the isProtectedComputer
function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
</retrieved_learning>
Learnt from: definitelynotagoblin
PR: SpecterOps/BloodHound#1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the `isProtectedComputer` function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1671
File: packages/go/analysis/ad/esc16.go:65-69
Timestamp: 2025-07-10T14:31:01.781Z
Learning: In BloodHound ADCS ESC analysis functions (PostADCSESC1, PostADCSESC4, PostADCSESC6, PostADCSESC9, PostADCSESC10, PostADCSESC13, PostADCSESC16, etc.), the established pattern for handling cert template validation errors and user filtering errors is to use slog.WarnContext to log warnings and continue processing with the next template, rather than returning errors to the caller. This pattern is consistently applied across all ADCS ESC edge implementations.
Learnt from: JonasBK
PR: SpecterOps/BloodHound#1434
File: packages/go/analysis/ad/gpos.go:102-126
Timestamp: 2025-06-18T08:27:18.317Z
Learning: In Active Directory's containment hierarchy, each user/computer has exactly one direct parent container, forming a tree structure. When processing GPO edges in packages/go/analysis/ad/gpos.go, the fetchDirectChildUsersAndComputers function only returns direct children, ensuring each user/computer is processed exactly once by its immediate parent container, eliminating the need for deduplication logic.
cmd/api/src/analysis/ad/ntlm_integration_test.go (2)
Learnt from: superlinkx
PR: SpecterOps/BloodHound#1503
File: cmd/api/src/services/job/jobs_test.go:19-143
Timestamp: 2025-05-27T16:58:33.295Z
Learning: Tests in cmd/api/src/services/job/jobs_test.go have been found to be flaky in the past and are due for rewrite. They should be skipped with t.Skip() until they can be properly rewritten.
Learnt from: definitelynotagoblin
PR: SpecterOps/BloodHound#1440
File: packages/go/analysis/ad/ntlm.go:123-138
Timestamp: 2025-05-23T19:56:13.423Z
Learning: In the BloodHound NTLM analysis code (packages/go/analysis/ad/ntlm.go), the `isProtectedComputer` function intentionally fails open (returns false/unprotected) when the Protected Users group cannot be retrieved, maintaining consistency with the original logic patterns in the codebase.
🧬 Code Graph Analysis (10)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
packages/go/graphschema/ad/ad.go (1)
CoerceAndRelayNTLMToADCSRPC
(117-117)
packages/go/analysis/ad/post.go (1)
packages/go/graphschema/ad/ad.go (1)
CoerceAndRelayNTLMToADCSRPC
(117-117)
packages/go/ein/incoming_models.go (1)
packages/go/graphschema/ad/ad.go (1)
RPCEncryptionEnforced
(265-265)
packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/CoerceAndRelayNTLMToADCSRPC.tsx (1)
packages/go/graphschema/ad/ad.go (1)
CoerceAndRelayNTLMToADCSRPC
(117-117)
packages/go/ein/ad.go (2)
packages/go/ein/incoming_models.go (2)
CARegistryData
(143-149)RPCEncryptionEnforced
(138-141)packages/go/graphschema/ad/ad.go (1)
RPCEncryptionEnforced
(265-265)
packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/General.tsx (1)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
EdgeInfoProps
(144-153)
packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/WindowsAbuse.tsx (2)
packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (1)
EdgeInfoProps
(144-153)packages/go/headers/headers.go (1)
Link
(112-112)
packages/go/analysis/ad/ad.go (2)
packages/go/graphschema/ad/ad.go (1)
CoerceAndRelayNTLMToADCSRPC
(117-117)packages/go/analysis/ad/ntlm.go (2)
GetCoerceAndRelayNTLMtoADCSRPCEdgeComposition
(344-391)GetVulnerableEnterpriseCAsForRelayNTLMtoADCSRPC
(780-800)
packages/go/graphschema/ad/ad.go (3)
packages/go/graphschema/common/common.go (2)
InboundRelationshipKinds
(42-44)OutboundRelationshipKinds
(45-47)cmd/api/src/test/integration/harnesses.go (3)
CoerceAndRelayNTLMToSMB
(8931-8952)CoerceAndRelayNTLMToLDAP
(9490-9510)CoerceAndRelayNTLMToLDAPS
(9637-9660)packages/go/ein/incoming_models.go (2)
RPCEncryptionEnforced
(138-141)GPLink
(245-248)
packages/go/analysis/ad/ntlm.go (4)
packages/go/analysis/analysis.go (2)
MaximumDatabaseParallelWorkers
(36-36)FetchNodeByObjectID
(91-101)packages/go/graphschema/ad/ad.go (21)
EnterpriseCA
(40-40)DomainSID
(154-154)MemberOf
(49-49)Group
(31-31)GenericAll
(45-45)Enroll
(92-92)AllExtendedRights
(51-51)CertTemplate
(42-42)RequiresManagerApproval
(194-194)SchemaVersion
(193-193)AuthorizedSignatures
(190-190)AuthenticationEnabled
(195-195)PublishedTo
(88-88)RPCEncryptionEnforced
(265-265)IssuedSignedBy
(99-99)EnterpriseCAFor
(98-98)AIACA
(38-38)RootCA
(39-39)RootCAFor
(86-86)CoerceAndRelayNTLMToADCS
(116-116)CoerceAndRelayNTLMToADCSRPC
(117-117)packages/go/analysis/ad/queries.go (1)
FetchAuthUsersAndEveryoneGroups
(1830-1840)packages/go/analysis/post.go (1)
CreatePostRelationshipJob
(151-157)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build BloodHound Container Image / Build and Package Container
- GitHub Check: run-analysis
- GitHub Check: run-tests
- GitHub Check: build-ui
🔇 Additional comments (60)
packages/javascript/bh-shared-ui/src/edgeTypes.tsx (1)
135-135
: LGTM! Proper categorization of the new edge type.The addition of
CoerceAndRelayNTLMToADCSRPC
to the NTLM Relay subcategory is correctly placed alongside other similar edge types and follows the established pattern.packages/javascript/bh-shared-ui/src/components/HelpTexts/index.tsx (2)
84-84
: LGTM! Proper import integration.The import statement follows the established pattern and is appropriately placed among the other CoerceAndRelayNTLM imports.
275-275
: LGTM! Component properly registered.The component is correctly added to the EdgeInfoComponents object, making it available for the new edge type help text functionality.
packages/go/analysis/ad/post.go (1)
67-67
: LGTM! Necessary addition for post-processing.The new relationship kind is correctly added to the PostProcessedRelationships list, ensuring it will be included in the post-processing pipeline alongside other NTLM relay relationships.
packages/go/ein/incoming_models.go (2)
138-141
: LGTM! Well-structured data model addition.The
RPCEncryptionEnforced
struct follows the established pattern of embeddingAPIResult
and including a booleanValue
field, consistent with other similar structures in the file.
148-148
: LGTM! Proper integration into CARegistryData.The new field is correctly added to the
CARegistryData
struct, maintaining consistency with the existing structure and supporting the ESC11 attack path requirements.packages/go/graphschema/common/common.go (2)
43-43
: LGTM! Proper schema integration for inbound relationships.The new relationship kind is correctly added to the InboundRelationshipKinds list. Since this is a generated file, the change reflects proper schema updates for the new edge type.
46-46
: LGTM! Proper schema integration for outbound relationships.The new relationship kind is correctly added to the OutboundRelationshipKinds list, ensuring complete schema support for the new edge type.
packages/go/analysis/ad/ad.go (2)
576-577
: LGTM! New edge type case added correctly.The new case for
CoerceAndRelayNTLMToADCSRPC
follows the established pattern and correctly calls the appropriate composition function.
607-608
: LGTM! New relay target case added correctly.The new case for
CoerceAndRelayNTLMToADCSRPC
follows the established pattern and correctly calls the appropriate relay target function.packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCS/LinuxAbuse.tsx (3)
17-17
: LGTM! Import statement updated correctly.The import statement properly includes the necessary Material-UI components for the updated implementation.
25-33
: LGTM! Improved security and consistency.The conversion from raw HTML anchor tags to Material-UI Link components improves consistency, while the
target='_blank'
andrel='noopener'
attributes ensure secure external link handling.
45-60
: LGTM! Better structured external links.The bulleted list format with Material-UI Link components improves readability and maintains consistent styling throughout the component.
packages/go/ein/ad.go (1)
1209-1212
: LGTM! Property parsing implemented correctly.The
RPCEncryptionEnforced
property parsing follows the established pattern and uses the correct property name from the schema.packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/CoerceAndRelayNTLMToADCSRPC.tsx (2)
17-23
: LGTM! Component imports are complete.All necessary help text components are properly imported for the new edge type.
25-33
: LGTM! Component mapping is well-structured.The object mapping follows the established pattern and provides clear, descriptive keys for each component type.
packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/References.tsx (2)
22-28
: LGTM! Secure external link implementation.The component properly uses Material-UI Link components with appropriate security attributes (
target='_blank'
andrel='noopener'
) for safe external link handling.
22-22
: LGTM! Scrollable container improves UX.The
overflowX: 'auto'
styling ensures the references remain accessible even on smaller screens.packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/LinuxAbuse.tsx (1)
17-79
: LGTM! Well-structured component with proper security practices.The component follows React best practices with consistent Material-UI usage and proper security attributes (
target='_blank'
,rel='noopener'
) for external links. The step-by-step Linux abuse instructions are clear and well-organized.packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/Composition.tsx (1)
17-50
: LGTM! Well-implemented component with proper async state handling.The component correctly implements loading states, error handling, and uses the
useEdgeInfoItems
hook properly. The Material-UI components are used consistently and the TypeScript typing is correct.packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/RelayTargets.tsx (1)
17-51
: LGTM! Well-implemented component with proper async state handling.The component correctly implements loading states, error handling, and uses the
useEdgeInfoItems
hook properly. The Material-UI components are used consistently, TypeScript typing is correct, and theonNodeClick
prop is properly handled for interactive functionality.packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/WindowsAbuse.tsx (3)
17-21
: LGTM! Component structure follows established patterns.The imports and component definition are correctly implemented following the established patterns for help text components in the codebase.
50-54
: Good technical content and formatting.The WebDAV connection string format and example are clearly presented with appropriate code formatting.
28-48
: Excellent external link security and relevance.The external links use proper security attributes and point to legitimate security research tools relevant to the attack scenario.
packages/cue/bh/ad/ad.cue (4)
973-985
: Well-structured new properties following established patterns.The new RPC encryption properties are correctly defined and follow the established naming conventions and structure used throughout the schema.
1119-1120
: Correct placement in Properties list.The new properties are appropriately added to the Properties list following the established ordering pattern.
1597-1600
: Correctly defined relationship kind.The new CoerceAndRelayNTLMToADCSRPC relationship kind follows established patterns and is placed logically with similar relationship types.
1732-1732
: Consistent integration across relationship lists.The new relationship kind is appropriately added to all necessary lists (RelationshipKinds, SharedRelationshipKinds, and EdgeCompositionRelationships) with consistent placement.
cmd/api/src/test/integration/harnesses/CoerceAndRelayNTLMToADCSRPC.json (3)
1-54
: Comprehensive style configuration for test harness.The style configuration is well-structured and follows established patterns for test harness visualization.
94-104
: Critical vulnerability configuration correctly set.The EnterpriseCA node is properly configured with
RPCEncryptionEnforced: "false"
, which is essential for testing the ESC11 attack scenario.
153-226
: Comprehensive relationship modeling for attack scenario.The relationships create a thorough test scenario that models the ESC11 attack path with appropriate connections between nodes for testing the attack composition logic.
cmd/api/src/test/integration/harnesses.go (3)
8876-8885
: LGTM! Well-structured test harness for ESC11 scenario.The struct definition follows existing patterns and includes all necessary Active Directory components for testing the RPC-based NTLM relay to ADCS attack scenario.
8887-8929
: Excellent implementation of ESC11 attack scenario setup.The setup method correctly implements the test scenario for the ESC11 attack:
- Creates appropriate AD nodes and relationships
- Sets
RPCEncryptionEnforced=false
to simulate the vulnerable CA configuration- Configures
RestrictOutboundNTLM=false
to enable NTLM relay attacks- Properly configures the AuthenticatedUsersGroup with the required ObjectID suffix
The property assignments align perfectly with the ESC11 attack requirements.
10233-10233
: Perfect integration with existing harness structure.The new harness is properly integrated into the
HarnessDetails
struct, maintaining consistency with naming conventions and logical grouping.packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/General.tsx (1)
21-48
: LGTM! Well-structured React component providing clear ESC11 documentation.The component follows React best practices with proper TypeScript typing, clear explanatory text, and appropriate use of Material-UI components. The technical content accurately describes the ESC11 attack vector and its differences from ESC8.
packages/javascript/bh-shared-ui/src/commonSearchesAGI.ts (3)
196-200
: LGTM! Correct ESC11 query for vulnerable RPC endpoints.The query correctly identifies certificate templates published to Enterprise CAs with RPC encryption disabled (
rpcencryptionenforced = False
), enabling detection of ESC11 vulnerabilities.
429-429
: LGTM! Proper inclusion of new RPC-based NTLM relay edge.The query correctly includes the new
CoerceAndRelayNTLMToADCSRPC
relationship type alongside existing NTLM relay edges, providing comprehensive coverage of all coercion and relay attack vectors.
435-438
: LGTM! Correct query for ESC11-vulnerable Enterprise CAs.The query properly identifies Enterprise CAs vulnerable to ESC11 attacks by checking for disabled RPC encryption enforcement.
cmd/api/src/analysis/ad/ntlm_integration_test.go (2)
614-659
: LGTM! Well-structured integration test for NTLM relay to ADCS RPC.The test follows the established pattern in the file, uses the correct test harness (
NTLMCoerceAndRelayNTLMToADCSRPC
), and properly verifies the creation of the newCoerceAndRelayNTLMToADCSRPC
relationship with correct start and end nodes.
661-714
: LGTM! Comprehensive composition test for ADCS RPC edge.The test correctly verifies the composition of the
CoerceAndRelayNTLMToADCSRPC
relationship, ensuring all expected nodes (Computer, CertTemplate, EnterpriseCA, RootCA, Domain, NTAuthStore, and AuthenticatedUsersGroup) are included in the composition.packages/javascript/bh-shared-ui/src/commonSearchesAGT.ts (3)
196-200
: LGTM! Consistent ESC11 query implementation.The query correctly identifies certificate templates published to Enterprise CAs with vulnerable RPC endpoints and is properly synchronized with the AGI version of the file.
429-429
: LGTM! Proper synchronization of NTLM relay edges.The query correctly includes the new
CoerceAndRelayNTLMToADCSRPC
relationship type and maintains consistency with the AGI file as required.
435-438
: LGTM! Consistent ESC11 vulnerability detection.The query properly identifies ESC11-vulnerable Enterprise CAs and maintains synchronization with the AGI file version.
packages/javascript/bh-shared-ui/src/graphSchema.ts (4)
146-146
: LGTM! Relationship kind addition follows established patterns.The new
CoerceAndRelayNTLMToADCSRPC
relationship kind is properly added to the enum and positioned logically with other NTLM relay relationships.
307-308
: LGTM! Display function correctly updated.The case for
CoerceAndRelayNTLMToADCSRPC
is properly added to the display function with the correct return value.
488-489
: LGTM! Properties properly integrated into schema.The new RPC encryption properties are correctly added to the enum and display function, following the established naming convention and pattern used by other EnterpriseCA properties.
Also applies to: 755-758
350-350
: LGTM! Relationship kind properly integrated into collections.The new relationship kind is correctly added to both
EdgeCompositionRelationships
andActiveDirectoryPathfindingEdges
, ensuring it's available for graph composition and pathfinding operations.Also applies to: 809-809
packages/go/graphschema/ad/ad.go (4)
117-117
: LGTM! Relationship kind constant properly defined.The new
CoerceAndRelayNTLMToADCSRPC
constant follows the established pattern and is correctly positioned with other NTLM relay relationships.
265-266
: LGTM! Property constants properly defined.The new RPC encryption properties are correctly defined following the established naming convention and pattern.
270-270
: LGTM! Properties comprehensively integrated into all functions.The new RPC encryption properties are properly integrated into all property-related functions:
- Added to
AllProperties()
slice- Parse cases added to
ParseProperty()
function- String conversion cases added to
String()
method- Display name cases added to
Name()
methodThis ensures complete functionality for the new properties.
Also applies to: 536-539, 808-811, 1080-1083
1100-1100
: LGTM! Relationship kind comprehensively integrated into all collections.The new
CoerceAndRelayNTLMToADCSRPC
relationship kind is properly added to all relevant relationship slices:
Relationships()
- for general relationship enumerationPathfindingRelationships()
- for pathfinding operationsInboundRelationshipKinds()
- for inbound relationship queriesOutboundRelationshipKinds()
- for outbound relationship queriesThis ensures the relationship is available for all graph operations and analysis.
Also applies to: 1106-1106, 1109-1109, 1112-1112
packages/go/analysis/ad/ntlm.go (5)
214-219
: LGTM! Well-structured result type.The
adcsTraversalResult
struct properly encapsulates the traversal results with clear field names and appropriate use of cardinality bitmaps for performance.
434-473
: LGTM! Traversal pattern correctly implements RPC-specific filtering.The pattern properly filters on
RPCEncryptionEnforced=false
to identify vulnerable RPC endpoints, maintaining consistency with the existing traversal pattern structure.
514-609
: LGTM! Comprehensive handling of both HTTP and RPC attack paths.The updated function correctly:
- Validates enterprise CAs for both attack paths separately
- Maintains separate result sets to avoid edge type mixing
- Efficiently skips processing when neither path is valid
- Creates the appropriate edge types based on CA validity
620-636
: LGTM! Consistent validation logic for both attack paths.Both validation functions correctly treat missing properties as "not valid" rather than errors, maintaining consistency with the codebase patterns.
780-800
: LGTM! Correctly implements RPC-specific vulnerability detection.The function properly identifies vulnerable Enterprise CAs by checking for disabled RPC encryption, maintaining consistency with the HTTP counterpart's structure and error handling.
packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCS/WindowsAbuse.tsx (4)
17-19
: LGTM!The import statements are clean and appropriate for the component's functionality.
29-46
: LGTM!The external links are properly secured with
target='_blank'
andrel='noopener'
attributes, and they point to legitimate security research repositories.
21-58
: LGTM!The component structure follows React best practices with proper TypeScript typing and consistent Material-UI component usage.
24-54
: LGTM!The technical content is accurate and provides clear, step-by-step instructions for the ADCS ESC11 attack path. The references to legitimate security tools and documentation are appropriate.
import { Typography } from '@mui/material'; | ||
import { FC } from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Import Material-UI Link component for consistency.
The component should import Link
from Material-UI to maintain consistency with other components in this feature set.
-import { Typography } from '@mui/material';
+import { Link, Typography } from '@mui/material';
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { Typography } from '@mui/material'; | |
import { FC } from 'react'; | |
import { Link, Typography } from '@mui/material'; | |
import { FC } from 'react'; |
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/Opsec.tsx
around lines 17 to 18, the Material-UI Link component is not imported. Add an
import statement for Link from '@mui/material' alongside the existing Typography
import to ensure consistent use of Material-UI components across this feature
set.
<a href={'https://posts.bluraven.io/detecting-ntlm-relay-attacks-d92e99e68fb9'}> | ||
Detecting NTLM Relay Attacks | ||
</a> | ||
. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use Material-UI Link component with proper security attributes.
For consistency with other components and security best practices, replace the plain <a>
tag with Material-UI Link
component.
- <a href={'https://posts.bluraven.io/detecting-ntlm-relay-attacks-d92e99e68fb9'}>
+ <Link target='_blank' rel='noopener' href='https://posts.bluraven.io/detecting-ntlm-relay-attacks-d92e99e68fb9'>
Detecting NTLM Relay Attacks
- </a>
+ </Link>
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
<a href={'https://posts.bluraven.io/detecting-ntlm-relay-attacks-d92e99e68fb9'}> | |
Detecting NTLM Relay Attacks | |
</a> | |
. | |
<Link target='_blank' rel='noopener' href='https://posts.bluraven.io/detecting-ntlm-relay-attacks-d92e99e68fb9'> | |
Detecting NTLM Relay Attacks | |
</Link> | |
. |
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/Opsec.tsx
around lines 28 to 31, replace the plain <a> tag with the Material-UI Link
component to maintain consistency and improve security. Import the Link
component from Material-UI if not already imported, then use it with the href
attribute set to the URL and include security attributes like target="_blank"
and rel="noopener noreferrer" to ensure safe external linking.
<a href={'https://specterops.io/wp-content/uploads/sites/3/2022/06/Certified_Pre-Owned.pdf'}> | ||
Certified Pre-Owned. | ||
</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Use Material-UI Link component with proper security attributes.
For consistency with other components and security best practices, replace the plain <a>
tag with Material-UI Link
component.
- <a href={'https://specterops.io/wp-content/uploads/sites/3/2022/06/Certified_Pre-Owned.pdf'}>
+ <Link target='_blank' rel='noopener' href='https://specterops.io/wp-content/uploads/sites/3/2022/06/Certified_Pre-Owned.pdf'>
Certified Pre-Owned.
- </a>
+ </Link>
🤖 Prompt for AI Agents
In
packages/javascript/bh-shared-ui/src/components/HelpTexts/CoerceAndRelayNTLMToADCSRPC/Opsec.tsx
around lines 43 to 45, replace the plain <a> tag with the Material-UI Link
component to maintain consistency and follow security best practices. Ensure to
add appropriate security attributes such as rel="noopener noreferrer" and
target="_blank" if the link opens in a new tab.
Description
Coverage of the ADCS ESC11 attack paths.
ESC11 attacks exploit misconfigured Enterprise Certificate Authorities that allow inbound relay attacks and impersonation of any domain principal.
This PR implements:
Motivation and Context
Resolves BED-6182
Using this dataset:
20250611101235_BloodHound_esc11.zip
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Style