refactor(dgw): CredSSP enclosure, supermarket store, thin synthetic KDC

[irvingouj@Devolutions (irvingoujAtDevolution)]

As previously mentioned, the current state of KDC via provisioning is complete from a functional standpoint. However, we can see that the words credential and provision are starting to blur into each other — the boundary isn't clear, and we've been using them interchangeably, which is just bad design.

I added comments to the critical lines in the PR diff; it should be clear how this makes sense from there.

[irvingouj@Devolutions (irvingoujAtDevolution)]

Create credential injection from provisioning.
The analogy, as well as the mindset I'd like to use here, is that the provisioning store is like a supermarket, where each handler — for example, GenericClient here — can purchase what he wants with a jti and start preparing the meal (credential injection here).

This grants better separation of concerns, as well as better extensibility.

[irvingouj@Devolutions (irvingoujAtDevolution)]

Now the Chef 1, the generic client has done the first round of preparation, we pass it to the second chef, which is the kdc proxy hereOnce Chef 1 (GenericClient) finishes the first round of prep, the dish gets handed off to the second chef — the KDC proxy — to continue the process.

[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.

[irvingouj@Devolutions (irvingoujAtDevolution)]

Notice it's a take here, so provisioning won't be reused, one of the benifits this refactoring brings

[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).