Skip to content

feat: add native Windows ADCS connenction logic#297

Open
carlosmonastyrski wants to merge 3 commits into
mainfrom
PKI-295
Open

feat: add native Windows ADCS connenction logic#297
carlosmonastyrski wants to merge 3 commits into
mainfrom
PKI-295

Conversation

@carlosmonastyrski

Copy link
Copy Markdown
Contributor

Description 📣

Type ✨

  • Bug fix
  • New feature
  • Improvement
  • Breaking change
  • Documentation

Tests 🛠️

# Here's some code block to paste some code snippets

@linear

linear Bot commented Jul 4, 2026

Copy link
Copy Markdown

PKI-295

@infisical-review-police

Copy link
Copy Markdown

💬 Discussion in Slack: #pr-review-cli-297-feat-add-native-windows-adcs-connenction-logic

Posted by Review Police — reviews, comments, new commits, and CI failures will stream into this channel.

@greptile-apps

greptile-apps Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds native Windows ADCS (Active Directory Certificate Services) support to the gateway by implementing an MS-WCCE/DCOM client (adcs.go) and an HTTP handler (adcs_handler.go) that exposes four operations — test, discover-ca, templates, and enroll — over the existing mTLS relay tunnel via the new infisical-adcs ALPN protocol.

  • adcs/adcs.go: Implements Dial (DCOM/RPC over port 135), DiscoverCAName (registry read over SMB/port 445), Templates, and Enroll. The \ \ injection guard added to Enroll addresses the previously flagged template-name injection issue.
  • adcs_handler.go: Routes inbound requests through a sync.OnceValue-initialised ServeMux (fixing the per-request allocation from the earlier review). Dial errors now return a generic message; however, env.Host is still accepted without allowlist validation (SSRF), and errors from the operation handlers are still returned verbatim to callers.
  • gateway.go and root.go: Wire the new mode into the channel dispatcher and fix a zerolog v1.35 console-output regression respectively.

Confidence Score: 4/5

Requires resolution of the unvalidated host field before merging; the rest of the change is structurally sound.

The new ADCS handler passes env.Host directly from the request body into outbound DCOM (port 135) and SMB (port 445) connections with no allowlist check. Because the gateway runs inside the customer's network, this lets a caller redirect the gateway to connect to any reachable internal host, turning it into a network scanner. The two previously flagged issues (per-request ServeMux allocation and template-name injection) have been addressed in this revision.

packages/gateway-v2/adcs_handler.go — the env.Host validation path needs attention before this is production-ready.

Security Review

  • SSRF — packages/gateway-v2/adcs_handler.go lines 149, 187: env.Host from the request body is used without any allowlist or IP-range guard to initiate DCOM (port 135) and SMB (port 445) connections. A caller who can reach the infisical-adcs endpoint can direct the gateway to probe arbitrary internal hosts.

Important Files Changed

Filename Overview
packages/gateway-v2/adcs/adcs.go New MS-WCCE/DCOM client implementing CA discovery, template listing, and certificate enrollment. Template injection (\r\n check) is fixed; registry key handles (hklm.Key, sub.ResultKey) are not explicitly closed via BaseRegCloseKey.
packages/gateway-v2/adcs_handler.go New HTTP handler routing ADCS operations over the mTLS tunnel. ServeMux is now correctly initialised once via sync.OnceValue. Two issues remain: env.Host is used without allowlist validation (SSRF), and operation handler errors are returned verbatim to callers.
packages/gateway-v2/gateway.go Adds ForwardModeADCS and the "infisical-adcs" ALPN protocol; wires serveAdcsOverTLS into the channel dispatcher. Change is minimal and follows the existing PKCS#11 pattern exactly.
packages/cmd/root.go Adds a FormatMessage override to zerolog ConsoleWriter to suppress the bold formatting introduced in zerolog v1.35, keeping CLI output consistent with prior releases.
go.mod Adds go-msrpc, go-smb2.fork, pkcs7, and related transitive dependencies for Windows ADCS support; bumps zerolog, golang.org/x/* packages, and go-colorable to current minors.

Reviews (2): Last reviewed commit: "Address bot PR comments" | Re-trigger Greptile

Comment thread packages/gateway-v2/adcs/adcs.go
@veria-ai

veria-ai Bot commented Jul 4, 2026

Copy link
Copy Markdown

PR overview

All previously flagged issues have been addressed. No open security concerns remain on this pull request.

Security review

No open security issues remain on this pull request.

Fixed/addressed: 2 · PR risk: 0/10

@carlosmonastyrski

Copy link
Copy Markdown
Contributor Author

@greptile can you re-review and update your original summary please

Comment thread packages/gateway-v2/adcs_handler.go
Comment thread packages/gateway-v2/adcs_handler.go
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.

1 participant