diff --git a/devolutions-gateway/src/listener.rs b/devolutions-gateway/src/listener.rs index 048d590aa..db5926be5 100644 --- a/devolutions-gateway/src/listener.rs +++ b/devolutions-gateway/src/listener.rs @@ -4,7 +4,6 @@ use anyhow::Context; use async_trait::async_trait; use devolutions_gateway_task::{ChildTask, ShutdownSignal, Task}; use futures::TryFutureExt as _; -use tap::Pipe as _; use tokio::io::{AsyncRead, AsyncWrite}; use tokio::net::{TcpListener, TcpSocket, TcpStream}; use tracing::Instrument as _; @@ -220,17 +219,39 @@ async fn run_https_listener(listener: TcpListener, state: DgwState) -> anyhow::R } } +/// Checks if an error represents a benign client disconnect. +/// +/// Walks the error chain and returns true if any cause is a `std::io::Error` +/// with kind `BrokenPipe`, `ConnectionReset`, or `UnexpectedEof`. +fn is_benign_disconnect(err: &anyhow::Error) -> bool { + use std::io::ErrorKind::{BrokenPipe, ConnectionReset, UnexpectedEof}; + + err.chain().any(|cause| { + if let Some(ioe) = cause.downcast_ref::() { + return matches!(ioe.kind(), BrokenPipe | ConnectionReset | UnexpectedEof); + } + false + }) +} + async fn handle_https_peer( stream: TcpStream, tls_acceptor: tokio_rustls::TlsAcceptor, state: DgwState, peer_addr: SocketAddr, ) -> anyhow::Result<()> { - let tls_stream = tls_acceptor - .accept(stream) - .await - .context("TLS handshake failed")? - .pipe(tokio_rustls::TlsStream::Server); + let tls_stream = match tls_acceptor.accept(stream).await { + Ok(stream) => tokio_rustls::TlsStream::Server(stream), + Err(e) => { + let e = anyhow::Error::from(e); + if is_benign_disconnect(&e) { + debug!(error = format!("{e:#}"), %peer_addr, "TLS handshake ended by peer"); + return Ok(()); + } else { + return Err(e.context("TLS handshake failed")); + } + } + }; handle_http_peer(tls_stream, state, peer_addr).await } @@ -259,13 +280,24 @@ where match result { Ok(()) => Ok(()), - Err(e) => match e.downcast_ref::() { - Some(e) if e.is_canceled() || e.is_incomplete_message() => { - debug!(error = %e, "Request was cancelled"); + Err(error) => { + // Check for hyper-specific benign cases first. + if let Some(hyper_err) = error.downcast_ref::() + && (hyper_err.is_canceled() || hyper_err.is_incomplete_message()) + { + debug!(error = format!("{:#}", anyhow::anyhow!(error)), %peer_addr, "Request was cancelled/incomplete"); + return Ok(()); + } + + // Then check for underlying io::Error kinds via anyhow chain. + let error = anyhow::Error::from_boxed(error); + if is_benign_disconnect(&error) { + debug!(error = format!("{error:#}"), %peer_addr, "Client disconnected"); Ok(()) + } else { + Err(error.context("HTTP server")) } - _ => Err(anyhow::anyhow!(e).context("HTTP server")), - }, + } } } diff --git a/testsuite/tests/cli/dgw/benign_disconnect.rs b/testsuite/tests/cli/dgw/benign_disconnect.rs new file mode 100644 index 000000000..a53802665 --- /dev/null +++ b/testsuite/tests/cli/dgw/benign_disconnect.rs @@ -0,0 +1,112 @@ +//! Tests for benign client disconnect handling. +//! +//! These tests verify that common client disconnects (e.g., health checks, aborted +//! connections) are logged at DEBUG level instead of ERROR. +//! +//! Note: HTTPS disconnect handling is implemented in handle_https_peer and uses the same +//! benign disconnect detection logic. However, testing HTTPS requires TLS configuration +//! which is not included in the default test setup. + +use anyhow::Context as _; +use rstest::rstest; +use testsuite::cli::{dgw_tokio_cmd, wait_for_tcp_port}; +use testsuite::dgw_config::{DgwConfig, DgwConfigHandle, VerbosityProfile}; +use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; +use tokio::process::Child; + +/// Starts a gateway instance and returns the process and a handle to collect stderr. +/// +/// The gateway is configured with DEBUG logging enabled to capture disconnect logs. +/// Stderr is collected in a background task and returned when the handle is awaited. +async fn start_gateway_with_logs( + config_handle: &DgwConfigHandle, +) -> anyhow::Result<(Child, tokio::task::JoinHandle>)> { + let mut process = dgw_tokio_cmd() + .env("DGATEWAY_CONFIG_PATH", config_handle.config_dir()) + .env("RUST_LOG", "devolutions_gateway=debug") + .kill_on_drop(true) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .context("failed to start Devolutions Gateway")?; + + let stderr = process.stderr.take().context("failed to take stderr")?; + let stderr_handle = tokio::spawn(async move { + let mut stderr_reader = BufReader::new(stderr); + let mut lines = Vec::new(); + let mut line_buf = String::new(); + loop { + match stderr_reader.read_line(&mut line_buf).await { + Ok(0) => break, + Ok(_) => { + lines.push(line_buf.clone()); + line_buf.clear(); + } + Err(_) => break, + } + } + lines + }); + + // Wait for HTTP port to be ready. + wait_for_tcp_port(config_handle.http_port()).await?; + + Ok((process, stderr_handle)) +} + +/// Test that benign HTTP disconnects log DEBUG, not ERROR. +/// +/// Tests various scenarios where clients disconnect without error: +/// - Connecting and immediately closing (e.g., health checks, port scanners) +/// - Sending partial request then closing (e.g., aborted browser requests) +#[rstest] +#[case::connect_and_close(None)] +#[case::abort_mid_request(Some("GET /jet/health HTTP/1.1\r\nHost: localhost\r\n".as_bytes()))] +#[tokio::test] +async fn benign_http_disconnect(#[case] payload: Option<&[u8]>) -> anyhow::Result<()> { + // 1) Start the gateway with DEBUG logging. + let config_handle = DgwConfig::builder() + .disable_token_validation(true) + .verbosity_profile(VerbosityProfile::DEBUG) + .build() + .init() + .context("init config")?; + + let (mut process, stderr_handle) = start_gateway_with_logs(&config_handle).await?; + + // 2) Connect to HTTP port. + let mut stream = tokio::net::TcpStream::connect(("127.0.0.1", config_handle.http_port())) + .await + .context("failed to connect to HTTP port")?; + + // 3) Send payload if provided. + if let Some(data) = payload { + stream.write_all(data).await.context("failed to send payload")?; + } + + // 4) Close the connection. + drop(stream); + + // Wait a bit for the log to be written. + tokio::time::sleep(tokio::time::Duration::from_millis(500)).await; + + // 5) Stop the gateway and collect logs. + let _ = process.start_kill(); + let stderr_lines = tokio::time::timeout(tokio::time::Duration::from_secs(5), stderr_handle) + .await + .context("timeout waiting for stderr")? + .context("wait for stderr collection")?; + let _ = process.wait().await; + + // 6) Verify no ERROR logs about "HTTP server" or "handle_http_peer failed". + let has_error = stderr_lines.iter().any(|line| { + line.contains("ERROR") && (line.contains("HTTP server") || line.contains("handle_http_peer failed")) + }); + + assert!( + !has_error, + "Expected no ERROR logs for benign HTTP disconnect, but found one" + ); + + Ok(()) +} diff --git a/testsuite/tests/cli/dgw/mod.rs b/testsuite/tests/cli/dgw/mod.rs index 7680745c8..5afb954f6 100644 --- a/testsuite/tests/cli/dgw/mod.rs +++ b/testsuite/tests/cli/dgw/mod.rs @@ -1,4 +1,5 @@ mod ai_gateway; +mod benign_disconnect; mod preflight; mod tls_anchoring; mod traffic_audit;