redirector: a port probe must not forge the signature of a TLS fault
"TLS HANDSHAKE FAILED: ... unexpected EOF" is exactly how the certificate mismatch presented -- the defect that cost three live gate attempts and was invisible everywhere else. It is the one line this project has learned to treat as serious. A reachability probe forges it for free: TcpStream::connect followed by a drop opens the connection and closes without sending a byte, and the acceptor reports that as "unexpected EOF". The launcher's preflight makes two such probes per run. On 2026-08-12 they produced ten of these lines and sent a whole session diagnosing a client-side fault that did not exist -- autopatch, ptrace_scope and client_arm.sh were all investigated before the pairing of the timestamps gave it away. Classify before the acceptor sees the connection: peek one byte, and treat EOF-before-any-byte as a probe with its own quiet line. A timeout is deliberately NOT a probe -- a slow or broken client must still reach the acceptor and produce a real diagnostic, since misclassifying a fault as benign would defeat the point. The counter exists because the test needs it. A probe produces no response either way, so a test written against client-visible behaviour passes with the classification deleted; asserting on a count is what makes the mutation detectable. Verified: all three mutations (drop the classification, treat undetermined as a probe, treat a speaking client as a probe) are killed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -127,8 +127,17 @@ pub struct Server {
|
||||
acceptor: Arc<SslAcceptor>,
|
||||
cfg: Arc<RedirectorConfig>,
|
||||
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<std::sync::atomic::AtomicUsize>;
|
||||
|
||||
/// 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<std::sync::Mutex<Vec<ConnOutcome>>>;
|
||||
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<Server> {
|
||||
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<usize>) -> 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) => {
|
||||
|
||||
@@ -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<u8> {
|
||||
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
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user