From 6e7867147d7d7b264453d2046aef6c0be303eb99 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 14 Aug 2026 19:09:40 +0700 Subject: [PATCH] birds stop blinking out, and the version check is answered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - an animated doodad is culled by its authored box, not its bind pose — the one-yard bird flying a 64-yard circuit stops vanishing with the camera angle — and the fix survives the engine's own bound recompute - the logon proof answers the server's version challenge instead of sending zeros, so a strict realmd stops rejecting benilla as a modified client - two instruments land: a frame-by-frame visibility trace for one model, and a probe that classifies whether a realmd enforces the version check Co-Authored-By: Claude Fable 5 --- Cargo.toml | 1 + crates/benilla-app/src/entities/wmo_props.rs | 6 +- .../src/bin/benilla-extract/main.rs | 14 ++ .../src/bin/benilla-extract/scan/geometry.rs | 107 +++++++++++++ .../src/bin/benilla-extract/scan/mod.rs | 2 +- .../m2_authored_box_is_the_animated_extent.rs | 90 +++++++++++ crates/benilla-protocol/Cargo.toml | 2 +- .../examples/srp_encoding_probe.rs | 3 +- .../examples/version_check_probe.rs | 137 +++++++++++++++++ crates/benilla-protocol/src/auth.rs | 141 +++++++++++++++++- crates/benilla-protocol/src/lib.rs | 1 + crates/benilla-protocol/tests/auth.rs | 10 +- crates/benilla-srp/Cargo.toml | 2 +- crates/benilla-world/src/doodad_anim/lazy.rs | 85 +++++++++++ crates/benilla-world/src/model_render.rs | 10 ++ .../src/model_render/visibility.rs | 110 ++++++++++++++ crates/benilla-world/src/terrain_stream.rs | 2 +- .../src/terrain_stream/spawn/assemble.rs | 110 +++++++++++++- .../src/terrain_stream/spawn/mod.rs | 35 +++++ 19 files changed, 848 insertions(+), 20 deletions(-) create mode 100644 crates/benilla-formats/tests/m2_authored_box_is_the_animated_extent.rs create mode 100644 crates/benilla-protocol/examples/version_check_probe.rs diff --git a/Cargo.toml b/Cargo.toml index e3b389e1..8fd170b6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,7 @@ authors = ["sam "] anyhow = "1.0.102" bevy = "0.18.1" clap = { version = "4.6.1", features = ["derive"] } +sha1 = "0.10.6" # The WoW format + protocol stack is fully in-repo (decision 0021), so there are no warcraft-rs / # wow_messages crates (or forks of them) anywhere in the tree. That is still true and is why this diff --git a/crates/benilla-app/src/entities/wmo_props.rs b/crates/benilla-app/src/entities/wmo_props.rs index 15814458..f4966650 100644 --- a/crates/benilla-app/src/entities/wmo_props.rs +++ b/crates/benilla-app/src/entities/wmo_props.rs @@ -60,8 +60,8 @@ use benilla_world::lighting::{PropProbeSlot, PropProbes}; use benilla_world::model_render::ShadeSel; use benilla_world::particles; use benilla_world::terrain_stream::{ - build_collider_task, fold_interior_probe, m2_fade, placement_collider_data, point_light, - spawn_model_entities, PendingCollider, PropLobeLight, + build_collider_task, fold_interior_probe, m2_anim_bound, m2_fade, placement_collider_data, + point_light, spawn_model_entities, PendingCollider, PropLobeLight, }; use super::{GameObjects, ModelHandle, VisualAttached}; @@ -228,6 +228,7 @@ pub(super) fn spawn_wmo_gameobject_props( anchor }); let (radius, center) = m2_fade(&m.bounds, prop.local.scale.x); + let anim_bound = m2_anim_bound(&m.bounds); // The interior lane (0474): fold the cabin prop's committed light ONCE, composed // through the host's current pose. Exact on a mover — see the module doc's // invariance argument. @@ -273,6 +274,7 @@ pub(super) fn spawn_wmo_gameobject_props( interior_slot, radius, center, + anim_bound, Some((m, now)), &mut uv_reg, &mut tint_reg, diff --git a/crates/benilla-formats/src/bin/benilla-extract/main.rs b/crates/benilla-formats/src/bin/benilla-extract/main.rs index dbdf3b8c..f2a55d8a 100644 --- a/crates/benilla-formats/src/bin/benilla-extract/main.rs +++ b/crates/benilla-formats/src/bin/benilla-extract/main.rs @@ -212,6 +212,19 @@ enum Command { /// Internal-path prefix filter (e.g. `spells`), case-insensitive; all models if omitted. prefix: Option, }, + /// Sweep every `.m2` (optionally under a path prefix) and measure how far its authored header + /// bounding box — the model's **all-animation** vertex extent, and the box the reference + /// derives its doodad cull sphere from (`rec+0x5c`/`rec+0x68`) — reaches past its **bind-pose** + /// vertex extent. The population instrument for decision 1259: an animated placement's submesh + /// keeps its transform at the placement origin while the joint palette moves its vertices, so a + /// bind-pose bound culls an object whose geometry is still on screen. The ambient critters are + /// the extreme — `World\critter\birds\Bird01.m2` authors a 67 yd box around a 1.2 yd body, + /// because its root bone flies it 64 yd along a circuit. Also reports the reverse (`SHORT`: + /// bind-pose geometry OUTSIDE the authored box), which is why the fix unions the two boxes. + Animboundscan { + /// Internal-path prefix filter (e.g. `world\critter`), case-insensitive; all if omitted. + prefix: Option, + }, /// Sweep every `.m2` (optionally under a path prefix) and census the geometry a /// **non-character** spawn draws that the reference may not: MULTI-GEOSET models (more than /// one `skinSectionId` — only the character compositor selects among them, so every other @@ -658,6 +671,7 @@ fn main() -> Result<()> { Command::Bbscan { prefix } => scan::bbscan(&mut chain, prefix.as_deref())?, Command::Bbfacescan { prefix } => scan::bbfacescan(&mut chain, prefix.as_deref())?, Command::Groundscan { prefix } => scan::groundscan(&mut chain, prefix.as_deref())?, + Command::Animboundscan { prefix } => scan::animboundscan(&mut chain, prefix.as_deref())?, Command::Geosetscan { prefix } => scan::geosetscan(&mut chain, prefix.as_deref())?, Command::Alphascan { prefix } => scan::alphascan(&mut chain, prefix.as_deref())?, Command::Fxlifescan { prefix } => scan::fxlifescan(&mut chain, prefix.as_deref())?, diff --git a/crates/benilla-formats/src/bin/benilla-extract/scan/geometry.rs b/crates/benilla-formats/src/bin/benilla-extract/scan/geometry.rs index b41ee9e0..100a19ab 100644 --- a/crates/benilla-formats/src/bin/benilla-extract/scan/geometry.rs +++ b/crates/benilla-formats/src/bin/benilla-extract/scan/geometry.rs @@ -507,3 +507,110 @@ pub fn groundscan(chain: &mut Chain, prefix: Option<&str>) -> Result<()> { } Ok(()) } + +/// Sweep every `.m2` (under `prefix`, if given) and measure how far its authored header bounding +/// box — the model's **all-animation** vertex extent, and the box the reference derives its doodad +/// cull sphere from — reaches past its **bind-pose** vertex extent. +/// +/// The population instrument for decision 1259. A placed model's submesh entity keeps its transform +/// at the placement origin while the joint palette moves its vertices, so a bind-pose mesh bound +/// stops describing what is drawn the moment the model animates: cull with it and the object blinks +/// out while its geometry is still on screen. `SLACK` is how many yards the authored box reaches +/// past the bind-pose box on its worst face — for the ambient critters (birds, bats, butterflies, +/// wasps) that is tens of yards against a sub-yard body, which is the whole bug. +/// +/// `SHORT` is the same measure with the signs reversed: yards of bind-pose geometry sticking out of +/// the *authored* box. It is not zero in the shipped corpus, which is why the fix unions the two +/// boxes instead of swapping one for the other — the reference never trips over it because it tests +/// the box's circumsphere, which swallows the overhang. +/// +/// `ANIM` marks the models the widened bound actually applies to: any bone track with more than one +/// key, or any global-sequence bone channel. A static model keeps its tighter per-batch bound. +pub fn animboundscan(chain: &mut Chain, prefix: Option<&str>) -> Result<()> { + let names = super::m2_names(chain, prefix)?; + // (slack, short, animates, bind-pose half-diagonal, name) + let mut rows: Vec<(f32, f32, bool, f32, String)> = Vec::new(); + let mut scanned = 0u32; + for name in names { + let Ok(bytes) = chain.read_file(&name) else { + continue; + }; + let Ok(bounds) = benilla_formats::parse_m2_bounds(&bytes) else { + continue; + }; + let Ok(fmt) = benilla_m2::parse_m2(&mut std::io::Cursor::new(bytes.as_slice())) else { + continue; + }; + let verts = &fmt.model().vertices; + if verts.is_empty() { + continue; + } + scanned += 1; + let mut lo = [f32::MAX; 3]; + let mut hi = [f32::MIN; 3]; + for v in verts { + for (a, c) in [v.position.x, v.position.y, v.position.z] + .into_iter() + .enumerate() + { + lo[a] = lo[a].min(c); + hi[a] = hi[a].max(c); + } + } + let (mut slack, mut short) = (0.0f32, 0.0f32); + for a in 0..3 { + slack = slack + .max(lo[a] - bounds.bbox_min[a]) + .max(bounds.bbox_max[a] - hi[a]); + short = short + .max(bounds.bbox_min[a] - lo[a]) + .max(hi[a] - bounds.bbox_max[a]); + } + let half_diag = + ((hi[0] - lo[0]).powi(2) + (hi[1] - lo[1]).powi(2) + (hi[2] - lo[2]).powi(2)).sqrt() + * 0.5; + // "Animates" the way `doodad_anim::classify` means it: geometry that MOVES relative to the + // placement transform — a keyed bone track, or a free-running global-sequence channel. + let keyed = benilla_formats::parse_m2_animations(&bytes) + .iter() + .any(|a| { + a.bones + .iter() + .any(|b| b.translation.len() > 1 || b.rotation.len() > 1 || b.scale.len() > 1) + }); + let gseq = !benilla_formats::parse_m2_global_sequence_bones(&bytes).is_empty(); + rows.push((slack, short, keyed || gseq, half_diag, name)); + } + rows.sort_by(|a, b| b.0.total_cmp(&a.0)); + let over = |t: f32| rows.iter().filter(|r| r.0 > t && r.2).count(); + println!("{scanned} models with geometry"); + println!( + " animated: {}", + rows.iter().filter(|r| r.2).count() + ); + for t in [1.0f32, 5.0, 10.0, 20.0] { + println!(" animated with SLACK > {t:>5.1} yd: {}", over(t)); + } + println!( + " models whose authored box does NOT contain their bind pose: {}", + rows.iter().filter(|r| r.1 > 0.01).count() + ); + println!(); + println!( + "{:>9} {:>8} {:>9} {:>4} MODEL", + "SLACK", "SHORT", "BIND-R", "ANIM" + ); + // The class the bug is about: the authored box dwarfs the body, so the animation carries the + // model clean out of any bind-pose bound. Ranked by that ratio, not by raw slack — a 200 yd + // waterfall with 200 yd of slack is never culled either way. + let mut ranked: Vec<&(f32, f32, bool, f32, String)> = + rows.iter().filter(|r| r.2 && r.0 > 1.0).collect(); + ranked.sort_by(|a, b| (b.0 / b.3.max(0.05)).total_cmp(&(a.0 / a.3.max(0.05)))); + for (slack, short, _, half_diag, name) in ranked.iter().take(40) { + println!( + "{slack:9.2} {short:8.2} {half_diag:9.2} {:>4} {name}", + "yes" + ); + } + Ok(()) +} diff --git a/crates/benilla-formats/src/bin/benilla-extract/scan/mod.rs b/crates/benilla-formats/src/bin/benilla-extract/scan/mod.rs index 0543e32b..a95a0350 100644 --- a/crates/benilla-formats/src/bin/benilla-extract/scan/mod.rs +++ b/crates/benilla-formats/src/bin/benilla-extract/scan/mod.rs @@ -27,7 +27,7 @@ mod sequence; mod skeleton; mod world; -pub use geometry::{bbfacescan, bbscan, geosetscan, groundscan}; +pub use geometry::{animboundscan, bbfacescan, bbscan, geosetscan, groundscan}; pub use lighting::{darkpropscan, m2lightscan, shadeat}; pub use material::{alphascan, blendscan, envmapscan, texmodescan, uvwrapscan}; pub use particles::{ diff --git a/crates/benilla-formats/tests/m2_authored_box_is_the_animated_extent.rs b/crates/benilla-formats/tests/m2_authored_box_is_the_animated_extent.rs new file mode 100644 index 00000000..d51941ee --- /dev/null +++ b/crates/benilla-formats/tests/m2_authored_box_is_the_animated_extent.rs @@ -0,0 +1,90 @@ +//! The birds that blink out when you turn the camera (decision 1259). +//! +//! `World\critter\birds\Bird01.m2` is the model that separates a **bind-pose** bound from an +//! **all-animation** one. Its geometry is a single 1.2 × 1.8 × 0.23 yd bird, modelled once at the +//! origin — and its only sequence keys bone 0 with a translation track that flies that bird 64 yd +//! along X and 17 yd along Y, a slow circuit over the treetops. A placed doodad's submesh entity +//! keeps its transform at the placement origin and lets the joint palette move the vertices, so a +//! bound derived from the bind-pose mesh describes a 1-yd box the bird has long since left: turn the +//! camera until that box exits the frustum and the bird vanishes while still on screen. +//! +//! The authored header box is the fix's source, and this pins that it really is the animated extent +//! — off the shipped file, so the premise can't rot. What consumes it (`m2_anim_bound` → the +//! per-submesh `Aabb` of an animated placement) is pinned by `benilla-world`'s own unit tests. +//! +//! Skips (passes) when the client isn't present at `/WoW/Data`. + +use benilla_formats::{open_chain, parse_m2_animations, parse_m2_bounds}; + +const BIRD: &str = "World\\critter\\birds\\Bird01.m2"; + +#[test] +fn the_birds_authored_box_covers_a_flight_path_its_bind_pose_never_hints_at() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = open_chain(&data).expect("open vanilla patch chain"); + let bytes = chain.read_file(BIRD).expect("Bird01.m2 in the chain"); + + // 1. The mechanism: bone 0 — the root every other bone hangs off — is keyed with a translation + // track tens of yards wide. Without this the model would need no special bound at all. + let anims = parse_m2_animations(&bytes); + let root = anims + .iter() + .flat_map(|a| a.bones.iter()) + .find(|b| b.bone == 0) + .expect("Bird01 keys bone 0"); + let (mut lo, mut hi) = ([f32::MAX; 3], [f32::MIN; 3]); + for (_, t) in &root.translation { + for a in 0..3 { + lo[a] = lo[a].min(t[a]); + hi[a] = hi[a].max(t[a]); + } + } + assert!( + hi[0] - lo[0] > 60.0, + "bone 0 should fly the bird >60 yd along X, got {}", + hi[0] - lo[0] + ); + + // 2. The premise: the AUTHORED header box is that flight path, not the bird. Its half-diagonal + // is the reference's own cull radius (`rec+0x68` = `bounding_sphere_radius × scale`, + // wow-5875-re `terrain/scratch/doodad-emitter-drawset-gate.md` §1c) — so a ~35 yd sphere + // around a 0.6 yd body is what the real client tests, and it never blinks. + let b = parse_m2_bounds(&bytes).expect("Bird01 bounds"); + let box_x = b.bbox_max[0] - b.bbox_min[0]; + assert!( + box_x > 60.0, + "the authored box should span the whole circuit, got {box_x} yd on X" + ); + assert!( + b.sphere_radius > 30.0, + "the authored sphere radius should be the circuit's, got {}", + b.sphere_radius + ); + + // 3. …and the bind pose is not remotely it: the vertex extent — the box Bevy's + // `calculate_bounds` derives, and what a placed submesh was culled with — is one small bird. + let model = benilla_m2::parse_m2(&mut std::io::Cursor::new(&bytes[..])).expect("parse Bird01"); + let (mut vlo, mut vhi) = ([f32::MAX; 3], [f32::MIN; 3]); + for v in &model.model().vertices { + for (a, c) in [v.position.x, v.position.y, v.position.z] + .into_iter() + .enumerate() + { + vlo[a] = vlo[a].min(c); + vhi[a] = vhi[a].max(c); + } + } + let widest = (0..3).fold(0.0f32, |w, a| w.max(vhi[a] - vlo[a])); + assert!( + widest < 3.0, + "the bind-pose geometry is one small bird, widest axis {widest} yd" + ); + // The gap IS the bug: the authored box reaches this far past the bind-pose box. + let slack = (0..3).fold(0.0f32, |s, a| { + s.max(vlo[a] - b.bbox_min[a]).max(b.bbox_max[a] - vhi[a]) + }); + assert!( + slack > 30.0, + "the authored box should reach tens of yards past the bind pose, got {slack} yd" + ); +} diff --git a/crates/benilla-protocol/Cargo.toml b/crates/benilla-protocol/Cargo.toml index 52739981..7bada359 100644 --- a/crates/benilla-protocol/Cargo.toml +++ b/crates/benilla-protocol/Cargo.toml @@ -10,9 +10,9 @@ anyhow.workspace = true clap.workspace = true benilla-srp = { path = "../benilla-srp" } flate2 = "1.0" +sha1.workspace = true [dev-dependencies] # The SRP probes re-derive the handshake by hand (to vary what `benilla-srp` fixes) — examples only. num-bigint = "0.4.6" rand = "0.8.6" -sha1 = "0.10.6" diff --git a/crates/benilla-protocol/examples/srp_encoding_probe.rs b/crates/benilla-protocol/examples/srp_encoding_probe.rs index b8908695..23e8d1ed 100644 --- a/crates/benilla-protocol/examples/srp_encoding_probe.rs +++ b/crates/benilla-protocol/examples/srp_encoding_probe.rs @@ -285,7 +285,8 @@ fn handshake( let attempt = compute(&reply, user, pass, enc, want, tries) .ok_or_else(|| anyhow!("ephemeral search did not reach the wanted shape"))?; - auth::write_logon_proof(&mut s, &attempt.a_pub, &attempt.m1).context("sending logon proof")?; + auth::write_logon_proof(&mut s, &attempt.a_pub, &attempt.m1, &reply.crc_salt) + .context("sending logon proof")?; let outcome = match auth::read_proof_reply(&mut s) { Ok(m2) => Outcome::Accepted { m2_fixed: m2 == attempt.expected_m2(Enc::Fixed), diff --git a/crates/benilla-protocol/examples/version_check_probe.rs b/crates/benilla-protocol/examples/version_check_probe.rs new file mode 100644 index 00000000..d05cdcc3 --- /dev/null +++ b/crates/benilla-protocol/examples/version_check_probe.rs @@ -0,0 +1,137 @@ +//! `version_check_probe` — the B241 instrument (decision 1263): does this realmd enforce +//! `StrictVersionCheck`, and does our logon proof satisfy it? +//! +//! Runs the same handshake **twice against the same server**, differing only in the proof packet's +//! `crc_hash`: +//! +//! - **zeros** — what benilla sent for its whole life before 1263. A strict realmd answers this with +//! `WOW_FAIL_VERSION_INVALID` (0x09) and logs *"tried to login with modified client!"*; 0x09 is the +//! code our login screen renders as `AUTH_VERSION_MISMATCH` / "Wrong client version", which is +//! exactly how B241 was reported. +//! - **computed** — `SHA1(A ‖ H)` with the integrity digest for our build and OS +//! ([`auth::version_proof`]). +//! +//! A non-strict server accepts both arms (it never looks), so the probe reports *what the server +//! enforces* as much as what we send. That is the point: it is the A/B that closes B241 and the one +//! to run against any server a strict-mode rejection is reported from. +//! +//! Usage: `cargo run -p benilla-protocol --example version_check_probe -- ` +//! (the local strict-mode realmd of decision 1263 is `127.0.0.1:3725`; the ordinary one is 3724.) + +use std::net::TcpStream; +use std::time::Duration; + +use anyhow::{anyhow, Context, Result}; +use benilla_protocol::{auth, host_port, AuthReject, AUTH_PORT, CLIENT_BUILD}; +use benilla_srp::{NormalizedString, PublicKey, SrpClientChallenge}; + +/// Which `crc_hash` the arm puts in the proof packet. +#[derive(Clone, Copy, PartialEq, Eq)] +enum Arm { + /// Twenty zeros — pre-1263 benilla. Forced by handing the writer a `crc_salt` we hold no digest + /// for, which is precisely the "no answer" path. + Zeros, + /// `SHA1(A ‖ H)` for the salt the server actually sent. + Computed, +} + +impl Arm { + fn label(self) -> &'static str { + match self { + Arm::Zeros => "zeros (pre-1263)", + Arm::Computed => "computed (1263) ", + } + } +} + +/// One full handshake, sending `arm`'s `crc_hash`. `Ok(None)` = accepted, `Ok(Some(code))` = the +/// server's `WOW_FAIL_*` byte. +fn attempt(host: &str, port: u16, user: &str, pass: &str, arm: Arm) -> Result> { + let user_n = NormalizedString::new(user).map_err(|e| anyhow!("invalid username: {e}"))?; + let pass_n = NormalizedString::new(pass).map_err(|e| anyhow!("invalid password: {e}"))?; + + // Redial past an ambiguously-serialized `B` exactly as `logon` does, so a 0x04 here means a bad + // password and never an encoding coin-flip (see `benilla-srp`, "Encoding-unambiguous handshakes"). + for _ in 0..8 { + let mut stream = TcpStream::connect((host, port)) + .with_context(|| format!("connecting to {host}:{port}"))?; + stream.set_read_timeout(Some(Duration::from_secs(10)))?; + auth::write_logon_challenge(&mut stream, &user.to_uppercase(), CLIENT_BUILD) + .context("sending logon challenge")?; + let reply = auth::read_challenge_reply(&mut stream).context("reading challenge reply")?; + let b = PublicKey::from_le_bytes(reply.server_public_key) + .map_err(|e| anyhow!("invalid server public key: {e}"))?; + if !b.is_width_stable() { + continue; + } + + let challenge = SrpClientChallenge::new( + user_n, + pass_n, + reply.generator, + reply.large_safe_prime, + b, + reply.salt, + ); + let salt = match arm { + Arm::Computed => reply.crc_salt, + Arm::Zeros => [0u8; 16], + }; + auth::write_logon_proof( + &mut stream, + challenge.client_public_key(), + challenge.client_proof(), + &salt, + ) + .context("sending logon proof")?; + + return match auth::read_proof_reply(&mut stream) { + Ok(_) => Ok(None), + Err(e) => match e.downcast_ref::() { + Some(r) => Ok(Some(r.code)), + None => Err(e), + }, + }; + } + Err(anyhow!("eight dials, every `B` ambiguous — try again")) +} + +fn main() -> Result<()> { + let args: Vec = std::env::args().skip(1).collect(); + let [host_arg, user, pass] = match args.as_slice() { + [h, u, p] => [h.clone(), u.clone(), p.clone()], + _ => { + eprintln!("usage: version_check_probe "); + std::process::exit(2); + } + }; + let (host, port) = host_port(&host_arg, AUTH_PORT); + + // Report the challenge itself first: our digest is only valid for the one salt every mangos-family + // realmd sends, so a server issuing anything else explains a `zeros` result on the computed arm. + let mut stream = + TcpStream::connect((host, port)).with_context(|| format!("connecting to {host}:{port}"))?; + stream.set_read_timeout(Some(Duration::from_secs(10)))?; + auth::write_logon_challenge(&mut stream, &user.to_uppercase(), CLIENT_BUILD)?; + let crc_salt = auth::read_challenge_reply(&mut stream)?.crc_salt; + drop(stream); // no proof follows, so realmd records nothing for this dial + let hex: String = crc_salt.iter().map(|b| format!("{b:02x}")).collect(); + let known = auth::version_proof(&crc_salt, &[0u8; 32]) != [0u8; 20]; + println!("{host}:{port} — build {CLIENT_BUILD}, crc_salt {hex}"); + println!( + " we {} an integrity digest for that challenge\n", + if known { "HOLD" } else { "hold NO" } + ); + + for arm in [Arm::Zeros, Arm::Computed] { + let verdict = match attempt(host, port, &user, &pass, arm)? { + None => "ACCEPTED".to_string(), + Some(0x09) => { + "REJECTED 0x09 WOW_FAIL_VERSION_INVALID (\"Wrong client version\")".into() + } + Some(c) => format!("REJECTED {c:#04x}"), + }; + println!(" crc_hash {} → {verdict}", arm.label()); + } + Ok(()) +} diff --git a/crates/benilla-protocol/src/auth.rs b/crates/benilla-protocol/src/auth.rs index e61fcfba..35abb741 100644 --- a/crates/benilla-protocol/src/auth.rs +++ b/crates/benilla-protocol/src/auth.rs @@ -9,6 +9,7 @@ use std::io::{Read, Write}; use std::net::Ipv4Addr; use anyhow::{bail, Result}; +use sha1::{Digest, Sha1}; use crate::wire::{read_array, read_cstring, read_f32_le, read_u16_le, read_u32_le, read_u8}; use crate::RealmInfo; @@ -66,6 +67,9 @@ pub struct ChallengeReply { pub generator: u8, pub large_safe_prime: [u8; 32], pub salt: [u8; 32], + /// The 16-byte version challenge the client answers with `crc_hash` — see [`version_proof`]. + /// Not an SRP6 input; it seeds the client's binary-integrity digest. + pub crc_salt: [u8; 16], } /// Send `CMD_AUTH_LOGON_CHALLENGE_Client`. `account_name` must already be uppercased (it is what the @@ -126,9 +130,10 @@ pub fn read_challenge_reply(r: &mut impl Read) -> Result { .map_err(|_| anyhow::anyhow!("large safe prime was {prime_len} bytes, expected 32"))?; let salt = read_array::<32>(r)?; // Consume the rest of the packet so the stream stays aligned for the next message — login packets - // are NOT length-framed, so leftover bytes desync the very next read. crc_salt[16] + security_flag - // (+ the PIN block if set) are unused for the SRP6 calculation but must still be read off. - let _crc_salt = read_array::<16>(r)?; + // are NOT length-framed, so leftover bytes desync the very next read. crc_salt[16] feeds the + // integrity proof (not SRP6); security_flag (+ the PIN block if set) is unused but must still be + // read off. + let crc_salt = read_array::<16>(r)?; let security_flag = read_u8(r)?; if security_flag & 0x01 != 0 { // PIN: pin_grid_seed (u32) + pin_salt[16]. (vmangos sends security_flag = 0; handled anyway.) @@ -140,21 +145,94 @@ pub fn read_challenge_reply(r: &mut impl Read) -> Result { generator, large_safe_prime, salt, + crc_salt, }) } -/// Send `CMD_AUTH_LOGON_PROOF_Client`. `crc_hash` is the client-binary checksum (vmangos accepts -/// zeros); no telemetry keys, no security flag. +// --- the version (client-integrity) proof --------------------------------------------------------- +// +// The proof packet's `crc_hash` answers the challenge's `crc_salt`: the real client scans its own +// binaries under that salt into a 20-byte digest `H`, and sends `SHA1(A ‖ H)`. realmd recomputes the +// same expression from a stored `H` and rejects a mismatch as a *modified client* +// (vmangos `AuthSocket::VerifyVersion`, cmangos ditto — byte-identical implementations). We sent +// twenty zeros here for the project's whole life, which is invisible until a server turns the check +// on: `StrictVersionCheck = 1` then answers a correct password with `WOW_FAIL_VERSION_INVALID` (0x09, +// our login screen's "Wrong client version") — B241. Note vmangos's own shipped +// `realmd.conf.example` sets 1, so this is a *default* on a fresh repack, not exotic hardening. + +/// The 16-byte `crc_salt` every mangos-family realmd sends — a **constant, not a nonce** (vmangos +/// `realmd/AuthSocket.cpp:65` and cmangos `realmd/AuthSocket.cpp:187`, two independent copies of the +/// same bytes). The whole scheme rests on that: because every server issues the same challenge, the +/// client's answer is a per-build constant a server can store without owning the client. +const MANGOS_VERSION_CHALLENGE: [u8; 16] = [ + 0xba, 0xa3, 0x1e, 0x99, 0xa0, 0x0b, 0x21, 0x57, 0xfc, 0x37, 0x3f, 0xb3, 0x69, 0xcd, 0xd2, 0xf1, +]; + +/// `H` for build 5875 under [`MANGOS_VERSION_CHALLENGE`], Windows client. Cross-checked between two +/// independent emulator lineages: vmangos ships it as DB data (`sql/migrations/20221117065844_logon.sql`, +/// `allowed_clients` row `1,12,1,'',5875,'Win','x86'`), cmangos as a hardcoded table +/// (`realmd/RealmList.cpp`, `ExpectedRealmdClientBuilds`) — byte-identical. +const INTEGRITY_HASH_5875_WINDOWS: [u8; 20] = [ + 0x95, 0xed, 0xb2, 0x7c, 0x78, 0x23, 0xb3, 0x63, 0xcb, 0xdd, 0xab, 0x56, 0xa3, 0x92, 0xe7, 0xcb, + 0x73, 0xfc, 0xca, 0x20, +]; + +/// `H` for build 5875 under [`MANGOS_VERSION_CHALLENGE`], Mac client — same two sources, and the one +/// we must answer with on macOS, where [`client_os`] says `OSX` (vmangos keys its lookup on the os +/// field, cmangos picks `MacHash` off the same field). +const INTEGRITY_HASH_5875_MACOS: [u8; 20] = [ + 0x8d, 0x17, 0x3c, 0xc3, 0x81, 0x96, 0x1e, 0xeb, 0xab, 0xf3, 0x36, 0xf5, 0xe6, 0x67, 0x5b, 0x10, + 0x1b, 0xb5, 0x13, 0xe5, +]; + +/// The 20-byte integrity digest to answer `crc_salt` with, or `None` when we have no answer for that +/// salt. +/// +/// We hold `H` only for the one salt above, so a server that issues a *different* one gets zeros +/// rather than a stale constant. That is not defensive padding: replaying a constant computed for +/// another challenge is exactly the tell a randomised challenge would be looking for, and it cannot +/// pass anyway. (No server in the mangos family randomises it — a per-connection salt would make +/// their own stored hashes useless — so this branch is the honest answer to a hypothetical, and free.) +fn integrity_hash(crc_salt: &[u8; 16]) -> Option<[u8; 20]> { + if *crc_salt != MANGOS_VERSION_CHALLENGE { + return None; + } + Some(if client_os() == OS_MACOS { + INTEGRITY_HASH_5875_MACOS + } else { + INTEGRITY_HASH_5875_WINDOWS + }) +} + +/// The proof packet's `crc_hash`: `SHA1(A ‖ H)` over the wire bytes of `A` (realmd hashes `lp->A` +/// as received, so this is the same 32 bytes we put in the packet). Twenty zeros when we have no +/// `H` for this salt — the pre-B241 behaviour, which a non-strict server ignores. +pub fn version_proof(crc_salt: &[u8; 16], client_public_key: &[u8; 32]) -> [u8; 20] { + match integrity_hash(crc_salt) { + Some(h) => { + let mut sha = Sha1::new(); + sha.update(client_public_key); + sha.update(h); + sha.finalize().into() + } + None => [0u8; 20], + } +} + +/// Send `CMD_AUTH_LOGON_PROOF_Client`. `crc_salt` is the challenge's — the packet's `crc_hash` is +/// computed here ([`version_proof`]) rather than passed in, so no caller can forget it. No telemetry +/// keys, no security flag. pub fn write_logon_proof( w: &mut impl Write, client_public_key: &[u8; 32], client_proof: &[u8; 20], + crc_salt: &[u8; 16], ) -> std::io::Result<()> { let mut packet = Vec::with_capacity(1 + 32 + 20 + 20 + 1 + 1); packet.push(CMD_AUTH_LOGON_PROOF); packet.extend_from_slice(client_public_key); packet.extend_from_slice(client_proof); - packet.extend_from_slice(&[0u8; 20]); // crc_hash + packet.extend_from_slice(&version_proof(crc_salt, client_public_key)); packet.push(0); // number_of_telemetry_keys packet.push(0); // security_flag = None w.write_all(&packet) @@ -242,4 +320,55 @@ mod tests { assert_eq!(client_os(), OS_WINDOWS); } } + + /// An arbitrary but fixed `A` for the version-proof vectors below. + fn test_public_key() -> [u8; 32] { + std::array::from_fn(|i| (i as u8).wrapping_mul(7).wrapping_add(9)) + } + + /// Known answers, computed *outside* this crate (Python `hashlib`) from realmd's own expression + /// `SHA1(A ‖ H)` and the emulator-sourced `H`. A regression here means either constant moved or + /// the hash grew an extra input — both of which read as "modified client" to a strict server and + /// are otherwise invisible until someone tries to log in. + #[test] + fn version_proof_matches_the_realmd_expression() { + let expected = if cfg!(target_os = "macos") { + "dccd0e67946fae221451513b1a59724090ff6096" + } else { + "9b95cd41edd719fddf237294b8aca17010e71703" + }; + let got = version_proof(&MANGOS_VERSION_CHALLENGE, &test_public_key()); + assert_eq!( + got.iter().map(|b| format!("{b:02x}")).collect::(), + expected + ); + } + + /// A salt we hold no `H` for is answered with zeros, not with the constant for another salt. + #[test] + fn an_unknown_version_challenge_is_answered_with_zeros() { + let mut salt = MANGOS_VERSION_CHALLENGE; + salt[0] ^= 0xff; + assert_eq!(version_proof(&salt, &test_public_key()), [0u8; 20]); + } + + /// The proof packet carries the answer at the offset realmd reads it from: `opcode · A[32] · + /// M1[20] · crc_hash[20] · num_keys · security_flag`, 75 bytes total. + #[test] + fn the_proof_packet_carries_the_version_proof() { + let a = test_public_key(); + let m1: [u8; 20] = std::array::from_fn(|i| (i as u8).wrapping_mul(13).wrapping_add(4)); + let mut packet = Vec::new(); + write_logon_proof(&mut packet, &a, &m1, &MANGOS_VERSION_CHALLENGE).unwrap(); + + assert_eq!(packet.len(), 1 + 32 + 20 + 20 + 1 + 1); + assert_eq!(packet[0], CMD_AUTH_LOGON_PROOF); + assert_eq!(&packet[1..33], &a); + assert_eq!(&packet[33..53], &m1); + assert_eq!( + &packet[53..73], + &version_proof(&MANGOS_VERSION_CHALLENGE, &a) + ); + assert_eq!(&packet[73..], &[0, 0]); + } } diff --git a/crates/benilla-protocol/src/lib.rs b/crates/benilla-protocol/src/lib.rs index 7512b0b3..665f7e7f 100644 --- a/crates/benilla-protocol/src/lib.rs +++ b/crates/benilla-protocol/src/lib.rs @@ -128,6 +128,7 @@ pub fn logon(host: &str, username: &str, password: &str) -> Result { &mut stream, challenge.client_public_key(), challenge.client_proof(), + &reply.crc_salt, ) .context("sending logon proof")?; diff --git a/crates/benilla-protocol/tests/auth.rs b/crates/benilla-protocol/tests/auth.rs index ee1c4126..49f1df1a 100644 --- a/crates/benilla-protocol/tests/auth.rs +++ b/crates/benilla-protocol/tests/auth.rs @@ -8,6 +8,13 @@ use benilla_protocol::auth; const N: [u8; 32] = benilla_srp::LARGE_SAFE_PRIME_LITTLE_ENDIAN; +/// The `crc_salt` a real mangos-family realmd sends (its fixed `VersionChallenge`). The fixture uses +/// the live bytes rather than zeros, so the reader meets what it actually meets — and so the `got +/// 0xba` in the proof-read comment below names a real byte of this array. +const CRC_SALT: [u8; 16] = [ + 0xba, 0xa3, 0x1e, 0x99, 0xa0, 0x0b, 0x21, 0x57, 0xfc, 0x37, 0x3f, 0xb3, 0x69, 0xcd, 0xd2, 0xf1, +]; + /// A successful `CMD_AUTH_LOGON_CHALLENGE_Server` body, including the `crc_salt` + `security_flag` /// tail that must be consumed. fn challenge_packet(server_public_key: &[u8; 32], salt: &[u8; 32]) -> Vec { @@ -18,7 +25,7 @@ fn challenge_packet(server_public_key: &[u8; 32], salt: &[u8; 32]) -> Vec { p.push(32); // large-safe-prime length p.extend_from_slice(&N); p.extend_from_slice(salt); - p.extend_from_slice(&[0u8; 16]); // crc_salt + p.extend_from_slice(&CRC_SALT); p.push(0); // security_flag = None p } @@ -72,6 +79,7 @@ fn logon_read_sequence_stays_aligned() { assert_eq!(reply.generator, 7); assert_eq!(reply.large_safe_prime, N); assert_eq!(reply.salt, salt); + assert_eq!(reply.crc_salt, CRC_SALT); // the proof's `crc_hash` is computed from this // 2. Proof — this is exactly where a challenge-reply under-read used to surface (`got 0xba`). let proof = diff --git a/crates/benilla-srp/Cargo.toml b/crates/benilla-srp/Cargo.toml index 2f192d2b..15111bc8 100644 --- a/crates/benilla-srp/Cargo.toml +++ b/crates/benilla-srp/Cargo.toml @@ -7,5 +7,5 @@ description = "WoW 1.12.1 (build 5875) SRP6 client + vanilla world-header crypto [dependencies] num-bigint = "0.4.6" -sha1 = "0.10.6" +sha1.workspace = true rand = "0.8.6" diff --git a/crates/benilla-world/src/doodad_anim/lazy.rs b/crates/benilla-world/src/doodad_anim/lazy.rs index dbe76c74..931b11b2 100644 --- a/crates/benilla-world/src/doodad_anim/lazy.rs +++ b/crates/benilla-world/src/doodad_anim/lazy.rs @@ -447,3 +447,88 @@ mod tests { assert_eq!(app.world().resource::().occupancy().0, 0); } } + +/// The Bevy contract this lane's `Aabb` depends on (decision 1261), pinned against the real +/// `calculate_bounds` system rather than a reading of it. +/// +/// `promote_lazy_rig`/`demote_lazy_rig` write `Mesh3d`. Bevy's `calculate_bounds` runs two +/// queries — an inserting one filtered `Without`, and an **updating** one filtered +/// `Or<(AssetChanged, Changed)>` that overwrites a bound the app authored. So the +/// authored all-animation bound decision 1259 puts on an animated placement survives exactly as +/// long as nothing swaps that placement's mesh — i.e. until its first draw-gate wake, at which +/// point it was silently recomputed from the skinned twin's bind-pose geometry and the birds +/// resumed blinking. `NoAutoAabb` (which both queries exclude) is the fix; this is the guard that +/// a Bevy upgrade cannot quietly take it back. +#[cfg(test)] +mod bound_survives_the_twin_swap { + use super::*; + use bevy::camera::primitives::Aabb; + use bevy::camera::visibility::NoAutoAabb; + + /// A 1 yd cube — what the skinned twin's bind-pose geometry computes to. + fn tiny_mesh() -> Mesh { + Mesh::from(bevy::math::primitives::Cuboid::new(1.0, 1.0, 1.0)) + } + + /// The authored all-animation bound: a bird's 67 yd circuit around that 1 yd body. + fn authored() -> Aabb { + Aabb::from_min_max(Vec3::new(-4.2, 8.8, -30.5), Vec3::new(13.6, 16.0, 36.6)) + } + + /// `(app, entity)` with Bevy's own bounds system on the schedule and the authored bound in + /// place, mid-swap: `Mesh3d` has just been rewritten the way `promote_lazy_rig` rewrites it. + fn swapped(guard: bool) -> (App, Entity) { + let mut app = App::new(); + app.add_plugins((bevy::MinimalPlugins, AssetPlugin::default())); + app.init_asset::(); + app.add_systems(Update, bevy::camera::visibility::calculate_bounds); + let stat = app + .world_mut() + .resource_mut::>() + .add(tiny_mesh()); + let skinned = app + .world_mut() + .resource_mut::>() + .add(tiny_mesh()); + let e = app.world_mut().spawn((Mesh3d(stat), authored())).id(); + if guard { + app.world_mut().entity_mut(e).insert(NoAutoAabb); + } + app.update(); + // The swap the lazy rig performs at the placement's first wake. + app.world_mut().entity_mut(e).get_mut::().unwrap().0 = skinned; + app.update(); + (app, e) + } + + fn bound(app: &App, e: Entity) -> Aabb { + *app.world().entity(e).get::().expect("a bound") + } + + /// The bug: unguarded, the twin swap hands the authored bound back to `compute_aabb`. + #[test] + fn an_unguarded_bound_is_clobbered_by_the_mesh_swap() { + let (app, e) = swapped(false); + let b = bound(&app, e); + assert!( + b.half_extents.max_element() < 1.0, + "expected the 1 yd cube's own bound, got half-extents {:?} — if this now keeps the \ + authored box, Bevy changed `calculate_bounds` and `NoAutoAabb` may be unnecessary", + b.half_extents + ); + } + + /// The fix: guarded, the authored bound is still the authored bound after the swap. + #[test] + fn a_guarded_bound_survives_the_mesh_swap() { + let (app, e) = swapped(true); + let b = bound(&app, e); + assert!( + (Vec3::from(b.min()) - Vec3::from(authored().min())).length() < 1e-3 + && (Vec3::from(b.max()) - Vec3::from(authored().max())).length() < 1e-3, + "the authored all-animation bound must survive the promote, got {:?}..{:?}", + b.min(), + b.max() + ); + } +} diff --git a/crates/benilla-world/src/model_render.rs b/crates/benilla-world/src/model_render.rs index 75ce1cd0..95bac375 100644 --- a/crates/benilla-world/src/model_render.rs +++ b/crates/benilla-world/src/model_render.rs @@ -976,6 +976,16 @@ pub fn plugin(app: &mut App) { .after(crate::wmo_portal::WmoPvsSet) .in_set(ModelVisSet), ); + // `WOW_VIS_TRACE=