From 0a8a62205a2fc8d52908f117af45314d5c3288d0 Mon Sep 17 00:00:00 2001 From: Eric Rodrigues Pires Date: Sat, 4 Jul 2026 08:15:13 -0300 Subject: [PATCH] Properly handle incomplete TLS handshakes --- CHANGELOG.md | 4 + src/entrypoint.rs | 54 ++++++-- src/tls.rs | 343 ++++++++++++++++++++++++++++------------------ 3 files changed, 257 insertions(+), 144 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 617f4f7..52a9cbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Fixed + +- Properly handle incomplete TLS handshakes. + ### Changed - Update dependencies. diff --git a/src/entrypoint.rs b/src/entrypoint.rs index b793c06..2b297b3 100644 --- a/src/entrypoint.rs +++ b/src/entrypoint.rs @@ -28,8 +28,7 @@ use rustls_pki_types::ServerName; use sandhole_socket::tcp_listener::{get_tcp_listener, get_tcp_listener_with_keepalive}; use socket2::TcpKeepalive; use sysinfo::{CpuRefreshKind, MemoryRefreshKind, Networks, RefreshKind, System}; -#[cfg_attr(not(feature = "acme"), allow(unused_imports))] -use tokio::io::AsyncWriteExt; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::{ fs, io::copy_bidirectional_with_sizes, @@ -70,7 +69,7 @@ use crate::{ TELEMETRY_COUNTER_SNI_CONNECTIONS, TELEMETRY_GAUGE_CPU_USAGE, TELEMETRY_GAUGE_TOTAL_MEMORY, TELEMETRY_GAUGE_USED_MEMORY, TELEMETRY_KEY_HOSTNAME, Telemetry, }, - tls::{TlsPeekData, peek_sni_and_alpn}, + tls::{ClientHelloStatus, RewindStream, TlsPeekData, parse_client_hello}, udp::UdpHandler, }; #[cfg_attr(not(feature = "prometheus"), allow(unused_imports))] @@ -1084,6 +1083,9 @@ struct HandleHttpsConnectionConfig { http11_server_config: Arc, } +// Maximum number of bytes to buffer while waiting for a complete ClientHello +const MAX_CLIENT_HELLO_SIZE: usize = 1 << 15; + async fn handle_https_connection( HandleHttpsConnectionConfig { mut stream, @@ -1097,11 +1099,16 @@ async fn handle_https_connection( http11_server_config, }: HandleHttpsConnectionConfig, ) { - let mut buf = [0u8; 4096]; - let Ok(Ok(n)) = timeout(sandhole.idle_connection_timeout, stream.peek(&mut buf)).await else { + let mut ssh_peek_buf = [0u8; 8]; + let Ok(Ok(n)) = timeout( + sandhole.idle_connection_timeout, + stream.peek(&mut ssh_peek_buf), + ) + .await + else { return; }; - if connect_ssh_on_https_port && buf[..n].starts_with(b"SSH-2.0-") { + if connect_ssh_on_https_port && ssh_peek_buf[..n].starts_with(b"SSH-2.0-") { // Handle as an SSH connection instead of HTTPS. handle_ssh_connection(HandleSshConnectionConfig { stream, @@ -1111,9 +1118,39 @@ async fn handle_https_connection( }); return; } - let Some(TlsPeekData { sni, alpn }) = peek_sni_and_alpn(&buf[..n]).await else { - return; + + let mut consumed: Vec = Vec::with_capacity(4096); + let mut read_buf = [0u8; 4096]; + let TlsPeekData { sni, alpn } = loop { + let n = match timeout(sandhole.idle_connection_timeout, stream.read(&mut read_buf)).await { + Ok(Ok(0)) | Ok(Err(_)) | Err(_) => return, + Ok(Ok(n)) => n, + }; + consumed.extend_from_slice(&read_buf[..n]); + match parse_client_hello(&consumed) { + ClientHelloStatus::Ready(peek_data) => break peek_data, + ClientHelloStatus::Incomplete if consumed.len() <= MAX_CLIENT_HELLO_SIZE => (), + ClientHelloStatus::Incomplete => { + #[cfg(not(coverage_nightly))] + tracing::debug!(%address, "Rejecting HTTPS connection: ClientHello too large."); + return; + } + ClientHelloStatus::Invalid(alert) => { + if !alert.is_empty() { + let _ = stream.write_all(&alert).await; + } + let _ = stream.shutdown().await; + #[cfg(not(coverage_nightly))] + tracing::debug!( + %address, + "Rejecting HTTPS connection: invalid ClientHello or missing SNI." + ); + return; + } + } }; + let mut stream = RewindStream::new(consumed, stream); + #[cfg(feature = "acme")] if alpn == [ACME_TLS_ALPN_NAME] { if let Some(challenge_config) = certificates.challenge_rustls_config() { @@ -1134,6 +1171,7 @@ async fn handle_https_connection( } #[cfg(not(feature = "acme"))] let _ = certificates; + let ip = address.ip().to_canonical(); if let Some(tunnel_handler) = sandhole.sni.get(&sni, ip) { let Ok(mut channel) = tunnel_handler.tunneling_channel(ip, address.port()).await else { diff --git a/src/tls.rs b/src/tls.rs index b3804e9..224c301 100644 --- a/src/tls.rs +++ b/src/tls.rs @@ -1,44 +1,145 @@ -use tokio::io::{empty, join}; -use tokio_rustls::LazyConfigAcceptor; +use std::{ + io::Cursor, + pin::Pin, + task::{Context, Poll}, +}; + +use rustls::server::Acceptor; +use tokio::io::{AsyncRead, AsyncWrite, ReadBuf}; pub(crate) struct TlsPeekData { pub(crate) sni: String, pub(crate) alpn: Vec>, } -// Get the SNI and ALPN from a peeked ClientHello if it's valid. -pub(crate) async fn peek_sni_and_alpn(buf: &[u8]) -> Option { - let handshake = LazyConfigAcceptor::new(Default::default(), join(buf, empty())) - .await - .ok()?; - let client_hello = handshake.client_hello(); - client_hello.server_name().map(|sni| TlsPeekData { - sni: sni.to_owned(), - alpn: client_hello - .alpn() - .map(|alpn_iter| alpn_iter.into_iter().map(|alpn| alpn.to_vec()).collect()) - .unwrap_or_default(), - }) +pub(crate) enum ClientHelloStatus { + Ready(TlsPeekData), + Incomplete, + Invalid(Vec), +} + +// Classify the bytes received since the start of the connection, +// extracting the SNI and ALPN from the ClientHello once it is complete +pub(crate) fn parse_client_hello(buf: &[u8]) -> ClientHelloStatus { + let mut acceptor = Acceptor::default(); + let mut cursor = Cursor::new(buf); + loop { + match acceptor.read_tls(&mut cursor) { + Ok(0) => break ClientHelloStatus::Incomplete, + Ok(_) => match acceptor.accept() { + Ok(Some(accepted)) => { + let client_hello = accepted.client_hello(); + break match client_hello.server_name() { + Some(sni) => ClientHelloStatus::Ready(TlsPeekData { + sni: sni.to_owned(), + alpn: client_hello + .alpn() + .map(|alpn_iter| { + alpn_iter.into_iter().map(|alpn| alpn.to_vec()).collect() + }) + .unwrap_or_default(), + }), + None => ClientHelloStatus::Invalid(Vec::new()), + }; + } + Ok(None) => continue, + Err((_error, mut alert)) => { + let mut alert_bytes = Vec::new(); + while let Ok(n) = alert.write(&mut alert_bytes) { + if n == 0 { + break; + } + } + break ClientHelloStatus::Invalid(alert_bytes); + } + }, + Err(_) => break ClientHelloStatus::Invalid(Vec::new()), + } + } +} + +// Wraps a stream. On read, replays any bytes that were consumed, +// then yields data from the underlying stream +pub(crate) struct RewindStream { + prefix: Vec, + offset: usize, + inner: S, +} + +impl RewindStream { + pub(crate) fn new(prefix: Vec, inner: S) -> Self { + RewindStream { + prefix, + offset: 0, + inner, + } + } +} + +impl AsyncRead for RewindStream { + fn poll_read( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &mut ReadBuf<'_>, + ) -> Poll> { + let this = self.get_mut(); + if this.offset < this.prefix.len() { + let len = (this.prefix.len() - this.offset).min(buf.remaining()); + buf.put_slice(&this.prefix[this.offset..this.offset + len]); + this.offset += len; + // Free the replay buffer once drained + if this.offset == this.prefix.len() { + this.prefix = Vec::new(); + this.offset = 0; + } + return Poll::Ready(Ok(())); + } + Pin::new(&mut this.inner).poll_read(cx, buf) + } +} + +impl AsyncWrite for RewindStream { + fn poll_write( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + buf: &[u8], + ) -> Poll> { + Pin::new(&mut self.get_mut().inner).poll_write(cx, buf) + } + + fn poll_write_vectored( + self: Pin<&mut Self>, + cx: &mut Context<'_>, + bufs: &[std::io::IoSlice<'_>], + ) -> Poll> { + Pin::new(&mut self.get_mut().inner).poll_write_vectored(cx, bufs) + } + + fn is_write_vectored(&self) -> bool { + self.inner.is_write_vectored() + } + + fn poll_flush(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.get_mut().inner).poll_flush(cx) + } + + fn poll_shutdown(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll> { + Pin::new(&mut self.get_mut().inner).poll_shutdown(cx) + } } #[cfg(test)] #[cfg_attr(coverage_nightly, coverage(off))] -mod peek_sni_and_alpn_tests { +mod parse_client_hello_tests { use std::{path::PathBuf, sync::Arc}; use rustls_pki_types::pem::PemObject; use tokio::io::{AsyncReadExt, duplex}; use tokio_rustls::TlsConnector; - use super::peek_sni_and_alpn; - - #[test_log::test(tokio::test)] - async fn fails_on_empty_buffer() { - assert!(peek_sni_and_alpn(b"").await.is_none()); - } + use super::{ClientHelloStatus, parse_client_hello}; - #[test_log::test(tokio::test)] - async fn fails_on_plain_message() { + fn root_store() -> rustls::RootCertStore { let mut root_store = rustls::RootCertStore::empty(); root_store.add_parsable_certificates( rustls_pki_types::CertificateDer::pem_file_iter( @@ -48,142 +149,112 @@ mod peek_sni_and_alpn_tests { .and_then(|iter| iter.collect::, _>>()) .expect("Failed to parse client certificates"), ); - let mut client_config = rustls::ClientConfig::builder_with_provider(Arc::new( + root_store + } + + fn client_config() -> rustls::ClientConfig { + rustls::ClientConfig::builder_with_provider(Arc::new( rustls::crypto::aws_lc_rs::default_provider(), )) .with_safe_default_protocol_versions() .unwrap() - .with_root_certificates(root_store) - .with_no_client_auth(); - client_config.enable_sni = false; - let connector = TlsConnector::from(Arc::new(client_config)); - let (mut server, client) = duplex(4096); - let jh = tokio::spawn(async move { - connector - .connect("plain.msg".try_into().unwrap(), client) - .await - }); - let mut buf = [0u8; 4096]; + .with_root_certificates(root_store()) + .with_no_client_auth() + } + + async fn client_hello_bytes(config: rustls::ClientConfig, domain: &'static str) -> Vec { + let connector = TlsConnector::from(Arc::new(config)); + let (mut server, client) = duplex(8192); + let jh = + tokio::spawn( + async move { connector.connect(domain.try_into().unwrap(), client).await }, + ); + let mut buf = [0u8; 8192]; let size = server .read(&mut buf) .await .expect("Failed to read from duplex stream"); jh.abort(); - let peek_data = peek_sni_and_alpn(&buf[..size]).await; - assert!(peek_data.is_none()); + buf[..size].to_vec() + } + + #[test_log::test(tokio::test)] + async fn empty_buffer_is_incomplete() { + assert!(matches!( + parse_client_hello(b""), + ClientHelloStatus::Incomplete + )); + } + + #[test_log::test(tokio::test)] + async fn truncated_client_hello_is_incomplete() { + let buf = client_hello_bytes(client_config(), "sandhole.com.br").await; + for len in [1, 5, buf.len() / 2, buf.len() - 1] { + assert!(matches!( + parse_client_hello(&buf[..len]), + ClientHelloStatus::Incomplete + )); + } } #[test_log::test(tokio::test)] async fn fails_on_missing_sni() { - let mut root_store = rustls::RootCertStore::empty(); - root_store.add_parsable_certificates( - rustls_pki_types::CertificateDer::pem_file_iter( - PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap()) - .join("tests/data/ca/rootCA.pem"), - ) - .and_then(|iter| iter.collect::, _>>()) - .expect("Failed to parse client certificates"), - ); - let mut client_config = rustls::ClientConfig::builder_with_provider(Arc::new( - rustls::crypto::aws_lc_rs::default_provider(), - )) - .with_safe_default_protocol_versions() - .unwrap() - .with_root_certificates(root_store) - .with_no_client_auth(); - client_config.enable_sni = false; - client_config.alpn_protocols.push(b"useless-alpn".to_vec()); - let connector = TlsConnector::from(Arc::new(client_config)); - let (mut server, client) = duplex(4096); - let jh = tokio::spawn(async move { - connector - .connect("sni.was.disabled".try_into().unwrap(), client) - .await - }); - let mut buf = [0u8; 4096]; - let size = server - .read(&mut buf) - .await - .expect("Failed to read from duplex stream"); - jh.abort(); - let peek_data = peek_sni_and_alpn(&buf[..size]).await; - assert!(peek_data.is_none()); + let mut config = client_config(); + config.enable_sni = false; + config.alpn_protocols.push(b"useless-alpn".to_vec()); + let buf = client_hello_bytes(config, "sni.was.disabled").await; + assert!(matches!( + parse_client_hello(&buf), + ClientHelloStatus::Invalid(_) + )); + } + + #[test_log::test(tokio::test)] + async fn fails_on_plain_message() { + assert!(matches!( + parse_client_hello(b"GET / HTTP/1.1\r\nHost: not.tls\r\n\r\n"), + ClientHelloStatus::Invalid(_) + )); } #[test_log::test(tokio::test)] async fn returns_sni_data() { - let mut root_store = rustls::RootCertStore::empty(); - root_store.add_parsable_certificates( - rustls_pki_types::CertificateDer::pem_file_iter( - PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap()) - .join("tests/data/ca/rootCA.pem"), - ) - .and_then(|iter| iter.collect::, _>>()) - .expect("Failed to parse client certificates"), - ); - let client_config = rustls::ClientConfig::builder_with_provider(Arc::new( - rustls::crypto::aws_lc_rs::default_provider(), - )) - .with_safe_default_protocol_versions() - .unwrap() - .with_root_certificates(root_store) - .with_no_client_auth(); - let connector = TlsConnector::from(Arc::new(client_config)); - let (mut server, client) = duplex(4096); - let jh = tokio::spawn(async move { - connector - .connect("sandhole.com.br".try_into().unwrap(), client) - .await - }); - let mut buf = [0u8; 4096]; - let size = server - .read(&mut buf) - .await - .expect("Failed to read from duplex stream"); - jh.abort(); - let peek_data = peek_sni_and_alpn(&buf[..size]).await; - assert!(peek_data.is_some()); - let peek_data = peek_data.unwrap(); + let buf = client_hello_bytes(client_config(), "sandhole.com.br").await; + let ClientHelloStatus::Ready(peek_data) = parse_client_hello(&buf) else { + panic!("Expected complete ClientHello"); + }; assert_eq!(peek_data.sni, "sandhole.com.br"); assert_eq!(peek_data.alpn, Vec::>::new()); } #[test_log::test(tokio::test)] async fn returns_sni_and_alpn_data() { - let mut root_store = rustls::RootCertStore::empty(); - root_store.add_parsable_certificates( - rustls_pki_types::CertificateDer::pem_file_iter( - PathBuf::from(std::env::var("CARGO_MANIFEST_DIR").unwrap()) - .join("tests/data/ca/rootCA.pem"), - ) - .and_then(|iter| iter.collect::, _>>()) - .expect("Failed to parse client certificates"), - ); - let mut client_config = rustls::ClientConfig::builder_with_provider(Arc::new( - rustls::crypto::aws_lc_rs::default_provider(), - )) - .with_safe_default_protocol_versions() - .unwrap() - .with_root_certificates(root_store) - .with_no_client_auth(); - client_config.alpn_protocols.push(b"example-alpn".to_vec()); - let connector = TlsConnector::from(Arc::new(client_config)); - let (mut server, client) = duplex(4096); - let jh = tokio::spawn(async move { - connector - .connect("foobar.tld".try_into().unwrap(), client) - .await - }); - let mut buf = [0u8; 4096]; - let size = server - .read(&mut buf) - .await - .expect("Failed to read from duplex stream"); - jh.abort(); - let peek_data = peek_sni_and_alpn(&buf[..size]).await; - assert!(peek_data.is_some()); - let peek_data = peek_data.unwrap(); + let mut config = client_config(); + config.alpn_protocols.push(b"example-alpn".to_vec()); + let buf = client_hello_bytes(config, "foobar.tld").await; + let ClientHelloStatus::Ready(peek_data) = parse_client_hello(&buf) else { + panic!("Expected complete ClientHello"); + }; assert_eq!(peek_data.sni, "foobar.tld"); assert_eq!(peek_data.alpn, vec![b"example-alpn".to_vec()]); } } + +#[cfg(test)] +#[cfg_attr(coverage_nightly, coverage(off))] +mod rewind_stream_tests { + use tokio::io::{AsyncReadExt, AsyncWriteExt, duplex}; + + use super::RewindStream; + + #[test_log::test(tokio::test)] + async fn replays_prefix_then_reads_inner() { + let (client, mut server) = duplex(64); + let mut rewind = RewindStream::new(b"hello ".to_vec(), client); + server.write_all(b"world").await.unwrap(); + drop(server); + let mut out = Vec::new(); + rewind.read_to_end(&mut out).await.unwrap(); + assert_eq!(out, b"hello world"); + } +} -- 2.51.2