f451406058
Mandatory OpenFUT architecture audit. Two real defects found and fixed, plus
the config surface tightened so neither class can recur.
DEFECT 1 -- hidden localhost fallback. The Rust host defaulted POW hosts to
127.0.0.1 while every other URL followed OPENFUT_ADVERTISE, so a remote
deployment would emit loopback POW URLs and fail far from the cause. It also
diverged from the deployed Python entrypoint, which derives them
(POW_HOST="${POW_HOST:-$ADV:8094}"). POW endpoints now derive from the
advertised address; explicit overrides still win.
DEFECT 2 -- Default gave loopback silently. `Endpoints::default()` and
`AdapterConfig::default()` supplied 127.0.0.1, so anything constructing a
config by omission got loopback with no signal. Both `Default` impls are
REMOVED. Loopback is now `Endpoints::loopback()` / `AdapterConfig::loopback()`:
an explicit, greppable decision. Production uses `advertising(host)`.
CONFIGURABILITY. `blaze_port` and `utas_port` are now config, not literals.
The advertised Blaze port is our choice -- the client goes wherever
<serverinstanceinfo> sends it -- and 8099 is the client's own built-in default
but still deployment config. A bad port value is an error, not a silent
fallback to the previous one.
TEST-NET EVERYWHERE. Committed fixtures and tests used the lab's real LAN
address; a test that passes because its constant matches the current lab
proves nothing about relocatability. Redirector fixtures regenerated on
RFC 5737 TEST-NET-1/2/3 plus loopback. Harness scripts no longer default the
client IP to the lab address -- client-state.sh now requires it.
SEVEN REQUIRED TESTS in tests/deployment_config.rs plus host-side coverage:
remote config never silently becomes localhost; missing advertise fails
clearly; bind may differ from advertise; changing the Blaze port changes the
redirect; changing the host updates all 200+ generated URLs with no
stragglers; no helper bypasses central config; mutations are detectable.
MUTATION TESTED, and it found a hole in the audit tests themselves. Hardcoding
utas_base, reverting the POW derivation and re-hardcoding the Blaze port were
all caught. Making the redirector read `bind` instead of `advertise` was NOT:
`advertising()` sets bind == advertise, so the two sources were
indistinguishable. That is the single most likely bypass -- the oracle really
does read bind for nucleusConnect -- so the test now forces bind != advertise
and asserts the bind address never reaches the wire. Re-mutated: caught.
Wire behaviour unchanged: oracle fixtures still current, 153 tests green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
219 lines
8.6 KiB
Rust
219 lines
8.6 KiB
Rust
//! Deployment-address audit: the configured address must reach every
|
|
//! client-visible endpoint, and nothing may quietly substitute its own.
|
|
//!
|
|
//! OpenFUT has to run on arbitrary addresses. The development topology is
|
|
//! deployment configuration, not architecture, so no crate may contain a
|
|
//! production destination, an advertised address, or a hidden localhost
|
|
//! fallback.
|
|
//!
|
|
//! Two TEST-NET addresses are used throughout (RFC 5737), deliberately not the
|
|
//! lab's real LAN addresses: a test that passes only because its constant
|
|
//! happens to match the current lab proves nothing about relocatability.
|
|
|
|
use openfut_adapter_fifa17::blaze::{client_config, AdapterConfig, Endpoints};
|
|
use openfut_adapter_fifa17::redirector;
|
|
|
|
/// TEST-NET-2 and TEST-NET-3. Never routable, never ours, and obviously not a
|
|
/// lab address to anyone reading a failure.
|
|
const ADDR_A: &str = "198.51.100.7";
|
|
const ADDR_B: &str = "203.0.113.42";
|
|
|
|
fn cfg(advertise: &str) -> AdapterConfig {
|
|
AdapterConfig::advertising(advertise)
|
|
}
|
|
|
|
/// Every client-visible string a config produces, for wholesale comparison.
|
|
fn client_visible_surface(cfg: &AdapterConfig) -> Vec<String> {
|
|
let mut out = vec![
|
|
cfg.utas_base(),
|
|
cfg.nucleus_base(),
|
|
cfg.pow_content_url(),
|
|
String::from_utf8(redirector::redirect_response(cfg)).unwrap(),
|
|
];
|
|
for section in client_config::known_sections() {
|
|
for (k, v) in client_config::rows_for(section, cfg) {
|
|
out.push(format!("{section}/{k}={v}"));
|
|
}
|
|
}
|
|
out
|
|
}
|
|
|
|
/// (5) Changing the advertised host must update every applicable generated URL,
|
|
/// with no recompilation and no leftovers.
|
|
#[test]
|
|
fn changing_the_advertised_host_updates_every_client_visible_url() {
|
|
let a = client_visible_surface(&cfg(ADDR_A));
|
|
let b = client_visible_surface(&cfg(ADDR_B));
|
|
|
|
assert_eq!(a.len(), b.len(), "the surface itself must not change shape");
|
|
|
|
let a_has = a.iter().filter(|s| s.contains(ADDR_A)).count();
|
|
let b_has = b.iter().filter(|s| s.contains(ADDR_B)).count();
|
|
assert!(a_has > 200, "expected the address throughout, saw {a_has}");
|
|
assert_eq!(a_has, b_has, "the same entries must carry the new address");
|
|
|
|
// Nothing may retain the old address after reconfiguration.
|
|
let stragglers: Vec<&String> = b.iter().filter(|s| s.contains(ADDR_A)).collect();
|
|
assert!(
|
|
stragglers.is_empty(),
|
|
"these kept the previous address: {:?}",
|
|
&stragglers[..stragglers.len().min(5)]
|
|
);
|
|
}
|
|
|
|
/// (1) A remote configuration must not silently become localhost anywhere.
|
|
#[test]
|
|
fn remote_configuration_never_silently_becomes_localhost() {
|
|
let c = cfg(ADDR_A);
|
|
// Allowlisted: two OAuth redirect targets that are literals in the oracle
|
|
// and are never dialled (see the compatibility exceptions in the vault).
|
|
const ALLOWED_LOOPBACK_KEYS: [&str; 2] = ["identityRedirectUri", "redirect_uri"];
|
|
|
|
for entry in client_visible_surface(&c) {
|
|
if entry.contains("127.0.0.1") || entry.contains("localhost") {
|
|
assert!(
|
|
ALLOWED_LOOPBACK_KEYS.iter().any(|k| entry.contains(k)),
|
|
"unexpected loopback in a remote configuration: {entry}"
|
|
);
|
|
}
|
|
}
|
|
|
|
// POW hosts in particular must derive from advertise, not fall back alone.
|
|
assert!(c.endpoints.pow_content_host.starts_with(ADDR_A));
|
|
assert!(c.endpoints.pow_host.starts_with(ADDR_A));
|
|
assert!(c.pow_content_url().contains(ADDR_A));
|
|
}
|
|
|
|
/// (3) Bind and advertise are different concepts and must never be conflated.
|
|
#[test]
|
|
fn bind_can_differ_from_advertise() {
|
|
let mut c = cfg(ADDR_A);
|
|
c.endpoints.bind = "0.0.0.0".into();
|
|
|
|
assert_eq!(c.endpoints.advertise, ADDR_A);
|
|
assert_eq!(c.endpoints.bind, "0.0.0.0");
|
|
// The advertised surface follows advertise, not bind.
|
|
assert!(c.utas_base().contains(ADDR_A));
|
|
assert!(!c.utas_base().contains("0.0.0.0"));
|
|
|
|
// COMPATIBILITY EXCEPTION, reproduced deliberately: nucleusConnect follows
|
|
// BIND in the oracle. Instrumentation showed the client never dials it, so
|
|
// this is cosmetic on the observed path. Asserted so the exception cannot
|
|
// be "fixed" by accident without this test failing and forcing the decision
|
|
// to be made explicitly.
|
|
assert_eq!(c.nucleus_base(), "http://0.0.0.0:42131");
|
|
}
|
|
|
|
/// (4) The advertised Blaze port must reach the redirect result.
|
|
#[test]
|
|
fn changing_the_blaze_port_changes_the_redirect() {
|
|
let mut c = cfg(ADDR_A);
|
|
let before = String::from_utf8(redirector::redirect_response(&c)).unwrap();
|
|
assert!(before.contains("<port>42130</port>"));
|
|
|
|
c.endpoints.blaze_port = 42999;
|
|
let after = String::from_utf8(redirector::redirect_response(&c)).unwrap();
|
|
assert!(after.contains("<port>42999</port>"), "{after}");
|
|
assert!(!after.contains("<port>42130</port>"));
|
|
// And the advertised host still follows config.
|
|
assert!(after.contains(&format!("<hostname>{ADDR_A}</hostname>")));
|
|
}
|
|
|
|
/// The UTAS port is deployment configuration too, not a constant we own.
|
|
#[test]
|
|
fn changing_the_utas_port_changes_every_rs4_url() {
|
|
let mut c = cfg(ADDR_A);
|
|
assert!(c.utas_base().contains(":8099/"));
|
|
|
|
c.endpoints.utas_port = 9099;
|
|
assert_eq!(c.utas_base(), format!("http://{ADDR_A}:9099/"));
|
|
|
|
let rows = client_config::rows_for("BlazeSDK", &c);
|
|
let base = rows.iter().find(|(k, _)| k == "FUT_RS4_BASE_URL").unwrap();
|
|
assert_eq!(base.1, format!("http://{ADDR_A}:9099/"));
|
|
assert!(!rows.iter().any(|(_, v)| v.contains(":8099")));
|
|
}
|
|
|
|
/// (6) No service-specific helper may construct an endpoint from a different
|
|
/// source of truth than the central configuration.
|
|
#[test]
|
|
fn no_helper_bypasses_the_central_configuration() {
|
|
// bind MUST differ from advertise here. With them equal, a helper that
|
|
// wrongly reads `bind` is indistinguishable from one that reads
|
|
// `advertise` — and reading `bind` is the single most likely bypass,
|
|
// because the oracle really does it for nucleusConnect. Mutation-tested:
|
|
// with bind == advertise this test could not detect that substitution.
|
|
let mut c = cfg(ADDR_B);
|
|
c.endpoints.bind = "0.0.0.0".into();
|
|
|
|
// Every URL-shaped helper resolves through the same Endpoints.
|
|
assert!(c.utas_base().contains(ADDR_B));
|
|
assert!(c.pow_content_url().contains(ADDR_B));
|
|
let ep = redirector::BlazeEndpoint::from_config(&c);
|
|
assert_eq!(
|
|
ep.host, c.endpoints.advertise,
|
|
"the redirector must advertise the ADVERTISED host, not the bind address"
|
|
);
|
|
assert_ne!(
|
|
ep.host, c.endpoints.bind,
|
|
"bind must not leak into the wire"
|
|
);
|
|
let xml = String::from_utf8(redirector::redirect_response(&c)).unwrap();
|
|
assert!(xml.contains(ADDR_B));
|
|
assert!(
|
|
!xml.contains("0.0.0.0"),
|
|
"the bind address must never reach the client"
|
|
);
|
|
assert_eq!(ep.port, c.endpoints.blaze_port);
|
|
|
|
// And the config table's URL tokens resolve through those same helpers,
|
|
// rather than re-deriving a URL shape of their own.
|
|
let rows = client_config::rows_for("BlazeSDK", &c);
|
|
let base = rows.iter().find(|(k, _)| k == "FUT_RS4_BASE_URL").unwrap();
|
|
assert_eq!(base.1, c.utas_base());
|
|
let nucleus = rows.iter().find(|(k, _)| k == "nucleusConnect").unwrap();
|
|
assert_eq!(nucleus.1, c.nucleus_base());
|
|
}
|
|
|
|
/// (7) Mutating the configuration must make these tests fail — a suite that
|
|
/// passes regardless of the configured address would prove nothing.
|
|
#[test]
|
|
fn configuration_mutations_are_detectable() {
|
|
let a = cfg(ADDR_A);
|
|
let b = cfg(ADDR_B);
|
|
|
|
// Each of these is what a mutation would have to defeat.
|
|
assert_ne!(a.utas_base(), b.utas_base());
|
|
assert_ne!(a.pow_content_url(), b.pow_content_url());
|
|
assert_ne!(
|
|
redirector::redirect_response(&a),
|
|
redirector::redirect_response(&b)
|
|
);
|
|
assert_ne!(
|
|
client_config::rows_for("BlazeSDK", &a),
|
|
client_config::rows_for("BlazeSDK", &b)
|
|
);
|
|
|
|
let mut port_changed = a.clone();
|
|
port_changed.endpoints.blaze_port += 1;
|
|
assert_ne!(
|
|
redirector::redirect_response(&a),
|
|
redirector::redirect_response(&port_changed)
|
|
);
|
|
}
|
|
|
|
/// Loopback must be a named, deliberate choice — not something a caller can
|
|
/// reach by omission.
|
|
#[test]
|
|
fn loopback_is_explicit_not_a_default() {
|
|
let l = Endpoints::loopback();
|
|
assert_eq!(l.advertise, "127.0.0.1");
|
|
assert!(l.pow_content_host.starts_with("127.0.0.1"));
|
|
|
|
// `Endpoints::default()` and `AdapterConfig::default()` deliberately do not
|
|
// exist; this test documents that, and the crate would not compile if they
|
|
// were reintroduced and used by accident elsewhere.
|
|
let explicit = AdapterConfig::loopback();
|
|
assert_eq!(explicit.endpoints.advertise, "127.0.0.1");
|
|
}
|