update: keep the remote HTTP agents and their redirects on HTTPS (#477)

The ureq agents for remote hosts now set https_only, so a redirect to http:// is refused before connecting instead of being followed in plain text. A download refused this way fails at once rather than retrying until it pauses. Tests and the local-mirror feature keep plain HTTP, and the direct agents still serve loopback links.

crates/update/src/http.rs

@@ -65,6 +65,15 @@ impl Http {
     /// `user_agent` names the product and its version (`PhotoCraft/0.9.0`) and nothing about the
     /// person using it.
     pub fn new(user_agent: &str) -> Result<Self, String> {
+        // Tests and a local mirror serve plain HTTP on loopback, which `get_api` and
+        // `redirect_target` reach through the remote agents.
+        Self::build(user_agent, !(cfg!(feature = "local-mirror") || cfg!(test)))
+    }
+
+    /// `https_only` keeps the agents for remote hosts, and every redirect they follow, on HTTPS:
+    /// a release download that GitHub's file host redirected to `http://` is refused, not read in
+    /// plain text. The `direct` agents only ever fetch `is_loopback` links, which are plain HTTP.
+    fn build(user_agent: &str, https_only: bool) -> Result<Self, String> {
         // The OS checks each certificate chain itself: Windows fetches roots it hasn't stored yet
         // and trusts what antivirus HTTPS scanning installs. Reading the store ourselves missed
         // both ("invalid peer certificate: UnknownIssuer").
@@ -74,6 +83,7 @@ impl Http {
         let small = ureq::Agent::config_builder()
             .tls_config(tls())
             .proxy(proxy.clone())
+            .https_only(https_only)
             .timeout_global(Some(Duration::from_secs(20)))
             .http_status_as_error(false)
             .user_agent(user_agent)
@@ -82,6 +92,7 @@ impl Http {
         let feed = ureq::Agent::config_builder()
             .tls_config(tls())
             .proxy(proxy.clone())
+            .https_only(https_only)
             .timeout_connect(Some(Duration::from_secs(5)))
             .timeout_global(Some(Duration::from_secs(10)))
             .http_status_as_error(false)
@@ -91,6 +102,7 @@ impl Http {
         let no_redirect = ureq::Agent::config_builder()
             .tls_config(tls())
             .proxy(proxy.clone())
+            .https_only(https_only)
             .timeout_global(Some(Duration::from_secs(20)))
             .http_status_as_error(false)
             .max_redirects(0)
@@ -101,6 +113,7 @@ impl Http {
         let download = ureq::Agent::config_builder()
             .tls_config(tls())
             .proxy(proxy.clone())
+            .https_only(https_only)
             .timeout_connect(Some(Duration::from_secs(20)))
             .timeout_recv_response(Some(Duration::from_secs(60)))
             // ureq has no idle timeout: cap each attempt instead, and resume where it stopped.
@@ -336,6 +349,10 @@ impl Http {
         }
         let mut resp = match req.call() {
             Ok(r) => r,
+            // Asking again gets the same answer.
+            Err(ureq::Error::RequireHttpsOnly(to)) => {
+                return Err(AttemptError::Fatal(format!("the download led to plain HTTP ({to}), so it wasn't fetched")));
+            }
             Err(e) => return Err(retry(format!("couldn't connect: {e}"), false)),
         };
         let status = resp.status().as_u16();
@@ -1116,6 +1133,32 @@ pub(crate) mod tests {
         assert_eq!(part_path(Path::new("/d/photocraft.dmg"), 9), Path::new("/d/photocraft.dmg.9.part"));
     }
 
+    #[test]
+    fn the_remote_agents_refuse_plain_http_before_connecting() {
+        // ureq checks every hop, redirects included, before it connects; `.invalid` never
+        // resolves, so an agent that tried would fail on DNS instead.
+        let http = Http::build(crate::TEST_AGENT, true).unwrap();
+        let refused = |r: Result<_, String>| r.is_err_and(|e| e.contains("https only"));
+        assert!(refused(http.get_api("http://api.example.invalid/repos/x/y/releases", None).map(|_| ())));
+        assert!(refused(http.get_text("http://example.invalid/SHA256SUMS.txt").map(|_| ())));
+        assert!(refused(http.get_bytes("http://example.invalid/icon.png", 100).map(|_| ())));
+        assert!(refused(http.redirect_target("http://example.invalid/releases/latest").map(|_| ())));
+        assert!(http.get_feed("http://example.invalid/feed.json").is_err_and(|e| e.why.contains("https only")));
+        // A download fails at once rather than retrying until it pauses.
+        let dir = tempfile::tempdir().unwrap();
+        let (_, sha) = body();
+        let result = http.download("http://example.invalid/file.bin", &dir.path().join("file.bin"), 10, &sha, &Stop::default(), &mut |_| {});
+        assert!(result.is_err_and(|e| e.contains("led to plain HTTP")));
+    }
+
+    #[test]
+    fn https_only_still_reaches_a_loopback_link() {
+        let (body, _) = body();
+        let (url, _) = serve(body.clone(), vec![Reply::Full]);
+        let http = Http::build(crate::TEST_AGENT, true).unwrap();
+        assert_eq!(http.get_bytes(&url, body.len()).unwrap(), Some(body));
+    }
+
     #[test]
     fn get_bytes_rejects_oversized_payload() {
         let listener = TcpListener::bind("127.0.0.1:0").unwrap();