Build 33: security audit fixes — remove the clearnet bridge probe, cap untrusted responses

- SECURITY (High): drop sort_bridges_by_reachability / bridge_probe_addr. The
  Build 30 probe did clearnet DNS + direct TCP to bridge endpoints outside Tor
  on every startup, deanonymizing bridge users. The consensus-cache keep + 120s
  timeout + pre-warm remain and are the real fix for slow first connect.
- SECURITY (Low): cap HTTP response bodies from the untrusted goblin.st server
  at 2 MiB, streamed so a lying/absent Content-Length can't OOM the wallet.
This commit is contained in:
2ro
2026-06-12 12:18:57 -04:00
parent b9ce88e996
commit 60e4e8b5a9
+30 -82
View File
@@ -27,7 +27,7 @@ use parking_lot::RwLock;
use safelog::DisplayRedacted;
use sha2::Sha512;
use std::collections::{BTreeMap, BTreeSet};
use std::net::{IpAddr, Ipv4Addr, SocketAddr, TcpStream, ToSocketAddrs};
use std::net::{IpAddr, Ipv4Addr, SocketAddr};
use std::str::FromStr;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
@@ -53,6 +53,11 @@ use crate::http::HttpClient;
use crate::tor::http::ArtiHttpConnector;
use crate::tor::{TorBridge, TorConfig, TorProxy};
/// Hard ceiling on an HTTP response body read over Tor from the untrusted
/// goblin.st server. NIP-05 JSON and profiles are a few KB; avatars are capped
/// at 1 MiB by the caller. 2 MiB leaves headroom while bounding memory.
const MAX_RESPONSE_BYTES: usize = 2 * 1024 * 1024;
lazy_static! {
/// Static thread-aware state of Tor to be updated from separate thread.
static ref TOR_STATE: Arc<Tor> = Arc::new(Tor::default());
@@ -172,81 +177,6 @@ impl Tor {
(config, bridges, max_two_bridges)
}
/// Endpoint (host, port) a bridge dials first, for a quick reachability probe.
/// Webtunnel and obfs4 need a working TCP path to this endpoint before anything
/// else can happen; snowflake is CDN-fronted, so it gets no probe (None).
fn bridge_probe_addr(bridge: &TorBridge) -> Option<(String, u16)> {
let line = bridge.connection_line();
match bridge {
TorBridge::Webtunnel(_, _) => {
let url = line
.split_whitespace()
.find_map(|t| t.strip_prefix("url="))?;
let rest = url
.strip_prefix("https://")
.or_else(|| url.strip_prefix("http://"))?;
let host_port = rest.split('/').next()?;
let mut parts = host_port.splitn(2, ':');
let host = parts.next()?.to_string();
let port = parts.next().and_then(|p| p.parse().ok()).unwrap_or(443);
Some((host, port))
}
TorBridge::Obfs4(_, _) => {
let addr = line.split_whitespace().nth(1)?;
let mut parts = addr.rsplitn(2, ':');
let port = parts.next()?.parse().ok()?;
let host = parts
.next()?
.trim_start_matches('[')
.trim_end_matches(']')
.to_string();
Some((host, port))
}
TorBridge::Snowflake(_, _) => None,
}
}
/// Order bridges live-endpoint-first via a parallel TCP probe. A dead bridge
/// otherwise costs a full bootstrap timeout (60s) per attempt, which reads as
/// the wallet being stuck on "connecting…" while the list is ground through.
fn sort_bridges_by_reachability(bridges: Vec<TorBridge>) -> Vec<TorBridge> {
if bridges.len() < 2 {
return bridges;
}
let probes: Vec<(TorBridge, thread::JoinHandle<bool>)> = bridges
.into_iter()
.map(|b| {
let addr = Self::bridge_probe_addr(&b);
let handle = thread::spawn(move || match addr {
// No probeable endpoint: assume reachable.
None => true,
Some((host, port)) => (host.as_str(), port)
.to_socket_addrs()
.ok()
.and_then(|mut addrs| addrs.next())
.map(|a| {
TcpStream::connect_timeout(&a, Duration::from_millis(4000)).is_ok()
})
.unwrap_or(false),
});
(b, handle)
})
.collect();
let mut alive = vec![];
let mut dead = vec![];
for (b, handle) in probes {
if handle.join().unwrap_or(true) {
alive.push(b);
} else {
dead.push(b);
}
}
// Probes can lie on hostile networks — keep dead ones as a last resort
// rather than dropping them.
alive.extend(dead);
alive
}
/// Build bootstrapped client from provided config.
fn build_client_bootstrap(config: TorClientConfig) -> Option<TorClient<TokioNativeTlsRuntime>> {
let client_res = TorClient::with_runtime(TOR_STATE.runtime.clone())
@@ -317,7 +247,7 @@ impl Tor {
bridge
})
.collect::<Vec<TorBridge>>();
Some(Self::sort_bridges_by_reachability(bridges))
Some(bridges)
} else {
None
};
@@ -363,8 +293,7 @@ impl Tor {
.iter()
.map(|b| TorBridge::Webtunnel(TorConfig::webtunnel_path(), b.to_string()))
.collect::<Vec<_>>();
config_bridges =
Self::build_config(Some(Self::sort_bridges_by_reachability(add_bridges)));
config_bridges = Self::build_config(Some(add_bridges));
continue;
} else if TorConfig::get_bridge().is_some() {
// Launch without bridges if all attempts failed.
@@ -566,10 +495,10 @@ impl Tor {
match http.request(req).await {
Ok(r) => {
let status = r.status().as_u16();
match hyper_tor::body::to_bytes(r.into_body()).await {
Ok(raw) => Some((status, raw.to_vec())),
match Self::read_body_capped(r.into_body(), MAX_RESPONSE_BYTES).await {
Ok(raw) => Some((status, raw)),
Err(e) => {
error!("Tor: http response parse error: {}", e);
error!("Tor: {}", e);
None
}
}
@@ -581,6 +510,25 @@ impl Tor {
}
}
/// Read a response body, aborting once it exceeds `cap` bytes. goblin.st is
/// untrusted (see nip05.rs): a malicious or breached server must not be able
/// to stream an unbounded body into memory and OOM the wallet. Streaming the
/// frames means a lying `Content-Length` (or none at all) can't get past the
/// cap by buffering everything up front.
async fn read_body_capped(body: hyper_tor::Body, cap: usize) -> Result<Vec<u8>, String> {
use futures::StreamExt;
let mut body = body;
let mut out: Vec<u8> = Vec::new();
while let Some(chunk) = body.next().await {
let chunk = chunk.map_err(|e| format!("http response read error: {e}"))?;
if out.len() + chunk.len() > cap {
return Err(format!("http response exceeds {cap} byte cap"));
}
out.extend_from_slice(&chunk);
}
Ok(out)
}
/// Check if Onion service is starting.
pub fn is_service_starting(id: &String) -> bool {
let r_services = TOR_STATE.start.read();