diff --git a/openfut-redirector-host/src/lib.rs b/openfut-redirector-host/src/lib.rs index 638bfcc..478018b 100644 --- a/openfut-redirector-host/src/lib.rs +++ b/openfut-redirector-host/src/lib.rs @@ -127,8 +127,17 @@ pub struct Server { acceptor: Arc, cfg: Arc, outcomes: Outcomes, + probes: ProbeCount, } +/// How many bare port probes have been classified. +/// +/// Counted rather than only logged for the same reason [`ConnOutcome`] exists: +/// a test that asserts on client-visible symptoms cannot tell a probe that was +/// classified from one that merely failed quietly, so removing the +/// classification would leave the suite green. This makes it assertable. +pub type ProbeCount = Arc; + /// What the host did on a connection, beyond the bytes it sent. /// /// Recorded because the failure that cost two live gate attempts — a request @@ -149,6 +158,11 @@ pub type Outcomes = Arc>>; const OUTCOME_HISTORY: usize = 64; impl Server { + /// A handle to the bare-probe counter, obtainable before [`Server::run`]. + pub fn probes(&self) -> ProbeCount { + self.probes.clone() + } + /// A handle to the connection record, obtainable before [`Server::run`] /// consumes the server. pub fn outcomes(&self) -> Outcomes { @@ -162,7 +176,8 @@ impl Server { let id = counter.fetch_add(1, Ordering::Relaxed) + 1; let (acceptor, cfg) = (self.acceptor.clone(), self.cfg.clone()); let outcomes = self.outcomes.clone(); - std::thread::spawn(move || handle(stream, id, &acceptor, &cfg, &outcomes)); + let probes = self.probes.clone(); + std::thread::spawn(move || handle(stream, id, &acceptor, &cfg, &outcomes, &probes)); } Ok(()) } @@ -209,6 +224,7 @@ pub fn bind(cfg: RedirectorConfig) -> std::io::Result { acceptor: Arc::new(acceptor), cfg: Arc::new(cfg), outcomes: Outcomes::default(), + probes: ProbeCount::default(), }) } @@ -217,12 +233,40 @@ pub fn serve(cfg: RedirectorConfig) -> std::io::Result<()> { bind(cfg)?.run() } +/// What a peer did with the connection before any TLS was attempted. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PeerOpening { + /// Connected and closed without sending anything: a reachability probe. + ClosedWithoutSpeaking, + /// Sent at least one byte, so a real handshake is under way. + Spoke, + /// Timed out or errored. Deliberately NOT treated as a probe — a slow or + /// broken client must still reach the acceptor and produce a real + /// diagnostic, because misclassifying a fault as a probe would hide + /// precisely what this distinction exists to protect. + Undetermined, +} + +/// Classify the result of peeking at the first byte. +/// +/// Split out as a pure function so the policy is testable without a socket — +/// the interesting cases (EOF vs timeout) are awkward to provoke live and easy +/// to get backwards. +pub fn classify_opening(peek: &std::io::Result) -> PeerOpening { + match peek { + Ok(0) => PeerOpening::ClosedWithoutSpeaking, + Ok(_) => PeerOpening::Spoke, + Err(_) => PeerOpening::Undetermined, + } +} + fn handle( stream: TcpStream, id: u64, acceptor: &SslAcceptor, cfg: &RedirectorConfig, outcomes: &Outcomes, + probes: &ProbeCount, ) { let peer = stream .peer_addr() @@ -231,6 +275,25 @@ fn handle( let _ = stream.set_read_timeout(Some(Duration::from_secs(15))); let _ = stream.set_write_timeout(Some(Duration::from_secs(15))); + // Classify a bare port probe BEFORE the acceptor sees it. + // + // A reachability probe — the launcher's preflight makes two per run, and + // any `TcpStream::connect` then drop is one — opens the connection and + // closes without sending a byte. Once that reaches the acceptor it fails + // as "unexpected EOF", which is byte-identical to the signature of a + // genuine TLS fault. That is not a cosmetic problem: "unexpected EOF" is + // exactly how the certificate mismatch that cost three live gates + // presented, so a benign probe forging it poisons the one channel this + // project gates on. Classified here, the two are never confused again. + let mut first = [0u8; 1]; + if classify_opening(&stream.peek(&mut first)) == PeerOpening::ClosedWithoutSpeaking { + probes.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + log(&format!( + "conn-{id:04} {peer} PROBE: closed before sending a ClientHello (not a TLS fault)" + )); + return; + } + let mut tls = match acceptor.accept(stream) { Ok(s) => s, Err(e) => { diff --git a/openfut-redirector-host/tests/probe_classification.rs b/openfut-redirector-host/tests/probe_classification.rs new file mode 100644 index 0000000..f54dabc --- /dev/null +++ b/openfut-redirector-host/tests/probe_classification.rs @@ -0,0 +1,145 @@ +//! A bare port probe must not be reported as a TLS fault. +//! +//! `TLS HANDSHAKE FAILED: ... unexpected EOF` is the exact signature the +//! certificate mismatch produced — the defect that cost three live gate +//! attempts and was invisible everywhere else. Anything that can forge that +//! line degrades the one channel this project gates on. +//! +//! A reachability probe forges it trivially: `TcpStream::connect` followed by a +//! drop opens the connection and closes it without sending a byte, and the +//! acceptor reports that as `unexpected EOF`. The launcher's preflight makes +//! two such probes per run, and on 2026-08-12 they produced ten of these lines +//! and sent this session chasing a client-side fault that did not exist. + +use std::io::Write; +use std::net::TcpStream; +use std::sync::atomic::Ordering; +use std::time::Duration; + +use openfut_adapter_fifa17::tls; +use openfut_redirector_host::{bind, classify_opening, PeerOpening, RedirectorConfig}; +use openssl::ssl::{SslConnector, SslMethod, SslVerifyMode}; + +/// Start a host on an ephemeral port; hand back its address and its counters. +fn start() -> ( + String, + openfut_redirector_host::ProbeCount, + openfut_redirector_host::Outcomes, +) { + let mut cfg = RedirectorConfig::for_test("198.51.100.7"); + cfg.listen_port = 0; + let server = bind(cfg).expect("host binds"); + let addr = server.local_addr.to_string(); + let (probes, outcomes) = (server.probes(), server.outcomes()); + std::thread::spawn(move || { + let _ = server.run(); + }); + (addr, probes, outcomes) +} + +fn tls_request(addr: &str) -> Vec { + let mut b = SslConnector::builder(SslMethod::tls()).expect("connector"); + b.set_cipher_list(tls::OBSERVED_CLIENT_SUITES) + .expect("list"); + b.set_verify(SslVerifyMode::NONE); + let sock = TcpStream::connect(addr).expect("connect"); + let ssl = b + .build() + .configure() + .expect("configure") + .verify_hostname(false) + .into_ssl("winter15.gosredirector.ea.com") + .expect("ssl"); + let mut s = openssl::ssl::SslStream::new(ssl, sock).expect("stream"); + s.connect().expect("handshake"); + s.write_all(b"POST /redirector/getServerInstance HTTP/1.1\r\nContent-Length: 0\r\n\r\n") + .expect("write"); + s.flush().ok(); + let mut out = Vec::new(); + let _ = std::io::Read::read_to_end(&mut s, &mut out); + out +} + +/// The whole point: connect, send nothing, close — and be classified as a +/// probe rather than reaching the acceptor. +/// +/// Asserting on the counter rather than on client-visible behaviour is +/// deliberate. A probe produces no response either way, so a test written +/// against what the client sees would pass with the classification deleted — +/// exactly the mutation that must not survive. +#[test] +fn a_bare_connect_and_close_is_classified_as_a_probe() { + let (addr, probes, outcomes) = start(); + + drop(TcpStream::connect(&addr).expect("probe connects")); + std::thread::sleep(Duration::from_millis(200)); + + assert_eq!( + probes.load(Ordering::Relaxed), + 1, + "a connect-and-close was not classified as a probe" + ); + assert!( + outcomes.lock().expect("lock").is_empty(), + "a probe must not be recorded as a served connection" + ); +} + +/// A probe must not disturb the host: the next real client still gets served. +#[test] +fn a_probe_does_not_break_the_connection_that_follows_it() { + let (addr, probes, _outcomes) = start(); + + drop(TcpStream::connect(&addr).expect("probe connects")); + std::thread::sleep(Duration::from_millis(100)); + + let response = tls_request(&addr); + assert!( + response.starts_with(b"HTTP/1.1 200 OK"), + "real client after a probe got: {:?}", + String::from_utf8_lossy(response.get(..64).unwrap_or(&response)) + ); + assert_eq!( + probes.load(Ordering::Relaxed), + 1, + "the real client was miscounted as a probe" + ); +} + +/// The dangerous direction: a client that DOES speak and then fails must still +/// reach the acceptor and be reported as a fault. Classifying it as a probe +/// would hide precisely what the distinction exists to protect. +#[test] +fn a_client_that_speaks_then_fails_is_not_a_probe() { + let (addr, probes, _outcomes) = start(); + + let mut sock = TcpStream::connect(&addr).expect("connect"); + // One byte of nonsense: enough that the peer has spoken, not enough to be + // a ClientHello, so the handshake genuinely fails. + sock.write_all(&[0x16]).expect("write"); + sock.flush().ok(); + drop(sock); + std::thread::sleep(Duration::from_millis(200)); + + assert_eq!( + probes.load(Ordering::Relaxed), + 0, + "a failing handshake was silently reclassified as a benign probe" + ); +} + +#[test] +fn the_classification_policy_is_explicit_about_each_case() { + assert_eq!( + classify_opening(&Ok(0)), + PeerOpening::ClosedWithoutSpeaking, + "EOF before any byte is a probe" + ); + assert_eq!(classify_opening(&Ok(1)), PeerOpening::Spoke); + // A timeout must NOT be a probe: a slow or broken client still deserves a + // real diagnostic from the acceptor. + assert_eq!( + classify_opening(&Err(std::io::Error::from(std::io::ErrorKind::WouldBlock))), + PeerOpening::Undetermined + ); +}