fix(dgw): separate credential injection runtime

[irvingouj@Devolutions (irvingoujAtDevolution)]

Separates pending provisioning data from the live credential-injection runtime so each handler owns the session state it consumes and the KDC proxy resolves only active synthetic KDC registrations.

Credential injection uses the association token's dynamic dst_hst for the client-facing TERMSRV SPN and target-side Kerberos hostname. Supported clients retain that logical destination when connecting through a Gateway listener; clients that derive the SPN from the Gateway transport hostname are outside the unstable Kerberos credential-injection contract.

Provisioned injection mappings are checked out atomically and consumed once. Reused JTIs fail explicitly instead of silently downgrading to ordinary forwarding, and callers must re-provision after a failed attempt.

Both generic RDP and RDCleanPath use the same CredSSP session orchestration and centralized Kerberos opt-in decision.

Issue: DVLS-14697

[irvingouj@Devolutions (irvingoujAtDevolution)]

Now everything the KDC Proxy need is in the synthetic_kdc_registry, there's no need for KDC handler to read provisioning store anymore.
More semantically clear

[Copilot]

Pull request overview

Refactors RDP credential injection by separating provisioning, synthetic KDC registration, and CredSSP orchestration.

Changes:

• Splits RDP proxy and CredSSP logic into dedicated modules.
• Replaces CredentialService with a one-shot provisioning store and live KDC registry.
• Updates all connection and API paths to use the new architecture.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
devolutions-gateway/src/service.rs	Initializes and wires the new stores.
devolutions-gateway/src/rdp_proxy/mod.rs	Adds RDP handshake and TLS orchestration.
devolutions-gateway/src/rdp_proxy/credssp.rs	Encapsulates CredSSP MITM behavior.
devolutions-gateway/src/rdp_proxy.rs	Removes the former monolithic proxy.
devolutions-gateway/src/rd_clean_path.rs	Integrates new credential-injection state.
devolutions-gateway/src/provisioning.rs	Makes provisioning retrieval one-shot.
devolutions-gateway/src/ngrok.rs	Updates state wiring.
devolutions-gateway/src/listener.rs	Updates TCP client construction.
devolutions-gateway/src/lib.rs	Replaces credential-service state fields.
devolutions-gateway/src/generic_client.rs	Builds session-scoped credential injection.
devolutions-gateway/src/credential_injection.rs	Adds credential and synthetic KDC runtime.
devolutions-gateway/src/credential_injection_kdc.rs	Removes the previous combined service.
devolutions-gateway/src/api/rdp.rs	Passes new state into RDP handling.
devolutions-gateway/src/api/preflight.rs	Writes directly to provisioning storage.
devolutions-gateway/src/api/kdc_proxy.rs	Resolves only live registered KDCs.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Benoît Cortier (CBenoit)]

Warning

Automated review. Human insight is required, especially to resolve the conflicting SPN assumptions described below.

Requesting changes on this stacked delta (chore/regenerate-openapi-provisioning -> refactor/rdp-proxy-credssp-extract).

1. Blocking - the existing SPN regression thread remains unresolved. credential_injection.rs:222,260,351,416 now derives the synthetic-KDC service hostname from the association token's dst_hst. The base branch deliberately used conf.hostname after merged fix #1856, and this PR deletes service_kdc_for_uses_gateway_hostname_for_spn, the regression test that pinned that behavior. The author has supplied contrary product-path evidence in the existing thread, so this needs a human protocol decision rather than another automated guess. Whichever hostname is correct must be explicit, tested with different Gateway and destination hostnames, and described as a behavior change if #1856 is intentionally being reversed.

2. The new one-shot store changes observable retry behavior without documenting it. ProvisioningStore::take (provisioning.rs:143-197) removes credentials and connection options before target connection, TLS, or CredSSP can succeed (generic_client.rs:158-197, rd_clean_path.rs:490-580). A transient failure consumes the data, and a retry inside the advertised TTL can silently fall through to non-injected forwarding. This conflicts with the current time_to_live wording in openapi.rs:396-400 ("Minimum persistence duration") and is not a behavior-preserving refactor:. Either retain entries across failed attempts, or document the single-use contract in OpenAPI/PR history and make skipped injection explicit instead of silently falling back.

3. The CredSSP enclosure is incomplete and leaves two orchestration implementations. credssp.rs:73-153 owns both CredSSP legs, Connect Confirm interception, leftover flushing, and forwarding, but rd_clean_path.rs:551-634 open-codes the same sequence. rdp_proxy/mod.rs:12-14 then re-exports three internals solely to sustain that duplicate path. Build CredsspSession/PreparedCredssp from RDCleanPath too and call run; that removes roughly 80 duplicated lines and makes the stated module boundary real.

4. generic_client destructively reads entries it does not use. At generic_client.rs:157-160, take runs before entry.mapping.is_some(), so an RDP connection consumes a token-only provision-token entry and discards it. Match the safer RDCleanPath shape: check has_mapping first, then take only for the injection path.

5. Protocol gating was duplicated while its regression coverage was deleted. The two-flag condition is repeated at generic_client.rs:162 and rd_clean_path.rs:495, while the prior flag-matrix test was removed. The empty test module at rdp_proxy/mod.rs:274-278 incorrectly claims protocol selection remains covered. Centralize the predicate, restore the opt-in matrix test, and remove the empty scaffolding.

6. The kitchen metaphor makes the new boundaries harder to learn. credential_injection.rs:1-5,101,108,205,524-531 and rd_clean_path.rs:668-669 require readers to translate "supermarket/groceries/chef/dish/pass window" back into ProvisioningStore, ProvisioningEntry, preparation, and live KDC registration. Use those technical terms directly; this module's purpose is separation of concerns, so it should not introduce a second vocabulary for them.

Non-blocking cleanup: CredentialInjectionKdcResolveError no longer has a discriminating consumer (credential_injection.rs:55-72), so its four variants and duplicated pre-return warnings should either regain a matching caller or collapse to contextual anyhow errors. Also capitalize the newly added log messages and terminate sentence comments per repository conventions.

Note

LLM-assisted content (no human feedback).

[irvingouj@Devolutions (irvingoujAtDevolution)]

Implementation notes:

• ProvisioningStore owns pending token, credential, and connection-option material.
• SyntheticKdcRegistry contains only KDCs published by active Kerberos injection sessions.
• Synthetic KDC ticket/SPN handling and target-side Kerberos both use association dst_hst; conf.hostname remains Gateway identity for TLS/configuration uses.
• RDCleanPath now calls CredsspSession::run instead of maintaining a second CredSSP orchestration.
• Generic RDP checks has_mapping before one-shot checkout, preserving token-only provisioning rows.
• Added explicit Kerberos/NTLM selection coverage and dynamic-destination tests.
• Local DVLS matrix: 22 Playwright cases passed, including live NTLM and Kerberos CredSSP through Gateway.
• CI packaging fix retries Chocolatey and verifies candle.exe/WIXSHARP_WIXDIR after transient 503 failures.

Note

Human-tuned, LLM-assisted content.

[irvingouj@Devolutions (irvingoujAtDevolution)]

Final review follow-up:

1. SPN decision: dst_hst is the explicit service-name contract for supported credential-injection clients. Synthetic KDC tickets, CredSSP acceptor properties, and the target Kerberos leg use the same dynamic destination. Gateway-addressed clients that derive TERMSRV/<conf.hostname> from the transport endpoint are explicitly outside this unstable feature contract.
2. One-shot behavior: checkout is atomic; a consumed-JTI tombstone remains until the original expiry, so reuse and concurrent losers fail closed instead of falling through to non-injected forwarding. Re-provisioning clears the tombstone.
3. CredSSP enclosure: RDCleanPath constructs CredsspSession / PreparedCredssp and calls the single run implementation.
4. Token-only provisioning: both entry points inspect MappingStatus before checkout; token-only rows are preserved.
5. Protocol gating: both flags are centralized and covered by the full 2x2 matrix, with target username selection tested separately.
6. Terminology: kitchen/supermarket metaphors are removed from source and docs.

Additional cleanup: removed the unused typed KDC resolve error, centralized checkout validation, kept generation-guarded KDC leases for explicit same-JTI re-provision overlap, and regenerated OpenAPI artifacts.

Validation: full workspace Clippy; workspace lib/bin/test targets plus colliding examples run separately; generated .NET client build; 22-case DVLS matrix, including live NTLM and Kerberos CredSSP over RDCleanPath.

Note

Human-tuned, LLM-assisted content.

[irvingouj@Devolutions (irvingoujAtDevolution)]

Benoît Cortier (@CBenoit), ready for review

Note

Human-tuned, LLM-assisted content.