From 7187e841b6a38b34851e16a7d17f10c499393afd Mon Sep 17 00:00:00 2001 From: LynithDev <61880709+LynithDev@users.noreply.github.com> Date: Fri, 31 Jul 2026 23:22:14 +0200 Subject: [PATCH] fix(core): attempt to fix requesting issues --- Cargo.lock | 30 ++++- Cargo.toml | 7 ++ packages/oneclient_auth/src/diagnostics.rs | 130 ++++++++++++++++++--- packages/oneclient_auth/src/error.rs | 5 +- packages/oneclient_net/Cargo.toml | 3 + packages/oneclient_net/src/error.rs | 27 ++++- packages/oneclient_net/src/lib.rs | 2 +- packages/oneclient_net/src/service.rs | 36 +++++- 8 files changed, 220 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ca671769..376cced3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3045,9 +3045,11 @@ dependencies = [ "percent-encoding", "pin-project-lite", "socket2 0.6.4", + "system-configuration", "tokio", "tower-service", "tracing", + "windows-registry", ] [[package]] @@ -3062,7 +3064,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.56.0", + "windows-core 0.62.2", ] [[package]] @@ -4809,6 +4811,7 @@ dependencies = [ "tracing", "url", "uuid 1.23.1", + "webpki-root-certs", ] [[package]] @@ -6815,6 +6818,27 @@ dependencies = [ "windows 0.62.2", ] +[[package]] +name = "system-configuration" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a13f3d0daba03132c0aa9767f98351b3488edc2c100cda2d2ec2b04f3d8d3c8b" +dependencies = [ + "bitflags 2.12.1", + "core-foundation 0.9.4", + "system-configuration-sys", +] + +[[package]] +name = "system-configuration-sys" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e1d1b10ced5ca923a1fcb8d03e96b8d3268065d724548c0211415ff6ac6bac4" +dependencies = [ + "core-foundation-sys", + "libc", +] + [[package]] name = "tagptr" version = "0.2.0" @@ -7864,9 +7888,9 @@ dependencies = [ [[package]] name = "webpki-root-certs" -version = "1.0.7" +version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f31141ce3fc3e300ae89b78c0dd67f9708061d1d2eda54b8209346fd6be9a92c" +checksum = "b96554aa2acc8ccdb7e1c9a58a7a68dd5d13bccc69cd124cb09406db612a1c9b" dependencies = [ "rustls-pki-types", ] diff --git a/Cargo.toml b/Cargo.toml index 486f2e78..967a9d25 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -85,8 +85,15 @@ reqwest = {version = "=0.13.3", default-features = false, features = [ "brotli", "form", "hickory-dns", + # Without this reqwest sees only the HTTP_PROXY/HTTPS_PROXY env vars, and + # ignores the proxy configured in Windows Internet Options or macOS network + # preferences — which is how school and workplace networks are set up. Those + # users could reach every service in a browser but not from here. + "system-proxy", ]} reqwest-websocket = {version = "=0.6.0"} +# Fallback trust roots for Windows. See `oneclient_net::service` for why. +webpki-root-certs = {version = "=1.0.9"} tokio = {version = "=1.52.3", features = ["full"]} tokio-stream = {version = "=0.1.18", features = ["full"]} tokio-util = {version = "=0.7.18", features = ["full"]} diff --git a/packages/oneclient_auth/src/diagnostics.rs b/packages/oneclient_auth/src/diagnostics.rs index 3e41cbda..46cf5367 100644 --- a/packages/oneclient_auth/src/diagnostics.rs +++ b/packages/oneclient_auth/src/diagnostics.rs @@ -31,7 +31,9 @@ pub fn diagnose_auth_error(err: &MinecraftAuthError) -> Option guidance_for_step(*step), - MinecraftAuthError::RequestError { .. } => Some(network_guidance()), + MinecraftAuthError::RequestError { source, .. } => { + Some(network_guidance(classify_network_failure(source))) + } MinecraftAuthError::BrowserAuthorizationExpired | MinecraftAuthError::BrowserLoginNotFound => Some(AuthErrorGuidance::new( @@ -271,7 +273,22 @@ pub fn preview_samples() -> Vec { out.push(AuthErrorSample { label: "Network unreachable", message: "error sending request for url (https://user.auth.xboxlive.com/user/authenticate): connection closed".to_string(), - guidance: Some(network_guidance()), + guidance: Some(network_guidance(NetworkFailure::Generic)), + }); + out.push(AuthErrorSample { + label: "TLS certificate rejected", + message: "error sending request for url (https://login.microsoftonline.com/consumers/oauth2/v2.0/devicecode): invalid peer certificate: UnknownIssuer".to_string(), + guidance: Some(network_guidance(NetworkFailure::Certificate)), + }); + out.push(AuthErrorSample { + label: "TLS handshake failed", + message: "error sending request for url (https://login.microsoftonline.com/consumers/oauth2/v2.0/devicecode): received fatal alert: HandshakeFailure".to_string(), + guidance: Some(network_guidance(NetworkFailure::Handshake)), + }); + out.push(AuthErrorSample { + label: "DNS lookup failed", + message: "error sending request for url (https://login.microsoftonline.com/consumers/oauth2/v2.0/devicecode): dns error: failed to lookup address information".to_string(), + guidance: Some(network_guidance(NetworkFailure::Dns)), }); out } @@ -285,15 +302,102 @@ fn xerr(code: u64) -> MinecraftAuthError { } } -fn network_guidance() -> AuthErrorGuidance { - AuthErrorGuidance::new( - "OneClient could not connect to a Microsoft, Xbox, or Minecraft service needed for sign-in. This is usually a local network, DNS, proxy, firewall, hosts file, VPN, or antivirus issue.", - &[ - "Restart OneClient and try signing in again", - "Check that your internet connection is working", - "Allow OneClient through your firewall, antivirus, proxy, VPN, and hosts file rules", - "Try a different network, or temporarily disable VPN/proxy software if you use one", - "If routing or DNS is the issue, a service like Cloudflare WARP can sometimes help", - ], - ) +/// Which layer of the connection failed. +/// +/// reqwest reports every one of these as a connect error, so the text of the +/// source chain is the only thing that separates "your antivirus is +/// intercepting TLS" from "your DNS is broken" — and they need different +/// instructions. A wrong guess here costs the user a wasted support round, so +/// anything unrecognised falls back to the generic advice. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum NetworkFailure { + Certificate, + Handshake, + Dns, + Generic, +} + +fn classify_network_failure(err: &reqwest::Error) -> NetworkFailure { + let chain = oneclient_net::error_chain(err).to_ascii_lowercase(); + + // Certificate before handshake: a rejected certificate also aborts the + // handshake, and the certificate wording is the more specific of the two. + const CERTIFICATE: &[&str] = &[ + "invalid peer certificate", + "unknownissuer", + "certificate", + "certexpired", + "notvalidforname", + ]; + const HANDSHAKE: &[&str] = &[ + "handshake", + "received fatal alert", + "peer misbehaved", + "no cipher suites in common", + "protocol version", + "unsupported protocol", + ]; + const DNS: &[&str] = &[ + "dns error", + "failed to lookup address", + "no record found", + "nodename nor servname", + "name or service not known", + "no such host", + ]; + + let matches = |markers: &[&str]| markers.iter().any(|marker| chain.contains(marker)); + + if matches(CERTIFICATE) { + NetworkFailure::Certificate + } else if matches(HANDSHAKE) { + NetworkFailure::Handshake + } else if matches(DNS) { + NetworkFailure::Dns + } else { + NetworkFailure::Generic + } +} + +fn network_guidance(failure: NetworkFailure) -> AuthErrorGuidance { + match failure { + NetworkFailure::Certificate => AuthErrorGuidance::new( + "OneClient reached the Microsoft sign-in service, but could not verify its security certificate. Something on this machine or network is intercepting the encrypted connection — usually antivirus HTTPS scanning, a school or workplace filter, or a VPN.", + &[ + "Turn off HTTPS/SSL scanning in your antivirus (often called \"encrypted connection scanning\", \"web shield\", or \"SSL interception\")", + "Temporarily disable your VPN or proxy and try signing in again", + "If you are on a school or workplace network, try a home network or phone hotspot to confirm", + "Check that your system date, time, and time zone are correct", + ], + ), + NetworkFailure::Handshake => AuthErrorGuidance::new( + "OneClient reached the Microsoft sign-in service, but the two could not agree on a secure connection. This normally means a proxy, filter, or antivirus in between is using outdated encryption settings.", + &[ + "Turn off HTTPS/SSL scanning in your antivirus and try signing in again", + "Temporarily disable your VPN or proxy software", + "If you are on a school or workplace network, try a home network or phone hotspot to confirm", + "Make sure OneClient is up to date", + ], + ), + NetworkFailure::Dns => AuthErrorGuidance::new( + "OneClient could not look up the address of the Microsoft sign-in service. This is a DNS problem on this machine or network, not a problem with your account.", + &[ + "Check that your internet connection is working", + "Temporarily disable your VPN or proxy and try signing in again", + "Check your hosts file for entries blocking Microsoft or Minecraft domains", + "Try a different DNS server, such as Cloudflare (1.1.1.1) or Google (8.8.8.8)", + "Restart OneClient and try signing in again", + ], + ), + NetworkFailure::Generic => AuthErrorGuidance::new( + "OneClient could not connect to a Microsoft, Xbox, or Minecraft service needed for sign-in. This is usually a local network, DNS, proxy, firewall, hosts file, VPN, or antivirus issue.", + &[ + "Restart OneClient and try signing in again", + "Check that your internet connection is working", + "Allow OneClient through your firewall, antivirus, proxy, VPN, and hosts file rules", + "Try a different network, or temporarily disable VPN/proxy software if you use one", + "If routing or DNS is the issue, a service like Cloudflare WARP can sometimes help", + ], + ), + } } diff --git a/packages/oneclient_auth/src/error.rs b/packages/oneclient_auth/src/error.rs index 9128a88d..376f00f1 100644 --- a/packages/oneclient_auth/src/error.rs +++ b/packages/oneclient_auth/src/error.rs @@ -33,7 +33,10 @@ pub enum MinecraftAuthError { source: serde_json::Error, status_code: reqwest::StatusCode, }, - #[error("failed to request using HTTP during MSA step {step:?}: {source}")] + // `source` is rendered through `error_chain` rather than with `{source}`: + // reqwest's `Display` names only the URL, and the cause we need to tell + // apart (certificate, handshake, DNS, refused) is under it. + #[error("failed to request using HTTP during MSA step {step:?}: {}", oneclient_net::error_chain(source))] RequestError { step: MinecraftAuthStep, #[source] diff --git a/packages/oneclient_net/Cargo.toml b/packages/oneclient_net/Cargo.toml index 50dd5502..490b16f2 100644 --- a/packages/oneclient_net/Cargo.toml +++ b/packages/oneclient_net/Cargo.toml @@ -34,3 +34,6 @@ thiserror.workspace = true tracing.workspace = true url.workspace = true uuid.workspace = true + +[target.'cfg(windows)'.dependencies] +webpki-root-certs.workspace = true diff --git a/packages/oneclient_net/src/error.rs b/packages/oneclient_net/src/error.rs index b0886c32..d64cf0f6 100644 --- a/packages/oneclient_net/src/error.rs +++ b/packages/oneclient_net/src/error.rs @@ -1,6 +1,31 @@ +/// Renders an error together with every `source()` beneath it. +/// +/// `reqwest::Error`'s own `Display` stops at "error sending request for url +/// (...)". The fact that actually names the failure — a rustls handshake +/// alert, an untrusted certificate, a DNS lookup failure, `ECONNREFUSED` — +/// lives further down the chain, so without this every transport failure in a +/// user report reads identically and is undiagnosable. +#[must_use] +pub fn error_chain(err: &dyn std::error::Error) -> String { + let mut rendered = err.to_string(); + let mut cursor = err.source(); + + while let Some(cause) = cursor { + let text = cause.to_string(); + // hyper and reqwest repeat the same sentence across several layers. + if !rendered.ends_with(&text) { + rendered.push_str(": "); + rendered.push_str(&text); + } + cursor = cause.source(); + } + + rendered +} + #[derive(Debug, thiserror::Error)] pub enum RequestError { - #[error(transparent)] + #[error("{}", error_chain(.0))] ReqwestError(#[from] reqwest::Error), #[error("IO Error: {0}")] diff --git a/packages/oneclient_net/src/lib.rs b/packages/oneclient_net/src/lib.rs index 60c87b52..1926a046 100644 --- a/packages/oneclient_net/src/lib.rs +++ b/packages/oneclient_net/src/lib.rs @@ -19,7 +19,7 @@ pub mod status; pub use cache::{EtagPolicy, Fetched, commit_etag, fetch_cached}; pub use config::NetConfig; pub use download::{download_verified, fetch_verified, matches_on_disk}; -pub use error::RequestError; +pub use error::{RequestError, error_chain}; pub use request::*; pub use response::*; pub use service::RequestClient; diff --git a/packages/oneclient_net/src/service.rs b/packages/oneclient_net/src/service.rs index 4950fd7f..002c1fe1 100644 --- a/packages/oneclient_net/src/service.rs +++ b/packages/oneclient_net/src/service.rs @@ -81,6 +81,36 @@ fn apply_curseforge_auth( Ok(()) } +/// Adds the bundled Mozilla roots as a fallback behind the Windows trust store. +/// +/// This does not replace the system store. `rustls-platform-verifier` builds +/// the chain against Windows first and only retries with these roots when that +/// came back untrusted or partial, so an intercepting proxy or an +/// enterprise-installed CA still works exactly as before. +/// +/// It matters because Windows populates its root store lazily from Windows +/// Update. On a machine where root auto-update is disabled by policy — routine +/// on managed school and workplace images — or on a trimmed-down install, a +/// root that the rest of the world has can simply be absent. Every other +/// client those users try carries its own roots (Chrome since 105, Firefox via +/// NSS, Java-based launchers via `cacerts`), so sign-in works everywhere +/// except here. macOS and Linux do not have this failure mode, and adding +/// roots there would be additive rather than a fallback, so this is Windows +/// only. +#[cfg(target_os = "windows")] +fn add_fallback_roots(mut builder: ClientBuilder) -> ClientBuilder { + for der in webpki_root_certs::TLS_SERVER_ROOT_CERTS { + match reqwest::Certificate::from_der(der) { + Ok(root) => builder = builder.add_root_certificate(root), + // A malformed entry would be a bug in the bundle, not something the + // user can act on. Dropping one root beats failing to build a client. + Err(err) => tracing::warn!("skipping malformed fallback root: {err}"), + } + } + + builder +} + #[derive(Clone)] pub struct RequestClient { client: reqwest::Client, @@ -109,7 +139,6 @@ impl RequestClient { } pub fn new(config: NetConfig) -> Result { - #[cfg_attr(target_os = "windows", allow(unused_mut))] let mut builder = ClientBuilder::new() .connect_timeout(std::time::Duration::from_secs(10)) .read_timeout(std::time::Duration::from_secs(30)) @@ -126,6 +155,11 @@ impl RequestClient { builder = builder.hickory_dns(true); } + #[cfg(target_os = "windows")] + { + builder = add_fallback_roots(builder); + } + let client = builder.build()?; Ok(Self {