fix(tls): share bare-probe classification across all FIFA-facing TLS hosts
A reachability probe (TcpStream::connect then drop; the launcher preflight makes them) reaches a TLS acceptor as 'unexpected EOF' — byte-identical to the certificate mismatch that cost three live gates. The redirector classified the opening before the acceptor to keep a benign probe from forging a TLS fault, but the roster host (the second FIFA-facing TLS host) did not, so the documented hazard 'remains in any other TLS host that has not adopted it' was live there. Lift the pure policy (PeerOpening + classify_opening) plus a peer_opening(&TcpStream) peek helper into the shared openfut-tls crate (game-independent; +unit tests). The redirector now re-exports them (public API + its probe_classification test unchanged; behaviour identical). The roster host adopts them: a ProbeCount, a probes() handle, and a pre-acceptor peek that logs PROBE and returns instead of failing the handshake. New roster probe_classification integration test (3 cases: bare probe classified, real client after a probe still served 200, speaks-then- fails still reported as a fault). Full workspace tests green; clippy -D clean.
This commit is contained in:
@@ -233,32 +233,11 @@ pub fn serve(cfg: RedirectorConfig) -> std::io::Result<()> {
|
|||||||
bind(cfg)?.run()
|
bind(cfg)?.run()
|
||||||
}
|
}
|
||||||
|
|
||||||
/// What a peer did with the connection before any TLS was attempted.
|
/// Re-exported from the shared TLS crate. The classification policy now lives in
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
/// `openfut-tls` so every FIFA-facing TLS host shares one implementation; this
|
||||||
pub enum PeerOpening {
|
/// host keeps naming them here so its public API and `probe_classification`
|
||||||
/// Connected and closed without sending anything: a reachability probe.
|
/// integration test are unaffected.
|
||||||
ClosedWithoutSpeaking,
|
pub use openfut_tls::{classify_opening, PeerOpening};
|
||||||
/// 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(
|
fn handle(
|
||||||
stream: TcpStream,
|
stream: TcpStream,
|
||||||
@@ -285,8 +264,7 @@ fn handle(
|
|||||||
// exactly how the certificate mismatch that cost three live gates
|
// exactly how the certificate mismatch that cost three live gates
|
||||||
// presented, so a benign probe forging it poisons the one channel this
|
// presented, so a benign probe forging it poisons the one channel this
|
||||||
// project gates on. Classified here, the two are never confused again.
|
// project gates on. Classified here, the two are never confused again.
|
||||||
let mut first = [0u8; 1];
|
if openfut_tls::peer_opening(&stream) == PeerOpening::ClosedWithoutSpeaking {
|
||||||
if classify_opening(&stream.peek(&mut first)) == PeerOpening::ClosedWithoutSpeaking {
|
|
||||||
probes.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
|
probes.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
|
||||||
log(&format!(
|
log(&format!(
|
||||||
"conn-{id:04} {peer} PROBE: closed before sending a ClientHello (not a TLS fault)"
|
"conn-{id:04} {peer} PROBE: closed before sending a ClientHello (not a TLS fault)"
|
||||||
|
|||||||
@@ -43,7 +43,7 @@
|
|||||||
|
|
||||||
use std::io::{Read, Write};
|
use std::io::{Read, Write};
|
||||||
use std::net::{TcpListener, TcpStream};
|
use std::net::{TcpListener, TcpStream};
|
||||||
use std::sync::atomic::{AtomicU64, Ordering};
|
use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering};
|
||||||
use std::sync::Arc;
|
use std::sync::Arc;
|
||||||
use std::time::{SystemTime, UNIX_EPOCH};
|
use std::time::{SystemTime, UNIX_EPOCH};
|
||||||
|
|
||||||
@@ -52,7 +52,7 @@ use openfut_adapter_fifa17::roster::{self, Method};
|
|||||||
use openfut_http::drain_body;
|
use openfut_http::drain_body;
|
||||||
pub use openfut_http::BodyRead;
|
pub use openfut_http::BodyRead;
|
||||||
// The acceptor comes from the shared crate, so this host never names OpenSSL.
|
// The acceptor comes from the shared crate, so this host never names OpenSSL.
|
||||||
use openfut_tls::SslAcceptor;
|
use openfut_tls::{peer_opening, PeerOpening, SslAcceptor};
|
||||||
|
|
||||||
pub mod config;
|
pub mod config;
|
||||||
pub use config::RosterConfig;
|
pub use config::RosterConfig;
|
||||||
@@ -112,6 +112,15 @@ pub struct ConnOutcome {
|
|||||||
|
|
||||||
pub type Outcomes = Arc<std::sync::Mutex<Vec<ConnOutcome>>>;
|
pub type Outcomes = Arc<std::sync::Mutex<Vec<ConnOutcome>>>;
|
||||||
|
|
||||||
|
/// How many bare port probes have been classified before reaching the acceptor.
|
||||||
|
///
|
||||||
|
/// Counted, not 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 — and
|
||||||
|
/// mirrors the redirector, the host that first needed the distinction.
|
||||||
|
pub type ProbeCount = Arc<AtomicUsize>;
|
||||||
|
|
||||||
/// Bounded: a host polled every few seconds must not accumulate forever.
|
/// Bounded: a host polled every few seconds must not accumulate forever.
|
||||||
const OUTCOME_HISTORY: usize = 64;
|
const OUTCOME_HISTORY: usize = 64;
|
||||||
|
|
||||||
@@ -130,6 +139,7 @@ pub struct Server {
|
|||||||
acceptor: Arc<SslAcceptor>,
|
acceptor: Arc<SslAcceptor>,
|
||||||
cfg: Arc<RosterConfig>,
|
cfg: Arc<RosterConfig>,
|
||||||
outcomes: Outcomes,
|
outcomes: Outcomes,
|
||||||
|
probes: ProbeCount,
|
||||||
}
|
}
|
||||||
|
|
||||||
impl Server {
|
impl Server {
|
||||||
@@ -137,6 +147,11 @@ impl Server {
|
|||||||
self.outcomes.clone()
|
self.outcomes.clone()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A handle to the bare-probe counter, obtainable before [`Server::run`].
|
||||||
|
pub fn probes(&self) -> ProbeCount {
|
||||||
|
self.probes.clone()
|
||||||
|
}
|
||||||
|
|
||||||
pub fn run(self) -> std::io::Result<()> {
|
pub fn run(self) -> std::io::Result<()> {
|
||||||
let counter = AtomicU64::new(0);
|
let counter = AtomicU64::new(0);
|
||||||
for incoming in self.listener.incoming() {
|
for incoming in self.listener.incoming() {
|
||||||
@@ -144,7 +159,8 @@ impl Server {
|
|||||||
let id = counter.fetch_add(1, Ordering::Relaxed) + 1;
|
let id = counter.fetch_add(1, Ordering::Relaxed) + 1;
|
||||||
let (acceptor, cfg) = (self.acceptor.clone(), self.cfg.clone());
|
let (acceptor, cfg) = (self.acceptor.clone(), self.cfg.clone());
|
||||||
let outcomes = self.outcomes.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(())
|
Ok(())
|
||||||
}
|
}
|
||||||
@@ -184,6 +200,7 @@ pub fn bind(cfg: RosterConfig) -> std::io::Result<Server> {
|
|||||||
acceptor: Arc::new(acceptor),
|
acceptor: Arc::new(acceptor),
|
||||||
cfg: Arc::new(cfg),
|
cfg: Arc::new(cfg),
|
||||||
outcomes: Arc::new(std::sync::Mutex::new(Vec::new())),
|
outcomes: Arc::new(std::sync::Mutex::new(Vec::new())),
|
||||||
|
probes: ProbeCount::default(),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -207,12 +224,25 @@ fn handle(
|
|||||||
acceptor: &SslAcceptor,
|
acceptor: &SslAcceptor,
|
||||||
cfg: &RosterConfig,
|
cfg: &RosterConfig,
|
||||||
outcomes: &Outcomes,
|
outcomes: &Outcomes,
|
||||||
|
probes: &ProbeCount,
|
||||||
) {
|
) {
|
||||||
let peer = stream
|
let peer = stream
|
||||||
.peer_addr()
|
.peer_addr()
|
||||||
.map(|a| a.to_string())
|
.map(|a| a.to_string())
|
||||||
.unwrap_or_else(|_| "?".into());
|
.unwrap_or_else(|_| "?".into());
|
||||||
|
|
||||||
|
// Classify a bare port probe BEFORE the acceptor sees it. A `connect` then
|
||||||
|
// drop (the launcher's preflight makes one per run) otherwise reaches the
|
||||||
|
// acceptor as `unexpected EOF` — byte-identical to the certificate mismatch
|
||||||
|
// that cost three live gates. Shared with the redirector via openfut-tls so
|
||||||
|
// the two hosts can never diverge on this.
|
||||||
|
if peer_opening(&stream) == PeerOpening::ClosedWithoutSpeaking {
|
||||||
|
probes.fetch_add(1, 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) {
|
let mut tls = match acceptor.accept(stream) {
|
||||||
Ok(s) => s,
|
Ok(s) => s,
|
||||||
Err(e) => {
|
Err(e) => {
|
||||||
|
|||||||
@@ -0,0 +1,132 @@
|
|||||||
|
//! A bare port probe must not be reported as a TLS fault on the roster host.
|
||||||
|
//!
|
||||||
|
//! `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. A reachability probe forges it trivially:
|
||||||
|
//! `TcpStream::connect` then drop opens the connection and closes it without
|
||||||
|
//! sending a byte, which the acceptor reports as `unexpected EOF`. The launcher's
|
||||||
|
//! preflight makes such probes, so the roster host — like the redirector — must
|
||||||
|
//! classify the opening before the acceptor sees it. The policy is shared via
|
||||||
|
//! `openfut-tls`; this proves the roster host actually applies it.
|
||||||
|
|
||||||
|
use std::io::{Read, Write};
|
||||||
|
use std::net::TcpStream;
|
||||||
|
use std::sync::atomic::Ordering;
|
||||||
|
use std::time::Duration;
|
||||||
|
|
||||||
|
use openfut_roster_host::{bind, RosterConfig};
|
||||||
|
|
||||||
|
fn cert_pair() -> (String, String) {
|
||||||
|
let base = concat!(env!("CARGO_MANIFEST_DIR"), "/../fifa17-recon/tools");
|
||||||
|
(
|
||||||
|
format!("{base}/redir_cert.pem"),
|
||||||
|
format!("{base}/redir_key.pem"),
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Start a host on an ephemeral port; hand back its address and its counters.
|
||||||
|
fn start() -> (
|
||||||
|
std::net::SocketAddr,
|
||||||
|
openfut_roster_host::ProbeCount,
|
||||||
|
openfut_roster_host::Outcomes,
|
||||||
|
) {
|
||||||
|
let (c, k) = cert_pair();
|
||||||
|
let server = bind(RosterConfig::for_test(&c, &k)).expect("host binds");
|
||||||
|
let addr = server.local_addr;
|
||||||
|
let (probes, outcomes) = (server.probes(), server.outcomes());
|
||||||
|
std::thread::spawn(move || {
|
||||||
|
let _ = server.run();
|
||||||
|
});
|
||||||
|
(addr, probes, outcomes)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// A FIFA-like client: legacy suites, no certificate checking. Sends a GET and
|
||||||
|
/// reads the response to EOF.
|
||||||
|
fn tls_get(addr: std::net::SocketAddr) -> Vec<u8> {
|
||||||
|
use openfut_tls::{SslConnector, SslMethod, SslVerifyMode};
|
||||||
|
let mut b = SslConnector::builder(SslMethod::tls()).expect("connector");
|
||||||
|
b.set_cipher_list(openfut_adapter_fifa17::tls::OBSERVED_CLIENT_SUITES)
|
||||||
|
.expect("ciphers");
|
||||||
|
b.set_verify(SslVerifyMode::NONE);
|
||||||
|
let sock = TcpStream::connect(addr).expect("connect");
|
||||||
|
let ssl = b
|
||||||
|
.build()
|
||||||
|
.configure()
|
||||||
|
.and_then(|c| c.verify_hostname(false).into_ssl("roster-test"))
|
||||||
|
.expect("ssl");
|
||||||
|
let mut s = openfut_tls::SslStream::new(ssl, sock).expect("stream");
|
||||||
|
s.connect().expect("handshake");
|
||||||
|
s.write_all(
|
||||||
|
b"GET /fifa17/fut/rosterupdate.xml HTTP/1.1\r\nHost: roster-test\r\nAccept: */*\r\n\r\n",
|
||||||
|
)
|
||||||
|
.expect("write");
|
||||||
|
s.flush().ok();
|
||||||
|
let mut out = Vec::new();
|
||||||
|
let _ = s.read_to_end(&mut out);
|
||||||
|
out
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The whole point: connect, send nothing, close — classified as a probe rather
|
||||||
|
/// than reaching the acceptor. Asserting on the counter (not on client-visible
|
||||||
|
/// behaviour) is deliberate: a probe produces no response either way, so a test
|
||||||
|
/// on what the client sees would pass with the classification deleted.
|
||||||
|
#[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_get(addr);
|
||||||
|
assert!(
|
||||||
|
response.starts_with(b"HTTP/1.0 200"),
|
||||||
|
"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, not silently reclassified as a
|
||||||
|
/// benign probe.
|
||||||
|
#[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: the peer has spoken, but it is not 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"
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -54,6 +54,52 @@ impl fmt::Display for TlsError {
|
|||||||
|
|
||||||
impl std::error::Error for TlsError {}
|
impl std::error::Error for TlsError {}
|
||||||
|
|
||||||
|
/// What a peer did with the connection before any TLS was attempted.
|
||||||
|
///
|
||||||
|
/// A bare reachability probe — `TcpStream::connect` then drop, which the
|
||||||
|
/// launcher's preflight makes twice per run — opens the connection and closes
|
||||||
|
/// without sending a byte. If that reaches the acceptor it fails as
|
||||||
|
/// `unexpected EOF`, which is **byte-identical** to the signature of the
|
||||||
|
/// certificate mismatch that cost three live gates. Classifying the opening
|
||||||
|
/// before the acceptor sees it keeps a benign probe from forging a TLS fault in
|
||||||
|
/// the one channel this project gates on. Every FIFA-facing TLS host shares this
|
||||||
|
/// policy so the distinction can never regress in just one of them.
|
||||||
|
#[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,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Peek one byte to classify a connection before the TLS acceptor sees it.
|
||||||
|
///
|
||||||
|
/// `MSG_PEEK` leaves the byte in the receive queue, so a subsequent
|
||||||
|
/// `acceptor.accept(stream)` reads the ClientHello intact — this is
|
||||||
|
/// non-destructive for a real handshake and only short-circuits a bare probe.
|
||||||
|
pub fn peer_opening(stream: &std::net::TcpStream) -> PeerOpening {
|
||||||
|
let mut first = [0u8; 1];
|
||||||
|
classify_opening(&stream.peek(&mut first))
|
||||||
|
}
|
||||||
|
|
||||||
/// A TLS protocol version, expressed without an OpenSSL type.
|
/// A TLS protocol version, expressed without an OpenSSL type.
|
||||||
///
|
///
|
||||||
/// Adapters name the version window their client was observed to use; keeping
|
/// Adapters name the version window their client was observed to use; keeping
|
||||||
@@ -299,6 +345,19 @@ pub fn self_test(cfg: &TlsConfig, client_ciphers: &str, sni: &str) -> Result<Neg
|
|||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn classify_opening_maps_each_case() {
|
||||||
|
// EOF before any byte is the reachability probe this exists to catch.
|
||||||
|
assert_eq!(classify_opening(&Ok(0)), PeerOpening::ClosedWithoutSpeaking);
|
||||||
|
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, not a silent probe reclassification.
|
||||||
|
assert_eq!(
|
||||||
|
classify_opening(&Err(std::io::Error::from(std::io::ErrorKind::WouldBlock))),
|
||||||
|
PeerOpening::Undetermined
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
/// A self-signed pair, generated here rather than borrowed from the game
|
/// A self-signed pair, generated here rather than borrowed from the game
|
||||||
/// stack: this crate is game-independent and its tests must not depend on
|
/// stack: this crate is game-independent and its tests must not depend on
|
||||||
/// FIFA material. Adapter crates test their own profiles.
|
/// FIFA material. Adapter crates test their own profiles.
|
||||||
|
|||||||
Reference in New Issue
Block a user