Skip to content

Conversation

@CBenoit
Copy link
Member

@CBenoit CBenoit commented Nov 12, 2025

Allow "unsecure" TLS if the client provides a thumbprint and the peer certificate matches it.

Issue: DGW-318

Allow "unsecure" TLS if the client provides a thumbprint and the peer
certificate matches it.

Issue: DGW-318
@CBenoit CBenoit enabled auto-merge (squash) November 12, 2025 09:31
@CBenoit CBenoit disabled auto-merge November 12, 2025 09:31
@CBenoit CBenoit enabled auto-merge (squash) November 12, 2025 09:31
Copilot finished reviewing on behalf of CBenoit November 12, 2025 09:34
Comment on lines 7 to 26
async fn start_gateway() -> anyhow::Result<(DgwConfigHandle, Child)> {
let config_handle = DgwConfig::builder()
.disable_token_validation(true)
.build()
.init()
.context("init config")?;

// Start JMUX server that will accept JMUX connections.
let process = dgw_tokio_cmd()
.env("DGATEWAY_CONFIG_PATH", config_handle.config_dir())
.kill_on_drop(true)
.stdout(std::process::Stdio::piped())
.spawn()
.context("failed to start Devolutions Gateway")?;

// Give the server a moment to start.
tokio::time::sleep(std::time::Duration::from_millis(100)).await;

Ok((config_handle, process))
}
Copy link
Member Author

Choose a reason for hiding this comment

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

E2E tests actually spawing a Devolutions Gateway process, and connecting to it the same way a WebBrowser would.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements TLS thumbprint anchoring for Devolutions Gateway, allowing "insecure" TLS connections when a client provides a certificate thumbprint that matches the server's certificate. This enables connections to self-signed or otherwise untrusted certificates while maintaining a degree of security through thumbprint validation.

  • Adds cert_thumb256 field to token claims and API endpoints to support thumbprint-based certificate validation
  • Implements ThumbprintAnchoredVerifier that attempts standard TLS verification first, then falls back to thumbprint matching if verification fails
  • Refactors TLS connection logic into safe_connect (with optional thumbprint) and dangerous_connect (no verification)
  • Adds comprehensive test suite for thumbprint anchoring scenarios

Reviewed Changes

Copilot reviewed 18 out of 19 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
devolutions-gateway/src/tls.rs Implements thumbprint anchoring logic with new verifier and connection methods; adds certificate info extraction for logging
devolutions-gateway/src/token.rs Adds cert_thumb256 field to AssociationTokenClaims for thumbprint passing
devolutions-gateway/src/api/fwd.rs Updates forwarding endpoint to use safe_connect with thumbprint from token
devolutions-gateway/src/rdp_proxy.rs Renames connect to dangerous_connect for clarity
devolutions-gateway/src/rd_clean_path.rs Renames connect to dangerous_connect for clarity
devolutions-gateway/src/api/webapp.rs Sets cert_thumb256: None for session token generation
tools/tokengen/src/lib.rs Adds cert_thumb256 field to token generation library
tools/tokengen/src/main.rs Adds CLI argument for certificate thumbprint
tools/tokengen/src/server/server_impl.rs Passes cert_thumb256 from request to token generation
testsuite/tests/cli/dgw/tls_anchoring.rs Adds integration tests for thumbprint anchoring scenarios
testsuite/tests/cli/dgw/mod.rs Registers new test module
testsuite/tests/cli/mod.rs Includes dgw test module
testsuite/src/dgw_config.rs Adds test infrastructure for DGW configuration
testsuite/src/cli.rs Adds DGW command builders for tests
testsuite/src/lib.rs Exposes dgw_config module
testsuite/Cargo.toml Adds dependencies for WebSocket testing
devolutions-gateway/Cargo.toml Adds dependencies for SHA-256 hashing and native cert loading
crates/devolutions-log/src/lib.rs Adds unrelated FIXME comment about stderr
Cargo.lock Updates lock file with new dependencies

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

Copy link
Contributor

Copilot AI commented Nov 12, 2025

@CBenoit I've opened a new pull request, #1571, to work on those changes. Once the pull request is ready, I'll request review from you.

@CBenoit CBenoit requested a review from Copilot November 12, 2025 12:45
@CBenoit CBenoit enabled auto-merge (squash) November 12, 2025 12:45
Copilot finished reviewing on behalf of CBenoit November 12, 2025 12:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

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


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

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.

2 participants