diff --git a/src/bin/cli.rs b/src/bin/cli.rs index c850832..708a98c 100644 --- a/src/bin/cli.rs +++ b/src/bin/cli.rs @@ -142,6 +142,15 @@ async fn serve(args: Args) -> Result<(), String> { "Ссылка для Telegram: {}", listen.telegram_link(&stats.telegram_secret()) )); + // Запись секрета могла провалиться — тогда после перезапуска ссылка + // изменится и Telegram скажет «прокси настроен неверно». Раньше это + // происходило молча (by-sonic/tglock#37). + if let Some(error) = stats.secret_write_error() { + say(&format!( + "Внимание: секрет НЕ сохранён ({error}). После перезапуска ссылка \ + изменится, и Telegram откажется подключаться к старой" + )); + } if matches!(settings.secret, config::SecretSource::Ephemeral) { say( "Внимание: секрет не закреплён и будет новым после перезапуска — \ @@ -202,13 +211,15 @@ async fn watch_status(stats: Arc) { stats.last_dc.load(Ordering::Relaxed), stats.last_route.load(Ordering::Relaxed), stats.ws_failures.load(Ordering::Relaxed), + stats.route_failures(), ); if previous.as_ref() == Some(¤t) { continue; } - let (active, tunnels, dc, route, failures) = current; + let (active, tunnels, dc, route, failures, route_failures) = current; let line = format!( - "соединений {active} · туннелей {tunnels} · {} · {} · сбоев {failures}", + "соединений {active} · туннелей {tunnels} · {} · {} · сбоев {failures} · \ + падений маршрутов {route_failures}", if dc > 0 { format!("DC{dc}") } else { diff --git a/src/cli_settings.rs b/src/cli_settings.rs index 85e84c6..9c1facf 100644 --- a/src/cli_settings.rs +++ b/src/cli_settings.rs @@ -88,7 +88,7 @@ impl Resolved { let stats = match &self.secret { SecretSource::Inline(secret) => proxy::Stats::with_secret(*secret), SecretSource::File(path) => { - proxy::Stats::with_secret(mtproto::load_or_create_secret_at(path)) + proxy::Stats::with_stored_secret(mtproto::load_or_create_secret_at(path)) } SecretSource::Ephemeral => proxy::Stats::new(), }; diff --git a/src/main.rs b/src/main.rs index 2938e3b..1ace319 100644 --- a/src/main.rs +++ b/src/main.rs @@ -44,6 +44,9 @@ struct StatusSnapshot { data_center: Option, route: String, failures: u32, + /// Падения отдельных маршрутов. Растёт даже когда соединение в итоге + /// состоялось через запасной адрес (by-sonic/tglock#32). + route_failures: u32, uptime_seconds: u64, port: u16, logs: Vec, @@ -96,6 +99,7 @@ impl AppState { data_center: (data_center > 0).then_some(data_center), route: route.to_owned(), failures: self.stats.ws_failures.load(Ordering::Relaxed), + route_failures: self.stats.route_failures(), uptime_seconds: self .started_at .lock() @@ -279,7 +283,17 @@ fn main() { .app_config_dir() .map_err(|error| error.to_string())? .join("settings.json"); - app.manage(AppState::new(settings_path)); + let state = AppState::new(settings_path); + // Если секрет не удалось записать, ссылка tg://proxy изменится после + // перезапуска и Telegram откажется подключаться к сохранённой. + // Раньше это происходило молча (by-sonic/tglock#37). + if let Some(error) = state.stats.secret_write_error() { + state.log( + format!("Секрет не сохранён ({error}). После перезапуска ссылка изменится"), + true, + ); + } + app.manage(state); Ok(()) }) .invoke_handler(tauri::generate_handler![ diff --git a/src/mtproto.rs b/src/mtproto.rs index 0ef6868..cfb3542 100644 --- a/src/mtproto.rs +++ b/src/mtproto.rs @@ -51,31 +51,64 @@ pub fn generate_secret() -> [u8; 16] { secret } -/// Reuse the secret stored at `path`, creating it if it is missing or unusable. +/// Секрет прокси и то, лежит ли он на диске. +pub struct StoredSecret { + pub value: [u8; 16], + /// Ошибка, из-за которой секрет не удалось сохранить. + /// + /// Если она есть, при следующем запуске секрет будет другим, ссылка + /// `tg://proxy` перестанет совпадать с сохранённой в Telegram, и Telegram + /// скажет «прокси настроен неверно и будет отключён». Раньше запись + /// провалившись молчала, и понять причину было невозможно + /// (by-sonic/tglock#37). + pub write_error: Option, +} + +impl StoredSecret { + /// Секрет действительно переживёт перезапуск. + pub fn is_persistent(&self) -> bool { + self.write_error.is_none() + } +} + +/// Взять секрет из файла, создав его, если файла нет или он испорчен. /// -/// A daemon needs this: the secret is half of the `tg://proxy` link, so a -/// service that invents a new one on every restart silently invalidates every -/// client that was already configured. -pub fn load_or_create_secret_at(path: &Path) -> [u8; 16] { +/// Секрет — половина ссылки `tg://proxy`, поэтому сервис, придумывающий новый +/// при каждом старте, отключает всех уже настроенных клиентов. +pub fn load_or_create_secret_at(path: &Path) -> StoredSecret { if let Ok(value) = std::fs::read_to_string(path) { - if let Some(secret) = parse_secret_hex(value.trim()) { - return secret; + if let Some(value) = parse_secret_hex(value.trim()) { + return StoredSecret { + value, + write_error: None, + }; } } - let secret = generate_secret(); + let value = generate_secret(); + let write_error = store_secret(path, &secret_hex(&value)) + .err() + .map(|error| format!("{}: {error}", path.display())); + StoredSecret { value, write_error } +} + +fn store_secret(path: &Path, value: &str) -> std::io::Result<()> { if let Some(parent) = path.parent() { - let _ = std::fs::create_dir_all(parent); + if !parent.as_os_str().is_empty() { + std::fs::create_dir_all(parent)?; + } } - write_secret_file(path, &secret_hex(&secret)); - secret + write_secret_file(path, value) } #[cfg(not(test))] -pub fn load_or_create_secret() -> [u8; 16] { +pub fn load_or_create_secret() -> StoredSecret { match secret_path() { Some(path) => load_or_create_secret_at(&path), - None => generate_secret(), + None => StoredSecret { + value: generate_secret(), + write_error: Some("не удалось определить папку для секрета в этой системе".to_owned()), + }, } } @@ -105,23 +138,21 @@ fn secret_path() -> Option { } #[cfg(unix)] -fn write_secret_file(path: &Path, value: &str) { +fn write_secret_file(path: &Path, value: &str) -> std::io::Result<()> { use std::io::Write; use std::os::unix::fs::OpenOptionsExt; - if let Ok(mut file) = std::fs::OpenOptions::new() + let mut file = std::fs::OpenOptions::new() .create(true) .truncate(true) .write(true) .mode(0o600) - .open(path) - { - let _ = file.write_all(value.as_bytes()); - } + .open(path)?; + file.write_all(value.as_bytes()) } #[cfg(not(unix))] -fn write_secret_file(path: &Path, value: &str) { - let _ = std::fs::write(path, value); +fn write_secret_file(path: &Path, value: &str) -> std::io::Result<()> { + std::fs::write(path, value) } pub fn secret_hex(secret: &[u8; 16]) -> String { @@ -514,6 +545,59 @@ mod tests { } } + #[test] + fn a_failed_write_is_reported_instead_of_swallowed() { + // Раньше ошибка записи выбрасывалась, секрет генерировался заново при + // каждом запуске, и Telegram говорил «прокси настроен неверно» без + // единой подсказки почему (by-sonic/tglock#37). + let blocker = std::env::temp_dir().join(format!( + "tglock-not-a-dir-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + std::fs::write(&blocker, "я файл, а не папка").unwrap(); + + // Родитель пути — обычный файл, поэтому создать каталог невозможно. + let stored = load_or_create_secret_at(&blocker.join("secret")); + + assert!( + !stored.is_persistent(), + "неудачная запись обязана быть видна" + ); + let error = stored.write_error.expect("должно быть сообщение об ошибке"); + assert!( + error.contains("secret"), + "в сообщении должен быть путь, получено: {error}" + ); + // Секрет всё равно выдан: прокси работает, просто до перезапуска. + assert_ne!(stored.value, [0; 16]); + + let _ = std::fs::remove_file(&blocker); + } + + #[test] + fn a_successful_write_reports_no_error() { + let path = std::env::temp_dir().join(format!( + "tglock-secret-ok-{}-{:?}", + std::process::id(), + std::thread::current().id() + )); + let _ = std::fs::remove_file(&path); + + let first = load_or_create_secret_at(&path); + assert!(first.is_persistent(), "запись в temp должна удаваться"); + + // Второй запуск читает готовый файл и тоже не жалуется. + let second = load_or_create_secret_at(&path); + assert!(second.is_persistent()); + assert_eq!( + first.value, second.value, + "секрет должен переживать перезапуск" + ); + + let _ = std::fs::remove_file(&path); + } + #[test] fn accepts_a_secret_copied_from_a_tg_link() { let expected = [ diff --git a/src/proxy.rs b/src/proxy.rs index 8eb166a..1e199e3 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -27,12 +27,23 @@ pub struct Stats { pub last_route: AtomicU8, transport: crate::transport::TransportEngine, secret: [u8; 16], + /// Почему секрет не удалось сохранить, если не удалось. + secret_write_error: Option, shutdown: Mutex>>, } impl Stats { pub fn new() -> Arc { - Self::with_secret(initial_secret()) + Self::with_stored_secret(initial_secret()) + } + + /// Построить с секретом, про который известно, сохранился он на диск или нет. + /// + /// Если не сохранился, при следующем запуске ссылка `tg://proxy` изменится и + /// Telegram скажет «прокси настроен неверно и будет отключён». Раньше это + /// происходило молча (by-sonic/tglock#37). + pub fn with_stored_secret(stored: crate::mtproto::StoredSecret) -> Arc { + Self::build(stored.value, stored.write_error) } /// Build with an explicit proxy secret. @@ -40,6 +51,15 @@ impl Stats { /// A daemon must pin this: the secret is half of the `tg://proxy` link, so /// generating a fresh one on restart breaks every configured client. pub fn with_secret(secret: [u8; 16]) -> Arc { + Self::build(secret, None) + } + + /// Сообщение о том, почему секрет не сохранён, если он не сохранён. + pub fn secret_write_error(&self) -> Option<&str> { + self.secret_write_error.as_deref() + } + + fn build(secret: [u8; 16], secret_write_error: Option) -> Arc { Arc::new(Self { running: AtomicBool::new(false), active: AtomicU32::new(0), @@ -50,6 +70,7 @@ impl Stats { last_route: AtomicU8::new(0), transport: crate::transport::TransportEngine::new(), secret, + secret_write_error, shutdown: Mutex::new(None), }) } @@ -58,6 +79,16 @@ impl Stats { crate::mtproto::telegram_secret(&self.secret) } + /// Сколько отдельных маршрутов не ответило. + /// + /// Отличается от `ws_failures`: тот растёт только когда упали все маршруты + /// и соединение не состоялось. Этот показывает перебор, который прошёл + /// незаметно — например, когда закреплённый адрес мёртв, а запасной + /// работает (by-sonic/tglock#32). + pub fn route_failures(&self) -> u32 { + self.transport.route_failures() + } + pub fn set_worker_domain(&self, domain: &str) { let domains = domain .split([',', ';', ' ']) @@ -75,13 +106,16 @@ impl Stats { } #[cfg(not(test))] -fn initial_secret() -> [u8; 16] { +fn initial_secret() -> crate::mtproto::StoredSecret { crate::mtproto::load_or_create_secret() } #[cfg(test)] -fn initial_secret() -> [u8; 16] { - crate::mtproto::generate_secret() +fn initial_secret() -> crate::mtproto::StoredSecret { + crate::mtproto::StoredSecret { + value: crate::mtproto::generate_secret(), + write_error: None, + } } /// Claim the local port. diff --git a/src/transport.rs b/src/transport.rs index 481581e..f0a2229 100644 --- a/src/transport.rs +++ b/src/transport.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Mutex; use std::time::{Duration, Instant}; @@ -113,6 +114,14 @@ struct HealthState { pub struct TransportEngine { health: Mutex, worker_domains: Mutex>, + /// Сколько раз отдельный маршрут не ответил. + /// + /// Считается отдельно от `Stats::ws_failures`, который растёт только когда + /// упали ВСЕ маршруты. Из-за этого диагностика показывала «сбоев 0», пока + /// закреплённый адрес был недоступен и каждое холодное соединение молча + /// откатывалось на следующий маршрут, тратя на это до восьми секунд + /// (by-sonic/tglock#32). + route_failures: AtomicU32, #[cfg(test)] forced_routes: Mutex>, } @@ -258,7 +267,13 @@ impl TransportEngine { health.preferred.insert(key, route.clone()); } + /// Сколько отдельных маршрутов не ответило за время работы. + pub fn route_failures(&self) -> u32 { + self.route_failures.load(Ordering::Relaxed) + } + fn record_failure(&self, route: &Route) { + self.route_failures.fetch_add(1, Ordering::Relaxed); let mut health = self.health.lock().unwrap(); let failures = health .routes @@ -656,6 +671,38 @@ mod tests { ); } + #[test] + fn every_route_failure_is_counted() { + // Диагностика показывала «сбоев 0», пока закреплённый адрес был мёртв и + // соединения молча откатывались на запасной. Счётчик маршрутов должен + // видеть каждое такое падение. + let engine = TransportEngine::new(); + let routes = routes_for_dc(2, false); + assert_eq!(engine.route_failures(), 0); + + engine.record_failure(&routes[0]); + assert_eq!(engine.route_failures(), 1); + + engine.record_failure(&routes[0]); + engine.record_failure(&routes[1]); + assert_eq!( + engine.route_failures(), + 3, + "считаются все падения, включая повторные по тому же маршруту" + ); + + // Успех не обнуляет историю: она нужна, чтобы понять, что маршруты + // перебирались, даже когда в итоге всё соединилось. + engine.record_success( + DcKey { + dc: 2, + media: false, + }, + &routes[1], + ); + assert_eq!(engine.route_failures(), 3); + } + #[test] fn documented_worker_contract_matches_the_requested_path() { // docs/CLOUDFLARE_WORKER.md promises exactly this shape. diff --git a/ui/main.ts b/ui/main.ts index 2d8f6f7..5d575a3 100644 --- a/ui/main.ts +++ b/ui/main.ts @@ -10,6 +10,7 @@ type Status = { dataCenter: number | null; route: string; failures: number; + routeFailures: number; uptimeSeconds: number; port: number; logs: LogLine[]; @@ -37,6 +38,7 @@ let status: Status = { dataCenter: null, route: "Маршрут ещё не выбран", failures: 0, + routeFailures: 0, uptimeSeconds: 0, port: 1080, logs: [], @@ -261,8 +263,18 @@ function renderDiagnostics(): void { Время работы ${formatUptime(status.uptimeSeconds)} +
+ Падений маршрутов + ${status.routeFailures} +
+

+ «Падений маршрутов» больше нуля при работающем Telegram — это норма: + значит закреплённый адрес недоступен и подключение идёт через запасной. + Число в багрепорте помогает понять, что именно перебиралось. +

+
Последние события