fix(dgw): use provisioned KDC for credential injection#1862
Open
irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 3 commits into
Open
fix(dgw): use provisioned KDC for credential injection#1862irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 3 commits into
irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 3 commits into
Conversation
Route target-side Kerberos authentication through the KDC provisioned with the credential mapping, and add an explicit unstable opt-in for the client-facing Kerberos acceptor. Issue: DVLS-14697
Add the optional krb_kdc field to the provision-credentials OpenAPI contract and synchronize the generated .NET and TypeScript client models. Issue: DVLS-14697
Let maintainers know that an action is required on their side
|
Copilot started reviewing on behalf of
irvingouj@Devolutions (irvingoujAtDevolution)
July 21, 2026 18:56
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Uses provisioned KDC addresses for target-side Kerberos credential injection while retaining NTLM fallback.
Changes:
- Adds
krb_kdccredential provisioning and client API support. - Replaces legacy Kerberos debug configuration with an unstable opt-in.
- Routes provisioned KDC settings through both RDP paths.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
devolutions-gateway/src/rdp_proxy.rs |
Builds target-side Kerberos configuration. |
devolutions-gateway/src/rd_clean_path.rs |
Uses provisioned KDC in clean-path sessions. |
devolutions-gateway/src/openapi.rs |
Documents the preflight KDC field. |
devolutions-gateway/src/credential/mod.rs |
Validates and stores provisioned KDC addresses. |
devolutions-gateway/src/credential_injection_kdc.rs |
Exposes the stored KDC address. |
devolutions-gateway/src/config.rs |
Adds the unstable credential-injection opt-in. |
devolutions-gateway/openapi/ts-angular-client/model/preflightOperation.ts |
Exposes krb_kdc to TypeScript clients. |
devolutions-gateway/openapi/gateway-api.yaml |
Updates the API schema and version. |
devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Model/PreflightOperation.cs |
Exposes KrbKdc to .NET clients. |
devolutions-gateway/openapi/dotnet-client/docs/PreflightOperation.md |
Documents the .NET property. |
config_schema.json |
Documents the new debug setting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+360
to
+363
| pub(crate) fn credential_injection_kerberos_client_config( | ||
| krb_kdc: Option<&TargetAddr>, | ||
| gateway_hostname: &str, | ||
| ) -> anyhow::Result<Option<ironrdp_connector::credssp::KerberosConfig>> { |
| /// <param name="timeToLive">Minimum persistance duration in seconds for the data provisioned via this operation. Optional parameter for \"provision-token\" and \"provision-credentials\" kinds..</param> | ||
| /// <param name="token">The token to be stored on the proxy-side. Required for \"provision-token\" and \"provision-credentials\" kinds..</param> | ||
| public PreflightOperation(string hostToResolve = default(string), Guid id = default(Guid), PreflightOperationKind kind = default(PreflightOperationKind), AppCredential proxyCredential = default(AppCredential), AppCredential targetCredential = default(AppCredential), int? timeToLive = default(int?), string token = default(string)) | ||
| public PreflightOperation(string hostToResolve = default(string), Guid id = default(Guid), PreflightOperationKind kind = default(PreflightOperationKind), string krbKdc = default(string), AppCredential proxyCredential = default(AppCredential), AppCredential targetCredential = default(AppCredential), int? timeToLive = default(int?), string token = default(string)) |
irvingouj@Devolutions (irvingoujAtDevolution)
requested review from
Benoît Cortier (CBenoit) and
Copilot
July 21, 2026 18:59
Copilot started reviewing on behalf of
irvingouj@Devolutions (irvingoujAtDevolution)
July 21, 2026 19:00
View session
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
devolutions-gateway/src/rdp_proxy.rs:139
- This enables target-side Kerberos whenever
krb_kdcis provisioned, even whenenable_unstableorkerberos_credential_injectionis false, whilecredential_injection_kerberos_server_configreturns no Kerberos acceptor in that case. The acceptor is documented to require the same auth package as the target leg, so this configuration cannot complete and the new opt-in does not actually gate the target-side behavior. Gate both legs with the same condition (or reject a Kerberos-provisioned session explicitly when the feature is disabled).
let krb_client_config =
credential_injection_kerberos_client_config(credential_injection_kdc.krb_kdc(), &gateway_hostname)?;
devolutions-gateway/src/rdp_proxy.rs:381
- When the opt-in is enabled but
krb_kdcis omitted (the documented NTLM case), this still callsclient_acceptor_protocol, which selects Kerberos for any domain-qualified target username. Meanwhile the target-side helper returnsNoneand uses NTLM, violating the required mirrored auth package and breaking existingDOMAIN\\user/UPN NTLM injections. Skip the Kerberos acceptor whenever no KDC was provisioned.
if !conf.debug.enable_unstable || !conf.debug.kerberos_credential_injection {
Comment on lines
+439
to
+442
| let krb_client_config = crate::rdp_proxy::credential_injection_kerberos_client_config( | ||
| credential_injection_kdc.krb_kdc(), | ||
| &gateway_hostname, | ||
| )?; |
Comment on lines
+114
to
+117
| /// Real KDC for Kerberos-enforced injection, provisioned in the same call as the credentials. | ||
| /// Optional: absent for NTLM targets. | ||
| #[serde(default, deserialize_with = "deserialize_optional_kdc_addr")] | ||
| pub krb_kdc: Option<crate::target_addr::TargetAddr>, |
…legs Review follow-up (Copilot + end-to-end review). The two CredSSP legs previously chose Kerberos vs NTLM from independent inputs and could disagree (e.g. a provisioned krb_kdc with the opt-in off gave a Kerberos target leg but an NTLM acceptor), which fails the handshake reading one package as the other. - Collapse the two per-leg helpers into a single credential_injection_kerberos_configs that makes one decision (enable_unstable && kerberos_credential_injection && domain-qualified target) and builds both legs from it; require the provisioned KDC when Kerberos is chosen, else NTLM on both legs. - A stray krb_kdc (feature off or domainless target) is now ignored with a warning instead of silently diverging or hard-failing an NTLM-capable session. - Keep the generated PreflightOperation ctor source-compatible by appending the new optional krbKdc parameter last. - Add unit coverage for the protocol decision and the krb_kdc round-trip.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uses the KDC provisioned with injected credentials for target-side Kerberos, while keeping NTLM as the fallback when no KDC is supplied. Also replaces the old debug Kerberos config with an explicit unstable credential-injection opt-in and exposes krb_kdc in the preflight clients.
Tests: cargo +nightly fmt --all -- --check; cargo check -p devolutions-gateway --all-targets; cargo clippy -p devolutions-gateway --all-targets -- -D warnings; cargo test -p devolutions-gateway credential_injection --lib
Issue: DVLS-14697