Skip to content

test(dgw): cover credential injection reconnect hops - #1938

Open
irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 40 commits into
masterfrom
test/cred-injection-reconnect-e2e
Open

test(dgw): cover credential injection reconnect hops#1938
irvingouj@Devolutions (irvingoujAtDevolution) wants to merge 40 commits into
masterfrom
test/cred-injection-reconnect-e2e

Conversation

@irvingoujAtDevolution

@irvingoujAtDevolution irvingouj@Devolutions (irvingoujAtDevolution) commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Add process-level Gateway tests for DVLS-like preflight, TCP and RDCleanPath injection, NTLM and Kerberos CredSSP, fail-closed missing mappings, and missing krb_kdc without dialing the target.

RDCleanPath cases drive the public ironrdp-agent 0.1.0 CLI over ws://127.0.0.1/jet/rdp.

Issue: DVLS-14697

@irvingoujAtDevolution

Copy link
Copy Markdown
Contributor Author

Implementation notes:

  • Tests live in testsuite/tests/cli/dgw/cred_injection.rs and start a real Gateway with disable_token_validation.
  • DVLS-like /jet/preflight posts provision-credentials and, for Kerberos, provision-connection-options.krb_kdc.
  • The client sends a PCB v2 blob (JWT in v2_payload) plus an X.224 Connection Request with Cookie: mstshash=client-cookie-user.
  • The destination is a loopback TCP peer that records the rewritten mstshash cookie. CredSSP is not completed; sspi-rs / IronRDP are not run as a fake KDC or RDP server.
  • First inject, jet_reuse reconnect (get not take), required-but-expired fail-closed, and unprovisioned ordinary forward are asserted from Gateway logs and the cookie.
  • Kerberos overlapping reconnects must publish the synthetic KDC once for the same provisioning generation; a later provision publishes again.

Note

LLM-assisted content (no human feedback).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds process-level coverage for Gateway RDP credential injection.

Changes:

  • Tests NTLM injection, reconnect attempts, expiration, and fail-closed routing.
  • Adds mock KDC and CredSSP integration coverage.
  • Extends test configuration and dependencies.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
Cargo.lock Locks new test dependencies.
testsuite/Cargo.toml Adds Kerberos and IronRDP test dependencies.
testsuite/src/dgw_config.rs Exposes Kerberos injection test configuration.
testsuite/tests/cli/dgw/mod.rs Registers new test modules.
testsuite/tests/cli/dgw/cred_injection.rs Adds credential injection lifecycle tests.
testsuite/tests/cli/dgw/cred_injection_kdc.rs Adds mock KDC/CredSSP integration testing.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread testsuite/tests/cli/dgw/cred_injection.rs Outdated
Comment thread testsuite/tests/cli/dgw/cred_injection.rs Outdated
Comment thread testsuite/tests/cli/dgw/cred_injection_kdc.rs Outdated
Comment thread testsuite/tests/cli/dgw/cred_injection_kdc.rs Outdated
Comment thread testsuite/Cargo.toml Outdated
Split rdp_proxy CredSSP into its own module. Delete CredentialService: DgwState holds ProvisioningStore + SyntheticKdcRegistry. from_provisioned builds PreparedCredentialInjection; register_if_kerberos publishes. take() consumes groceries once. Synthetic KDC keeps only fake-KDC runtime; credentials and target_kdc stay on the dish.
Authorize CleanPath tokens before one-shot take, use a registry-wide generation counter, and replace bare clippy allow with expect. SPN remains association-token dst_hst for client-facing CredSSP.
Use association dst_hst for synthetic KDC SPN and target-leg Kerberos
hostname instead of conf.hostname. Route RDCleanPath through
CredsspSession, peek before one-shot take, and document checkout TTL.

Issue: DGW review #1900

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep the preinstalled WiX toolset until Chocolatey successfully installs the pinned version. Retry transient feed failures, validate candle.exe, and expose WIXSHARP_WIXDIR so installer builds cannot continue with an empty WiX path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep PR #1900 scoped to the Gateway provisioning and CredSSP refactor.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep consumed credential mappings visible until their original expiry so a reused JTI fails explicitly instead of silently falling back to ordinary forwarding.

Centralize atomic checkout and CredSSP orchestration, preserve token-only provisioning, and simplify KDC error handling. Record the one-shot contract in PR history without regenerating unchanged OpenAPI artifacts.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Describe time_to_live as the first-checkout window for credential-injection mappings and state that failed attempts require re-provisioning. Regenerate the published specification and clients.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
State that supported credential-injection clients retain association dst_hst as their logical TERMSRV service name even when the transport endpoint is a Gateway listener.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore the pre-existing provisioning TTL wording and generated artifacts. The OpenAPI documentation update was outside the requested PR scope.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Token validation already accepts the same association JWT inside jet_reuse, but checkout consumed the mapping on first use. Native RDM reconnects reuse that JWT without DVLS, so injection failed or silently forwarded.

Keep encrypted mappings until the token acceptance deadline, authorize before choosing injection, fail closed when required material is gone, and reuse one synthetic KDC per provisioning generation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Mapped insert now caps retention to the association token acceptance deadline, so unsigned preflight fixtures without exp fail as invalid-parameters.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Missing Kerberos krb_kdc must fail closed without dialing the target.

Issue: DGW-1900

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@irvingoujAtDevolution irvingouj@Devolutions (irvingoujAtDevolution) changed the title test(dgw): cover credential injection reconnect test(dgw): cover credential injection reconnect hops Aug 21, 2026
@irvingoujAtDevolution
irvingouj@Devolutions (irvingoujAtDevolution) marked this pull request as ready for review August 25, 2026 16:31
@irvingoujAtDevolution
irvingouj@Devolutions (irvingoujAtDevolution) changed the base branch from refactor/rdp-proxy-credssp-extract to feat/enable-kerberos-injection August 25, 2026 16:41
Routing tests now decode X.224 Cookie instead of raw-byte search.
Client-leg Kerberos asserts synthetic-KDC AS-REQ cname. Fail-closed
Kerberos paths require inject-started then no Finished identity.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Decode X.224 cookies, KdcProxy AS/TGS principals, and attribute
KDC-down TCP to Gateway via a separate refusing listener.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Checkout-before-connect on the stack below lets the fail-closed path
prove accepted==0.

Issue: DGW-1900

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pin the public CLI release and complete NTLM and Kerberos target
CredSSP over ws://127.0.0.1/jet/rdp.

Issue: DGW-1900

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Read framed X.224 cookies, bound CredSSP handshakes, and share
the localhost TLS fixture.

Issue: DVLS-14697

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep credential-injection tests focused on public behavior instead of
serializing the removed Kerberos debug option.

Issue: DVLS-14697

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Treat an expired staging mapping as absent because association tokens do
not carry an injection requirement. Verify ordinary forwarding preserves
the client X.224 cookie.

Issue: DVLS-14697

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use the direct rustls dependency added on master so the credential-injection
E2E compiles under the workspace unused-qualifications lint.

Issue: DVLS-14697

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
irvingouj@Devolutions (irvingoujAtDevolution) added a commit that referenced this pull request Aug 26, 2026
Keep provisioned injection mappings alive for as long as Gateway still
accepts the association token, so native RDP reconnects reuse the same
JWT and get injected again. If injection was provisioned but the
material is gone, checkout fails closed before we dial the target.

Generic RDP and RDCleanPath now share one CredSSP session path. Kerberos
uses the token `dst_hst` for the client-facing TERMSRV SPN.

I wrote the lifecycle contract down in
[src/credential/INTENT.md](https://github.com/Devolutions/devolutions-gateway/blob/85545af64a3f911d0a7e3d3d68ecb8a105a1121f/devolutions-gateway/src/credential/INTENT.md)
(following
[intent-files.md](https://github.com/Devolutions/devolutions-gateway/blob/master/docs/intent-files.md))
— that's what this PR implements.

### Why this design

Credentials live in two phases. Before the first connect, the preflight
`time_to_live` bounds how long staged secrets wait in memory. After the
first checkout, retention switches to the token acceptance deadline
(`exp` + leeway). If Gateway accepts the token for a reconnect, the
mapping has to still be there — the old one-shot `take_mapping` broke
exactly that. `jet_reuse` starts at disconnect and the TTL starts at
preflight, so neither can cap the other; the token deadline is the one
clock every connection attempt shares.

### Where to look hardest

- [Generation-guarded KDC
publication](https://github.com/Devolutions/devolutions-gateway/blob/85545af64a3f911d0a7e3d3d68ecb8a105a1121f/devolutions-gateway/src/credential_injection.rs#L667-L698):
re-provisioning the same JTI bumps a generation, and an older in-flight
lease can never overwrite a newer published KDC, so the newest
provisioning wins deterministically.
- [Deadline-driven
cleanup](https://github.com/Devolutions/devolutions-gateway/blob/85545af64a3f911d0a7e3d3d68ecb8a105a1121f/devolutions-gateway/src/credential_injection.rs#L774-L835):
expired session KDC material is removed by a background task, not lazily
on the next checkout, and secrets are zeroized on drop.
- [Release after
CredSSP](85545af):
the per-connection mapping and KDC lease drop as soon as both CredSSP
legs finish, so plain forwarding never holds secret material.

Tests cover staging expiry, reconnect reuse, generation replacement, and
physical removal without a later checkout. Manual run: 22-case DVLS
matrix with live NTLM and Kerberos CredSSP through Gateway. E2E hops are
stacked on #1938.

Issue: DVLS-14697

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@CBenoit Benoît Cortier (CBenoit) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, I really appreciate the e2e tests here.

Architecturally-wise, I think the "helpers" are getting big enough that it may warrant some of the code to be moved somewhere under the lib.rs of the testsuite crate. One example could be FakeRdpTarget. If the "TLS fixtures" are meant to be shared, then I think the library side could be the best place.

It’s already pretty close, but make things as self-contained as possible so it’s possible to use the INTENT.md / .intent.md convention for providing some human documentation about the most meaningful abstractions and important properties under test. I would appreciate such files before we merge this PR.

Comment thread testsuite/Cargo.toml Outdated
Comment thread testsuite/tests/cli/dgw/cred_injection_kdc.rs Outdated
Base automatically changed from feat/enable-kerberos-injection to master August 26, 2026 17:48
Move GatewayProc, provisioning, fake RDP target, mock KDC/RDP, CredSSP
client leg, agent harness, and TLS fixtures from the test files into
testsuite/src so the abstractions are self-contained, per review.

Add empty INTENT.md placeholders to be filled by hand.

Issue: DVLS-14697
…-reconnect-e2e

# Conflicts:
#	config_schema.json
#	devolutions-gateway/src/config.rs
#	devolutions-gateway/src/credential_injection.rs
Record the intended NTLM, Kerberos, reconnect, and failure scenarios.

Name test tokens and identifiers by their protocol roles.
The behavioral contract already lives in
devolutions-gateway/src/credential/INTENT.md; the suite-level
INTENT.md covers the test scenarios, so the per-file placeholders
are unnecessary.

Issue: DVLS-14697
The RDCleanPath injection tests are the only coverage of that path;
skipping them when the binary is missing silently drops it. Missing
binary is now a failure, with IRONRDP_AGENT_SKIP=1 as explicit
opt-out. CI installs a pinned ironrdp-agent (cached per version).

Issue: DVLS-14697
Any Gateway test can drive a real RDP client, not just credential
injection, so the harness lives at testsuite::ironrdp_agent and
connect_rdcleanpath takes the credentials as parameters.

Issue: DVLS-14697
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants