diff --git a/swarmctl/src/remote.rs b/swarmctl/src/remote.rs index 541d388d..33fc150b 100644 --- a/swarmctl/src/remote.rs +++ b/swarmctl/src/remote.rs @@ -14,6 +14,7 @@ use std::fs::{self, DirBuilder, File, OpenOptions}; use std::io::Write as _; use std::os::unix::fs::{DirBuilderExt as _, OpenOptionsExt as _}; use std::path::{Path, PathBuf}; +use std::time::Duration; use anyhow::{Context as _, Result, anyhow, bail}; use serde::{Deserialize, Serialize}; @@ -25,6 +26,11 @@ pub(crate) const CLIENT_ID: &str = "swarmctl"; /// doesn't lapse between the check and the gateway's look at it. const EXPIRY_MARGIN_SECS: i64 = 60; +/// Bound on reaching the gateway or authelia, as swarm-controller's clients of swarm services use. +const HTTP_CONNECT_TIMEOUT: Duration = Duration::from_secs(5); +/// Bound on one whole request, body included; a refresh holds the cache lock for that long at most. +const HTTP_TIMEOUT: Duration = Duration::from_secs(30); + /// Where a controller request goes. pub(crate) enum Target { /// swarm-controller's unix socket on this host. @@ -74,7 +80,11 @@ impl Swarm { /// An HTTP client that never follows a redirect: from the gateway one /// means "log in", and following it would land on the login page. pub(crate) fn http(&self) -> Result { - let mut builder = reqwest::Client::builder().redirect(reqwest::redirect::Policy::none()); + let mut builder = bounded( + reqwest::Client::builder(), + HTTP_CONNECT_TIMEOUT, + HTTP_TIMEOUT, + ); if let Some(path) = &self.ca_file { let pem = fs::read(path) .with_context(|| format!("reading the CA certificate {}", path.display()))?; @@ -192,6 +202,19 @@ impl Swarm { } } +/// `builder` following no redirect, with every request bounded by `connect` +/// and `total`. No caller streams a response, so a total bound fits them all. +fn bounded( + builder: reqwest::ClientBuilder, + connect: Duration, + total: Duration, +) -> reqwest::ClientBuilder { + builder + .redirect(reqwest::redirect::Policy::none()) + .connect_timeout(connect) + .timeout(total) +} + /// `$XDG_STATE_HOME/swarmctl`, else `~/.local/state/swarmctl`. pub(crate) fn state_dir() -> Result { std::env::var_os("XDG_STATE_HOME") @@ -608,6 +631,43 @@ mod tests { fs::remove_dir_all(&dir).expect("cleanup"); } + /// authelia takes the refresh and never answers: the refresh fails at the + /// client's bound instead of holding the cache lock forever. + #[test] + fn a_stalled_refresh_fails_instead_of_hanging() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind"); + let origin = url::Url::parse(&format!("http://{}/", listener.local_addr().expect("addr"))) + .expect("url"); + // Accepted and kept open, so the client sees neither an answer nor a close. + std::thread::spawn(move || { + let mut held = Vec::new(); + while let Ok((stream, _)) = listener.accept() { + held.push(stream); + } + }); + let dir = scratch("stalled"); + let swarm = logged_in(&origin, &dir, "at-0", 0); + let http = bounded( + reqwest::Client::builder().tls_certs_only([]), + Duration::from_secs(1), + Duration::from_millis(200), + ) + .build() + .expect("client"); + + let refresh = block_on(async { + tokio::time::timeout(Duration::from_secs(10), swarm.access_token(&http, None)).await + }); + let err = refresh + .expect("the client's bound fires, not this one") + .expect_err("a silent authelia issues no token"); + assert!( + format!("{err:#}").contains("refreshing the login"), + "{err:#}" + ); + fs::remove_dir_all(&dir).expect("cleanup"); + } + /// A 401 after the refresh means the login itself is gone: say so, once. #[test] fn a_second_401_says_to_log_in_and_does_not_retry_again() {