diff --git a/README.md b/README.md index d77354f..28fc44a 100644 --- a/README.md +++ b/README.md @@ -126,7 +126,7 @@ fully-qualified server such as `http://openqa.internal`. > TLS certificate verification and exposes the connection to > man-in-the-middle attacks. Use it only against trusted instances on > trusted networks; prefer `TlsMode::CustomCa` with the internal CA bundle -> instead. +> instead. Building a client with this mode logs a `tracing::warn!`. A warning is also logged via `tracing::warn!` if credentials would be sent over plaintext `http` to a non-loopback host. @@ -196,8 +196,10 @@ responsibility on an injected client, so calling either alongside `multiplier` must be finite and `>= 1.0`; [`ClientBuilder::build`] rejects anything else. Backoff is exponential with full jitter (`uniform(0, backoff)`). Call -[`RetryPolicy::upstream_compat`] for a more lenient profile (5 retries, -10 s initial backoff, 60 s cap, no deadline). `deadline` is a budget for the +[`RetryPolicy::upstream_compat`] for the `openQA-python-client`'s numbers +(5 retries, 10 s initial backoff, 60 s cap, no deadline); the jitter, method +restriction, and `Retry-After` handling are `ruoqa`'s own hardening, not +that client's. `deadline` is a budget for the whole [`Client::execute`] call — every attempt, every backoff, and every redirect hop — and a request still in flight when it expires is aborted with [`Error::DeadlineExceeded`]; the response body is then read under diff --git a/src/policy.rs b/src/policy.rs index c1ef58e..fe08327 100644 --- a/src/policy.rs +++ b/src/policy.rs @@ -188,8 +188,18 @@ impl Default for RetryPolicy { } impl RetryPolicy { - /// A more lenient retry profile: 5 retries, 10 s initial backoff, 60 s - /// cap, no overall deadline. + /// The `openQA-python-client` defaults at revision + /// [`b808edb`](https://github.com/os-autoinst/openQA-python-client/blob/b808edb0e10b73d432d0950290759c63c220a313/src/openqa_client/client.py#L199-L250): + /// 5 retries, a 10 s initial backoff doubling to a 60 s cap, no overall + /// deadline. Three behaviours are `ruoqa` hardening, not that client's: + /// full jitter (it sleeps a deterministic duration), replay restricted + /// to `idempotent_methods` unless the server signals backpressure or + /// [`RetryPolicy::retry_non_idempotent`] is set (it replays `POST` + /// unconditionally), and a `Retry-After` header honored and capped by + /// `max_retry_after` (it ignores the header). Canonical `openqa-cli` + /// (`OpenQA::Command::retry_tx`) is a different profile — 0 retries by + /// default, a flat 3 s delay, only 502/503 or connection errors — so + /// this is not that. #[must_use] pub fn upstream_compat() -> Self { Self { @@ -386,6 +396,8 @@ mod tests { assert!(p.idempotent_methods.contains(&Method::GET)); } + // Pinned to the `openQA-python-client` defaults at revision `b808edb`; + // changing these values is a conscious re-derivation, not a drift fix. #[test] fn upstream_compat_values() { let p = RetryPolicy::upstream_compat(); diff --git a/src/tls.rs b/src/tls.rs index 3c8e42a..86b4108 100644 --- a/src/tls.rs +++ b/src/tls.rs @@ -21,21 +21,17 @@ pub enum TlsMode { /// If `true`, trust *only* `certs`, discarding the platform roots. replace_roots: bool, }, - /// Disable certificate verification entirely. Dangerous: only reachable - /// via [`TlsMode::danger_accept_invalid_certs`], which cannot be - /// triggered by accident. + /// Disable certificate verification entirely. Dangerous: applying this + /// mode logs a `tracing::warn!`, however it was constructed. DangerAcceptInvalid, } impl TlsMode { /// Builds a [`TlsMode::DangerAcceptInvalid`]. Named loudly so it cannot - /// be enabled by accident; emits a `tracing::warn!` once at build time. + /// be enabled by accident; applying it warns via `tracing::warn!` when + /// the client is built. #[must_use] pub fn danger_accept_invalid_certs() -> Self { - tracing::warn!( - "TLS certificate verification is disabled: connections are not \ - protected against man-in-the-middle attacks" - ); Self::DangerAcceptInvalid } @@ -52,13 +48,21 @@ impl TlsMode { certs, replace_roots: false, } => builder.tls_certs_merge(certs), - Self::DangerAcceptInvalid => builder.tls_danger_accept_invalid_certs(true), + Self::DangerAcceptInvalid => { + tracing::warn!( + "TLS certificate verification is disabled: connections are not \ + protected against man-in-the-middle attacks" + ); + builder.tls_danger_accept_invalid_certs(true) + } } } } #[cfg(test)] mod tests { + use tracing_test::traced_test; + use super::*; #[test] @@ -93,4 +97,30 @@ mod tests { fn danger_accept_invalid_builds() { let _ = TlsMode::danger_accept_invalid_certs().apply(ClientBuilder::new()); } + + #[test] + #[traced_test] + fn applying_the_danger_variant_directly_warns() { + let _ = TlsMode::DangerAcceptInvalid.apply(ClientBuilder::new()); + assert!(logs_contain("TLS certificate verification is disabled")); + } + + #[test] + #[traced_test] + fn constructing_the_danger_variant_does_not_warn() { + let _ = TlsMode::danger_accept_invalid_certs(); + assert!(!logs_contain("TLS certificate verification is disabled")); + } + + #[test] + #[traced_test] + fn safe_modes_do_not_warn() { + let _ = TlsMode::PlatformVerifier.apply(ClientBuilder::new()); + let _ = TlsMode::CustomCa { + certs: vec![], + replace_roots: false, + } + .apply(ClientBuilder::new()); + assert!(!logs_contain("TLS certificate verification is disabled")); + } }