swarmctl: bound every gateway and authelia request to 5 s connect, 30 s total

Swarm::http() built a reqwest client with no timeout. A token refresh runs
under the exclusive flock on the login cache (kept: authelia rotates the
refresh token, and a second refresher presenting the used one revokes the
login), so an authelia that accepted the refresh and never answered held
that lock forever and every other `swarmctl --swarm` invocation blocked
behind it in flock.

The client now has a 5 s connect timeout and a 30 s total timeout, the
pair swarm-controller's own clients of swarm services use. No caller
streams a response (every gateway call and every login POST reads one
whole body), so a total bound fits all of them. The lock is unchanged; it
is now held at most for one bounded request.

Closes #5308

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<reqwest::Client> {
-        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<PathBuf> {
     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() {